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

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 3/31/26 18:31, stern@rowland.harvard.edu wrote:
> 
> How about this instead?  We add a "halted" flag to the usb_host_endpoint
> structure, and the core will set this flag whenever a bulk or interrupt
> URB gets a status other than 0 (before putting the URB on the bh list).
> If an URB has one of these statuses, when its completion handler returns
> the core will unlink all the URBs queued to the same endpoint.  Finally,
> the "halted" flag should be cleared after a completion handler returns
> if there are no more unlinked URBs still in the queue or URBs waiting on
> the bh list to be given back.
> 
> The result of this is that any URB remaining in the queue when the flag
> is cleared must have been submitted by the class driver _after_ the
> failing URB's completion handler has run.  We can assume the class
> driver knows what it's doing in this case.
> 
> The endpoint queue shouldn't be restarted until the "halted" flag is
> cleared.  Either right away, if there are any URBs in the queue, or not
> until the next URB is submitted.  Doing this might require a new HCD
> callback.  (It would also mean the kerneldoc for usb_unlink_urb() would
> need to be updated, because the endpoint might restart before all the
> completion handlers for the unlinked URBs have run.)
> 
> What I'm trying to do here is come up with a single, consistent proposal
> for exactly when halted endpoint queues should restart.  Maybe someone
> else has a better suggestion.

Sounds like a possible solution to me.

Just to clarify, core should unlink the remaining URBs queued to that endpoint
after setting the "halted" flag, but before URB completion is called.
"Halted" flag should be cleared after URB completion returns, and endpoint
should be restarted if there are any pending URBs.

This allows the class driver URB completion handler to re-queue the halted URB
without core unlinking it.

> 
>>> Here's a troubling consequence for people to consider: Suppose an
>>> endpoint queue stops with -EPROTO or -EPIPE, and before the class driver
>>> gets around to calling usb_clear_halt(), it is unbound.  What happens
>>> the next time a driver binds to the device and tries to use the
>>> endpoint?
>>
>> The disable/enable interface and set config calls during unbind/bind should,
>> if I remember correctly flush pending URBs and drop and re-add the endpoint,
>> clearing the xhci side halt and reset toggle.
> 
> usb_probe_interface() doesn't do any of that stuff, other than a
> deferred switch to altsetting 0 if needed.
> 
> usb_unbind_interface() does call usb_enable_interface() if the interface
> is already in altsetting 0, but the reset_ep argument is false so the
> endpoint state doesn't get affected.  Should that be changed?

Looks like this needs more attention. Interface driver unbind/bind with
halted endpoint could be an issue. I don't have an answer right now.

Thanks
Mathias