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

Thinh Nguyen
2026-04-05T03:10:39Z

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.