Re: [PATCH 3/7] usb: musb: core: move babble recovery inside babble check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felipe,

On Thu, Feb 26, 2015 at 11:40 AM, Felipe Balbi <balbi@xxxxxx> wrote:
> Hi,
>
> On Thu, Feb 26, 2015 at 11:20:29AM -0600, Bin Liu wrote:
>> >> > >> > There was already a proper place where we were
>> >> > >> > checking for babble interrupts, move babble
>> >> > >> > recovery there.
>> >> > >> >
>> >> > >> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
>> >> > >> > ---
>> >> > >> >  drivers/usb/musb/musb_core.c | 13 ++++++-------
>> >> > >> >  1 file changed, 6 insertions(+), 7 deletions(-)
>> >> > >> >
>> >> > >> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> >> > >> > index 2767ce1bf016..0569b24719e6 100644
>> >> > >> > --- a/drivers/usb/musb/musb_core.c
>> >> > >> > +++ b/drivers/usb/musb/musb_core.c
>> >> > >> > @@ -892,6 +892,12 @@ b_host:
>> >> > >> >                         } else {
>> >> > >> >                                 ERR("Stopping host session -- babble\n");
>> >> > >> >                                 musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>> >> > >> > +
>> >> > >> > +                               if (is_host_active(musb)) {
>> >> > >> > +                                       musb_generic_disable(musb);
>> >> > >> > +                                       schedule_delayed_work(&musb->recover_work,
>> >> > >> > +                                                       msecs_to_jiffies(100));
>> >> > >> > +                               }
>> >> > >>
>> >> > >> This change breaks babble recovery, because the following lines above here
>> >> > >>
>> >> > >> 873                         if (devctl & (MUSB_DEVCTL_FSDEV |
>> >> > >> MUSB_DEVCTL_LSDEV)) {
>> >> > >> 874                                 dev_dbg(musb->controller, "BABBLE
>> >> > >> devctl: %02x\n", devctl);
>> >> > >>
>> >> > >> have a bug - DEVCTL_FSDEV bit will be set for high-speed too, so this
>> >> > >> 'if' traps babble handling for all cases, never hit on 'else'.
>> >> > >
>> >> > > We might as well drop that check altogether. Let me see what happens
>> >> > > here.
>> >> >
>> >> > It is good to clean it up, but I guess the babble storm you see is
>> >> > caused by something else. I debugged the storm last year in an older
>> >> > kernel, it was due to the babble recovery routine does not maintain a
>> >> > bit in MUSB_BABBLE_CTL, though I forgot the details now. I am looking
>> >> > at this part in the upstream kernel right now.
>>
>> I am unable to recall why this bug causes the storm, but here is the
>> bug fix - SW_SESSION_CTRL bit gets cleared after reset. Please let me
>> know if I need to send an seperate patch email.
>
> please send it as a patch, but please rebase on top of my testing/next.
> I've just pushed quite a few patches fixing a bunch of weird
> inconsistencies with babble recovery.

I can do that. But the hw reset is unnecessary for babble recover, it
is also a problem due to AM335x Errata Adversary 1.0.34. If not reset,
SW_SESSION_CTRL bit will not be cleared.

Do you want me to send this patch or a new patch to not reset hw in recovery?

>
> Basically, even though we set SW_SESSION_CONTROL, we were still writing
> 0 to devctl which was dropping the session anyway!
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux