Re: [PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

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

On 10/12/26 11:41, Michal Pecio wrote:
> On Fri,  9 Oct 2026 23:42:19 -0700, Yiyu Zhou wrote:
>> Commit 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
>> disconnect") makes xhci_handle_halted_endpoint() return early when the
>> root port is disconnected, and commit 7e0ef4332ed9 ("xhci: avoid xHC
>> endpoint changes after disconnect or link error.") also sets
>> EP_DROP_PENDING there, which keeps the doorbell from being rung until
>> the device is reset.
>>
>> The callers rely on xhci_handle_halted_endpoint() to take over the
>> halted TD.  For example, finish_td() returns right after calling it,
>> expecting the TD to be given back once the Reset Endpoint command
>> completes.  With the early return, nobody gives the TD back.  Its URB
>> stays pending until the submitter times out and unlinks it.  Every URB
>> queued on the endpoint afterwards never starts, because the doorbell
>> is blocked, and also has to time out.
> 
> Before 7.3 there was a similar device flag VDEV_PORT_ERROR which stuck
> until the device is reset or deleted. It only applied to USB3 devices.
> URBs were left hanging, but nobody cared, because disconnect() handler
> of the class driver would unlink them.
>   
>> Devices that disconnect and reconnect themselves during enumeration hit
>> this.  A PFU HHKB Professional HYBRID Type-S keyboard (04fe:0021) drops
>> off the bus about 300 ms after its first enumeration, while usbhid is
>> still probing its third interface.  The probe runs in hub_wq, so the
>> disconnect is not handled until the probe returns.
> 
> But probe breaks this scheme, it seems we do have no choice but to stop
> being lazy and give back the URB right there.
> 
> Regression occurred when the mechanism has been generalized to USB2 by
> also looking at the PORTSC.CCS bit, even though the problems it tries
> to solve have been unique to USB3 so far.
> 
> But this change seems sensible, because a USB3 port may autonomously
> exit the Error state to Disconnected on receiver termination loss, see
> xHCI 4.19.1.2.6, and PLS changes to RxDetect. It may even reach Enabled
> before software intervention; seems we should look at PORTSC.CSC too.
> 
> (On a USB2 port, like this keyboard uses, such race doesn't exist).
> 
> And since xhci_irq() clears these per-port flags when USB3 link comes
> back, there is no guarantee that the permanent EP_DROP_PENDING state
> will be latched on the lost device. Whether the HW bugs caused by link
> loss will still be happening on the "new" link is unclear; I only know
> about Renesas: it doesn't mind and actually recovers on reconnection.
> But the device is reset, which may or may not catch some xHC off guard.
> 
> Not sure if going back to a sticky per-device bit wouldn't be better.
> One downside is that we need to flag all affected devices, which means
> a loop in IRQ. Not great, but there is never more than 255 devices, so
> maybe a few microseconds at worst. Likely not enough latency to break
> anything, and it saves some repetitive checks in other places.
> 
> I could cook up a patch like that, I think it would make things easier,
> as there is no need for those very similar checks in urb_enqueue() for
> example, as seen below. But let's see what Mathias thinks.
> 

The solution in this patch seems reasonable to me.
It's simple enough and should solve this, and similar cases.

We should probably give back all TDs of the halted endpoint,
not just the one we halted on.

Agree with the minor changes you suggested to this patch.

-Mathias