Date: Thu, 11 Sep 2025 16:08:36 +0200
From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>, Greg Kroah-Hartman
 <gregkh@linuxfoundation.org>, linux-usb@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: [PATCH 3/3] usb: xhci: Clean up TD status passing
Message-ID: <20250911160836.57e1d159.michal.pecio@gmail.com>
In-Reply-To: <20250911160632.46245dc4.michal.pecio@gmail.com>
References: <20250911160632.46245dc4.michal.pecio@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

xhci_td_cleanup() has a 'status' parameter, which determines completion
status of the URB if this happens to be the last TD to be given back.
Most callers simply pass td->status here.

One bogus case is skip_isoc_td(), which is called with status determined
by some TD which triggered skipping, not the one being skipped, and the
status is later ignored anyway because it's an isochronous URB.

All those parameters can be removed and td->status used automatically by
xhci_td_cleanup(). This makes the URB status policy immediately clear.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/usb/host/xhci-ring.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 34905c8dee25..e0814282732b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -870,10 +870,10 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
 	seg->bounce_offs = 0;
 }
 
-static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
-			    struct xhci_ring *ep_ring, int status)
+static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ring *ep_ring)
 {
 	struct urb *urb = NULL;
+	int status = td->status;
 
 	/* Clean up the endpoint's TD list */
 	urb = td->urb;
@@ -917,14 +917,13 @@ static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
 }
 
 /* Give back previous TD and move on to the next TD. */
-static void xhci_dequeue_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ring *ring,
-			    u32 status)
+static void xhci_dequeue_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ring *ring)
 {
 	ring->dequeue = td->end_trb;
 	ring->deq_seg = td->end_seg;
 	inc_deq(xhci, ring);
 
-	xhci_td_cleanup(xhci, td, ring, status);
+	xhci_td_cleanup(xhci, td, ring);
 }
 
 /* Complete the cancelled URBs we unlinked from td_list. */
@@ -941,7 +940,7 @@ static void xhci_giveback_invalidated_tds(struct xhci_virt_ep *ep)
 		if (td->cancel_status == TD_CLEARED) {
 			xhci_dbg(ep->xhci, "%s: Giveback cancelled URB %p TD\n",
 				 __func__, td->urb);
-			xhci_td_cleanup(ep->xhci, td, ring, td->status);
+			xhci_td_cleanup(ep->xhci, td, ring);
 		} else {
 			xhci_dbg(ep->xhci, "%s: Keep cancelled URB %p TD as cancel_status is %d\n",
 				 __func__, td->urb, td->cancel_status);
@@ -1305,7 +1304,8 @@ static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring)
 	struct xhci_td *tmp;
 
 	list_for_each_entry_safe(cur_td, tmp, &ring->td_list, td_list) {
-		xhci_td_cleanup(xhci, cur_td, ring, -ESHUTDOWN);
+		cur_td->status = -ESHUTDOWN;
+		xhci_td_cleanup(xhci, cur_td, ring);
 	}
 }
 
@@ -1347,8 +1347,8 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
 	}
 
 	list_for_each_entry_safe(cur_td, tmp, &ep->cancelled_td_list, cancelled_td_list) {
-		ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb);
-		xhci_td_cleanup(xhci, cur_td, ring, -ESHUTDOWN);
+		cur_td->status = -ESHUTDOWN;
+		xhci_td_cleanup(xhci, cur_td, xhci_urb_to_transfer_ring(xhci, cur_td->urb));
 	}
 }
 
@@ -1510,7 +1510,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
 			td->cancel_status = TD_CLEARED;
 			xhci_dbg(ep->xhci, "%s: Giveback cancelled URB %p TD\n",
 				 __func__, td->urb);
-			xhci_td_cleanup(ep->xhci, td, ep_ring, td->status);
+			xhci_td_cleanup(ep->xhci, td, ep_ring);
 		} else {
 			xhci_dbg(ep->xhci, "%s: Keep cancelled URB %p TD as cancel_status is %d\n",
 				 __func__, td->urb, td->cancel_status);
@@ -2278,7 +2278,7 @@ static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 		break;
 	}
 
-	xhci_dequeue_td(xhci, td, ep_ring, td->status);
+	xhci_dequeue_td(xhci, td, ep_ring);
 }
 
 /* sum trb lengths from the first trb up to stop_trb, _excluding_ stop_trb */
@@ -2500,8 +2500,7 @@ static void process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 	finish_td(xhci, ep, ep_ring, td, trb_comp_code);
 }
 
-static void skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
-			 struct xhci_virt_ep *ep, int status)
+static void skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ring *ep_ring)
 {
 	struct urb_priv *urb_priv;
 	struct usb_iso_packet_descriptor *frame;
@@ -2517,7 +2516,7 @@ static void skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	/* calc actual length */
 	frame->actual_length = 0;
 
-	xhci_dequeue_td(xhci, td, ep->ring, status);
+	xhci_dequeue_td(xhci, td, ep_ring);
 }
 
 /*
@@ -2822,7 +2821,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 
 	if (td && td->error_mid_td && !trb_in_td(td, ep_trb_dma)) {
 		xhci_dbg(xhci, "Missing TD completion event after mid TD error\n");
-		xhci_dequeue_td(xhci, td, ep_ring, td->status);
+		xhci_dequeue_td(xhci, td, ep_ring);
 	}
 
 	/* If the TRB pointer is NULL, missed TDs will be skipped on the next event */
@@ -2862,7 +2861,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 				if (trb_comp_code == COMP_STOPPED_LENGTH_INVALID)
 					return 0;
 
-				skip_isoc_td(xhci, td, ep, status);
+				skip_isoc_td(xhci, td, ep_ring);
 
 				if (!list_empty(&ep_ring->td_list)) {
 					if (ring_xrun_event) {
-- 
2.48.1