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

Thinh Nguyen
2026-03-26T23:24:58Z

On Thu, Mar 26, 2026, Mathias Nyman wrote:
> 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."
>

Fair point. Then the core will need to track the endpoint's STALL state
and parse the clear-halt request to know which endpoint and when to
clear the STALL before it can accept new URB. So the first usb_enqueue()
call after clear-halt can start the endpoint again. The xhci will also
need to have access to this state.

Currently you have the xhci driver to "retire" the halted URBs. However,
you also noted that class/core may attempt to retire the pending URBs.
Who's expected to handle the retirement here?

On a separate note, will you plan to implement the clear-halt for EPROTO
in xhci?

Thanks!
Thinh