On Thu, Nov 13, 2014 at 10:31:55AM -0500, Alan Stern wrote: > On Wed, 12 Nov 2014, Felipe Balbi wrote: > > > > By the way, does the same sort of thing happen after a transfer > > > error (such as a CRC mismatch)? Does the xHCI controller change the > > > state to EP_STATE_HALTED? Or does it instead go directly to > > > > There are a few conditions in which XHCI will change EP state to > > EP_STATE_HALTED, one of them is a STALL token from the peripheral and > > the others would be really error conditions: Babble, CRC error, etc. > > > > The spec even has a note about it, which I quote: > > > > " > > A Halt condition or USB Transaction error detected on a USB pipe > > shall cause a Running Endpoint to transition to the Halted > > state. A Reset Endpoint Command shall be used to clear the Halt > > condition on the endpoint and transition the endpoint to the > > Stopped state. A Stop Endpoint Command received while an > > endpoint is in the Halted state shall have no effect and shall > > generate a Command Completion Event with the Completion Code set > > to Context State Error. > > " > > > > > EP_STATE_STOPPED? You probably want to treat that case and the STALL > > > case as similarly as possible. > > > > There's already code to deal with that, take a look at > > xhci_requires_manual_halt_cleanup() and its callers: > > Then shouldn't the recovery from a STALL be exactly the same as the > recovery from any other sort of transfer error? and it is :-) if (requres_manual_halt_cleanup()) xhci_endpoint_cleanup_halt(); that's basically what happens. > > > Right. In theory you could do it any time up until the completion > > > routine returns. Doing it when you process the failed TD seems like a > > > good choice -- advance the dequeue pointer and issue the command at the > > > same time. > > > > My concern here is that this will happen when the first usb_submit_urb() > > completes. I wonder if moving this to when the following > > usb_submit_urb() is about to start would be better ? > > No, because there may be some other URBs already on the endpoint queue. > If no further URBs are submitted then the queue won't get cleaned up. oh, ok... makes sense. > > I think this has a higher probability of being correct. Class driver > > might not queue any URB to a particular after the first Stall, so why > > would be move the endpoint away from EP_STATE_HALTED prematurely ? > > In order to handle the queue. Of course, if the queue is empty then > there's no harm in leaving the ep in EP_STATE_HALTED until another URB > is submitted. > > By the same reasoning, when the state is changed to EP_STATE_STOPPED, > the doorbell should be rung. right. -- balbi
Attachment:
signature.asc
Description: Digital signature
