Felipe,
On Thu, Feb 26, 2015 at 10:54 AM, Felipe Balbi <ba...@ti.com> wrote:
> 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.
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.
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 5965ed6..b4a92e2 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -607,6 +607,14 @@ static int dsps_musb_reset(struct musb *musb)
session_restart = 1;
}
+ if (glue->sw_babble_enabled) {
+ u32 val;
+
+ val = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+ val |= MUSB_BABBLE_SW_SESSION_CTRL;
+ dsps_writeb(musb->mregs, MUSB_BABBLE_CTL, val);
+ }
+
return !session_restart;
}
>>
>> 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) {
I should mention earlier that I have seen many cases that babble
happened with full-speed devices, for example 3G modems, which require
recovery too. I have not seen low-speed babble case yet, but talking
to our hw experts that regardless the speed, certain type of babble
could cause the controller malfunction, so reset is required.
So guess we have to invoke babble recovery routine regardless?
> 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.
SW_SESSION_CTRL is still set after reset?
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
