Re: [PATCH] usb: host: xhci: fix HALTED endpoint handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 21, 2014 at 05:19:13PM -0500, Alan Stern wrote:
> On Tue, 21 Jan 2014, Sarah Sharp wrote:
> 
> > 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.
> 
> In theory, errors caused by the device sending a STALL should be
> handled just like other sorts of errors.  After the URB which got the
> STALL has been given back, the HCD should automatically restart the
> endpoint ring.  It should not wait for usb_clear_halt.

Mathias, can you look into fixing this?  I know you're working on the
code to use a configure endpoint command when we can't issue a reset
endpoint command, and this change is definitely related.  It may even
simplify the code to make this change first.

Thread, for reference:

http://marc.info/?l=linux-usb&m=139025060301432&w=2

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]