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 <ba...@ti.com> 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 <ba...@ti.com> wrote:
> > >> > There was already a proper place where we were
> > >> > checking for babble interrupts, move babble
> > >> > recovery there.
> > >> >
> > >> > Signed-off-by: Felipe Balbi <ba...@ti.com>
> > >> > ---
> > >> > 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
signature.asc
Description: Digital signature
