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]

 



On Thu, Feb 26, 2015 at 11:56 AM, Felipe Balbi <balbi@xxxxxx> wrote:
> Hi again,
>
> On Thu, Feb 26, 2015 at 11:54:43AM -0600, Felipe Balbi 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?
>>
>> I would rather not reset the IP if we don't have to :-s
>
> what I see though, is that we're only resetting if sw_babble_control()
> decided taht we need a session to be restarted, if we managed to recover
> from babble, then we're not resetting, right ? here's the relevant piece
> of code:

That is correct.

>
> 593         if (glue->sw_babble_enabled)
> 594                 session_restart = dsps_sw_babble_control(musb);
> 595         /*
> 596          * In case of new silicon version babble condition can be recovered
> 597          * without resetting the MUSB. But for older silicon versions, MUSB
> 598          * reset is needed
> 599          */
> 600         if (session_restart || !glue->sw_babble_enabled) {
> 601                 dev_info(musb->controller, "Restarting MUSB to recover from Babble\n");
> 602                 dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
> 603                 usleep_range(100, 200);
> 604                 usb_phy_shutdown(musb->xceiv);
> 605                 usleep_range(100, 200);
> 606                 usb_phy_init(musb->xceiv);
> 607                 session_restart = 1;
> 608         }
>
> --
> 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