diff mbox series

[4/5] usb: xhci: Simplify the TD skipping loop further

Message ID 20240910131609.20434c3c@foxbook (mailing list archive)
State New
Headers show
Series Quick and effective handle_tx_event() cleanup | expand

Commit Message

MichaƂ Pecio Sept. 10, 2024, 11:16 a.m. UTC
Now that everything not related to skipping has been removed from the
loop, its role becomes more clear. It repeatedly skips the first TD on
the ring, as long as one exists, doesn't match the event, and is isoc.

Rewrite this logic more succintly and unify the various exit paths.
The skip flag is always cleared, so do it only once (nitpick: it used
not to be cleared on non-isoc exit, no big deal). Print one universal
debug message too, and count how many TDs were skipped.

Note: the test for non-isoc URB is valuable because skip_isoc_td()
writes to URB fields which don't exist on non-isoc URBs. This avoids
memory corruption in the unlucky event of a severe HW or SW bug.

Add a helper function for getting the first TD from the ring's queue
to make the code less of an eyesore.

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

Patch

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 56b0c0e85293..db9bc7db5aac 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2591,6 +2591,15 @@  static int handle_transferless_tx_event(struct xhci_hcd *xhci, struct xhci_virt_
 	return 0;
 }
 
+/*
+ * Return the first TD pending on a transfer ring, or NULL if none.
+ */
+static struct xhci_td *xhci_ring_pending_td(struct xhci_ring *ring)
+{
+	return list_first_entry_or_null(&ring->td_list,
+					struct xhci_td, td_list);
+}
+
 /*
  * If this function returns an error condition, it means it got a Transfer
  * event with a corrupted Slot ID, Endpoint ID, or TRB DMA address.
@@ -2777,8 +2786,7 @@  static int handle_tx_event(struct xhci_hcd *xhci,
 	 * We wait for the final IOC event, but if we get an event
 	 * anywhere outside this TD, just give it back already.
 	 */
-	td = list_first_entry_or_null(&ep_ring->td_list,
-						struct xhci_td, td_list);
+	td = xhci_ring_pending_td(ep_ring);
 
 	if (td && td->error_mid_td && !trb_in_td(xhci, td, ep_trb_dma, false)) {
 		xhci_dbg(xhci, "Missing TD completion event after mid TD error\n");
@@ -2786,48 +2794,35 @@  static int handle_tx_event(struct xhci_hcd *xhci,
 		ep_ring->deq_seg = td->last_trb_seg;
 		inc_deq(xhci, ep_ring);
 		xhci_td_cleanup(xhci, td, ep_ring, td->status);
+
+		td = xhci_ring_pending_td(ep_ring);
 	}
 
-	do {
-		/* This TRB should be in the TD at the head of this ring's
-		 * TD list.
-		 */
-		if (list_empty(&ep_ring->td_list)) {
-			if (ep->skip) {
-				ep->skip = false;
-				xhci_dbg(xhci, "td_list is empty while skip flag set. Clear skip flag for slot %u ep %u.\n",
-					 slot_id, ep_index);
-			}
-			break;
-		}
-
-		td = list_first_entry(&ep_ring->td_list, struct xhci_td,
-				      td_list);
-
-		/* Is this a TRB in the currently executing TD? */
+	/* Is this a TRB in the currently executing TD? */
+	if (td)
 		ep_seg = trb_in_td(xhci, td, ep_trb_dma, false);
 
+	if (ep->skip) {
+		int skipped = 0;
 		/*
 		 * If ep->skip is set, it means there are missed tds on the
 		 * endpoint ring need to take care of.
 		 * Process them as short transfer until reach the td pointed by
 		 * the event.
 		 */
-		if (ep->skip) {
-			if (ep_seg) {
-				xhci_dbg(xhci,
-					 "Found td. Clear skip flag for slot %u ep %u.\n",
-					 slot_id, ep_index);
-				ep->skip = false;
-			} else {
-				if (usb_endpoint_xfer_isoc(&td->urb->ep->desc))
-					skip_isoc_td(xhci, td, ep, status);
-				else
-					break;
-			}
-		}
 
-	} while (ep->skip);
+		while (td && !ep_seg &&
+				usb_endpoint_xfer_isoc(&td->urb->ep->desc)) {
+			skip_isoc_td(xhci, td, ep, status);
+			td = xhci_ring_pending_td(ep_ring);
+			if (td)
+				ep_seg = trb_in_td(xhci, td, ep_trb_dma, false);
+			skipped++;
+		}
+		ep->skip = false;
+		xhci_dbg(xhci, "Skipped %d isoc TDs, TD %sfound, clear skip flag for slot %u ep %u\n",
+				skipped, td ? "":"not ", slot_id, ep_index);
+	}
 
 	if (!ep_seg) {