On Wed, 23 Jan 2019, Bin Liu wrote: > 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...) Yeah, I know. If you really want to do this, you could update the documentation along with the driver. I don't recommend it, but it's a possibility. > > > > > > 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... I agree. However, it might make sense. In theory you could enable SOF interrupts whenever one of these errors occurs, just temporarily. But I suppose that's not much better than using a timer... > > > > > 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? I don't have a multi-port serial device to test with. Can the test be carried out with a simple single-port device? It shouldn't be too hard to find a PC with an EHCI controller. xHCI might use -EPROTO too, I don't know. Alan Stern > > > 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.
