usb: xhci: move debug capabilities from trb_in_td() to handle_tx_event()
authorNiklas Neronin <niklas.neronin@linux.intel.com>
Thu, 6 Mar 2025 14:49:50 +0000 (16:49 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 6 Mar 2025 15:46:16 +0000 (16:46 +0100)
Function trb_in_td() currently includes debug capabilities that are
triggered when its debug argument is set to true. The only consumer of
these debug capabilities is handle_tx_event(), which calls trb_in_td()
twice, once for its primary functionality and a second time solely for
debugging purposes if the first call returns 'NULL'.

This approach is inefficient and can lead to confusion, as trb_in_td()
executes the same code with identical arguments twice, differing only in
the debug output during the second execution.

To enhance clarity and efficiency, move the debug capabilities out of
trb_in_td() and integrates them directly into handle_tx_event().
This change reduces the argument count of trb_in_td() and ensures that
debug steps are executed only when necessary, streamlining the function's
operation.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20250306144954.3507700-12-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/host/xhci-ring.c

index 8c7258afb6bfe8bfc37ea83956b6c78fce1119ec..c2e15a27338ba5f7a9fb67bd3a4dbe9b88681509 100644 (file)
@@ -281,8 +281,7 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
  * If the suspect DMA address is a TRB in this TD, this function returns that
  * TRB's segment. Otherwise it returns 0.
  */
-static struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_td *td,
-                                     dma_addr_t suspect_dma, bool debug)
+static struct xhci_segment *trb_in_td(struct xhci_td *td, dma_addr_t suspect_dma)
 {
        dma_addr_t start_dma;
        dma_addr_t end_seg_dma;
@@ -301,15 +300,6 @@ static struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_td *td,
                /* If the end TRB isn't in this segment, this is set to 0 */
                end_trb_dma = xhci_trb_virt_to_dma(cur_seg, td->end_trb);
 
-               if (debug)
-                       xhci_warn(xhci,
-                               "Looking for event-dma %016llx trb-start %016llx trb-end %016llx seg-start %016llx seg-end %016llx\n",
-                               (unsigned long long)suspect_dma,
-                               (unsigned long long)start_dma,
-                               (unsigned long long)end_trb_dma,
-                               (unsigned long long)cur_seg->dma,
-                               (unsigned long long)end_seg_dma);
-
                if (end_trb_dma > 0) {
                        /* The end TRB is in this segment, so suspect should be here */
                        if (start_dma <= end_trb_dma) {
@@ -1075,7 +1065,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
                                         td->urb->stream_id);
                hw_deq &= ~0xf;
 
-               if (td->cancel_status == TD_HALTED || trb_in_td(xhci, td, hw_deq, false)) {
+               if (td->cancel_status == TD_HALTED || trb_in_td(td, hw_deq)) {
                        switch (td->cancel_status) {
                        case TD_CLEARED: /* TD is already no-op */
                        case TD_CLEARING_CACHE: /* set TR deq command already queued */
@@ -1165,7 +1155,7 @@ static struct xhci_td *find_halted_td(struct xhci_virt_ep *ep)
                hw_deq = xhci_get_hw_deq(ep->xhci, ep->vdev, ep->ep_index, 0);
                hw_deq &= ~0xf;
                td = list_first_entry(&ep->ring->td_list, struct xhci_td, td_list);
-               if (trb_in_td(ep->xhci, td, hw_deq, false))
+               if (trb_in_td(td, hw_deq))
                        return td;
        }
        return NULL;
@@ -2800,7 +2790,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
         */
        td = list_first_entry_or_null(&ep_ring->td_list, struct xhci_td, td_list);
 
-       if (td && td->error_mid_td && !trb_in_td(xhci, td, ep_trb_dma, false)) {
+       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);
        }
@@ -2833,7 +2823,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
                                      td_list);
 
                /* Is this a TRB in the currently executing TD? */
-               ep_seg = trb_in_td(xhci, td, ep_trb_dma, false);
+               ep_seg = trb_in_td(td, ep_trb_dma);
 
                if (!ep_seg) {
 
@@ -2893,12 +2883,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
                        }
 
                        /* HC is busted, give up! */
-                       xhci_err(xhci,
-                                "ERROR Transfer event TRB DMA ptr not part of current TD ep_index %d comp_code %u\n",
-                                ep_index, trb_comp_code);
-                       trb_in_td(xhci, td, ep_trb_dma, true);
-
-                       return -ESHUTDOWN;
+                       goto debug_finding_td;
                }
 
                if (ep->skip) {
@@ -2955,6 +2940,17 @@ check_endpoint_halted:
 
        return 0;
 
+debug_finding_td:
+       xhci_err(xhci, "Event dma %pad for ep %d status %d not part of TD at %016llx - %016llx\n",
+                &ep_trb_dma, ep_index, trb_comp_code,
+                (unsigned long long)xhci_trb_virt_to_dma(td->start_seg, td->start_trb),
+                (unsigned long long)xhci_trb_virt_to_dma(td->end_seg, td->end_trb));
+
+       xhci_for_each_ring_seg(ep_ring->first_seg, ep_seg)
+               xhci_warn(xhci, "Ring seg %u dma %pad\n", ep_seg->num, &ep_seg->dma);
+
+       return -ESHUTDOWN;
+
 err_out:
        xhci_err(xhci, "@%016llx %08x %08x %08x %08x\n",
                 (unsigned long long) xhci_trb_virt_to_dma(