On Fri, 11 Feb 2005, Wolfgang Mües wrote: > If the HC hardware retry logik is "rotten", yes, then the fix belongs to > the HC driver, I think.
Maybe "rotten" isn't the best word, but there's no question that the hardware retry logic doesn't cope well with transient errors (spikes) lasting more than a millisecond or two. > > Should > > there be a change to usbcore's API allowing drivers to specify > > unlimited retries for their URBs? > > No. It should be handled inside the HC driver. > > Think about it: will a driver who submits an URB ever expect that the > host controler _dont_ do its very best? > > The UHCI controler does 3 tries in hardware. I have made some tests and > found that sometimes about 8 x 3 tries were neccessary until the paket > could be sent. So there seem to be a large gap between the ability of > the host controler and the duration of spikes on the USB lines. > > > Or maybe only for control URBs? > > Or should every non-iso URB always have unlimited retries? > > Control: yes > Bulk: yes > Interrupt: yes > ISO: obviously not usefull. You make a good case that we should always use unlimited retries whenever it makes sense. At first sight, the only disadvantage to this would be that we will saturate the bus bandwidth when a device stops working or is unplugged, until all URBs for that device have been unlinked. This might not be so bad, given that full-speed bandwidth reclamation tends to saturate the bus anyhow. (Does the same thing happen at high speed?) What do other people think? > > Drivers _can_ stop and restart a queue; it happens every time you > > unlink an URB or an URB terminates with an error. (Okay, UHCI > > doesn't do this correctly when you unlink an URB, but it's going to > > be fixed eventually.) > > But this is not a function of the API. It is only by implementation. Do > you want to code a driver against such a weak fact? It _is_ a function of the API, and it is explicitly documented in comments at the start of usb_unlink_urb in core/urb.c. > > Inserting an URB at the front of a stopped queue is a luxury. You > > can live without it, by unlinking everything from the queue first. > > ... except for endpoint 0. I don't know all URBs... True enough. But if you're doing something that will affect URBs sent to ep0 by drivers other than your own (they won't send much more than GET-DESCRIPTOR requests), there must be something wrong. > > Besides, aren't you going to want to decide how to proceed based on > > the result of the URB inserted at the front? But you can't get a > > result without restarting the queue, and once it's restarted the > > queue won't stop after handling just that one URB. That's not what > > you want. > > Right. So I want to stop the queue, send another URB, examine the result > and restart the queue again. In essence, you want to start a second, single-element queue for an endpoint. It could be done, but it would be a lot of trouble for not much gain. I doubt anyone will want to implement it... > > > Another lacking feature is un-stalling an > > > endpoint from inside the callback function.... before we can > > > attempt to retry the packet.... > > > > There's nothing unique to USB about this. No interrupt-time code is > > allowed to block or sleep, ever. > > Please explain?!? Do you refer to the fact that un-stalling an endpoint > is a blocking function? Yes, exactly _this_ is the problem. There > should be some sort of asynchronous endpoint clear also... Would that help? You could start the asynchronous routine from within your completion handler, but you wouldn't be able to wait for it to finish. It would have to have its own completion handler, which could then retry the original packet. In principle there's no reason you can't write your own asynchronous routine for clearing endpoints. (usb-storage has something much like that already.) > Why have I mentioned it here? Because some devices do a STALL if they > receive garbage, and you have to un-stall the endpoint before you can > retry the URB.... > > Talking about STALLs... I assume that every driver knows 100% about the > command structure of the USB device. That's not a good assumption. Many drivers are written for a "class" specification which devices aren't always careful about adhering to. Also a specification may leave it undefined whether or not some feature is implemented, and the only way to find out if a device implements that feature is to try it. If the feature is not implemented, a stall is the likely result. > So a "protocol stall" (because of > a malformated command) will not happen in practice. (And if: how should > the driver cope with this? Anyway, he doesn't know better!). So STALLs > do happen because of communication errors.... maybe it's the best if > USB core does an automatic unstall command here? In my experience "protocol stalls" are quite rare, although they do sometimes happen. "Functional stalls" are much more common, and they usually occur for reasons other than communication errors. I don't know if automatically clearing stalls is a good thing. On the other hand, I can't think of any occasion where it would be bad... > Device drivers are many. Host controler drivers are many. USB core is > one. So where to put the reliability in? Clearing stalls isn't really related to reliability. It's a separate issue. > How about > > int unlink_all(....); > #define unlink_urb() unlink_all() // not recommended for new designs > > But I think that unlink_urb() can't omitted for ep0. What if a driver is > removed from memory? I see your point. My thought was that we would keep unlink_urb, and it would continue to function the same as now for ep0. But for other endpoints it would do the equivalent of unlink_all. Alan Stern ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
