MUSB interrupt storm on device removal

[Bin Liu <b-liu@ti.com>]

On Wed, Jan 23, 2019 at 12:42:36PM -0500, Alan Stern wrote:
> On Wed, 23 Jan 2019, Bin Liu wrote:
> 
> > > One possibility is to giveback URBs with certain errors (such as
> > > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > > like a very artificial solution, though.
> > 
> > My plan is to add an error counter in musb driver endpoint struct, if
> > -EPROTO has happened consequentially for a certain times, for example 3,
> > giveback URBs with -EPIPE instead -EPROTO. This is the simplest solution
> > I can think of, though I hate expending struct unnecessarily, this is
> > one of the cases.
> 
> But -EPIPE is documented to mean that the device replied with a STALL.  
> You would be violating the documentation.

(sigh...)

> > > > > I do see now that of all USB drivers we have two drivers that handles
> > > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > > all errors, but the majority appear to resubmit on -EPROTO).
> > > 
> > > Any driver which immediately retries an URB after getting -EPROTO or
> > > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > > retries, is buggy.  As far as I can see, that's all there is to it.
> > 
> > Agreed, but given that majority appear to resubmit on -EPROTO as Johan
> > said, I think better to handle it in HCD.
> 
> Do all those other drivers handle -EPIPE correctly?  A logical way to 
> respond to -EPIPE is to issue a Clear-Halt request; what will happen 
> when that also fails?
> 
> I think it makes more sense to continue using -EPROTO but slow down the 
> exchange of packets so that there is no interrupt storm.

the musb driver doesn't use SOF interrupts, so SOF interrupt is
disabled. Another way to add delay for -EPROTO would be moving such URBs
into a new linked list and using a timer to reschedule the list. The
musb driver already has enough mess, I really don't want to add this
logic if there is other simpler option...

> 
> > > > Thanks for the info.
> > > > I will handle this case in musb driver.
> > > 
> > > Why doesn't the same problem occur with other types of host controller?
> > 
> > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> > giveback URBs with -EPROTO in such error case.
> 
> ehci-hcd also uses -EPROTO.

Is it possible to test the use case on ehci?

- connect a multi-ports usb serial device to a hub;
- open multiple ports with cat command;
- remove the usb serial device from the hub;
- console lockup happens?

> > musb controller has a register bit telling the controller has tried the
> > transaction 3 times but didn't receive any response, then the musb
> > driver just giveback this URB with -EPROTO.
> 
> Same with EHCI.

Regards,
-Bin.