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

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

On 3/26/26 03:19, Thinh Nguyen wrote:
> On Wed, Mar 25, 2026, Mathias Nyman wrote:
>> On 3/25/26 03:52, Thinh Nguyen wrote:
>>> On Mon, Mar 23, 2026, Mathias Nyman wrote:
>>>> Avoid automatically restarting bulk or interrupt transfers after a
>>>> URB is given back due to stall or error.
>>>>
>>>> Introduce a 'TD_TAINTED' state for pending TDs queued on a endpoint when
>>>> it halted. The actual TD the endpoint halted on is marked TD_HALTED,
>>>> and its URB is given back with proper EPROTO or EPIPE error code.
>>>>
>>>> Don't automatically restart an endpoint if the next queued TD after
>>>> the TD_HALTED one is marked tainted.
>>>
>>> Sounds good for -EPROTO, but will a clear-halt ring the corresponding
>>> the endpoint's doorbell for STALL endpoint?
>>>
>>
>> With this change xhci would not restart the stalled endpoint after a clear-halt
>> request. The first usb_enqueue() call after clear-halt would start it.
>>
>> Could make sense to restart the endpoint after a clear-halt, and just add a small
>> debug message if the next queued URB is marked 'tainted'.
>>
> 
> The -EPROTO should be handled differently than -EPIPE. A STALL endpoint
> is part of a normal usb flow. Should the class driver submit a new URB
> while the endpoint is STALL, we would always expect a STALL error
> response after the endpoint is restarted. That's not the case with
> -EPROTO where the data may be corrupted and/or the host and device are
> out of sync. We should not continue until the class driver do some
> recovery. IMHO, we can keep the handling of -EPIPE how it was before.
> Let the xHC tell whether the STALL error still persists instead of
> managing it by the xhci driver.
> 
I agree that that we should restart the endpoint if class/core enqueues a new
URB _after_ xhci gave back an URB with EPIPE after endpoint STALL.

But I don't think we should restart the ring to continue processing URBs that
were queued before the endpoint stalled. This would prevent the class/core
from even attempting to retire the pending URBs, something USB2.0 spec,
'5.8.5 Bulk Transfer Data Sequences' requires:

"If a halt condition is detected on a bulk pipe due to transmission errors or
  a STALL handshake being returned from the endpoint, all pending IRPs are
  retired.  Removal of the halt condition is achieved via software intervention
  through a separate control pipe."

Thanks
Mathias