[PATCH 0/5] usb: xhci: fix endpoint halt and stall recovery

[PATCH 0/5] usb: xhci: fix endpoint halt and stall recovery

[Stephano Cetola]

Recovering from a halted or stalled USB endpoint is broken in the
XHCI driver in several independent ways. An interrupt endpoint
transfer never gets a real chance to complete. Its own timeout
always defeats the hardware's autonomous polling before it can
succeed. When that or any other transfer times out, the resulting
cleanup can hit a BUG_ON in the wrong completion state, corrupt a
pointer used in the recovery command, or leave the endpoint looking
halted even after recovery actually succeeded.

In practice this shows up two ways. Most keypresses still get
through by racing the cleanup against the hardware's real response,
so it looks like occasional dropped keystrokes rather than a dead
keyboard. When the rarer failure paths trigger instead, the keyboard
stops responding entirely until reboot.

This series fixes each of those problems in the order they are
actually hit during recovery. Patch order matters.

reset_ep()'s timeout_ms parameter was inherited from an unrelated
feature (b310b08f087e, "usb: xhci: Honor transfer timeouts") meant
to let data polls like network RX return quickly, not to describe
how long a hardware recovery command needs. Recovery should always
run to completion regardless of the original transfer's timeout, so
this series gives it a fixed one instead.

Found and fixed during USB bring-up on the MNT Pocket Reform
(RK3588S), which appears to be the first board in this tree to
combine an XHCI controller with a polled USB keyboard. Testers on
the official RCORE RK3588 module independently report the same
symptom. USB polling errors appear on screen, and only a reboot
recovers the keyboard.

Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
Stephano Cetola (5):
      usb: xhci: tolerate COMP_CTX_STATE in abort_td's final completion check
      usb: xhci: reset_ep: wait for real completion, not the caller's timeout
      usb: xhci: reset_ep: fix misaligned pointer in Set TR Dequeue Pointer
      usb: xhci: xhci_bulk_tx: re-fetch ep_ctx after resetting a halted endpoint
      usb: xhci: wait a real interval for interrupt endpoint transfers

 drivers/usb/host/xhci-ring.c | 29 +++++++++++++++++++----------
 drivers/usb/host/xhci.c      |  2 +-
 2 files changed, 20 insertions(+), 11 deletions(-)
---
base-commit: 9bc1a26592a59919d2ee8c60c273d87d3d9f2e81
change-id: 20260822-send-xhci-fixes-bf1812c6c27e

[PATCH 1/5] usb: xhci: tolerate COMP_CTX_STATE in abort_td's final completion check

[Stephano Cetola]

abort_td() issues a Stop Ring command followed by a Set TR Dequeue
Pointer command to cancel a pending transfer. The completion check
for the Stop Ring command already tolerates COMP_CTX_STATE as an
acceptable outcome alongside COMP_SUCCESS. The completion check for
the following Set TR Dequeue Pointer command did not, and hit its
BUG_ON when the endpoint had transitioned to a state where that
command legitimately returns COMP_CTX_STATE instead.

Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
 drivers/usb/host/xhci-ring.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 00344e8074..bd38f0de91 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -594,9 +594,13 @@ static void abort_td(struct usb_device *udev, int ep_index)
 	if (!event)
 		return;
 
+	comp = GET_COMP_CODE(le32_to_cpu(event->event_cmd.status));
+	if (comp == COMP_CTX_STATE)
+		dev_dbg(ctrl->dev, "%s: Set TR Dequeue Pointer got CTX_STATE, endpoint was already in the target state\n",
+			__func__);
 	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
-		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
-		event->event_cmd.status)) != COMP_SUCCESS);
+		!= udev->slot_id || (comp != COMP_SUCCESS && comp
+		!= COMP_CTX_STATE));
 	xhci_acknowledge_event(ctrl);
 }
 

-- 
2.55.0

[PATCH 2/5] usb: xhci: reset_ep: wait for real completion, not the caller's timeout

[Stephano Cetola]

reset_ep() recovers a halted endpoint by issuing a Reset Endpoint
command followed by a Set TR Dequeue Pointer command, each followed
by a wait for the command's completion event. It waited using the
caller's timeout_ms.

For an interrupt endpoint poll, timeout_ms is 0 (a non-blocking
check for pending data). Passed through to reset_ep(), that same 0ms
value gives the actual recovery commands almost no time to complete.
xhci_wait_for_event() gives up immediately, reset_ep() returns before
the halt is ever cleared, and the endpoint stays halted forever no
matter how many times the caller retries.

Recovering from a halt is a command completion wait, not a data
transfer wait, and should not inherit the transfer's timeout.

Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
 drivers/usb/host/xhci-ring.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bd38f0de91..0b11da88a2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -506,7 +506,7 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
  * Send reset endpoint command for given endpoint. This recovers from a
  * halted endpoint (e.g. due to a stall error).
  */
-static void reset_ep(struct usb_device *udev, int ep_index, unsigned int timeout_ms)
+static void reset_ep(struct usb_device *udev, int ep_index)
 {
 	struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
 	struct xhci_ring *ring =  ctrl->devs[udev->slot_id]->eps[ep_index].ring;
@@ -517,7 +517,7 @@ static void reset_ep(struct usb_device *udev, int ep_index, unsigned int timeout
 	dev_info(&udev->dev, "Resetting EP %d...\n", ep_index);
 
 	xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_RESET_EP);
-	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, timeout_ms);
+	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	if (!event)
 		return;
 
@@ -528,7 +528,7 @@ static void reset_ep(struct usb_device *udev, int ep_index, unsigned int timeout
 	addr = xhci_trb_virt_to_dma(ring->enq_seg,
 		(void *)((uintptr_t)ring->enqueue | ring->cycle_state));
 	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
-	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, timeout_ms);
+	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	if (!event)
 		return;
 
@@ -706,7 +706,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 	 * have dealt with whatever caused the error.
 	 */
 	if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) == EP_STATE_HALTED)
-		reset_ep(udev, ep_index, timeout_ms);
+		reset_ep(udev, ep_index);
 
 	ring = virt_dev->eps[ep_index].ring;
 	/*
@@ -1055,7 +1055,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 		dma_unmap_single(ctrl->host.hw_dev, map, length, direction);
 
 	if (udev->status == USB_ST_STALLED) {
-		reset_ep(udev, ep_index, timeout_ms);
+		reset_ep(udev, ep_index);
 		return -EPIPE;
 	}
 

-- 
2.55.0

[PATCH 3/5] usb: xhci: reset_ep: fix misaligned pointer in Set TR Dequeue Pointer

[Stephano Cetola]

reset_ep() computed the Set TR Dequeue Pointer command's address by
ORing the ring's cycle bit into ring->enqueue before passing it to
xhci_trb_virt_to_dma(). That function does pointer arithmetic that
requires a properly aligned TRB pointer. Tainting the low bit first
breaks that arithmetic whenever cycle_state==1, which happens
naturally as the ring wraps, producing a garbage segment offset and
tripping the BUG_ON in xhci_trb_virt_to_dma().

Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0b11da88a2..522ac7c45f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -525,8 +525,8 @@ static void reset_ep(struct usb_device *udev, int ep_index)
 	BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
 	xhci_acknowledge_event(ctrl);
 
-	addr = xhci_trb_virt_to_dma(ring->enq_seg,
-		(void *)((uintptr_t)ring->enqueue | ring->cycle_state));
+	addr = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
+	addr |= ring->cycle_state;
 	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION, XHCI_TIMEOUT_DEFAULT);
 	if (!event)

-- 
2.55.0

[PATCH 4/5] usb: xhci: xhci_bulk_tx: re-fetch ep_ctx after resetting a halted endpoint

[Stephano Cetola]

xhci_bulk_tx() reads the endpoint's context once at the top of the
function and reuses that same value both to decide whether to call
reset_ep() and, further down, to pass to prepare_ring(). reset_ep()
clears the halt by updating the endpoint's output context via DMA,
but the earlier ep_ctx read is never invalidated or refetched after
that, so prepare_ring() always sees the stale pre-reset
EP_STATE_HALTED value.

prepare_ring() rejects the transfer with -EINVAL, regardless of
whether reset_ep() actually succeeded. The USB keyboard driver's
poller treats any error other than -EAGAIN as fatal. One spurious
rejection is enough to stop it for good, and nothing calls
xhci_bulk_tx() again after that. The keyboard stays dead until the
next boot.

Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
 drivers/usb/host/xhci-ring.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 522ac7c45f..a3b78052a4 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -705,8 +705,13 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 	 * the next transfer. It is the responsibility of the upper layer to
 	 * have dealt with whatever caused the error.
 	 */
-	if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) == EP_STATE_HALTED)
+	if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK) == EP_STATE_HALTED) {
 		reset_ep(udev, ep_index);
+		/* reset_ep() updates the context via DMA. Re-fetch it here. */
+		xhci_inval_cache((uintptr_t)virt_dev->out_ctx->bytes,
+				 virt_dev->out_ctx->size);
+		ep_ctx = xhci_get_ep_ctx(ctrl, virt_dev->out_ctx, ep_index);
+	}
 
 	ring = virt_dev->eps[ep_index].ring;
 	/*

-- 
2.55.0

[PATCH 5/5] usb: xhci: wait a real interval for interrupt endpoint transfers

[Stephano Cetola]

_xhci_submit_int_msg() ignored its interval parameter and always
waited 0ms for the transfer to complete. With a 0ms wait,
xhci_wait_for_event() does essentially a single instant check of the
event ring and gives up if nothing is pending yet, then abort_td()
cancels the just-queued transfer.

The xHC polls the device autonomously at the endpoint's bInterval
once a TRB is queued and the doorbell is rung, entirely in hardware,
independent of how long the driver waits. A 0ms wait cancels that
transfer before the hardware's own polling cycle ever gets a chance
to complete, even when the device has real data ready to send. Every
call would time out regardless of actual device state.

Because abort_td()'s own cleanup commands wait on a real timeout,
five seconds, not the caller's zero, the aborted transfer is often
still racing the hardware's actual response when the driver moves
on. Some polls still pick up real data by accident. The result reads
as intermittent. A keypress sometimes registers and sometimes
doesn't, rather than a clean, consistent failure.

Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 27d4353597..661dc0dde0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1110,7 +1110,7 @@ static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
 	 * (at most) one TD. A TD (comprised of sg list entries) can
 	 * take several service intervals to transmit.
 	 */
-	return xhci_bulk_tx(udev, pipe, length, buffer, 0);
+	return xhci_bulk_tx(udev, pipe, length, buffer, interval);
 }
 
 /**

-- 
2.55.0

Re: [PATCH 0/5] usb: xhci: fix endpoint halt and stall recovery

[Sascha Hauer]

On Sat, 22 Aug 2026 15:33:59 -0700, Stephano Cetola wrote:
> Recovering from a halted or stalled USB endpoint is broken in the
> XHCI driver in several independent ways. An interrupt endpoint
> transfer never gets a real chance to complete. Its own timeout
> always defeats the hardware's autonomous polling before it can
> succeed. When that or any other transfer times out, the resulting
> cleanup can hit a BUG_ON in the wrong completion state, corrupt a
> pointer used in the recovery command, or leave the endpoint looking
> halted even after recovery actually succeeded.
> 
> [...]

Applied, thanks!

[1/5] usb: xhci: tolerate COMP_CTX_STATE in abort_td's final completion check
      https://git.pengutronix.de/cgit/barebox/commit/?id=f34158c2f6a7 (link may not be stable)
[2/5] usb: xhci: reset_ep: wait for real completion, not the caller's timeout
      https://git.pengutronix.de/cgit/barebox/commit/?id=90794a34e0d5 (link may not be stable)
[3/5] usb: xhci: reset_ep: fix misaligned pointer in Set TR Dequeue Pointer
      https://git.pengutronix.de/cgit/barebox/commit/?id=f4b7aebfe14d (link may not be stable)
[4/5] usb: xhci: xhci_bulk_tx: re-fetch ep_ctx after resetting a halted endpoint
      https://git.pengutronix.de/cgit/barebox/commit/?id=b5bd959ef11e (link may not be stable)
[5/5] usb: xhci: wait a real interval for interrupt endpoint transfers
      https://git.pengutronix.de/cgit/barebox/commit/?id=c9c00f47f571 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>