Linux USB
 help / color / mirror / Atom feed
* [PATCH] xhci: give back TDs and clean up ep0 after disconnect
@ 2026-10-29 15:39 Mathias Nyman
  2026-10-29 16:18 ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Mathias Nyman @ 2026-10-29 15:39 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, regressions, michal.pecio, Yiyu Zhou, Dennis Wissel,
	Mathias Nyman

From: Yiyu Zhou <yiyu@yiyuzhou.io>

Fix a regression in v7.3 introduced by the commits in the Fixes tags.
Those patches avoid endpoint recovery and rapid state changes after
the root port link is lost.

This caused a regression in device enumeration.

It turns out that a subset of USB HID devices briefly disconnect during
enumeration. USB core manages to resuscitate these devices without
reallocating them, and thus continue using the same udev, xhci vdev and
default control endpoint (ep0) of the device.
xhci didn't give back or clean up halted TDs behind disconnected ports
as it assumed that the disconnect cancels all URBs, and drops and frees
all endpoints.

This is not the case, so cancel and give back TDs on halted endpoint
behind disconnected ports, make sure ep0 is cleared and in a proper state
to continue working even after a link error or disconnect is detected.
Also prevent new URBs until device is reset.

One issue related to the TDs not being given back was seen on a PFU
HHKB Professional HYBRID Type-S keyboard (04fe:0021).
It 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.
On a root port the HID descriptor request and its four retries in
hid_get_class_descriptor() each wait for the full 5 second
USB_CTRL_GET_TIMEOUT:

  [57685.006036] usb 3-2: new full-speed USB device number 24 using xhci_hcd
  ...
  [57685.291304] hid-generic 0003:04FE:0021.0017: input,hidraw1: USB HID v1.11 Keyboard [PFU Limited HHKB-Hybrid] on usb-0000:00:14.0-2/input1
  [57711.221083] usbhid 3-2:1.2: can't add hid device: -110
  [57711.221613] usbhid 3-2:1.2: probe with driver usbhid failed with error -110
  [57711.221842] usb 3-2: USB disconnect, device number 24

[mn: merge EP_DROP_PENDING and link_inactive checks in xhci_urb_enqueue(),
 return TDs and clean up ep0 on disconnect, rewrite commit message]

Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub disconnect")
Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes after disconnect or link error.")
Closes: https://lore.kernel.org/all/20261010071811.20483-1-yiyu@yiyuzhou.io/
Closes: https://lore.kernel.org/linux-usb/4MjB9K-2SoiA85WbI9iEig@dsine.de/
Assisted-by: LLM
Tested-by: Yiyu Zhou <yiyu@yiyuzhou.io>
Tested-by: Dennis Wissel <dennis@dsine.de>
Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>
Co-developed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 73 ++++++++++++++++++++++++++++++------
 drivers/usb/host/xhci.c      | 13 ++++---
 2 files changed, 69 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ec278a9f9540..93cbb70d82eb 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -65,6 +65,7 @@
 static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
 			 u32 field1, u32 field2,
 			 u32 field3, u32 field4, bool command_must_succeed);
+static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, int slot_id, int ep_index);
 
 /*
  * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA
@@ -955,6 +956,24 @@ static void xhci_giveback_invalidated_tds(struct xhci_hcd *xhci, struct xhci_vir
 	}
 }
 
+static void xhci_cancel_ring_tds(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+				 struct xhci_ring *ring, int td_status)
+{
+	struct xhci_td *tmp_td;
+	struct xhci_td *td;
+
+	/* Add all TDs on ring to cancelled td list */
+	list_for_each_entry_safe(td, tmp_td, &ring->td_list, td_list) {
+		if (list_empty(&td->cancelled_td_list)) {
+			list_add_tail(&td->cancelled_td_list,
+				      &ep->cancelled_td_list);
+			td->cancel_status = TD_DIRTY;
+			if (!td->status)
+				td->status = td_status;
+		}
+	}
+}
+
 static int xhci_reset_halted_ep(struct xhci_hcd *xhci, unsigned int slot_id,
 				unsigned int ep_index, enum xhci_ep_reset_type reset_type)
 {
@@ -986,25 +1005,53 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
 {
 	struct xhci_port *rhub_port = ep->vdev->rhub_port;
 	unsigned int slot_id = ep->vdev->slot_id;
+	bool disconnected = false;
+	struct xhci_ring *ring;
 	int err;
 
-	/*
-	 * Avoid resetting endpoint if link is inactive or device disonnected.
-	 * Can cause host hang.
-	 * Device will be reset to recover an inactive link, so don't do anything
-	 */
+	/* link is inactive or device disconnected */
 	if (rhub_port->link_inactive || !rhub_port->connected) {
 		ep->ep_state |= EP_DROP_PENDING;
-		return -ENODEV;
+		disconnected = true;
+		if (!td && ep->ep_index != 0) {
+			xhci_kill_endpoint_urbs(xhci, slot_id, ep->ep_index);
+			return -ENODEV;
+		}
 	}
 
-	/* add td to cancelled list and let reset ep handler take care of it */
-	if (reset_type == EP_HARD_RESET) {
-		ep->ep_state |= EP_HARD_CLEAR_TOGGLE;
+	/* Add halted td to cancelled list and let reset ep handler take care of it */
+	if (reset_type == EP_HARD_RESET || disconnected) {
 		if (td && list_empty(&td->cancelled_td_list)) {
 			list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list);
 			td->cancel_status = TD_HALTED;
+			if (disconnected && !td->status)
+				td->status = -EPROTO;
+		}
+	}
+
+	/*
+	 * Avoid resetting non-ep0 endpoints if link is inactive or device
+	 * disconnected. Can cause host hang.
+	 * Endpoints other than ep0 won't restart before the device is reset or
+	 * the endpoint is dropped, so cancel all TDs, and give back non-ep0 TDs
+	 * now instead of letting them time out. Ep0 can survive a short device
+	 * disconnect during enumeration so reset ep0 normally and clean it up.
+	 */
+	if (disconnected) {
+		ring = td ? xhci_urb_to_transfer_ring(xhci, td->urb) : ep->ring;
+		xhci_cancel_ring_tds(xhci, ep, ring, -ESHUTDOWN);
+		if (ep->ep_index != 0) {
+			xhci_dbg(xhci, "Not resetting ep %u slot %u after disconnect\n",
+				 ep->ep_index, slot_id);
+
+			xhci_process_cancelled_tds(xhci, ep);
+			xhci_kill_endpoint_urbs(xhci, slot_id, ep->ep_index);
+			return -ENODEV;
 		}
+		/* hard-reset a halted disconnected ep0 */
+		xhci_dbg(xhci, "Reset and clean up ep0 slot %u after disconnect\n",
+			 slot_id);
+		reset_type = EP_HARD_RESET;
 	}
 
 	if (ep->ep_state & EP_HALTED) {
@@ -1013,6 +1060,9 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
 		return 0;
 	}
 
+	if (reset_type == EP_HARD_RESET)
+		ep->ep_state |= EP_HARD_CLEAR_TOGGLE;
+
 	err = xhci_reset_halted_ep(xhci, slot_id, ep->ep_index, reset_type);
 	if (err)
 		return err;
@@ -1063,8 +1113,9 @@ static int xhci_invalidate_cancelled_tds(struct xhci_hcd *xhci, struct xhci_virt
 			continue;
 		}
 
-		/* device disconnected or link error, ep will be dropped */
-		if (ep->ep_state & EP_DROP_PENDING) {
+		/* device disconnected or link error, non-ep0 endpoints will be dropped */
+		if (ep->ep_state & EP_DROP_PENDING && ep->ep_index != 0) {
+			td_to_noop(xhci, ep, td, false);
 			td->cancel_status = TD_CLEARED;
 			continue;
 		}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 7f7babe9d681..fa3012bf4c0f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1665,12 +1665,6 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
 		goto free_priv;
 	}
 
-	if (xhci->devs[slot_id]->rhub_port->link_inactive) {
-		xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
-		ret = -ENODEV;
-		goto free_priv;
-	}
-
 	if (xhci->xhc_state & XHCI_STATE_DYING) {
 		xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for non-responsive xHCI host.\n",
 			 urb->ep->desc.bEndpointAddress, urb);
@@ -1680,6 +1674,13 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
 
 	ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state;
 
+	if (xhci->devs[slot_id]->rhub_port->link_inactive ||
+	    *ep_state & EP_DROP_PENDING) {
+		xhci_dbg(xhci, "Can't queue urb, port disconnect or link inactive\n");
+		ret = -ENODEV;
+		goto free_priv;
+	}
+
 	if (*ep_state & (EP_GETTING_STREAMS | EP_GETTING_NO_STREAMS)) {
 		xhci_warn(xhci, "WARN: Can't enqueue URB, ep in streams transition state %x\n",
 			  *ep_state);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] xhci: give back TDs and clean up ep0 after disconnect
  2026-10-29 15:39 [PATCH] xhci: give back TDs and clean up ep0 after disconnect Mathias Nyman
@ 2026-10-29 16:18 ` Greg KH
  2026-10-29 20:18   ` Mathias Nyman
  0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2026-10-29 16:18 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: linux-usb, regressions, michal.pecio, Yiyu Zhou, Dennis Wissel

On Thu, Oct 29, 2026 at 05:39:10PM +0200, Mathias Nyman wrote:
> From: Yiyu Zhou <yiyu@yiyuzhou.io>
> 
> Fix a regression in v7.3 introduced by the commits in the Fixes tags.
> Those patches avoid endpoint recovery and rapid state changes after
> the root port link is lost.
> 
> This caused a regression in device enumeration.
> 
> It turns out that a subset of USB HID devices briefly disconnect during
> enumeration. USB core manages to resuscitate these devices without
> reallocating them, and thus continue using the same udev, xhci vdev and
> default control endpoint (ep0) of the device.
> xhci didn't give back or clean up halted TDs behind disconnected ports
> as it assumed that the disconnect cancels all URBs, and drops and frees
> all endpoints.
> 
> This is not the case, so cancel and give back TDs on halted endpoint
> behind disconnected ports, make sure ep0 is cleared and in a proper state
> to continue working even after a link error or disconnect is detected.
> Also prevent new URBs until device is reset.
> 
> One issue related to the TDs not being given back was seen on a PFU
> HHKB Professional HYBRID Type-S keyboard (04fe:0021).
> It 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.
> On a root port the HID descriptor request and its four retries in
> hid_get_class_descriptor() each wait for the full 5 second
> USB_CTRL_GET_TIMEOUT:
> 
>   [57685.006036] usb 3-2: new full-speed USB device number 24 using xhci_hcd
>   ...
>   [57685.291304] hid-generic 0003:04FE:0021.0017: input,hidraw1: USB HID v1.11 Keyboard [PFU Limited HHKB-Hybrid] on usb-0000:00:14.0-2/input1
>   [57711.221083] usbhid 3-2:1.2: can't add hid device: -110
>   [57711.221613] usbhid 3-2:1.2: probe with driver usbhid failed with error -110
>   [57711.221842] usb 3-2: USB disconnect, device number 24
> 
> [mn: merge EP_DROP_PENDING and link_inactive checks in xhci_urb_enqueue(),
>  return TDs and clean up ep0 on disconnect, rewrite commit message]
> 
> Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub disconnect")
> Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes after disconnect or link error.")
> Closes: https://lore.kernel.org/all/20261010071811.20483-1-yiyu@yiyuzhou.io/
> Closes: https://lore.kernel.org/linux-usb/4MjB9K-2SoiA85WbI9iEig@dsine.de/
> Assisted-by: LLM
> Tested-by: Yiyu Zhou <yiyu@yiyuzhou.io>
> Tested-by: Dennis Wissel <dennis@dsine.de>
> Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>
> Co-developed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci-ring.c | 73 ++++++++++++++++++++++++++++++------
>  drivers/usb/host/xhci.c      | 13 ++++---
>  2 files changed, 69 insertions(+), 17 deletions(-)

Doesn't apply here, what was this against:

checking file drivers/usb/host/xhci-ring.c
Hunk #2 succeeded at 963 (offset 7 lines).
Hunk #3 FAILED at 1005.
Hunk #4 succeeded at 1039 (offset 7 lines).
Hunk #5 succeeded at 1092 (offset 7 lines).
1 out of 5 hunks FAILED
checking file drivers/usb/host/xhci.c

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] xhci: give back TDs and clean up ep0 after disconnect
  2026-10-29 16:18 ` Greg KH
@ 2026-10-29 20:18   ` Mathias Nyman
  0 siblings, 0 replies; 3+ messages in thread
From: Mathias Nyman @ 2026-10-29 20:18 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-usb, regressions, michal.pecio, Yiyu Zhou, Dennis Wissel

On 10/29/26 18:18, Greg KH wrote:
> Doesn't apply here, what was this against:
> 
> checking file drivers/usb/host/xhci-ring.c
> Hunk #2 succeeded at 963 (offset 7 lines).
> Hunk #3 FAILED at 1005.
> Hunk #4 succeeded at 1039 (offset 7 lines).
> Hunk #5 succeeded at 1092 (offset 7 lines).
> 1 out of 5 hunks FAILED
> checking file drivers/usb/host/xhci.c
> 
> thanks,

Ah, right, was on top of a couple other patches in my for-usb-linus
branch intended to be sent after rc1.

I'll send v2
Thanks
Mathias


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-29 20:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 15:39 [PATCH] xhci: give back TDs and clean up ep0 after disconnect Mathias Nyman
2026-10-29 16:18 ` Greg KH
2026-10-29 20:18   ` Mathias Nyman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox