On Tue, Jan 21, 2014 at 12:11:39PM -0600, Felipe Balbi wrote: > Hi, > > On Tue, Jan 21, 2014 at 01:06:19PM -0500, Alan Stern wrote: > > > You could try to differentiate between an endpoint halt that requires a > > > manual cleanup, and ones that the upper layer should handle, perhaps by > > > adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for > > > the manual cleanup case, and reject URBs with -EPIPE for the ones the > > > upper layers should handle. > > > > That should not be necessary. The HCD should always accept URB > > submissions. Nothing will get sent to the device until the HCD clears > > any host-side halts, but the upper layers don't worry about that > > because the HCD takes care of it automatically. > > > > Contrariwise, the upper layers are responsible for clearing device-side > > halts. The HCD should ignore that end of things as much as it can. > > right, but there's still the bug that usb_submit_urb() times out with > xhci and it doesn't with ehci. So there _is_ a bug in xhci. > > I'll try to debug more and figure out what's really going on, but one > thing is for sure. usbtest issued a SetFeature(ENDPOINT_HALT) as part of > the test and it tries to usb_submit_urb() as means to verify that the > device is really responding with Stalls. ... > I suppose it doesn't make a difference, considering it > wait_for_completion_interruptible(). As long as the transfer actually > completes within 10 seconds, it doesn't matter. > > What I have seen, though, is the transfer never completes and the 10 > second timeout always kicks in when trying two consecutive > usb_submit_urb(). > > Here's, in a nut-shell, what happens: > > -> SetFeature(ENDPOINT_HALT) > -> GetStatus() > -> usb_submit_urb() > -> this one completes with -EPIPE as it should > -> usb_submit_urb() > -> this one always times out :-( Ah, I think I see what's happening. The xHCI driver is not designed to execute transfers after a stall on a non-control endpoint until xhci_endpoint_reset is called and the driver fixes up the endpoint ring. That will happen in usb_clear_halt(). I expect that if an URB is submitted to a stalled non-control endpoint before usb_clear_halt is called, the xHCI driver should see EP_HALTED is set and refuse to ring the doorbell in that case. Once usb_clear_halt calls endpoint_reset, the xHCI driver will clean up the endpoint ring. The way the code was designed doesn't account for the device driver telling the endpoint to halt via a control transfer. The xHC doesn't snoop that particular control transfer, so it allows the first URB to be submitted. When that transfer stalls, the driver sets EP_HALTED. Then the second URB hangs as I expect, because the xHCI driver is waiting for usb_clear_halt() to be called. If it really matters that a driver be able to get an immediate -EPIPE response back from a device when the endpoint is halted, I can rip the code out of xhci_endpoint_reset() and make the driver manually clean up the ring at the time of the stall, like it does for control endpoints and babble/transfer/split TX errors. If it doesn't matter, test #13 should be changed to only submit one URB and get back an -EPIPE response. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
