From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: <gregkh@linuxfoundation.org>
Cc: <linux-usb@vger.kernel.org>,
Michal Pecio <michal.pecio@gmail.com>,
Mathias Nyman <mathias.nyman@linux.intel.com>
Subject: [PATCH 14/15] usb: xhci: Unify duplicate inc_enq() code
Date: Thu, 6 Mar 2025 16:49:53 +0200 [thread overview]
Message-ID: <20250306144954.3507700-15-mathias.nyman@linux.intel.com> (raw)
In-Reply-To: <20250306144954.3507700-1-mathias.nyman@linux.intel.com>
From: Michal Pecio <michal.pecio@gmail.com>
Extract a block of code copied from inc_enq() into a separate function
and call it from inc_enq() and the other function which used this code.
Remove the pointless 'next' variable which only aliases ring->enqueue.
Note: I don't know if any 0.95 xHC ever reached series production, but
"AMD 0.96 host" appears to be the "Llano" family APU. Example dmesg at
https://linux-hardware.org/?probe=79d5cfd4fd&log=dmesg
pci 0000:00:10.0: [1022:7812] type 00 class 0x0c0330
hcc params 0x014042c3 hci version 0x96 quirks 0x0000000000000608
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 130 +++++++++++++++--------------------
1 file changed, 55 insertions(+), 75 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7643ab9ec3b4..2df94ed3152c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -204,79 +204,84 @@ void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
}
/*
- * See Cycle bit rules. SW is the consumer for the event ring only.
- *
- * If we've just enqueued a TRB that is in the middle of a TD (meaning the
- * chain bit is set), then set the chain bit in all the following link TRBs.
- * If we've enqueued the last TRB in a TD, make sure the following link TRBs
- * have their chain bit cleared (so that each Link TRB is a separate TD).
- *
- * Section 6.4.4.1 of the 0.95 spec says link TRBs cannot have the chain bit
- * set, but other sections talk about dealing with the chain bit set. This was
- * fixed in the 0.96 specification errata, but we have to assume that all 0.95
- * xHCI hardware can't handle the chain bit being cleared on a link TRB.
- *
- * @more_trbs_coming: Will you enqueue more TRBs before calling
- * prepare_transfer()?
+ * If enqueue points at a link TRB, follow links until an ordinary TRB is reached.
+ * Toggle the cycle bit of passed link TRBs and optionally chain them.
*/
-static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
- bool more_trbs_coming)
+static void inc_enq_past_link(struct xhci_hcd *xhci, struct xhci_ring *ring, u32 chain)
{
- u32 chain;
- union xhci_trb *next;
unsigned int link_trb_count = 0;
- chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
-
- if (last_trb_on_seg(ring->enq_seg, ring->enqueue)) {
- xhci_err(xhci, "Tried to move enqueue past ring segment\n");
- return;
- }
-
- next = ++(ring->enqueue);
-
- /* Update the dequeue pointer further if that was a link TRB */
- while (trb_is_link(next)) {
+ while (trb_is_link(ring->enqueue)) {
/*
- * If the caller doesn't plan on enqueueing more TDs before
- * ringing the doorbell, then we don't want to give the link TRB
- * to the hardware just yet. We'll give the link TRB back in
- * prepare_ring() just before we enqueue the TD at the top of
- * the ring.
- */
- if (!chain && !more_trbs_coming)
- break;
-
- /* If we're not dealing with 0.95 hardware or isoc rings on
- * AMD 0.96 host, carry over the chain bit of the previous TRB
- * (which may mean the chain bit is cleared).
+ * Section 6.4.4.1 of the 0.95 spec says link TRBs cannot have the chain bit
+ * set, but other sections talk about dealing with the chain bit set. This was
+ * fixed in the 0.96 specification errata, but we have to assume that all 0.95
+ * xHCI hardware can't handle the chain bit being cleared on a link TRB.
+ *
+ * On 0.95 and some 0.96 HCs the chain bit is set once at segment initalization
+ * and never changed here. On all others, modify it as requested by the caller.
*/
if (!xhci_link_chain_quirk(xhci, ring->type)) {
- next->link.control &= cpu_to_le32(~TRB_CHAIN);
- next->link.control |= cpu_to_le32(chain);
+ ring->enqueue->link.control &= cpu_to_le32(~TRB_CHAIN);
+ ring->enqueue->link.control |= cpu_to_le32(chain);
}
+
/* Give this link TRB to the hardware */
wmb();
- next->link.control ^= cpu_to_le32(TRB_CYCLE);
+ ring->enqueue->link.control ^= cpu_to_le32(TRB_CYCLE);
/* Toggle the cycle bit after the last ring segment. */
- if (link_trb_toggles_cycle(next))
+ if (link_trb_toggles_cycle(ring->enqueue))
ring->cycle_state ^= 1;
ring->enq_seg = ring->enq_seg->next;
ring->enqueue = ring->enq_seg->trbs;
- next = ring->enqueue;
trace_xhci_inc_enq(ring);
if (link_trb_count++ > ring->num_segs) {
- xhci_warn(xhci, "%s: Ring link TRB loop\n", __func__);
+ xhci_warn(xhci, "Link TRB loop at enqueue\n");
break;
}
}
}
+/*
+ * See Cycle bit rules. SW is the consumer for the event ring only.
+ *
+ * If we've just enqueued a TRB that is in the middle of a TD (meaning the
+ * chain bit is set), then set the chain bit in all the following link TRBs.
+ * If we've enqueued the last TRB in a TD, make sure the following link TRBs
+ * have their chain bit cleared (so that each Link TRB is a separate TD).
+ *
+ * @more_trbs_coming: Will you enqueue more TRBs before calling
+ * prepare_transfer()?
+ */
+static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
+ bool more_trbs_coming)
+{
+ u32 chain;
+
+ chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
+
+ if (last_trb_on_seg(ring->enq_seg, ring->enqueue)) {
+ xhci_err(xhci, "Tried to move enqueue past ring segment\n");
+ return;
+ }
+
+ ring->enqueue++;
+
+ /*
+ * If we are in the middle of a TD or the caller plans to enqueue more
+ * TDs as one transfer (eg. control), traverse any link TRBs right now.
+ * Otherwise, enqueue can stay on a link until the next prepare_ring().
+ * This avoids enqueue entering deq_seg and simplifies ring expansion.
+ */
+ if (trb_is_link(ring->enqueue) && (chain || more_trbs_coming))
+ inc_enq_past_link(xhci, ring, chain);
+}
+
/*
* If the suspect DMA address is a TRB in this TD, this function returns that
* TRB's segment. Otherwise it returns 0.
@@ -3213,7 +3218,6 @@ static void queue_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
u32 ep_state, unsigned int num_trbs, gfp_t mem_flags)
{
- unsigned int link_trb_count = 0;
unsigned int new_segs = 0;
/* Make sure the endpoint has been added to xHC schedule */
@@ -3261,33 +3265,9 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
}
}
- while (trb_is_link(ep_ring->enqueue)) {
- /* If we're not dealing with 0.95 hardware or isoc rings
- * on AMD 0.96 host, clear the chain bit.
- */
- if (!xhci_link_chain_quirk(xhci, ep_ring->type))
- ep_ring->enqueue->link.control &=
- cpu_to_le32(~TRB_CHAIN);
- else
- ep_ring->enqueue->link.control |=
- cpu_to_le32(TRB_CHAIN);
-
- wmb();
- ep_ring->enqueue->link.control ^= cpu_to_le32(TRB_CYCLE);
-
- /* Toggle the cycle bit after the last ring segment. */
- if (link_trb_toggles_cycle(ep_ring->enqueue))
- ep_ring->cycle_state ^= 1;
-
- ep_ring->enq_seg = ep_ring->enq_seg->next;
- ep_ring->enqueue = ep_ring->enq_seg->trbs;
-
- /* prevent infinite loop if all first trbs are link trbs */
- if (link_trb_count++ > ep_ring->num_segs) {
- xhci_warn(xhci, "Ring is an endless link TRB loop\n");
- return -EINVAL;
- }
- }
+ /* Ensure that new TRBs won't overwrite a link */
+ if (trb_is_link(ep_ring->enqueue))
+ inc_enq_past_link(xhci, ep_ring, 0);
if (last_trb_on_seg(ep_ring->enq_seg, ep_ring->enqueue)) {
xhci_warn(xhci, "Missing link TRB at end of ring segment\n");
--
2.43.0
next prev parent reply other threads:[~2025-03-06 14:49 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-06 14:49 [PATCH 00/15] xhci features for usb-next Mathias Nyman
2025-03-06 14:49 ` [PATCH 01/15] xhci: show correct U1 and U2 timeout values in debug messages Mathias Nyman
2025-03-06 14:49 ` [PATCH 02/15] usb: xhci: remove redundant update_ring_for_set_deq_completion() function Mathias Nyman
2025-03-06 14:49 ` [PATCH 03/15] usb: xhci: Don't skip on Stopped - Length Invalid Mathias Nyman
2025-03-06 14:52 ` Greg KH
2025-03-06 15:29 ` Mathias Nyman
2025-03-06 15:42 ` Greg KH
2025-03-06 14:49 ` [PATCH 04/15] usb: xhci: Complete 'error mid TD' transfers when handling Missed Service Mathias Nyman
2025-03-06 14:49 ` [PATCH 05/15] usb: xhci: Fix isochronous Ring Underrun/Overrun event handling Mathias Nyman
2025-03-06 14:49 ` [PATCH 06/15] usb: xhci: Expedite skipping missed isoch TDs on modern HCs Mathias Nyman
2025-03-06 14:49 ` [PATCH 07/15] usb: xhci: Skip only one TD on Ring Underrun/Overrun Mathias Nyman
2025-03-06 14:49 ` [PATCH 08/15] usb: xhci: correct debug message page size calculation Mathias Nyman
2025-03-06 14:49 ` [PATCH 09/15] usb: xhci: set page size to the xHCI-supported size Mathias Nyman
2025-03-06 14:49 ` [PATCH 10/15] usb: xhci: refactor trb_in_td() to be static Mathias Nyman
2025-03-06 14:49 ` [PATCH 11/15] usb: xhci: move debug capabilities from trb_in_td() to handle_tx_event() Mathias Nyman
2025-03-06 14:49 ` [PATCH 12/15] xhci: Prevent early endpoint restart when handling STALL errors Mathias Nyman
2025-03-07 6:54 ` Michał Pecio
2025-03-07 14:23 ` Mathias Nyman
2025-03-07 15:44 ` Michał Pecio
2025-03-07 16:18 ` Mathias Nyman
2025-03-06 14:49 ` [PATCH 13/15] usb: xhci: Apply the link chain quirk on NEC isoc endpoints Mathias Nyman
2025-03-06 14:49 ` Mathias Nyman [this message]
2025-03-06 14:49 ` [PATCH 15/15] xhci: Handle spurious events on Etron host isoc enpoints Mathias Nyman
2025-03-07 8:27 ` Michał Pecio
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=20250306144954.3507700-15-mathias.nyman@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-usb@vger.kernel.org \
--cc=michal.pecio@gmail.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