Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1640007 > unrolled thread

Re: [PATCH] usb-musb: keep VBUS on when device is disconnected

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Tony Lindgren <tony@atomide.com> - 2017-05-11 22:30 +0200
    Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Bin Liu <b-liu@ti.com> - 2017-05-11 22:50 +0200
      Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Tony Lindgren <tony@atomide.com> - 2017-05-11 23:10 +0200
        Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Bin Liu <b-liu@ti.com> - 2017-05-12 15:50 +0200
          Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Tony Lindgren <tony@atomide.com> - 2017-05-12 17:00 +0200
            Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Bin Liu <b-liu@ti.com> - 2017-05-12 17:30 +0200
              Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Moreno Bartalucci <moreno.bartalucci@tecnorama.it> - 2017-05-12 18:10 +0200
              Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Tony Lindgren <tony@atomide.com> - 2017-05-12 19:30 +0200
                Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Bin Liu <b-liu@ti.com> - 2017-05-12 19:50 +0200
                  Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Tony Lindgren <tony@atomide.com> - 2017-05-12 19:50 +0200
              Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Moreno Bartalucci <moreno.bartalucci@tecnorama.it> - 2017-05-15 09:10 +0200
                Re: [PATCH] usb-musb: keep VBUS on when device is disconnected Bin Liu <b-liu@ti.com> - 2017-05-15 14:30 +0200

#1640007 — Re: [PATCH] usb-musb: keep VBUS on when device is disconnected

* Bin Liu <b-liu@ti.com> [170511 13:05]:
> On Thu, May 11, 2017 at 12:38:11PM -0700, Tony Lindgren wrote:
> > 
> > I wonder if just keeping VBUS on longer in OTG_STATE_A_WAIT_BCON
> > solves this issue?
> 
> We don't cut VBUS intentionally for host mode (when devctl=0x19). The
> VBUS got cut in this case only because when a device is detached, the
> otg state changes from A_HOST -> A_WAIT_BCON, then otg_timer() cuts VBUS
> and sets the state to A_IDLE, then next otg_timer() turns on VBUS (I
> haven't check how the otg state becomes A_WAIT_VRISE from here).
> 
> Not sure how to *easily* keep VBUS here, without adding condition check
> in otg_timer() for TG_STATE_A_WAIT_BCON.
> 
> > solves this issue? It seems the issue is with modems that get
> > reconfigured after the initial enumeration?
> 
> Idealy we shouldn't cut VBUS at all in this case for host-only. I am
> looking for a small patch to solve this, if possilbe ;)

Maybe try something like below, compile tested only. I don't
think I have any USB modem here to test with.

> > We could poll for new devices every 2 seconds, if anything is seen
> > on the bus, keep VBUS on at least 20 seconds, then if nothing is
> > found, poll every 2 seconds again.
> 
> I am not sure this is relevant, VBUS is constantly on once the otg stage
> becomes A_HOST -> A_WAIT_BCON -> A_IDLE -> A_WAIT_VRISE, within a
> second.

Yup, but it sounds like once the modem changes mode, it disappears
from the USB bus for a long enough time where we go to A_WAIT_BCON
again. Or else I'm misunderstanding what's going on.

It's also possible that we have dsps_check_status() getting called
again on disconnect before the new 20 second period is over, I did
not check for that yet.

Regards,

Tony

8< -----------------------
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -270,6 +270,10 @@ static int dsps_check_status(struct musb *musb, void *unused)
 		musb_writel(musb->ctrl_base, wrp->coreintr_set,
 			    MUSB_INTR_VBUSERROR << wrp->usb_shift);
 		break;
+	case OTG_STATE_A_HOST:
+		if (glue->vbus_irq)
+			dsps_mod_timer(glue, 20000);	/* 20s */
+		break;
 	default:
 		break;
 	}
-- 
2.13.0

[toc] | [next] | [standalone]

#1640012

On Thu, May 11, 2017 at 01:23:06PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu@ti.com> [170511 13:05]:
> > On Thu, May 11, 2017 at 12:38:11PM -0700, Tony Lindgren wrote:
> > > 
> > > I wonder if just keeping VBUS on longer in OTG_STATE_A_WAIT_BCON
> > > solves this issue?
> > 
> > We don't cut VBUS intentionally for host mode (when devctl=0x19). The
> > VBUS got cut in this case only because when a device is detached, the
> > otg state changes from A_HOST -> A_WAIT_BCON, then otg_timer() cuts VBUS
> > and sets the state to A_IDLE, then next otg_timer() turns on VBUS (I
> > haven't check how the otg state becomes A_WAIT_VRISE from here).
> > 
> > Not sure how to *easily* keep VBUS here, without adding condition check
> > in otg_timer() for TG_STATE_A_WAIT_BCON.
> > 
> > > solves this issue? It seems the issue is with modems that get
> > > reconfigured after the initial enumeration?
> > 
> > Idealy we shouldn't cut VBUS at all in this case for host-only. I am
> > looking for a small patch to solve this, if possilbe ;)
> 
> Maybe try something like below, compile tested only. I don't
> think I have any USB modem here to test with.

The patch below doesn't help. In device detach by the time
dsps_check_status() is called, the otg state is already A_WAIT_BCON, set
by musb_root_disconnect() in musb_stage0_irq() when handling DISCONNECT
interrupt.

> 
> > > We could poll for new devices every 2 seconds, if anything is seen
> > > on the bus, keep VBUS on at least 20 seconds, then if nothing is
> > > found, poll every 2 seconds again.
> > 
> > I am not sure this is relevant, VBUS is constantly on once the otg stage
> > becomes A_HOST -> A_WAIT_BCON -> A_IDLE -> A_WAIT_VRISE, within a
> > second.
> 
> Yup, but it sounds like once the modem changes mode, it disappears
> from the USB bus for a long enough time where we go to A_WAIT_BCON
> again. Or else I'm misunderstanding what's going on.

I don't think it is about how long the modem disappears. When detach happens,
DISCONNECT interrupt happens, then dsps_check_status() is scheduled with
state A_WAIT_BCON, then VBUS got cut.

BTY, I didn't debug with a modem, just with a device detach. Tring to
see how to not cut vbus at all in dsps_check_status().

> 
> It's also possible that we have dsps_check_status() getting called
> again on disconnect before the new 20 second period is over, I did
> not check for that yet.

After a few trials, I start to think about a little cleanup in the otg
state machine in the musb drivers, I think we need to somehow
distinguish between normal and error conditions for A_WAIT_BCON state.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
> 
> 8< -----------------------
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -270,6 +270,10 @@ static int dsps_check_status(struct musb *musb, void *unused)
>  		musb_writel(musb->ctrl_base, wrp->coreintr_set,
>  			    MUSB_INTR_VBUSERROR << wrp->usb_shift);
>  		break;
> +	case OTG_STATE_A_HOST:
> +		if (glue->vbus_irq)
> +			dsps_mod_timer(glue, 20000);	/* 20s */
> +		break;
>  	default:
>  		break;
>  	}
> -- 
> 2.13.0

[toc] | [prev] | [next] | [standalone]

#1640037

* Bin Liu <b-liu@ti.com> [170511 13:47]:
> On Thu, May 11, 2017 at 01:23:06PM -0700, Tony Lindgren wrote:
> > 
> > Maybe try something like below, compile tested only. I don't
> > think I have any USB modem here to test with.
> 
> The patch below doesn't help. In device detach by the time
> dsps_check_status() is called, the otg state is already A_WAIT_BCON, set
> by musb_root_disconnect() in musb_stage0_irq() when handling DISCONNECT
> interrupt.

Oh OK.

> I don't think it is about how long the modem disappears. When detach happens,
> DISCONNECT interrupt happens, then dsps_check_status() is scheduled with
> state A_WAIT_BCON, then VBUS got cut.
> 
> BTY, I didn't debug with a modem, just with a device detach. Tring to
> see how to not cut vbus at all in dsps_check_status().

OK

> > It's also possible that we have dsps_check_status() getting called
> > again on disconnect before the new 20 second period is over, I did
> > not check for that yet.
> 
> After a few trials, I start to think about a little cleanup in the otg
> state machine in the musb drivers, I think we need to somehow
> distinguish between normal and error conditions for A_WAIT_BCON state.

Well maybe the minimal fix for now is just pretty much back to
square one of this thread. This should keep VBUS always on.
Then we can figure out some logic to cut VBUS later on.

And yeah, the state machine is really hard to follow so some kind
of clean up would be nice.

Regards,

Tony

8< -------------------
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -245,7 +245,6 @@ static int dsps_check_status(struct musb *musb, void *unused)
 		dsps_mod_timer_optional(glue);
 		break;
 	case OTG_STATE_A_WAIT_BCON:
-		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 		skip_session = 1;
 		/* fall */
 

[toc] | [prev] | [next] | [standalone]

#1640451

On Thu, May 11, 2017 at 02:06:28PM -0700, Tony Lindgren wrote:
> 
> Well maybe the minimal fix for now is just pretty much back to
> square one of this thread. This should keep VBUS always on.
> Then we can figure out some logic to cut VBUS later on.
> 
> And yeah, the state machine is really hard to follow so some kind
> of clean up would be nice.

Okay, figured out why clearing session in OTG_STATE_A_WAIT_BCON, it is
not for error condition handling (which is done in musb-core), but for
going back to b_idle state from a_host for dual-role mode. otg_timer()
(now is dsps_check_status()) was only called for otg port originally, so
it wasn't an issue, until started calling it for host mode as well when
runtime PM was added.
> 
> Regards,
> 
> Tony
> 
> 8< -------------------
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -245,7 +245,6 @@ static int dsps_check_status(struct musb *musb, void *unused)
>  		dsps_mod_timer_optional(glue);
>  		break;
>  	case OTG_STATE_A_WAIT_BCON:
> -		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>  		skip_session = 1;
>  		/* fall */
>  

So the above patch breaks otg port when switching from host to device
mode. The following change should solve it. But Tony do you see any way
to improve it with glue->vbus_irq?

Regards,
-Bin.

8< --------------------
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 9c7ee26ef388..465281244596 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -245,9 +245,14 @@ static int dsps_check_status(struct musb *musb, void *unused)
                dsps_mod_timer_optional(glue);
                break;
        case OTG_STATE_A_WAIT_BCON:
+               /* keep VBUS on for host-only mode */
+               if (musb->port_mode == MUSB_PORT_MODE_HOST) {
+                       dsps_mod_timer_optional(glue);
+                       break;
+               }
                musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
                skip_session = 1;
-               /* fall */
+               /* fall through */
 
        case OTG_STATE_A_IDLE:
        case OTG_STATE_B_IDLE:

[toc] | [prev] | [next] | [standalone]

#1640521

* Bin Liu <b-liu@ti.com> [170512 06:43]:
> On Thu, May 11, 2017 at 02:06:28PM -0700, Tony Lindgren wrote:
> > 
> > Well maybe the minimal fix for now is just pretty much back to
> > square one of this thread. This should keep VBUS always on.
> > Then we can figure out some logic to cut VBUS later on.
> > 
> > And yeah, the state machine is really hard to follow so some kind
> > of clean up would be nice.
> 
> Okay, figured out why clearing session in OTG_STATE_A_WAIT_BCON, it is
> not for error condition handling (which is done in musb-core), but for
> going back to b_idle state from a_host for dual-role mode. otg_timer()
> (now is dsps_check_status()) was only called for otg port originally, so
> it wasn't an issue, until started calling it for host mode as well when
> runtime PM was added.

OK makes sense.

> > 8< -------------------
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -245,7 +245,6 @@ static int dsps_check_status(struct musb *musb, void *unused)
> >  		dsps_mod_timer_optional(glue);
> >  		break;
> >  	case OTG_STATE_A_WAIT_BCON:
> > -		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> >  		skip_session = 1;
> >  		/* fall */
> >  
> 
> So the above patch breaks otg port when switching from host to device
> mode. The following change should solve it. But Tony do you see any way
> to improve it with glue->vbus_irq?

OK. No better ideas except I think we should probably have a separate
timer for keeping VBUS on after state changes eventually.

I guess the real test here would be to connect a USB modem that
changes state to the am335x-evm OTG port and make sure it works
with commit similar to d680414d0f42 ("ARM: dts: Configure BeagleBone
peripheral USB VBUS irq"). And also without configuring the vusb_irq.

For the patch below, seems like the way to go for the fix assuming
it fixes the $subject issue:

Acked-by: Tony Lindgren <tony@atomide.com>

> 8< --------------------
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 9c7ee26ef388..465281244596 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -245,9 +245,14 @@ static int dsps_check_status(struct musb *musb, void *unused)
>                 dsps_mod_timer_optional(glue);
>                 break;
>         case OTG_STATE_A_WAIT_BCON:
> +               /* keep VBUS on for host-only mode */
> +               if (musb->port_mode == MUSB_PORT_MODE_HOST) {
> +                       dsps_mod_timer_optional(glue);
> +                       break;
> +               }
>                 musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>                 skip_session = 1;
> -               /* fall */
> +               /* fall through */
>  
>         case OTG_STATE_A_IDLE:
>         case OTG_STATE_B_IDLE:

[toc] | [prev] | [next] | [standalone]

#1640542

On Fri, May 12, 2017 at 07:58:49AM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu@ti.com> [170512 06:43]:
> > On Thu, May 11, 2017 at 02:06:28PM -0700, Tony Lindgren wrote:
> > > 
> > > Well maybe the minimal fix for now is just pretty much back to
> > > square one of this thread. This should keep VBUS always on.
> > > Then we can figure out some logic to cut VBUS later on.
> > > 
> > > And yeah, the state machine is really hard to follow so some kind
> > > of clean up would be nice.
> > 
> > Okay, figured out why clearing session in OTG_STATE_A_WAIT_BCON, it is
> > not for error condition handling (which is done in musb-core), but for
> > going back to b_idle state from a_host for dual-role mode. otg_timer()
> > (now is dsps_check_status()) was only called for otg port originally, so
> > it wasn't an issue, until started calling it for host mode as well when
> > runtime PM was added.
> 
> OK makes sense.
> 
> > > 8< -------------------
> > > --- a/drivers/usb/musb/musb_dsps.c
> > > +++ b/drivers/usb/musb/musb_dsps.c
> > > @@ -245,7 +245,6 @@ static int dsps_check_status(struct musb *musb, void *unused)
> > >  		dsps_mod_timer_optional(glue);
> > >  		break;
> > >  	case OTG_STATE_A_WAIT_BCON:
> > > -		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> > >  		skip_session = 1;
> > >  		/* fall */
> > >  
> > 
> > So the above patch breaks otg port when switching from host to device
> > mode. The following change should solve it. But Tony do you see any way
> > to improve it with glue->vbus_irq?
> 
> OK. No better ideas except I think we should probably have a separate
> timer for keeping VBUS on after state changes eventually.

Currently with the patch below, VBUS is constantly on for host-only
mode, and this is what we want. Why we need a separate timer? No one
cuts VBUs now for host-only mode.

> 
> I guess the real test here would be to connect a USB modem that
> changes state to the am335x-evm OTG port and make sure it works
> with commit similar to d680414d0f42 ("ARM: dts: Configure BeagleBone
> peripheral USB VBUS irq"). And also without configuring the vusb_irq.

I will test w/ and w/o vbus_irq on BeagleBone.

Moreno, would you mind to test the patch below with your modem?

Thanks,
-Bin.

> 
> For the patch below, seems like the way to go for the fix assuming
> it fixes the $subject issue:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>
> 
> > 8< --------------------
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 9c7ee26ef388..465281244596 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -245,9 +245,14 @@ static int dsps_check_status(struct musb *musb, void *unused)
> >                 dsps_mod_timer_optional(glue);
> >                 break;
> >         case OTG_STATE_A_WAIT_BCON:
> > +               /* keep VBUS on for host-only mode */
> > +               if (musb->port_mode == MUSB_PORT_MODE_HOST) {
> > +                       dsps_mod_timer_optional(glue);
> > +                       break;
> > +               }
> >                 musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> >                 skip_session = 1;
> > -               /* fall */
> > +               /* fall through */
> >  
> >         case OTG_STATE_A_IDLE:
> >         case OTG_STATE_B_IDLE:

[toc] | [prev] | [next] | [standalone]

#1640562

> Il giorno 12 mag 2017, alle ore 17:21, Bin Liu <b-liu@ti.com> ha scritto:
> 
> On Fri, May 12, 2017 at 07:58:49AM -0700, Tony Lindgren wrote:
>> * Bin Liu <b-liu@ti.com> [170512 06:43]:
>>> On Thu, May 11, 2017 at 02:06:28PM -0700, Tony Lindgren wrote:
>>>> 
>>>> Well maybe the minimal fix for now is just pretty much back to
>>>> square one of this thread. This should keep VBUS always on.
>>>> Then we can figure out some logic to cut VBUS later on.
>>>> 
>>>> And yeah, the state machine is really hard to follow so some kind
>>>> of clean up would be nice.
>>> 
>>> Okay, figured out why clearing session in OTG_STATE_A_WAIT_BCON, it is
>>> not for error condition handling (which is done in musb-core), but for
>>> going back to b_idle state from a_host for dual-role mode. otg_timer()
>>> (now is dsps_check_status()) was only called for otg port originally, so
>>> it wasn't an issue, until started calling it for host mode as well when
>>> runtime PM was added.
>> 
>> OK makes sense.
>> 
>>>> 8< -------------------
>>>> --- a/drivers/usb/musb/musb_dsps.c
>>>> +++ b/drivers/usb/musb/musb_dsps.c
>>>> @@ -245,7 +245,6 @@ static int dsps_check_status(struct musb *musb, void *unused)
>>>> 		dsps_mod_timer_optional(glue);
>>>> 		break;
>>>> 	case OTG_STATE_A_WAIT_BCON:
>>>> -		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>>>> 		skip_session = 1;
>>>> 		/* fall */
>>>> 
>>> 
>>> So the above patch breaks otg port when switching from host to device
>>> mode. The following change should solve it. But Tony do you see any way
>>> to improve it with glue->vbus_irq?
>> 
>> OK. No better ideas except I think we should probably have a separate
>> timer for keeping VBUS on after state changes eventually.
> 
> Currently with the patch below, VBUS is constantly on for host-only
> mode, and this is what we want. Why we need a separate timer? No one
> cuts VBUs now for host-only mode.
> 
>> 
>> I guess the real test here would be to connect a USB modem that
>> changes state to the am335x-evm OTG port and make sure it works
>> with commit similar to d680414d0f42 ("ARM: dts: Configure BeagleBone
>> peripheral USB VBUS irq"). And also without configuring the vusb_irq.
> 
> I will test w/ and w/o vbus_irq on BeagleBone.
> 
> Moreno, would you mind to test the patch below with your modem?
> 
> Thanks,
> -Bin.
> 
>> 
>> For the patch below, seems like the way to go for the fix assuming
>> it fixes the $subject issue:
>> 
>> Acked-by: Tony Lindgren <tony@atomide.com>
>> 
>>> 8< --------------------
>>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>>> index 9c7ee26ef388..465281244596 100644
>>> --- a/drivers/usb/musb/musb_dsps.c
>>> +++ b/drivers/usb/musb/musb_dsps.c
>>> @@ -245,9 +245,14 @@ static int dsps_check_status(struct musb *musb, void *unused)
>>>                dsps_mod_timer_optional(glue);
>>>                break;
>>>        case OTG_STATE_A_WAIT_BCON:
>>> +               /* keep VBUS on for host-only mode */
>>> +               if (musb->port_mode == MUSB_PORT_MODE_HOST) {
>>> +                       dsps_mod_timer_optional(glue);
>>> +                       break;
>>> +               }
>>>                musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>>>                skip_session = 1;
>>> -               /* fall */
>>> +               /* fall through */
>>> 
>>>        case OTG_STATE_A_IDLE:
>>>        case OTG_STATE_B_IDLE:

Hello Bin,

today is too late, here.

I’ll try to test it on Monday.

Best regards,

Moreno

[toc] | [prev] | [next] | [standalone]

#1640651

* Bin Liu <b-liu@ti.com> [170512 08:24]:
> On Fri, May 12, 2017 at 07:58:49AM -0700, Tony Lindgren wrote:
> > OK. No better ideas except I think we should probably have a separate
> > timer for keeping VBUS on after state changes eventually.
> 
> Currently with the patch below, VBUS is constantly on for host-only
> mode, and this is what we want. Why we need a separate timer? No one
> cuts VBUs now for host-only mode.

Oh I was just thinking what we might want to do in the future if
we want to cut off VBUS when no devices are connected. If we have
a USB modem for example it might first enumerate as some boot device,
then nothing for 20 seconds while it's booting, and then we have a
different device enumerating after the modem has booted. During this
period we want to keep VBUS on and will go through multiple
OTG_STATE_A_WAIT_BCON states. So we can't really control VBUS using
the OTG_STATE_WHATEVER alone.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1640665

On Fri, May 12, 2017 at 10:21:35AM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu@ti.com> [170512 08:24]:
> > On Fri, May 12, 2017 at 07:58:49AM -0700, Tony Lindgren wrote:
> > > OK. No better ideas except I think we should probably have a separate
> > > timer for keeping VBUS on after state changes eventually.
> > 
> > Currently with the patch below, VBUS is constantly on for host-only
> > mode, and this is what we want. Why we need a separate timer? No one
> > cuts VBUs now for host-only mode.
> 
> Oh I was just thinking what we might want to do in the future if
> we want to cut off VBUS when no devices are connected. If we have

Okay, I see. But I don't think we will ever want to turn off VBUS when
no devices attached for host-only mode. Any other controllers do this?

Turning off VBUS doesn't save us much, because it comes from an external
power rail, and no one consumes it when no devices are attached.

I believe keeping the controller idle as what we have now is sufficient.

Regards,
-Bin.

> a USB modem for example it might first enumerate as some boot device,
> then nothing for 20 seconds while it's booting, and then we have a
> different device enumerating after the modem has booted. During this
> period we want to keep VBUS on and will go through multiple
> OTG_STATE_A_WAIT_BCON states. So we can't really control VBUS using
> the OTG_STATE_WHATEVER alone.
> 
> Regards,
> 
> Tony

[toc] | [prev] | [next] | [standalone]

#1640666

* Bin Liu <b-liu@ti.com> [170512 10:43]:
> On Fri, May 12, 2017 at 10:21:35AM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu@ti.com> [170512 08:24]:
> > > On Fri, May 12, 2017 at 07:58:49AM -0700, Tony Lindgren wrote:
> > > > OK. No better ideas except I think we should probably have a separate
> > > > timer for keeping VBUS on after state changes eventually.
> > > 
> > > Currently with the patch below, VBUS is constantly on for host-only
> > > mode, and this is what we want. Why we need a separate timer? No one
> > > cuts VBUs now for host-only mode.
> > 
> > Oh I was just thinking what we might want to do in the future if
> > we want to cut off VBUS when no devices are connected. If we have
> 
> Okay, I see. But I don't think we will ever want to turn off VBUS when
> no devices attached for host-only mode. Any other controllers do this?
> 
> Turning off VBUS doesn't save us much, because it comes from an external
> power rail, and no one consumes it when no devices are attached.
> 
> I believe keeping the controller idle as what we have now is sufficient.

OK fine with me.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1641319

> Il giorno 12 mag 2017, alle ore 17:21, Bin Liu <b-liu@ti.com> ha scritto:
> 
> […]
> 
> Moreno, would you mind to test the patch below with your modem?
> 
> […]
>> 
>>> 8< --------------------
>>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>>> index 9c7ee26ef388..465281244596 100644
>>> --- a/drivers/usb/musb/musb_dsps.c
>>> +++ b/drivers/usb/musb/musb_dsps.c
>>> @@ -245,9 +245,14 @@ static int dsps_check_status(struct musb *musb, void *unused)
>>>                dsps_mod_timer_optional(glue);
>>>                break;
>>>        case OTG_STATE_A_WAIT_BCON:
>>> +               /* keep VBUS on for host-only mode */
>>> +               if (musb->port_mode == MUSB_PORT_MODE_HOST) {
>>> +                       dsps_mod_timer_optional(glue);
>>> +                       break;
>>> +               }
>>>                musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>>>                skip_session = 1;
>>> -               /* fall */
>>> +               /* fall through */
>>> 
>>>        case OTG_STATE_A_IDLE:
>>>        case OTG_STATE_B_IDLE:

Hello Bin,

I tested the above patch with my device and it seems to work correctly with the current mainline kernel.

I tested it with my “production” kernel as well (4.9.20) but, for it to work, I had to change it slightly.

This is what I tested for kernel 4.9.20:

--- a/drivers/usb/musb/musb_dsps.c	2017-05-15 08:40:23.000000000 +0200
+++ b/drivers/usb/musb/musb_dsps.c	2017-05-15 08:49:17.000000000 +0200
@@ -213,6 +213,12 @@ static int dsps_check_status(struct musb
 				msecs_to_jiffies(wrp->poll_timeout));
 		break;
 	case OTG_STATE_A_WAIT_BCON:
+		/* keep VBUS on for host-only mode */
+		if (musb->port_mode == MUSB_PORT_MODE_HOST) {
+			mod_timer(&glue->timer, jiffies +
+					msecs_to_jiffies(wrp->poll_timeout));
+			break;
+		}
 		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 		skip_session = 1;
 		/* fall */

In this form, it appears to work properly for 4.9.20 too.

Best regards,

Moreno

[toc] | [prev] | [next] | [standalone]

#1641618

On Mon, May 15, 2017 at 09:07:10AM +0200, Moreno Bartalucci wrote:
> 
> > Il giorno 12 mag 2017, alle ore 17:21, Bin Liu <b-liu@ti.com> ha scritto:
> > 
> > […]
> > 
> > Moreno, would you mind to test the patch below with your modem?
> > 
> > […]
> >> 
> >>> 8< --------------------
> >>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> >>> index 9c7ee26ef388..465281244596 100644
> >>> --- a/drivers/usb/musb/musb_dsps.c
> >>> +++ b/drivers/usb/musb/musb_dsps.c
> >>> @@ -245,9 +245,14 @@ static int dsps_check_status(struct musb *musb, void *unused)
> >>>                dsps_mod_timer_optional(glue);
> >>>                break;
> >>>        case OTG_STATE_A_WAIT_BCON:
> >>> +               /* keep VBUS on for host-only mode */
> >>> +               if (musb->port_mode == MUSB_PORT_MODE_HOST) {
> >>> +                       dsps_mod_timer_optional(glue);
> >>> +                       break;
> >>> +               }
> >>>                musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> >>>                skip_session = 1;
> >>> -               /* fall */
> >>> +               /* fall through */
> >>> 
> >>>        case OTG_STATE_A_IDLE:
> >>>        case OTG_STATE_B_IDLE:
> 
> Hello Bin,
> 
> I tested the above patch with my device and it seems to work correctly with the current mainline kernel.
> 
> I tested it with my “production” kernel as well (4.9.20) but, for it to work, I had to change it slightly.

Thanks for testing.

> 
> This is what I tested for kernel 4.9.20:
> 
> --- a/drivers/usb/musb/musb_dsps.c	2017-05-15 08:40:23.000000000 +0200
> +++ b/drivers/usb/musb/musb_dsps.c	2017-05-15 08:49:17.000000000 +0200
> @@ -213,6 +213,12 @@ static int dsps_check_status(struct musb
>  				msecs_to_jiffies(wrp->poll_timeout));
>  		break;
>  	case OTG_STATE_A_WAIT_BCON:
> +		/* keep VBUS on for host-only mode */
> +		if (musb->port_mode == MUSB_PORT_MODE_HOST) {
> +			mod_timer(&glue->timer, jiffies +
> +					msecs_to_jiffies(wrp->poll_timeout));
> +			break;
> +		}
>  		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>  		skip_session = 1;
>  		/* fall */
> 
> In this form, it appears to work properly for 4.9.20 too.

Yeah, the mod_timer() call is wrapped into dsps_mod_timer_optional() by
369469a92393d ("usb: musb: Add support for optional VBUS irq to dsps
glue layer"), introduced in v4.11-rc1.

Regards,
-Bin.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web