Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Sat, Apr 04, 2026, Alan Stern wrote:
> On Sat, Apr 04, 2026 at 08:41:36PM +0000, Thinh Nguyen wrote:
> > On Fri, Apr 03, 2026, stern@rowland.harvard.edu wrote:
> > > Summarizing:
> > > 
> > > If the class driver wants to unlink queued URBs when a transaction error 
> > > occurs, it has to do so itself in the failed URB's completion handler.  
> > > We can make this easier by adding a usb_flush_endpoint_queue() routine 
> > > to the core.  In the meantime, the HCD ensures that the queue is stopped 
> > > before giving back the URB.  (Note: -EPIPE, -ENOENT, -ECONNRESET, and 
> > > -EREMOTEIO aren't considered to be transaction errors.)
> > > 
> > > When the completion handler returns, the core will automatically call 
> > > usb_clear_halt(), which will also reset the endpoint on the host side 
> > > and will restart the queue.  This also happens after SetConfiguration 
> > > and SetInterface, either explicitly or implicitly.
> > 
> > I like that the core will handle this automatically. But one concern:
> > How will the class driver know when the clear-halt complete so it can
> > perform the recovery? (ie. it shouldn't perform recovery immediately
> > after seeing -EPROTO)
> 
> It doesn't know, and it doesn't need to know.  Any recovery URBs the 
> class driver wants to send can be submitted as usual, and they will be 
> added onto the queue.  When the core resets the endpoint, the queue will 
> start going again and the URBs will run.
> 
> If the class driver wants to take some other action (like submitting 
> URBs to a different endpoint) before using the endpoint that stopped, 
> it's free to do so.  It only has to make sure that it doesn't submit any 
> URBs to the stopped endpoint until after the other action is finished -- 
> which is what it would do anyway.  (And maybe it has to unlink any URBs 
> that are already queued, which can be done with a simple function call.)
> 

Then the xhci must make sure that it should not ring the doorbell to
restart the endpoint when giving back the canceled URBs. It should only
do so on newly submitted URBs.

We can add a requirement such that if the class driver submitted the
recovery URBs prior to completing the usb_reset_endpoint (which should
be done after clear-halt), then the HCD may keep those URBs on a queue
and only process those URBs and restart the endpoint afterward.

BR,
Thinh

> > > For -EPIPE (device sent a STALL token), the class driver has to clear 
> > > the halt itself.  This is because stalls aren't errors; they are an 
> > > intentional part of the USB protocol.
> > > 
> > > -ENOENT and -ECONNRESET (URB was unlinked) and -EREMOTEIO (short packet 
> > > received with URB_SHORT_NOT_OK set) are a little trickier.  The HCD may 
> > > or may not need to stop the queue for an unlink, possibly depending on 
> > > whether the URB being unlinked is the active one.  When a short packet 
> > > is received, the HC hardware may or may not stop its queue.  Either way, 
> > > the class driver shouldn't need to take any special recovery action; any 
> > > necessary actions should be taken automatically by the HCD and the core.
> > > 
> > > All of this applies only to bulk and interrupt endpoints.  Control 
> > > endpoints generally need error recovery only on the host side, because 
> > > the device resets automatically when it gets a new SETUP packet, and so 
> > > the HCD should handle whatever is needed.  Isochronous endpoints don't 
> > > ever halt and they shouldn't need to be reset when an error occurs.
> > > 
> > > Overall, this seems simpler than anything else we have discussed.
> > > 
> > 
> > The rest sounds good to me!
> 
> Good!  Let's wait to hear from Michal, Mathias, and Oliver.
> 
> Alan