From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/3] usb: xhci: Simplify update_ring_for_set_deq_completion()
Date: Thu, 20 Feb 2025 23:44:58 +0100 [thread overview]
Message-ID: <20250220234458.4bf8dcba@foxbook> (raw)
In-Reply-To: <20250220234355.2386cb6d@foxbook>
This function checks if the queued Set Deq pointer really belongs to the
ring it was queued on and updates the ring's dequeue state. It also used
to count free TRBs, but that part has been removed.
The code is needlessly complex and inefficent, walking TRBs one by one.
And it could "jump off the segment into la-la-land" if a segment doesn't
end with a link TRB or if the link points to another link.
Make if faster, simpler and more robust. Upgrade xhci_dbg() to xhci_err()
because this situation is a bug and shouldn't happen.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
drivers/usb/host/xhci-ring.c | 43 ++++++++++++++----------------------
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 965bffce301e..c983d22842dc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -92,6 +92,11 @@ static bool trb_is_link(union xhci_trb *trb)
return TRB_TYPE_LINK_LE32(trb->link.control);
}
+static bool trb_on_seg(struct xhci_segment *seg, union xhci_trb *trb)
+{
+ return seg->trbs <= trb && trb < seg->trbs + TRBS_PER_SEGMENT;
+}
+
static bool last_trb_on_seg(struct xhci_segment *seg, union xhci_trb *trb)
{
return trb == &seg->trbs[TRBS_PER_SEGMENT - 1];
@@ -1332,41 +1337,25 @@ void xhci_hc_died(struct xhci_hcd *xhci)
usb_hc_died(xhci_to_hcd(xhci));
}
+/* Check if queued pointer is on the ring and update dequeue state */
static void update_ring_for_set_deq_completion(struct xhci_hcd *xhci,
struct xhci_virt_device *dev,
struct xhci_ring *ep_ring,
unsigned int ep_index)
{
- union xhci_trb *dequeue_temp;
+ union xhci_trb *deq = dev->eps[ep_index].queued_deq_ptr;
+ struct xhci_segment *seg;
- dequeue_temp = ep_ring->dequeue;
-
- /* If we get two back-to-back stalls, and the first stalled transfer
- * ends just before a link TRB, the dequeue pointer will be left on
- * the link TRB by the code in the while loop. So we have to update
- * the dequeue pointer one segment further, or we'll jump off
- * the segment into la-la-land.
- */
- if (trb_is_link(ep_ring->dequeue)) {
- ep_ring->deq_seg = ep_ring->deq_seg->next;
- ep_ring->dequeue = ep_ring->deq_seg->trbs;
- }
-
- while (ep_ring->dequeue != dev->eps[ep_index].queued_deq_ptr) {
- /* We have more usable TRBs */
- ep_ring->dequeue++;
- if (trb_is_link(ep_ring->dequeue)) {
- if (ep_ring->dequeue ==
- dev->eps[ep_index].queued_deq_ptr)
- break;
- ep_ring->deq_seg = ep_ring->deq_seg->next;
- ep_ring->dequeue = ep_ring->deq_seg->trbs;
- }
- if (ep_ring->dequeue == dequeue_temp) {
- xhci_dbg(xhci, "Unable to find new dequeue pointer\n");
- break;
+ /* Search starting from the last known position */
+ xhci_for_each_ring_seg(ep_ring->deq_seg, seg) {
+ if (seg == dev->eps[ep_index].queued_deq_seg && trb_on_seg(seg, deq)) {
+ ep_ring->deq_seg = seg;
+ ep_ring->dequeue = deq;
+ return;
}
}
+
+ xhci_err(xhci, "Set Deq pointer not on ring\n");
}
/*
--
2.48.1
next prev parent reply other threads:[~2025-02-20 22:45 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-20 22:43 [PATCH 0/3] xhci: ring queuing cleanups Michal Pecio
2025-02-20 22:44 ` Michal Pecio [this message]
2025-02-21 13:23 ` [PATCH 1/3] usb: xhci: Simplify update_ring_for_set_deq_completion() Mathias Nyman
2025-03-05 8:24 ` Michał Pecio
2025-02-20 22:46 ` [PATCH 2/3] usb: xhci: Simplify moving HW Dequeue Pointer past cancelled TDs Michal Pecio
2025-02-20 22:47 ` [PATCH 3/3] usb: xhci: Unify duplicate inc_enq() code Michal Pecio
2025-02-21 14:54 ` Mathias Nyman
2025-02-23 23:45 ` Michał Pecio
2025-02-24 11:49 ` Mathias Nyman
2025-02-24 21:01 ` Michał Pecio
2025-02-25 9:33 ` Mathias Nyman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250220234458.4bf8dcba@foxbook \
--to=michal.pecio@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox