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 10:44:15AM -0600, Felipe Balbi wrote:
> On Thu, Feb 26, 2015 at 10:37:50AM -0600, Bin Liu wrote:
> > Felipe,
> > 
> > On Thu, Feb 26, 2015 at 10:31 AM, Felipe Balbi <balbi@xxxxxx> wrote:
> > > On Thu, Feb 26, 2015 at 10:21:38AM -0600, Bin Liu wrote:
> > >> Felipe,
> > >>
> > >> On Thu, Feb 26, 2015 at 9:07 AM, Felipe Balbi <balbi@xxxxxx> 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.
> 
> alright, I'll have a look, let's see.

I'll split below into two patches, but here you go:

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index e23eb3e517de..c3c5a6462600 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -862,16 +862,23 @@ b_host:
 	if (int_usb & MUSB_INTR_RESET) {
 		handled = IRQ_HANDLED;
 		if (devctl & MUSB_DEVCTL_HM) {
+			u8 power = musb_readl(musb->mregs, MUSB_POWER);
+
 			/*
 			 * Looks like non-HS BABBLE can be ignored, but
-			 * HS BABBLE is an error condition. For HS the solution
-			 * is to avoid babble in the first place and fix what
-			 * caused BABBLE. When HS BABBLE happens we can only
-			 * stop the session.
+			 * HS BABBLE is an error condition.
+			 *
+			 * For HS the solution is to avoid babble in the first
+			 * place and fix what caused BABBLE.
+			 *
+			 * When HS BABBLE happens what we can depends on which
+			 * platform MUSB is running, because some platforms
+			 * implemented proprietary means for 'recovering' from
+			 * Babble conditions. One such platform is AM335x. In
+			 * most cases, however, the only thing we can do is drop
+			 * the session.
 			 */
-			if (devctl & (MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV)) {
-				dev_dbg(musb->controller, "BABBLE devctl: %02x\n", devctl);
-			} else {
+			if (power & MUSB_POWER_HSMODE) {
 				ERR("Stopping host session -- babble\n");
 				musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 5965ed69e457..b79202c3dd65 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -607,7 +607,7 @@ static int dsps_musb_reset(struct musb *musb)
 		session_restart = 1;
 	}
 
-	return !session_restart;
+	return session_restart ? 0 : -EPIPE;
 }
 
 static struct musb_platform_ops dsps_ops = {

When I look at this with g_zero, BABBLE_CTL always reads as 0x44
(SW_SESSION_CTRL | RCV_DISABLE), I see a reset happening and g_zero
renumerating. Still no IRQ storm.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


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

  Powered by Linux