Gadget regression with enabling of MUSB babble interrupt handling

Gadget regression with enabling of MUSB babble interrupt handling

[Tony Lindgren]

Hi,

Looks like commit ca88fc2ef0d7 (usb: musb: add a work_struct
to recover from babble errors) causes MUSB gadgets to stop
enumerating at least on omap3. Reverting the the commit fixes
the issue.

Regards,

Tony

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Daniel Mack]

(+ George)

On 06/19/2014 11:56 AM, Tony Lindgren wrote:
> Looks like commit ca88fc2ef0d7 (usb: musb: add a work_struct
> to recover from babble errors) causes MUSB gadgets to stop
> enumerating at least on omap3. Reverting the the commit fixes
> the issue.

Hmm, so do you see babble errors occuring?

Also, there are some more patches for musb and babble error recovery on
the usb list, namely the ones starting here in v6:

  http://marc.info/?l=linux-usb&m=140109627505065&w=4

Care to give them a try?


Thanks,
Daniel


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Tony Lindgren]

* Daniel Mack <daniel@zonque.org> [140619 03:10]:
> (+ George)
> 
> On 06/19/2014 11:56 AM, Tony Lindgren wrote:
> > Looks like commit ca88fc2ef0d7 (usb: musb: add a work_struct
> > to recover from babble errors) causes MUSB gadgets to stop
> > enumerating at least on omap3. Reverting the the commit fixes
> > the issue.
> 
> Hmm, so do you see babble errors occuring?

Not that I noticed of.
 
> Also, there are some more patches for musb and babble error recovery on
> the usb list, namely the ones starting here in v6:
> 
>   http://marc.info/?l=linux-usb&m=140109627505065&w=4
> 
> Care to give them a try?

I can confirm that "[PATCH v6 1/5] usb: musb: core: Handle Babble
condition only in HOST mode" fixes the issue for me. Also the
explanation in the patch description explains why it breaks, and
probably for all gadgets too because "BABBLE and RESET share the
same interrupt" so it's RESET in peripheral mode.

But that also raises a question: Were these patches merged for
v3.16 ever even tested in peripheral mode?

Regards,

Tony

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Daniel Mack]

Hi Tony,

On 06/19/2014 12:31 PM, Tony Lindgren wrote:
> * Daniel Mack <daniel@zonque.org> [140619 03:10]:
>> On 06/19/2014 11:56 AM, Tony Lindgren wrote:
>>> Looks like commit ca88fc2ef0d7 (usb: musb: add a work_struct
>>> to recover from babble errors) causes MUSB gadgets to stop
>>> enumerating at least on omap3. Reverting the the commit fixes
>>> the issue.
>>
>> Hmm, so do you see babble errors occuring?
> 
> Not that I noticed of.
>  
>> Also, there are some more patches for musb and babble error recovery on
>> the usb list, namely the ones starting here in v6:
>>
>>   http://marc.info/?l=linux-usb&m=140109627505065&w=4
>>
>> Care to give them a try?
> 
> I can confirm that "[PATCH v6 1/5] usb: musb: core: Handle Babble
> condition only in HOST mode" fixes the issue for me. Also the
> explanation in the patch description explains why it breaks, and
> probably for all gadgets too because "BABBLE and RESET share the
> same interrupt" so it's RESET in peripheral mode.

Ok, thanks for testing. I was told Felipe was out for some weeks, so I
don't know when these patches will be reviewed and merged.

> But that also raises a question: Were these patches merged for
> v3.16 ever even tested in peripheral mode?

At the time, I had no such hardware to test this on, so I was hoping for
more testers to give them a try in different environments, which
apparently didn't happen. It fixed a dead USB port condition on
host-mode enabled hardware, though.

We should make sure at least patch 1/5 from the series mentioned above
makes it to 3.16.


Thanks,
Daniel

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Tony Lindgren]

* Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> [140619 03:38]:
> Hi Tony,
> 
> On 06/19/2014 12:31 PM, Tony Lindgren wrote:
> > * Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> [140619 03:10]:
> >> On 06/19/2014 11:56 AM, Tony Lindgren wrote:
> >>> Looks like commit ca88fc2ef0d7 (usb: musb: add a work_struct
> >>> to recover from babble errors) causes MUSB gadgets to stop
> >>> enumerating at least on omap3. Reverting the the commit fixes
> >>> the issue.
> >>
> >> Hmm, so do you see babble errors occuring?
> > 
> > Not that I noticed of.
> >  
> >> Also, there are some more patches for musb and babble error recovery on
> >> the usb list, namely the ones starting here in v6:
> >>
> >>   http://marc.info/?l=linux-usb&m=140109627505065&w=4
> >>
> >> Care to give them a try?
> > 
> > I can confirm that "[PATCH v6 1/5] usb: musb: core: Handle Babble
> > condition only in HOST mode" fixes the issue for me. Also the
> > explanation in the patch description explains why it breaks, and
> > probably for all gadgets too because "BABBLE and RESET share the
> > same interrupt" so it's RESET in peripheral mode.
> 
> Ok, thanks for testing. I was told Felipe was out for some weeks, so I
> don't know when these patches will be reviewed and merged.

He seems to be back now :)
 
> > But that also raises a question: Were these patches merged for
> > v3.16 ever even tested in peripheral mode?
> 
> At the time, I had no such hardware to test this on, so I was hoping for
> more testers to give them a try in different environments, which
> apparently didn't happen. It fixed a dead USB port condition on
> host-mode enabled hardware, though.

Well we probably should not merge patches without proper acks and
tested-by:s in general as things just seem to keep breaking
constantly otherwise. And things not working will keep people from
using linux next which will lead into even less testing..

Anyways, this driver is the only way to get a console on many
devices, so we should ensure it always works properly.

> We should make sure at least patch 1/5 from the series mentioned above
> makes it to 3.16.

Agreed.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Daniel Mack]

On 06/19/2014 12:43 PM, Tony Lindgren wrote:
> * Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> [140619 03:38]:
>> On 06/19/2014 12:31 PM, Tony Lindgren wrote:
>>> * Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> [140619 03:10]:
>>>> On 06/19/2014 11:56 AM, Tony Lindgren wrote:

>>> But that also raises a question: Were these patches merged for
>>> v3.16 ever even tested in peripheral mode?
>>
>> At the time, I had no such hardware to test this on, so I was hoping for
>> more testers to give them a try in different environments, which
>> apparently didn't happen. It fixed a dead USB port condition on
>> host-mode enabled hardware, though.
> 
> Well we probably should not merge patches without proper acks and
> tested-by:s in general as things just seem to keep breaking
> constantly otherwise. And things not working will keep people from
> using linux next which will lead into even less testing..

I'm fairly sure the patch causing your trouble has been in linux-next
for a while before they hit the merge window, so people with gadget
enabled musb could have noticed the breakage early enough. The feedback
rate for patches to this driver posted to linux-usb is also usually low,
unfortunately.

Anyway, breaking things is certainly not good, and I'm sorry for that.
I'm just uncertain what detail in the procedure should be tweaked in
order to prevent that from happening in the future.


Thanks,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Tony Lindgren]

* Daniel Mack <daniel@zonque.org> [140619 03:51]:
> On 06/19/2014 12:43 PM, Tony Lindgren wrote:
> > * Daniel Mack <daniel@zonque.org> [140619 03:38]:
> >> On 06/19/2014 12:31 PM, Tony Lindgren wrote:
> >>> * Daniel Mack <daniel@zonque.org> [140619 03:10]:
> >>>> On 06/19/2014 11:56 AM, Tony Lindgren wrote:
> 
> >>> But that also raises a question: Were these patches merged for
> >>> v3.16 ever even tested in peripheral mode?
> >>
> >> At the time, I had no such hardware to test this on, so I was hoping for
> >> more testers to give them a try in different environments, which
> >> apparently didn't happen. It fixed a dead USB port condition on
> >> host-mode enabled hardware, though.
> > 
> > Well we probably should not merge patches without proper acks and
> > tested-by:s in general as things just seem to keep breaking
> > constantly otherwise. And things not working will keep people from
> > using linux next which will lead into even less testing..
> 
> I'm fairly sure the patch causing your trouble has been in linux-next
> for a while before they hit the merge window, so people with gadget
> enabled musb could have noticed the breakage early enough. The feedback
> rate for patches to this driver posted to linux-usb is also usually low,
> unfortunately.

Right but the problem is that people are not touching linux next
because it's constantly broken :)
 
> Anyway, breaking things is certainly not good, and I'm sorry for that.
> I'm just uncertain what detail in the procedure should be tweaked in
> order to prevent that from happening in the future.

Well I guess somebody should run basic tests on this driver in
linux next, that would probably solve the issues.

Regards,

Tony

Re: Gadget regression with enabling of MUSB babble interrupt handling

[George Cherian]

On 6/19/2014 4:54 PM, Tony Lindgren wrote:
> * Daniel Mack <daniel@zonque.org> [140619 03:51]:
>> On 06/19/2014 12:43 PM, Tony Lindgren wrote:
>>> * Daniel Mack <daniel@zonque.org> [140619 03:38]:
>>>> On 06/19/2014 12:31 PM, Tony Lindgren wrote:
>>>>> * Daniel Mack <daniel@zonque.org> [140619 03:10]:
>>>>>> On 06/19/2014 11:56 AM, Tony Lindgren wrote:
>>>>> But that also raises a question: Were these patches merged for
>>>>> v3.16 ever even tested in peripheral mode?
>>>> At the time, I had no such hardware to test this on, so I was hoping for
>>>> more testers to give them a try in different environments, which
>>>> apparently didn't happen. It fixed a dead USB port condition on
>>>> host-mode enabled hardware, though.
>>> Well we probably should not merge patches without proper acks and
>>> tested-by:s in general as things just seem to keep breaking
>>> constantly otherwise. And things not working will keep people from
>>> using linux next which will lead into even less testing..
>> I'm fairly sure the patch causing your trouble has been in linux-next
>> for a while before they hit the merge window, so people with gadget
>> enabled musb could have noticed the breakage early enough. The feedback
>> rate for patches to this driver posted to linux-usb is also usually low,
>> unfortunately.
I blame myself for not explicitly pointing out the fix.
Instead I clubbed it with this series.

http://marc.info/?l=linux-usb&m=140109627505065&w=4

Sorry for that.

> Right but the problem is that people are not touching linux next
> because it's constantly broken :)
>   
>> Anyway, breaking things is certainly not good, and I'm sorry for that.
>> I'm just uncertain what detail in the procedure should be tweaked in
>> order to prevent that from happening in the future.
> Well I guess somebody should run basic tests on this driver in
> linux next, that would probably solve the issues.

I am doing it.
> Regards,
>
> Tony


-- 
-George

Re: Gadget regression with enabling of MUSB babble interrupt handling

[Tony Lindgren]

* George Cherian <george.cherian@ti.com> [140619 20:43]:
> On 6/19/2014 4:54 PM, Tony Lindgren wrote:
> >* Daniel Mack <daniel@zonque.org> [140619 03:51]:
> >>On 06/19/2014 12:43 PM, Tony Lindgren wrote:
> >>>* Daniel Mack <daniel@zonque.org> [140619 03:38]:
> >>>>On 06/19/2014 12:31 PM, Tony Lindgren wrote:
> >>>>>* Daniel Mack <daniel@zonque.org> [140619 03:10]:
> >>>>>>On 06/19/2014 11:56 AM, Tony Lindgren wrote:
> >>>>>But that also raises a question: Were these patches merged for
> >>>>>v3.16 ever even tested in peripheral mode?
> >>>>At the time, I had no such hardware to test this on, so I was hoping for
> >>>>more testers to give them a try in different environments, which
> >>>>apparently didn't happen. It fixed a dead USB port condition on
> >>>>host-mode enabled hardware, though.
> >>>Well we probably should not merge patches without proper acks and
> >>>tested-by:s in general as things just seem to keep breaking
> >>>constantly otherwise. And things not working will keep people from
> >>>using linux next which will lead into even less testing..
> >>I'm fairly sure the patch causing your trouble has been in linux-next
> >>for a while before they hit the merge window, so people with gadget
> >>enabled musb could have noticed the breakage early enough. The feedback
> >>rate for patches to this driver posted to linux-usb is also usually low,
> >>unfortunately.
> I blame myself for not explicitly pointing out the fix.
> Instead I clubbed it with this series.
> 
> http://marc.info/?l=linux-usb&m=140109627505065&w=4
> 
> Sorry for that.
> 
> >Right but the problem is that people are not touching linux next
> >because it's constantly broken :)
> >>Anyway, breaking things is certainly not good, and I'm sorry for that.
> >>I'm just uncertain what detail in the procedure should be tweaked in
> >>order to prevent that from happening in the future.
> >Well I guess somebody should run basic tests on this driver in
> >linux next, that would probably solve the issues.
> 
> I am doing it.

Great, good to hear thanks :)

Tony