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 10:28:24PM +0000, Thinh Nguyen wrote:
> > On Sat, Apr 04, 2026, Thinh Nguyen wrote:
> > > On Sat, Apr 04, 2026, Alan Stern wrote:
> > > > 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.
> > 
> > Ignore this comment, it's not restarting the endpoint in the case of
> > unlinking.
> 
> I was going to say that xhci-hcd shouldn't restart the endpoint until 
> the usb_reset_endpoint() call is made.  Whether or not it rings the 
> doorbell at that time may depend on whether there are any URBs on the 
> queue; that's a relatively unimportant implementation detail.
> 
> > > 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.
> > > 
> > 
> > Actually, adding this new requirement would be tricky because we don't
> > know whether it's recovery URBs or not.
> 
> The purpose of the submitted URBs doesn't matter.  The HCD shouldn't 
> restart the endpoint until the usb_reset_endpoint() call occurs.
> 
> Also, I should point out that usbcore will call usb_clear_halt() and 
> usb_reset_endpoint(), presumably using a work queue for the calls.  The 
> class driver doesn't need to do it -- in fact, doing those things could 
> lead to errors because the endpoint may already be running (the core may 
> already have made the calls).
> 

That's good. This is what I wanted to confirm.

May need to update how xhci handles usb_reset_endpoint() because I
believe it's expected the endpoint transfer ring to be drained prior (by
the class driver unlinking URBs).

Thanks for the clarifications,
Thinh

ps. Have a good weekend! I'll be back a week after.