Date: Thu, 11 Sep 2025 16:08:04 +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 2/3] usb: xhci: Deduplicate TD cleanup code
Message-ID: <20250911160804.2584c3ce.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

One case is almost a copy of xhci_td_cleanup(), the other isn't but it
should - a TD still on this list hasn't had its bounce buffer unmapped
yet or else xhci_td_cleanup() would have removed it from the list.

A side effect is that those TDs will now also be checked for transfer
length mismatch and their giveback will be logged. Should be harmless,
and the logging helped confirm that the patched code executes.

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

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5233ed3e4ed6..34905c8dee25 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -901,6 +901,10 @@ static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
 	inc_td_cnt(urb);
 	/* Giveback the urb when all the tds are completed */
 	if (last_td_in_urb(td)) {
+		/* set isoc urb status to 0 just as EHCI, UHCI, and OHCI */
+		if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS && status != -ESHUTDOWN)
+			status = 0;
+
 		if ((urb->actual_length != urb->transfer_buffer_length &&
 		     (urb->transfer_flags & URB_SHORT_NOT_OK)) ||
 		    (status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc)))
@@ -908,9 +912,6 @@ static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
 				 urb, urb->actual_length,
 				 urb->transfer_buffer_length, status);
 
-		/* set isoc urb status to 0 just as EHCI, UHCI, and OHCI */
-		if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS)
-			status = 0;
 		xhci_giveback_urb_in_irq(xhci, td, status);
 	}
 }
@@ -1304,16 +1305,7 @@ 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) {
-		list_del_init(&cur_td->td_list);
-
-		if (!list_empty(&cur_td->cancelled_td_list))
-			list_del_init(&cur_td->cancelled_td_list);
-
-		xhci_unmap_td_bounce_buffer(xhci, ring, cur_td);
-
-		inc_td_cnt(cur_td->urb);
-		if (last_td_in_urb(cur_td))
-			xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN);
+		xhci_td_cleanup(xhci, cur_td, ring, -ESHUTDOWN);
 	}
 }
 
@@ -1354,13 +1346,9 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
 		xhci_kill_ring_urbs(xhci, ring);
 	}
 
-	list_for_each_entry_safe(cur_td, tmp, &ep->cancelled_td_list,
-			cancelled_td_list) {
-		list_del_init(&cur_td->cancelled_td_list);
-		inc_td_cnt(cur_td->urb);
-
-		if (last_td_in_urb(cur_td))
-			xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN);
+	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);
 	}
 }
 
-- 
2.48.1