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