* [PATCH 01/33] xhci: Add Isochronous TRB fields to TRB tracer
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 02/33] usb: xhci: Remove unused parameters of next_trb() Mathias Nyman
` (31 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
In addition to Normal TRB fields the Isoch TRBs have a SIA flag,
Frame ID, TLBPC and TBC fields.
Add these fields to tracing output
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.h | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 55a81073cf1e..7d81645c2c5d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1023,9 +1023,6 @@ enum xhci_setup_dev {
/* Interrupter Target - which MSI-X vector to target the completion event at */
#define TRB_INTR_TARGET(p) (((p) & 0x3ff) << 22)
#define GET_INTR_TARGET(p) (((p) >> 22) & 0x3ff)
-/* Total burst count field, Rsvdz on xhci 1.1 with Extended TBC enabled (ETE) */
-#define TRB_TBC(p) (((p) & 0x3) << 7)
-#define TRB_TLBPC(p) (((p) & 0xf) << 16)
/* Cycle bit - indicates TRB ownership by HC or HCD */
#define TRB_CYCLE (1<<0)
@@ -1059,6 +1056,12 @@ enum xhci_setup_dev {
/* Isochronous TRB specific fields */
#define TRB_SIA (1<<31)
#define TRB_FRAME_ID(p) (((p) & 0x7ff) << 20)
+#define GET_FRAME_ID(p) (((p) >> 20) & 0x7ff)
+/* Total burst count field, Rsvdz on xhci 1.1 with Extended TBC enabled (ETE) */
+#define TRB_TBC(p) (((p) & 0x3) << 7)
+#define GET_TBC(p) (((p) >> 7) & 0x3)
+#define TRB_TLBPC(p) (((p) & 0xf) << 16)
+#define GET_TLBPC(p) (((p) >> 16) & 0xf)
/* TRB cache size for xHC with TRB cache */
#define TRB_CACHE_SIZE_HS 8
@@ -2072,7 +2075,6 @@ static inline const char *xhci_decode_trb(char *str, size_t size,
field3 & TRB_CYCLE ? 'C' : 'c');
break;
case TRB_NORMAL:
- case TRB_ISOC:
case TRB_EVENT_DATA:
case TRB_TR_NOOP:
snprintf(str, size,
@@ -2089,7 +2091,25 @@ static inline const char *xhci_decode_trb(char *str, size_t size,
field3 & TRB_ENT ? 'E' : 'e',
field3 & TRB_CYCLE ? 'C' : 'c');
break;
-
+ case TRB_ISOC:
+ snprintf(str, size,
+ "Buffer %08x%08x length %d TD size/TBC %d intr %d type '%s' TBC %u TLBPC %u frame_id %u flags %c:%c:%c:%c:%c:%c:%c:%c:%c",
+ field1, field0, TRB_LEN(field2), GET_TD_SIZE(field2),
+ GET_INTR_TARGET(field2),
+ xhci_trb_type_string(type),
+ GET_TBC(field3),
+ GET_TLBPC(field3),
+ GET_FRAME_ID(field3),
+ field3 & TRB_SIA ? 'S' : 's',
+ field3 & TRB_BEI ? 'B' : 'b',
+ field3 & TRB_IDT ? 'I' : 'i',
+ field3 & TRB_IOC ? 'I' : 'i',
+ field3 & TRB_CHAIN ? 'C' : 'c',
+ field3 & TRB_NO_SNOOP ? 'S' : 's',
+ field3 & TRB_ISP ? 'I' : 'i',
+ field3 & TRB_ENT ? 'E' : 'e',
+ field3 & TRB_CYCLE ? 'C' : 'c');
+ break;
case TRB_CMD_NOOP:
case TRB_ENABLE_SLOT:
snprintf(str, size,
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 02/33] usb: xhci: Remove unused parameters of next_trb()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
2024-11-06 10:14 ` [PATCH 01/33] xhci: Add Isochronous TRB fields to TRB tracer Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 03/33] usb: xhci: Fix sum_trb_lengths() Mathias Nyman
` (30 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, Mathias Nyman
From: Michal Pecio <michal.pecio@gmail.com>
The function has two parameters which it doesn't use and hasn't ever
used. One caller even puts NULL there, knowing it will work anyway.
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 | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9f1e150a1c76..36ccf0dac7fa 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -145,10 +145,8 @@ static void trb_to_noop(union xhci_trb *trb, u32 noop_type)
* TRB is in a new segment. This does not skip over link TRBs, and it does not
* effect the ring dequeue or enqueue pointers.
*/
-static void next_trb(struct xhci_hcd *xhci,
- struct xhci_ring *ring,
- struct xhci_segment **seg,
- union xhci_trb **trb)
+static void next_trb(struct xhci_segment **seg,
+ union xhci_trb **trb)
{
if (trb_is_link(*trb) || last_trb_on_seg(*seg, *trb)) {
*seg = (*seg)->next;
@@ -446,9 +444,9 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
* avoiding corrupting the command ring pointer in case the command ring
* is stopped by the time the upper dword is written.
*/
- next_trb(xhci, NULL, &new_seg, &new_deq);
+ next_trb(&new_seg, &new_deq);
if (trb_is_link(new_deq))
- next_trb(xhci, NULL, &new_seg, &new_deq);
+ next_trb(&new_seg, &new_deq);
crcr = xhci_trb_virt_to_dma(new_seg, new_deq);
xhci_write_64(xhci, crcr | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
@@ -678,7 +676,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
link_trb_toggles_cycle(new_deq))
new_cycle ^= 0x1;
- next_trb(xhci, ep_ring, &new_seg, &new_deq);
+ next_trb(&new_seg, &new_deq);
/* Search wrapped around, bail out */
if (new_deq == ep->ring->dequeue) {
@@ -756,7 +754,7 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
if (trb == td->last_trb)
break;
- next_trb(xhci, ep_ring, &seg, &trb);
+ next_trb(&seg, &trb);
}
}
@@ -2273,7 +2271,7 @@ static int sum_trb_lengths(struct xhci_hcd *xhci, struct xhci_ring *ring,
union xhci_trb *trb = ring->dequeue;
struct xhci_segment *seg = ring->deq_seg;
- for (sum = 0; trb != stop_trb; next_trb(xhci, ring, &seg, &trb)) {
+ for (sum = 0; trb != stop_trb; next_trb(&seg, &trb)) {
if (!trb_is_noop(trb) && !trb_is_link(trb))
sum += TRB_LEN(le32_to_cpu(trb->generic.field[2]));
}
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 03/33] usb: xhci: Fix sum_trb_lengths()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
2024-11-06 10:14 ` [PATCH 01/33] xhci: Add Isochronous TRB fields to TRB tracer Mathias Nyman
2024-11-06 10:14 ` [PATCH 02/33] usb: xhci: Remove unused parameters of next_trb() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 04/33] xhci: Cleanup Candence controller PCI device and vendor ID usage Mathias Nyman
` (29 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, Mathias Nyman
From: Michal Pecio <michal.pecio@gmail.com>
This function is supposed to sum the lengths of all transfer TRBs in
a TD up to a point, but it starts summing at the current dequeue since
it only ever gets called on the first pending TD.
This won't work when there are cancelled TDs at the beginning of the
ring. The function tries to exclude No-Ops from the count, but not all
cancelled TDs are No-Op'ed - not those the HW stopped on.
The absolutely obvious fix is to start counting at the TD's first TRB.
And remove the now-useless 'ring' parameter, and 'xhci' too.
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 | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 36ccf0dac7fa..f1176c270c43 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2263,13 +2263,12 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
return xhci_td_cleanup(xhci, td, ep_ring, td->status);
}
-/* sum trb lengths from ring dequeue up to stop_trb, _excluding_ stop_trb */
-static int sum_trb_lengths(struct xhci_hcd *xhci, struct xhci_ring *ring,
- union xhci_trb *stop_trb)
+/* sum trb lengths from the first trb up to stop_trb, _excluding_ stop_trb */
+static u32 sum_trb_lengths(struct xhci_td *td, union xhci_trb *stop_trb)
{
u32 sum;
- union xhci_trb *trb = ring->dequeue;
- struct xhci_segment *seg = ring->deq_seg;
+ union xhci_trb *trb = td->first_trb;
+ struct xhci_segment *seg = td->start_seg;
for (sum = 0; trb != stop_trb; next_trb(&seg, &trb)) {
if (!trb_is_noop(trb) && !trb_is_link(trb))
@@ -2460,7 +2459,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
goto finish_td;
if (sum_trbs_for_length)
- frame->actual_length = sum_trb_lengths(xhci, ep->ring, ep_trb) +
+ frame->actual_length = sum_trb_lengths(td, ep_trb) +
ep_trb_len - remaining;
else
frame->actual_length = requested;
@@ -2540,7 +2539,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
goto finish_td;
case COMP_STOPPED_LENGTH_INVALID:
/* stopped on ep trb with invalid length, exclude it */
- td->urb->actual_length = sum_trb_lengths(xhci, ep_ring, ep_trb);
+ td->urb->actual_length = sum_trb_lengths(td, ep_trb);
goto finish_td;
case COMP_USB_TRANSACTION_ERROR:
if (xhci->quirks & XHCI_NO_SOFT_RETRY ||
@@ -2561,7 +2560,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->urb->actual_length = requested - remaining;
else
td->urb->actual_length =
- sum_trb_lengths(xhci, ep_ring, ep_trb) +
+ sum_trb_lengths(td, ep_trb) +
ep_trb_len - remaining;
finish_td:
if (remaining > requested) {
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 04/33] xhci: Cleanup Candence controller PCI device and vendor ID usage
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (2 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 03/33] usb: xhci: Fix sum_trb_lengths() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 05/33] xhci: show DMA address of TRB when tracing TRBs Mathias Nyman
` (28 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman, Andy Shevchenko
Use predefined PCI vendor ID constant for Cadence controller in pci_ids.h
Rename the Candence device ID to match the pattern other PCI vendor and
device IDs use
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Closes: https://lore.kernel.org/linux-usb/ZuMOfHp9j_6_3-WC@surfacebook.localdomain
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 8c4f3e7507ba..7803ff1f1c9f 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -82,8 +82,7 @@
#define PCI_DEVICE_ID_ASMEDIA_3042_XHCI 0x3042
#define PCI_DEVICE_ID_ASMEDIA_3242_XHCI 0x3242
-#define PCI_DEVICE_ID_CADENCE 0x17CD
-#define PCI_DEVICE_ID_CADENCE_SSP 0x0200
+#define PCI_DEVICE_ID_CDNS_SSP 0x0200
static const char hcd_name[] = "xhci_hcd";
@@ -481,9 +480,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
if (pdev->device == 0x9203)
xhci->quirks |= XHCI_ZHAOXIN_TRB_FETCH;
}
-
- if (pdev->vendor == PCI_DEVICE_ID_CADENCE &&
- pdev->device == PCI_DEVICE_ID_CADENCE_SSP)
+ if (pdev->vendor == PCI_VENDOR_ID_CDNS &&
+ pdev->device == PCI_DEVICE_ID_CDNS_SSP)
xhci->quirks |= XHCI_CDNS_SCTX_QUIRK;
/* xHC spec requires PCI devices to support D3hot and D3cold */
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 05/33] xhci: show DMA address of TRB when tracing TRBs
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (3 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 04/33] xhci: Cleanup Candence controller PCI device and vendor ID usage Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 06/33] xhci: Don't trace ring at every enqueue or dequeue increase Mathias Nyman
` (27 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
The DMA address of a queued TRB is essential when looking at traces as
both transfer events and command completion events refer to the command
or transfer based on its DMA address.
Previously the TRB address was figured out from xhci_inc_enq and
xhci_inc_deq trace entries seen after queuing or handlong a TRB.
Now that DMA address is shown in TRB tracing we can get rid of most of the
xhci_inc_enq and xhci_inc_deq traces thus decreasing trace size.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-dbgcap.c | 11 ++++++----
drivers/usb/host/xhci-ring.c | 11 ++++++----
drivers/usb/host/xhci-trace.h | 38 +++++++++++++++++++---------------
3 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 241d7aa1fbc2..408082372be1 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -248,8 +248,9 @@ xhci_dbc_queue_trb(struct xhci_ring *ring, u32 field1,
trb->generic.field[2] = cpu_to_le32(field3);
trb->generic.field[3] = cpu_to_le32(field4);
- trace_xhci_dbc_gadget_ep_queue(ring, &trb->generic);
-
+ trace_xhci_dbc_gadget_ep_queue(ring, &trb->generic,
+ xhci_trb_virt_to_dma(ring->enq_seg,
+ ring->enqueue));
ring->num_trbs_free--;
next = ++(ring->enqueue);
if (TRB_TYPE_LINK_LE32(next->link.control)) {
@@ -747,7 +748,7 @@ static void dbc_handle_xfer_event(struct xhci_dbc *dbc, union xhci_trb *event)
return;
}
- trace_xhci_dbc_handle_transfer(ring, &req->trb->generic);
+ trace_xhci_dbc_handle_transfer(ring, &req->trb->generic, req->trb_dma);
switch (comp_code) {
case COMP_SUCCESS:
@@ -898,7 +899,9 @@ static enum evtreturn xhci_dbc_do_handle_events(struct xhci_dbc *dbc)
*/
rmb();
- trace_xhci_dbc_handle_event(dbc->ring_evt, &evt->generic);
+ trace_xhci_dbc_handle_event(dbc->ring_evt, &evt->generic,
+ xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
+ dbc->ring_evt->dequeue));
switch (le32_to_cpu(evt->event_cmd.flags) & TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_PORT_STATUS):
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f1176c270c43..3c19f58fcefd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1714,7 +1714,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_trb = xhci->cmd_ring->dequeue;
- trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic);
+ trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic, cmd_dma);
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
@@ -2886,7 +2886,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
ep_ring->last_td_was_short = false;
ep_trb = &ep_seg->trbs[(ep_trb_dma - ep_seg->dma) / sizeof(*ep_trb)];
- trace_xhci_handle_transfer(ep_ring, (struct xhci_generic_trb *) ep_trb);
+ trace_xhci_handle_transfer(ep_ring, (struct xhci_generic_trb *) ep_trb, ep_trb_dma);
/*
* No-op TRB could trigger interrupts in a case where a URB was killed
@@ -2936,7 +2936,9 @@ static int xhci_handle_event_trb(struct xhci_hcd *xhci, struct xhci_interrupter
{
u32 trb_type;
- trace_xhci_handle_event(ir->event_ring, &event->generic);
+ trace_xhci_handle_event(ir->event_ring, &event->generic,
+ xhci_trb_virt_to_dma(ir->event_ring->deq_seg,
+ ir->event_ring->dequeue));
/*
* Barrier between reading the TRB_CYCLE (valid) flag before, and any
@@ -3159,7 +3161,8 @@ static void queue_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
wmb();
trb->field[3] = cpu_to_le32(field4);
- trace_xhci_queue_trb(ring, trb);
+ trace_xhci_queue_trb(ring, trb,
+ xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue));
inc_enq(xhci, ring, more_trbs_coming);
}
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 24405315ffe6..cc916e58a329 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -108,9 +108,10 @@ DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx,
);
DECLARE_EVENT_CLASS(xhci_log_trb,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb),
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma),
TP_STRUCT__entry(
+ __field(dma_addr_t, dma)
__field(u32, type)
__field(u32, field0)
__field(u32, field1)
@@ -118,51 +119,54 @@ DECLARE_EVENT_CLASS(xhci_log_trb,
__field(u32, field3)
),
TP_fast_assign(
+ __entry->dma = dma;
__entry->type = ring->type;
__entry->field0 = le32_to_cpu(trb->field[0]);
__entry->field1 = le32_to_cpu(trb->field[1]);
__entry->field2 = le32_to_cpu(trb->field[2]);
__entry->field3 = le32_to_cpu(trb->field[3]);
),
- TP_printk("%s: %s", xhci_ring_type_string(__entry->type),
+ TP_printk("%s: @%pad %s",
+ xhci_ring_type_string(__entry->type), &__entry->dma,
xhci_decode_trb(__get_buf(XHCI_MSG_MAX), XHCI_MSG_MAX, __entry->field0,
__entry->field1, __entry->field2, __entry->field3)
)
);
DEFINE_EVENT(xhci_log_trb, xhci_handle_event,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
);
DEFINE_EVENT(xhci_log_trb, xhci_handle_command,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
);
DEFINE_EVENT(xhci_log_trb, xhci_handle_transfer,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
);
DEFINE_EVENT(xhci_log_trb, xhci_queue_trb,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
+
);
DEFINE_EVENT(xhci_log_trb, xhci_dbc_handle_event,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
);
DEFINE_EVENT(xhci_log_trb, xhci_dbc_handle_transfer,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
);
DEFINE_EVENT(xhci_log_trb, xhci_dbc_gadget_ep_queue,
- TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
- TP_ARGS(ring, trb)
+ TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb, dma_addr_t dma),
+ TP_ARGS(ring, trb, dma)
);
DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 06/33] xhci: Don't trace ring at every enqueue or dequeue increase
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (4 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 05/33] xhci: show DMA address of TRB when tracing TRBs Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 07/33] xhci: add stream context tracing Mathias Nyman
` (26 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
Don't trace ring pointers every time driver increases enqueue pointer
after queuing a TRB, or dequeue pointer after handling an event.
These xhci_inc_deq: and xhci_inc_enq: trace entries fill up the trace and
provides little useful info now that the TRB DMA address is printed with
the TRB itself.
Only trace ring during xhci_inc_enq() and xhci_inc_deq() in case we move
to a new ring segment.
Also don't show both segment and TRB addess in trace.
Segment is just TRB with 0xfff masked off.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 16 +++++++++-------
drivers/usb/host/xhci-trace.h | 10 +++-------
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3c19f58fcefd..f7f7b0a818f8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -167,13 +167,16 @@ void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
if (ring->type == TYPE_EVENT) {
if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
ring->dequeue++;
- goto out;
+ return;
}
if (last_trb_on_ring(ring, ring->deq_seg, ring->dequeue))
ring->cycle_state ^= 1;
ring->deq_seg = ring->deq_seg->next;
ring->dequeue = ring->deq_seg->trbs;
- goto out;
+
+ trace_xhci_inc_deq(ring);
+
+ return;
}
/* All other rings have link trbs */
@@ -188,14 +191,13 @@ void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
ring->deq_seg = ring->deq_seg->next;
ring->dequeue = ring->deq_seg->trbs;
+ trace_xhci_inc_deq(ring);
+
if (link_trb_count++ > ring->num_segs) {
xhci_warn(xhci, "Ring is an endless link TRB loop\n");
break;
}
}
-out:
- trace_xhci_inc_deq(ring);
-
return;
}
@@ -264,13 +266,13 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
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__);
break;
}
}
-
- trace_xhci_inc_enq(ring);
}
/*
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index cc916e58a329..57b8cb5a8d19 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -458,8 +458,6 @@ DECLARE_EVENT_CLASS(xhci_log_ring,
__field(void *, ring)
__field(dma_addr_t, enq)
__field(dma_addr_t, deq)
- __field(dma_addr_t, enq_seg)
- __field(dma_addr_t, deq_seg)
__field(unsigned int, num_segs)
__field(unsigned int, stream_id)
__field(unsigned int, cycle_state)
@@ -470,17 +468,15 @@ DECLARE_EVENT_CLASS(xhci_log_ring,
__entry->type = ring->type;
__entry->num_segs = ring->num_segs;
__entry->stream_id = ring->stream_id;
- __entry->enq_seg = ring->enq_seg->dma;
- __entry->deq_seg = ring->deq_seg->dma;
__entry->cycle_state = ring->cycle_state;
__entry->bounce_buf_len = ring->bounce_buf_len;
__entry->enq = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
__entry->deq = xhci_trb_virt_to_dma(ring->deq_seg, ring->dequeue);
),
- TP_printk("%s %p: enq %pad(%pad) deq %pad(%pad) segs %d stream %d bounce %d cycle %d",
+ TP_printk("%s %p: enq %pad deq %pad segs %d stream %d bounce %d cycle %d",
xhci_ring_type_string(__entry->type), __entry->ring,
- &__entry->enq, &__entry->enq_seg,
- &__entry->deq, &__entry->deq_seg,
+ &__entry->enq,
+ &__entry->deq,
__entry->num_segs,
__entry->stream_id,
__entry->bounce_buf_len,
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 07/33] xhci: add stream context tracing
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (5 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 06/33] xhci: Don't trace ring at every enqueue or dequeue increase Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 08/33] xhci: trace stream context at Set TR Deq command completion Mathias Nyman
` (25 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
Show stream id, stream context type (SCT), ring dequeue pointer and
the DMA address of the stream context during stream allocation
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 2 ++
drivers/usb/host/xhci-trace.h | 26 ++++++++++++++++++++++++++
drivers/usb/host/xhci.h | 1 +
3 files changed, 29 insertions(+)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d2900197a49e..2952737ccf6c 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -658,6 +658,8 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
xhci_dbg(xhci, "Setting stream %d ring ptr to 0x%08llx\n", cur_stream, addr);
ret = xhci_update_stream_mapping(cur_ring, mem_flags);
+
+ trace_xhci_alloc_stream_info_ctx(stream_info, cur_stream);
if (ret) {
xhci_ring_free(xhci, cur_ring);
stream_info->stream_rings[cur_stream] = NULL;
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 57b8cb5a8d19..56af9f62916a 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -314,6 +314,32 @@ DEFINE_EVENT(xhci_log_urb, xhci_urb_dequeue,
TP_ARGS(urb)
);
+DECLARE_EVENT_CLASS(xhci_log_stream_ctx,
+ TP_PROTO(struct xhci_stream_info *info, unsigned int stream_id),
+ TP_ARGS(info, stream_id),
+ TP_STRUCT__entry(
+ __field(unsigned int, stream_id)
+ __field(u64, stream_ring)
+ __field(dma_addr_t, ctx_array_dma)
+
+ ),
+ TP_fast_assign(
+ __entry->stream_id = stream_id;
+ __entry->stream_ring = le64_to_cpu(info->stream_ctx_array[stream_id].stream_ring);
+ __entry->ctx_array_dma = info->ctx_array_dma + stream_id * 16;
+
+ ),
+ TP_printk("stream %u ctx @%pad: SCT %llu deq %llx", __entry->stream_id,
+ &__entry->ctx_array_dma, CTX_TO_SCT(__entry->stream_ring),
+ __entry->stream_ring
+ )
+);
+
+DEFINE_EVENT(xhci_log_stream_ctx, xhci_alloc_stream_info_ctx,
+ TP_PROTO(struct xhci_stream_info *info, unsigned int stream_id),
+ TP_ARGS(info, stream_id)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ep_ctx,
TP_PROTO(struct xhci_ep_ctx *ctx),
TP_ARGS(ctx),
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7d81645c2c5d..89337562440b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -554,6 +554,7 @@ struct xhci_stream_ctx {
/* Stream Context Types (section 6.4.1) - bits 3:1 of stream ctx deq ptr */
#define SCT_FOR_CTX(p) (((p) & 0x7) << 1)
+#define CTX_TO_SCT(p) (((p) >> 1) & 0x7)
/* Secondary stream array type, dequeue pointer is to a transfer ring */
#define SCT_SEC_TR 0
/* Primary stream array type, dequeue pointer is to a transfer ring */
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 08/33] xhci: trace stream context at Set TR Deq command completion
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (6 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 07/33] xhci: add stream context tracing Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 09/33] xhci: debugfs: Add virt endpoint state to xhci debugfs Mathias Nyman
` (24 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
A successful 'Set TR Deq' command writes a new dequeue pointer to the
stream context for stream rings, show the content of the stream ctx
at command completion.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 14 +++++++++-----
drivers/usb/host/xhci-trace.h | 5 +++++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f7f7b0a818f8..f62b243d0fc4 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1338,6 +1338,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
struct xhci_virt_ep *ep;
struct xhci_ep_ctx *ep_ctx;
struct xhci_slot_ctx *slot_ctx;
+ struct xhci_stream_ctx *stream_ctx;
struct xhci_td *td, *tmp_td;
bool deferred = false;
@@ -1360,6 +1361,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
trace_xhci_handle_cmd_set_deq(slot_ctx);
trace_xhci_handle_cmd_set_deq_ep(ep_ctx);
+ if (ep->ep_state & EP_HAS_STREAMS) {
+ stream_ctx = &ep->stream_info->stream_ctx_array[stream_id];
+ trace_xhci_handle_cmd_set_deq_stream(ep->stream_info, stream_id);
+ }
+
if (cmd_comp_code != COMP_SUCCESS) {
unsigned int ep_state;
unsigned int slot_state;
@@ -1396,9 +1402,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
u64 deq;
/* 4.6.10 deq ptr is written to the stream ctx for streams */
if (ep->ep_state & EP_HAS_STREAMS) {
- struct xhci_stream_ctx *ctx =
- &ep->stream_info->stream_ctx_array[stream_id];
- deq = le64_to_cpu(ctx->stream_ring) & SCTX_DEQ_MASK;
+ deq = le64_to_cpu(stream_ctx->stream_ring) & SCTX_DEQ_MASK;
/*
* Cadence xHCI controllers store some endpoint state
@@ -1410,8 +1414,8 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
* To fix this issue driver must clear Rsvd0 field.
*/
if (xhci->quirks & XHCI_CDNS_SCTX_QUIRK) {
- ctx->reserved[0] = 0;
- ctx->reserved[1] = 0;
+ stream_ctx->reserved[0] = 0;
+ stream_ctx->reserved[1] = 0;
}
} else {
deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK;
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 56af9f62916a..bfb5c5c17012 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -340,6 +340,11 @@ DEFINE_EVENT(xhci_log_stream_ctx, xhci_alloc_stream_info_ctx,
TP_ARGS(info, stream_id)
);
+DEFINE_EVENT(xhci_log_stream_ctx, xhci_handle_cmd_set_deq_stream,
+ TP_PROTO(struct xhci_stream_info *info, unsigned int stream_id),
+ TP_ARGS(info, stream_id)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ep_ctx,
TP_PROTO(struct xhci_ep_ctx *ctx),
TP_ARGS(ctx),
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 09/33] xhci: debugfs: Add virt endpoint state to xhci debugfs
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (7 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 08/33] xhci: trace stream context at Set TR Deq command completion Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 10/33] usb: xhci: introduce macro for ring segment list iteration Mathias Nyman
` (23 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, WangYuli, Xu Rao, Mathias Nyman
From: WangYuli <wangyuli@uniontech.com>
The ring data structure of each xHCI endpoint might stop sending
data due to the virt endpoint state.
Show the virt endpoint state within the endpoint context via debugfs
to facilitate debugging.
Co-developed-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-debugfs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index f8ba15e7c225..35247cd50c74 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -291,12 +291,13 @@ static int xhci_endpoint_context_show(struct seq_file *s, void *unused)
for (ep_index = 0; ep_index < 31; ep_index++) {
ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
dma = dev->out_ctx->dma + (ep_index + 1) * CTX_SIZE(xhci->hcc_params);
- seq_printf(s, "%pad: %s\n", &dma,
+ seq_printf(s, "%pad: %s, virt_state:%#x\n", &dma,
xhci_decode_ep_context(str,
le32_to_cpu(ep_ctx->ep_info),
le32_to_cpu(ep_ctx->ep_info2),
le64_to_cpu(ep_ctx->deq),
- le32_to_cpu(ep_ctx->tx_info)));
+ le32_to_cpu(ep_ctx->tx_info)),
+ dev->eps[ep_index].ep_state);
}
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 10/33] usb: xhci: introduce macro for ring segment list iteration
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (8 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 09/33] xhci: debugfs: Add virt endpoint state to xhci debugfs Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 11/33] usb: xhci: remove option to change a default ring's TRB cycle bit Mathias Nyman
` (22 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Andy Shevchenko, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Add macro to streamline and standardize the iteration over ring
segment list.
xhci_for_each_ring_seg(): Iterates over the entire ring segment list.
The xhci_free_segments_for_ring() function's while loop has not been
updated to use the new macro. This function has some underlying issues,
and as a result, it will be handled separately in a future patch.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-debugfs.c | 5 +----
drivers/usb/host/xhci-mem.c | 24 +++++++-----------------
drivers/usb/host/xhci.c | 20 ++++++++------------
drivers/usb/host/xhci.h | 3 +++
4 files changed, 19 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 35247cd50c74..4f0c1b96e208 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -214,14 +214,11 @@ static void xhci_ring_dump_segment(struct seq_file *s,
static int xhci_ring_trb_show(struct seq_file *s, void *unused)
{
- int i;
struct xhci_ring *ring = *(struct xhci_ring **)s->private;
struct xhci_segment *seg = ring->first_seg;
- for (i = 0; i < ring->num_segs; i++) {
+ xhci_for_each_ring_seg(ring->first_seg, seg)
xhci_ring_dump_segment(s, seg);
- seg = seg->next;
- }
return 0;
}
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2952737ccf6c..7aee76f846bc 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -224,7 +224,6 @@ static int xhci_update_stream_segment_mapping(
struct radix_tree_root *trb_address_map,
struct xhci_ring *ring,
struct xhci_segment *first_seg,
- struct xhci_segment *last_seg,
gfp_t mem_flags)
{
struct xhci_segment *seg;
@@ -234,28 +233,22 @@ static int xhci_update_stream_segment_mapping(
if (WARN_ON_ONCE(trb_address_map == NULL))
return 0;
- seg = first_seg;
- do {
+ xhci_for_each_ring_seg(first_seg, seg) {
ret = xhci_insert_segment_mapping(trb_address_map,
ring, seg, mem_flags);
if (ret)
goto remove_streams;
- if (seg == last_seg)
- return 0;
- seg = seg->next;
- } while (seg != first_seg);
+ }
return 0;
remove_streams:
failed_seg = seg;
- seg = first_seg;
- do {
+ xhci_for_each_ring_seg(first_seg, seg) {
xhci_remove_segment_mapping(trb_address_map, seg);
if (seg == failed_seg)
return ret;
- seg = seg->next;
- } while (seg != first_seg);
+ }
return ret;
}
@@ -267,17 +260,14 @@ static void xhci_remove_stream_mapping(struct xhci_ring *ring)
if (WARN_ON_ONCE(ring->trb_address_map == NULL))
return;
- seg = ring->first_seg;
- do {
+ xhci_for_each_ring_seg(ring->first_seg, seg)
xhci_remove_segment_mapping(ring->trb_address_map, seg);
- seg = seg->next;
- } while (seg != ring->first_seg);
}
static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags)
{
return xhci_update_stream_segment_mapping(ring->trb_address_map, ring,
- ring->first_seg, ring->last_seg, mem_flags);
+ ring->first_seg, mem_flags);
}
/* XXX: Do we need the hcd structure in all these functions? */
@@ -438,7 +428,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
if (ring->type == TYPE_STREAM) {
ret = xhci_update_stream_segment_mapping(ring->trb_address_map,
- ring, first, last, flags);
+ ring, first, flags);
if (ret)
goto free_segments;
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fdb1b71eeec2..44e4ae201048 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -40,15 +40,15 @@ MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
static bool td_on_ring(struct xhci_td *td, struct xhci_ring *ring)
{
- struct xhci_segment *seg = ring->first_seg;
+ struct xhci_segment *seg;
if (!td || !td->start_seg)
return false;
- do {
+
+ xhci_for_each_ring_seg(ring->first_seg, seg) {
if (seg == td->start_seg)
return true;
- seg = seg->next;
- } while (seg && seg != ring->first_seg);
+ }
return false;
}
@@ -785,14 +785,10 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
struct xhci_segment *seg;
ring = xhci->cmd_ring;
- seg = ring->deq_seg;
- do {
- memset(seg->trbs, 0,
- sizeof(union xhci_trb) * (TRBS_PER_SEGMENT - 1));
- seg->trbs[TRBS_PER_SEGMENT - 1].link.control &=
- cpu_to_le32(~TRB_CYCLE);
- seg = seg->next;
- } while (seg != ring->deq_seg);
+ xhci_for_each_ring_seg(ring->deq_seg, seg) {
+ memset(seg->trbs, 0, sizeof(union xhci_trb) * (TRBS_PER_SEGMENT - 1));
+ seg->trbs[TRBS_PER_SEGMENT - 1].link.control &= cpu_to_le32(~TRB_CYCLE);
+ }
xhci_initialize_ring_info(ring, 1);
/*
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 89337562440b..753d9343a4b1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1263,6 +1263,9 @@ static inline const char *xhci_trb_type_string(u8 type)
#define AVOID_BEI_INTERVAL_MIN 8
#define AVOID_BEI_INTERVAL_MAX 32
+#define xhci_for_each_ring_seg(head, seg) \
+ for (seg = head; seg != NULL; seg = (seg->next != head ? seg->next : NULL))
+
struct xhci_segment {
union xhci_trb *trbs;
/* private to HCD */
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 11/33] usb: xhci: remove option to change a default ring's TRB cycle bit
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (9 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 10/33] usb: xhci: introduce macro for ring segment list iteration Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 12/33] usb: xhci: adjust xhci_alloc_segments_for_ring() arguments Mathias Nyman
` (21 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The TRB cycle bit indicates TRB ownership by the Host Controller (HC) or
Host Controller Driver (HCD). New rings are initialized with 'cycle_state'
equal to one, and all its TRBs' cycle bits are set to zero. When handling
ring expansion, set the source ring cycle bits to the same value as the
destination ring.
Move the cycle bit setting from xhci_segment_alloc() to xhci_link_rings(),
and remove the 'cycle_state' argument from xhci_initialize_ring_info().
The xhci_segment_alloc() function uses kzalloc_node() to allocate segments,
ensuring that all TRB cycle bits are initialized to zero.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-dbgcap.c | 2 +-
drivers/usb/host/xhci-mem.c | 50 ++++++++++++++++------------------
drivers/usb/host/xhci.c | 2 +-
drivers/usb/host/xhci.h | 6 ++--
4 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 408082372be1..227e513867dd 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -472,7 +472,7 @@ xhci_dbc_ring_alloc(struct device *dev, enum xhci_ring_type type, gfp_t flags)
trb->link.control = cpu_to_le32(LINK_TOGGLE | TRB_TYPE(TRB_LINK));
}
INIT_LIST_HEAD(&ring->td_list);
- xhci_initialize_ring_info(ring, 1);
+ xhci_initialize_ring_info(ring);
return ring;
dma_fail:
kfree(seg);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 7aee76f846bc..164b22d0b475 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -27,14 +27,12 @@
* "All components of all Command and Transfer TRBs shall be initialized to '0'"
*/
static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
- unsigned int cycle_state,
unsigned int max_packet,
unsigned int num,
gfp_t flags)
{
struct xhci_segment *seg;
dma_addr_t dma;
- int i;
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
seg = kzalloc_node(sizeof(*seg), flags, dev_to_node(dev));
@@ -56,11 +54,6 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
return NULL;
}
}
- /* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */
- if (cycle_state == 0) {
- for (i = 0; i < TRBS_PER_SEGMENT; i++)
- seg->trbs[i].link.control = cpu_to_le32(TRB_CYCLE);
- }
seg->num = num;
seg->dma = dma;
seg->next = NULL;
@@ -138,6 +131,14 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring,
chain_links = xhci_link_chain_quirk(xhci, ring->type);
+ /* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */
+ if (ring->cycle_state == 0) {
+ xhci_for_each_ring_seg(ring->first_seg, seg) {
+ for (int i = 0; i < TRBS_PER_SEGMENT; i++)
+ seg->trbs[i].link.control |= cpu_to_le32(TRB_CYCLE);
+ }
+ }
+
next = ring->enq_seg->next;
xhci_link_segments(ring->enq_seg, first, ring->type, chain_links);
xhci_link_segments(last, next, ring->type, chain_links);
@@ -287,8 +288,7 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring)
kfree(ring);
}
-void xhci_initialize_ring_info(struct xhci_ring *ring,
- unsigned int cycle_state)
+void xhci_initialize_ring_info(struct xhci_ring *ring)
{
/* The ring is empty, so the enqueue pointer == dequeue pointer */
ring->enqueue = ring->first_seg->trbs;
@@ -302,7 +302,7 @@ void xhci_initialize_ring_info(struct xhci_ring *ring,
* New rings are initialized with cycle state equal to 1; if we are
* handling ring expansion, set the cycle state equal to the old ring.
*/
- ring->cycle_state = cycle_state;
+ ring->cycle_state = 1;
/*
* Each segment has a link TRB, and leave an extra TRB for SW
@@ -317,7 +317,6 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
struct xhci_segment **first,
struct xhci_segment **last,
unsigned int num_segs,
- unsigned int cycle_state,
enum xhci_ring_type type,
unsigned int max_packet,
gfp_t flags)
@@ -328,7 +327,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
chain_links = xhci_link_chain_quirk(xhci, type);
- prev = xhci_segment_alloc(xhci, cycle_state, max_packet, num, flags);
+ prev = xhci_segment_alloc(xhci, max_packet, num, flags);
if (!prev)
return -ENOMEM;
num++;
@@ -337,8 +336,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
while (num < num_segs) {
struct xhci_segment *next;
- next = xhci_segment_alloc(xhci, cycle_state, max_packet, num,
- flags);
+ next = xhci_segment_alloc(xhci, max_packet, num, flags);
if (!next)
goto free_segments;
@@ -363,9 +361,8 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
* Set the end flag and the cycle toggle bit on the last segment.
* See section 4.9.1 and figures 15 and 16.
*/
-struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
- unsigned int num_segs, unsigned int cycle_state,
- enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
+struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
+ enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
{
struct xhci_ring *ring;
int ret;
@@ -383,7 +380,7 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
return ring;
ret = xhci_alloc_segments_for_ring(xhci, &ring->first_seg, &ring->last_seg, num_segs,
- cycle_state, type, max_packet, flags);
+ type, max_packet, flags);
if (ret)
goto fail;
@@ -393,7 +390,7 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
ring->last_seg->trbs[TRBS_PER_SEGMENT - 1].link.control |=
cpu_to_le32(LINK_TOGGLE);
}
- xhci_initialize_ring_info(ring, cycle_state);
+ xhci_initialize_ring_info(ring);
trace_xhci_ring_alloc(ring);
return ring;
@@ -421,8 +418,8 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
struct xhci_segment *last;
int ret;
- ret = xhci_alloc_segments_for_ring(xhci, &first, &last, num_new_segs, ring->cycle_state,
- ring->type, ring->bounce_buf_len, flags);
+ ret = xhci_alloc_segments_for_ring(xhci, &first, &last, num_new_segs, ring->type,
+ ring->bounce_buf_len, flags);
if (ret)
return -ENOMEM;
@@ -632,8 +629,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
for (cur_stream = 1; cur_stream < num_streams; cur_stream++) {
stream_info->stream_rings[cur_stream] =
- xhci_ring_alloc(xhci, 2, 1, TYPE_STREAM, max_packet,
- mem_flags);
+ xhci_ring_alloc(xhci, 2, TYPE_STREAM, max_packet, mem_flags);
cur_ring = stream_info->stream_rings[cur_stream];
if (!cur_ring)
goto cleanup_rings;
@@ -976,7 +972,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
}
/* Allocate endpoint 0 ring */
- dev->eps[0].ring = xhci_ring_alloc(xhci, 2, 1, TYPE_CTRL, 0, flags);
+ dev->eps[0].ring = xhci_ring_alloc(xhci, 2, TYPE_CTRL, 0, flags);
if (!dev->eps[0].ring)
goto fail;
@@ -1453,7 +1449,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
/* Set up the endpoint ring */
virt_dev->eps[ep_index].new_ring =
- xhci_ring_alloc(xhci, 2, 1, ring_type, max_packet, mem_flags);
+ xhci_ring_alloc(xhci, 2, ring_type, max_packet, mem_flags);
if (!virt_dev->eps[ep_index].new_ring)
return -ENOMEM;
@@ -2263,7 +2259,7 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
if (!ir)
return NULL;
- ir->event_ring = xhci_ring_alloc(xhci, segs, 1, TYPE_EVENT, 0, flags);
+ ir->event_ring = xhci_ring_alloc(xhci, segs, TYPE_EVENT, 0, flags);
if (!ir->event_ring) {
xhci_warn(xhci, "Failed to allocate interrupter event ring\n");
kfree(ir);
@@ -2465,7 +2461,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
/* Set up the command ring to have one segments for now. */
- xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, 0, flags);
+ xhci->cmd_ring = xhci_ring_alloc(xhci, 1, TYPE_COMMAND, 0, flags);
if (!xhci->cmd_ring)
goto fail;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 44e4ae201048..aa8c877f47ac 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -790,7 +790,7 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
seg->trbs[TRBS_PER_SEGMENT - 1].link.control &= cpu_to_le32(~TRB_CYCLE);
}
- xhci_initialize_ring_info(ring, 1);
+ xhci_initialize_ring_info(ring);
/*
* Reset the hardware dequeue pointer.
* Yes, this will need to be re-written after resume, but we're paranoid
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 753d9343a4b1..d3b250c736b8 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1795,14 +1795,12 @@ void xhci_slot_copy(struct xhci_hcd *xhci,
int xhci_endpoint_init(struct xhci_hcd *xhci, struct xhci_virt_device *virt_dev,
struct usb_device *udev, struct usb_host_endpoint *ep,
gfp_t mem_flags);
-struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
- unsigned int num_segs, unsigned int cycle_state,
+struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
enum xhci_ring_type type, unsigned int max_packet, gfp_t flags);
void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring);
int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
unsigned int num_trbs, gfp_t flags);
-void xhci_initialize_ring_info(struct xhci_ring *ring,
- unsigned int cycle_state);
+void xhci_initialize_ring_info(struct xhci_ring *ring);
void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
struct xhci_virt_device *virt_dev,
unsigned int ep_index);
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 12/33] usb: xhci: adjust xhci_alloc_segments_for_ring() arguments
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (10 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 11/33] usb: xhci: remove option to change a default ring's TRB cycle bit Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 13/33] usb: xhci: rework xhci_free_segments_for_ring() Mathias Nyman
` (20 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The function xhci_alloc_segments_for_ring() currently takes 7 arguments,
5 of which are components of the 'xhci_ring' struct. Refactor the function
to accept a pointer to the 'xhci_ring' struct instead of passing these
components separately.
The change reduces the number of arguments, making the function signature
cleaner and easier to understand.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 48 ++++++++++++++++---------------------
1 file changed, 21 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 164b22d0b475..fa77a15dfde6 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -313,44 +313,38 @@ void xhci_initialize_ring_info(struct xhci_ring *ring)
EXPORT_SYMBOL_GPL(xhci_initialize_ring_info);
/* Allocate segments and link them for a ring */
-static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
- struct xhci_segment **first,
- struct xhci_segment **last,
- unsigned int num_segs,
- enum xhci_ring_type type,
- unsigned int max_packet,
- gfp_t flags)
+static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring *ring, gfp_t flags)
{
struct xhci_segment *prev;
unsigned int num = 0;
bool chain_links;
- chain_links = xhci_link_chain_quirk(xhci, type);
+ chain_links = xhci_link_chain_quirk(xhci, ring->type);
- prev = xhci_segment_alloc(xhci, max_packet, num, flags);
+ prev = xhci_segment_alloc(xhci, ring->bounce_buf_len, num, flags);
if (!prev)
return -ENOMEM;
num++;
- *first = prev;
- while (num < num_segs) {
+ ring->first_seg = prev;
+ while (num < ring->num_segs) {
struct xhci_segment *next;
- next = xhci_segment_alloc(xhci, max_packet, num, flags);
+ next = xhci_segment_alloc(xhci, ring->bounce_buf_len, num, flags);
if (!next)
goto free_segments;
- xhci_link_segments(prev, next, type, chain_links);
+ xhci_link_segments(prev, next, ring->type, chain_links);
prev = next;
num++;
}
- xhci_link_segments(prev, *first, type, chain_links);
- *last = prev;
+ xhci_link_segments(prev, ring->first_seg, ring->type, chain_links);
+ ring->last_seg = prev;
return 0;
free_segments:
- xhci_free_segments_for_ring(xhci, *first);
+ xhci_free_segments_for_ring(xhci, ring->first_seg);
return -ENOMEM;
}
@@ -379,8 +373,7 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
if (num_segs == 0)
return ring;
- ret = xhci_alloc_segments_for_ring(xhci, &ring->first_seg, &ring->last_seg, num_segs,
- type, max_packet, flags);
+ ret = xhci_alloc_segments_for_ring(xhci, ring, flags);
if (ret)
goto fail;
@@ -414,23 +407,24 @@ void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
unsigned int num_new_segs, gfp_t flags)
{
- struct xhci_segment *first;
- struct xhci_segment *last;
- int ret;
+ struct xhci_ring new_ring;
+ int ret;
- ret = xhci_alloc_segments_for_ring(xhci, &first, &last, num_new_segs, ring->type,
- ring->bounce_buf_len, flags);
+ new_ring.num_segs = num_new_segs;
+ new_ring.bounce_buf_len = ring->bounce_buf_len;
+ new_ring.type = ring->type;
+ ret = xhci_alloc_segments_for_ring(xhci, &new_ring, flags);
if (ret)
return -ENOMEM;
if (ring->type == TYPE_STREAM) {
- ret = xhci_update_stream_segment_mapping(ring->trb_address_map,
- ring, first, flags);
+ ret = xhci_update_stream_segment_mapping(ring->trb_address_map, ring,
+ new_ring.first_seg, flags);
if (ret)
goto free_segments;
}
- xhci_link_rings(xhci, ring, first, last, num_new_segs);
+ xhci_link_rings(xhci, ring, new_ring.first_seg, new_ring.last_seg, num_new_segs);
trace_xhci_ring_expansion(ring);
xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
"ring expansion succeed, now has %d segments",
@@ -439,7 +433,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
return 0;
free_segments:
- xhci_free_segments_for_ring(xhci, first);
+ xhci_free_segments_for_ring(xhci, new_ring.first_seg);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 13/33] usb: xhci: rework xhci_free_segments_for_ring()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (11 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 12/33] usb: xhci: adjust xhci_alloc_segments_for_ring() arguments Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 14/33] usb: xhci: refactor xhci_link_rings() to use source and destination rings Mathias Nyman
` (19 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The segment list is only relevant within the context of the ring.
Thus, it is more logical to pass the ring itself when freeing all segments
from a ring, rather than passing the ring list.
Rename the function to "xhci_ring_segments_free" to align with the naming
convention of xhci_segment_free().
To make the freeing process more intuitive, free segments sequentially
from start to end (i.e., 0, 1, 2, 3, ...) instead of freeing the first
segment last (i.e., 1, 2, 3, ... 0).
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index fa77a15dfde6..e46be4c49b2f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -71,18 +71,18 @@ static void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
kfree(seg);
}
-static void xhci_free_segments_for_ring(struct xhci_hcd *xhci,
- struct xhci_segment *first)
+static void xhci_ring_segments_free(struct xhci_hcd *xhci, struct xhci_ring *ring)
{
- struct xhci_segment *seg;
+ struct xhci_segment *seg, *next;
+
+ ring->last_seg->next = NULL;
+ seg = ring->first_seg;
- seg = first->next;
- while (seg && seg != first) {
- struct xhci_segment *next = seg->next;
+ while (seg) {
+ next = seg->next;
xhci_segment_free(xhci, seg);
seg = next;
}
- xhci_segment_free(xhci, first);
}
/*
@@ -282,7 +282,7 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring)
if (ring->first_seg) {
if (ring->type == TYPE_STREAM)
xhci_remove_stream_mapping(ring);
- xhci_free_segments_for_ring(xhci, ring->first_seg);
+ xhci_ring_segments_free(xhci, ring);
}
kfree(ring);
@@ -344,7 +344,8 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
return 0;
free_segments:
- xhci_free_segments_for_ring(xhci, ring->first_seg);
+ ring->last_seg = prev;
+ xhci_ring_segments_free(xhci, ring);
return -ENOMEM;
}
@@ -433,7 +434,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
return 0;
free_segments:
- xhci_free_segments_for_ring(xhci, new_ring.first_seg);
+ xhci_ring_segments_free(xhci, &new_ring);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 14/33] usb: xhci: refactor xhci_link_rings() to use source and destination rings
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (12 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 13/33] usb: xhci: rework xhci_free_segments_for_ring() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 15/33] usb: xhci: rework xhci_link_segments() Mathias Nyman
` (18 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Refactor the xhci_link_rings() function to accept two rings: a source ring
and a destination ring. Previously, the function accepted a ring and a
segment list as arguments, now the function splices the source ring segment
list into the destination ring. This new approach reduces the number of
arguments and simplifies the code, making it easier to follow.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 40 ++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index e46be4c49b2f..feeaafc59a39 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -116,45 +116,42 @@ static void xhci_link_segments(struct xhci_segment *prev,
}
/*
- * Link the ring to the new segments.
+ * Link the src ring segments to the dst ring.
* Set Toggle Cycle for the new ring if needed.
*/
-static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring,
- struct xhci_segment *first, struct xhci_segment *last,
- unsigned int num_segs)
+static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *src, struct xhci_ring *dst)
{
- struct xhci_segment *next, *seg;
+ struct xhci_segment *seg;
bool chain_links;
- if (!ring || !first || !last)
+ if (!src || !dst)
return;
- chain_links = xhci_link_chain_quirk(xhci, ring->type);
+ chain_links = xhci_link_chain_quirk(xhci, dst->type);
/* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */
- if (ring->cycle_state == 0) {
- xhci_for_each_ring_seg(ring->first_seg, seg) {
+ if (dst->cycle_state == 0) {
+ xhci_for_each_ring_seg(src->first_seg, seg) {
for (int i = 0; i < TRBS_PER_SEGMENT; i++)
seg->trbs[i].link.control |= cpu_to_le32(TRB_CYCLE);
}
}
- next = ring->enq_seg->next;
- xhci_link_segments(ring->enq_seg, first, ring->type, chain_links);
- xhci_link_segments(last, next, ring->type, chain_links);
- ring->num_segs += num_segs;
+ xhci_link_segments(src->last_seg, dst->enq_seg->next, dst->type, chain_links);
+ xhci_link_segments(dst->enq_seg, src->first_seg, dst->type, chain_links);
+ dst->num_segs += src->num_segs;
- if (ring->enq_seg == ring->last_seg) {
- if (ring->type != TYPE_EVENT) {
- ring->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
+ if (dst->enq_seg == dst->last_seg) {
+ if (dst->type != TYPE_EVENT) {
+ dst->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
&= ~cpu_to_le32(LINK_TOGGLE);
- last->trbs[TRBS_PER_SEGMENT-1].link.control
+ src->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
|= cpu_to_le32(LINK_TOGGLE);
}
- ring->last_seg = last;
+ dst->last_seg = src->last_seg;
}
- for (seg = ring->enq_seg; seg != ring->last_seg; seg = seg->next)
+ for (seg = dst->enq_seg; seg != dst->last_seg; seg = seg->next)
seg->next->num = seg->num + 1;
}
@@ -411,6 +408,9 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
struct xhci_ring new_ring;
int ret;
+ if (num_new_segs == 0)
+ return 0;
+
new_ring.num_segs = num_new_segs;
new_ring.bounce_buf_len = ring->bounce_buf_len;
new_ring.type = ring->type;
@@ -425,7 +425,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
goto free_segments;
}
- xhci_link_rings(xhci, ring, new_ring.first_seg, new_ring.last_seg, num_new_segs);
+ xhci_link_rings(xhci, ring, &new_ring);
trace_xhci_ring_expansion(ring);
xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
"ring expansion succeed, now has %d segments",
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 15/33] usb: xhci: rework xhci_link_segments()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (13 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 14/33] usb: xhci: refactor xhci_link_rings() to use source and destination rings Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 16/33] usb: xhci: add xhci_initialize_ring_segments() Mathias Nyman
` (17 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Prepare for splitting ring segments allocation and initialization by
reworking the xhci_link_segments() function. Segment linking and ring type
checks are moved out of xhci_link_segments(), and the function is renamed
to "xhci_set_link_trb()".
The goal is to keep ring linking within xhci_alloc_segments_for_ring() and
move initialization into a separate function.
Additionally, reorder and simplify xhci_set_link_trb() for better
readability.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 54 ++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index feeaafc59a39..41a5e67e1c4f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -86,33 +86,31 @@ static void xhci_ring_segments_free(struct xhci_hcd *xhci, struct xhci_ring *rin
}
/*
- * Make the prev segment point to the next segment.
+ * Only for transfer and command rings where driver is the producer, not for
+ * event rings.
*
- * Change the last TRB in the prev segment to be a Link TRB which points to the
+ * Change the last TRB in the segment to be a Link TRB which points to the
* DMA address of the next segment. The caller needs to set any Link TRB
* related flags, such as End TRB, Toggle Cycle, and no snoop.
*/
-static void xhci_link_segments(struct xhci_segment *prev,
- struct xhci_segment *next,
- enum xhci_ring_type type, bool chain_links)
+static void xhci_set_link_trb(struct xhci_segment *seg, bool chain_links)
{
+ union xhci_trb *trb;
u32 val;
- if (!prev || !next)
+ if (!seg || !seg->next)
return;
- prev->next = next;
- if (type != TYPE_EVENT) {
- prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
- cpu_to_le64(next->dma);
- /* Set the last TRB in the segment to have a TRB type ID of Link TRB */
- val = le32_to_cpu(prev->trbs[TRBS_PER_SEGMENT-1].link.control);
- val &= ~TRB_TYPE_BITMASK;
- val |= TRB_TYPE(TRB_LINK);
- if (chain_links)
- val |= TRB_CHAIN;
- prev->trbs[TRBS_PER_SEGMENT-1].link.control = cpu_to_le32(val);
- }
+ trb = &seg->trbs[TRBS_PER_SEGMENT - 1];
+
+ /* Set the last TRB in the segment to have a TRB type ID of Link TRB */
+ val = le32_to_cpu(trb->link.control);
+ val &= ~TRB_TYPE_BITMASK;
+ val |= TRB_TYPE(TRB_LINK);
+ if (chain_links)
+ val |= TRB_CHAIN;
+ trb->link.control = cpu_to_le32(val);
+ trb->link.segment_ptr = cpu_to_le64(seg->next->dma);
}
/*
@@ -127,8 +125,6 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *src, struct
if (!src || !dst)
return;
- chain_links = xhci_link_chain_quirk(xhci, dst->type);
-
/* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */
if (dst->cycle_state == 0) {
xhci_for_each_ring_seg(src->first_seg, seg) {
@@ -137,8 +133,13 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *src, struct
}
}
- xhci_link_segments(src->last_seg, dst->enq_seg->next, dst->type, chain_links);
- xhci_link_segments(dst->enq_seg, src->first_seg, dst->type, chain_links);
+ src->last_seg->next = dst->enq_seg->next;
+ dst->enq_seg->next = src->first_seg;
+ if (dst->type != TYPE_EVENT) {
+ chain_links = xhci_link_chain_quirk(xhci, dst->type);
+ xhci_set_link_trb(dst->enq_seg, chain_links);
+ xhci_set_link_trb(src->last_seg, chain_links);
+ }
dst->num_segs += src->num_segs;
if (dst->enq_seg == dst->last_seg) {
@@ -331,13 +332,18 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
if (!next)
goto free_segments;
- xhci_link_segments(prev, next, ring->type, chain_links);
+ prev->next = next;
+ if (ring->type != TYPE_EVENT)
+ xhci_set_link_trb(prev, chain_links);
prev = next;
num++;
}
- xhci_link_segments(prev, ring->first_seg, ring->type, chain_links);
ring->last_seg = prev;
+ ring->last_seg->next = ring->first_seg;
+ if (ring->type != TYPE_EVENT)
+ xhci_set_link_trb(prev, chain_links);
+
return 0;
free_segments:
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 16/33] usb: xhci: add xhci_initialize_ring_segments()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (14 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 15/33] usb: xhci: rework xhci_link_segments() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 17/33] xhci: Combine two if statements for Etron xHCI host Mathias Nyman
` (16 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
A ring consists of a list of segments, each containing a specific number of
TRBs. The xhci driver allocates and initializes ring segments and TRBs in
the same functions. This combined allocation and initialization process
leads to an issue where, after hibernation (S4 state), the xhci driver
frees all its memory and re-creates the rings, segments, and TRBs from
scratch.
Move all default ring segment initialization into function
xhci_initialize_ring_segments(). This function can be called to
reinitialize a ring without freeing and reallocating it.
Since xhci_alloc_segments_for_ring() no longer initializes segment TRBs,
xhci_initialize_ring_segments() is added to xhci_ring_expansion(). This
results in the last segment of the source ring having the 'LINK_TOGGLE'
bit set. Therefore, if the last source ring segment is not the last in
the destination ring, the 'LINK_TOGGLE' bit must be cleared.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 41 +++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 41a5e67e1c4f..4295e9a4de50 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -113,6 +113,22 @@ static void xhci_set_link_trb(struct xhci_segment *seg, bool chain_links)
trb->link.segment_ptr = cpu_to_le64(seg->next->dma);
}
+static void xhci_initialize_ring_segments(struct xhci_hcd *xhci, struct xhci_ring *ring)
+{
+ struct xhci_segment *seg;
+ bool chain_links;
+
+ if (ring->type == TYPE_EVENT)
+ return;
+
+ chain_links = xhci_link_chain_quirk(xhci, ring->type);
+ xhci_for_each_ring_seg(ring->first_seg, seg)
+ xhci_set_link_trb(seg, chain_links);
+
+ /* See section 4.9.2.1 and 6.4.4.1 */
+ ring->last_seg->trbs[TRBS_PER_SEGMENT - 1].link.control |= cpu_to_le32(LINK_TOGGLE);
+}
+
/*
* Link the src ring segments to the dst ring.
* Set Toggle Cycle for the new ring if needed.
@@ -143,13 +159,13 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *src, struct
dst->num_segs += src->num_segs;
if (dst->enq_seg == dst->last_seg) {
- if (dst->type != TYPE_EVENT) {
+ if (dst->type != TYPE_EVENT)
dst->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
&= ~cpu_to_le32(LINK_TOGGLE);
- src->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
- |= cpu_to_le32(LINK_TOGGLE);
- }
+
dst->last_seg = src->last_seg;
+ } else if (dst->type != TYPE_EVENT) {
+ src->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control &= ~cpu_to_le32(LINK_TOGGLE);
}
for (seg = dst->enq_seg; seg != dst->last_seg; seg = seg->next)
@@ -315,9 +331,6 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
{
struct xhci_segment *prev;
unsigned int num = 0;
- bool chain_links;
-
- chain_links = xhci_link_chain_quirk(xhci, ring->type);
prev = xhci_segment_alloc(xhci, ring->bounce_buf_len, num, flags);
if (!prev)
@@ -333,17 +346,12 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci, struct xhci_ring
goto free_segments;
prev->next = next;
- if (ring->type != TYPE_EVENT)
- xhci_set_link_trb(prev, chain_links);
prev = next;
num++;
}
ring->last_seg = prev;
ring->last_seg->next = ring->first_seg;
- if (ring->type != TYPE_EVENT)
- xhci_set_link_trb(prev, chain_links);
-
return 0;
free_segments:
@@ -381,12 +389,7 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
if (ret)
goto fail;
- /* Only event ring does not use link TRB */
- if (type != TYPE_EVENT) {
- /* See section 4.9.2.1 and 6.4.4.1 */
- ring->last_seg->trbs[TRBS_PER_SEGMENT - 1].link.control |=
- cpu_to_le32(LINK_TOGGLE);
- }
+ xhci_initialize_ring_segments(xhci, ring);
xhci_initialize_ring_info(ring);
trace_xhci_ring_alloc(ring);
return ring;
@@ -424,6 +427,8 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
if (ret)
return -ENOMEM;
+ xhci_initialize_ring_segments(xhci, &new_ring);
+
if (ring->type == TYPE_STREAM) {
ret = xhci_update_stream_segment_mapping(ring->trb_address_map, ring,
new_ring.first_seg, flags);
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 17/33] xhci: Combine two if statements for Etron xHCI host
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (15 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 16/33] usb: xhci: add xhci_initialize_ring_segments() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 18/33] xhci: Don't issue Reset Device command to " Mathias Nyman
` (15 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Kuangyi Chiang, stable, Mathias Nyman
From: Kuangyi Chiang <ki.chiang65@gmail.com>
Combine two if statements, because these hosts have the same
quirk flags applied.
[Mathias: has stable tag because other fixes in series depend on this]
Fixes: 91f7a1524a92 ("xhci: Apply broken streams quirk to Etron EJ188 xHCI host")
Cc: <stable@vger.kernel.org>
Signed-off-by: Kuangyi Chiang <ki.chiang65@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7803ff1f1c9f..db3c7e738213 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -394,12 +394,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
- pdev->device == PCI_DEVICE_ID_EJ168) {
- xhci->quirks |= XHCI_RESET_ON_RESUME;
- xhci->quirks |= XHCI_BROKEN_STREAMS;
- }
- if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
- pdev->device == PCI_DEVICE_ID_EJ188) {
+ (pdev->device == PCI_DEVICE_ID_EJ168 ||
+ pdev->device == PCI_DEVICE_ID_EJ188)) {
xhci->quirks |= XHCI_RESET_ON_RESUME;
xhci->quirks |= XHCI_BROKEN_STREAMS;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 18/33] xhci: Don't issue Reset Device command to Etron xHCI host
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (16 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 17/33] xhci: Combine two if statements for Etron xHCI host Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 19/33] xhci: Fix control transfer error on " Mathias Nyman
` (14 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Kuangyi Chiang, stable, Mathias Nyman
From: Kuangyi Chiang <ki.chiang65@gmail.com>
Sometimes the hub driver does not recognize the USB device connected
to the external USB2.0 hub when the system resumes from S4.
After the SetPortFeature(PORT_RESET) request is completed, the hub
driver calls the HCD reset_device callback, which will issue a Reset
Device command and free all structures associated with endpoints
that were disabled.
This happens when the xHCI driver issue a Reset Device command to
inform the Etron xHCI host that the USB device associated with a
device slot has been reset. Seems that the Etron xHCI host can not
perform this command correctly, affecting the USB device.
To work around this, the xHCI driver should obtain a new device slot
with reference to commit 651aaf36a7d7 ("usb: xhci: Handle USB transaction
error on address command"), which is another way to inform the Etron
xHCI host that the USB device has been reset.
Add a new XHCI_ETRON_HOST quirk flag to invoke the workaround in
xhci_discover_or_reset_device().
Fixes: 2a8f82c4ceaf ("USB: xhci: Notify the xHC when a device is reset.")
Cc: <stable@vger.kernel.org>
Signed-off-by: Kuangyi Chiang <ki.chiang65@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 1 +
drivers/usb/host/xhci.c | 19 +++++++++++++++++++
drivers/usb/host/xhci.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index db3c7e738213..4b8c93e59d6d 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -396,6 +396,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
(pdev->device == PCI_DEVICE_ID_EJ168 ||
pdev->device == PCI_DEVICE_ID_EJ188)) {
+ xhci->quirks |= XHCI_ETRON_HOST;
xhci->quirks |= XHCI_RESET_ON_RESUME;
xhci->quirks |= XHCI_BROKEN_STREAMS;
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index aa8c877f47ac..ae16253b53fb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3733,6 +3733,8 @@ void xhci_free_device_endpoint_resources(struct xhci_hcd *xhci,
xhci->num_active_eps);
}
+static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev);
+
/*
* This submits a Reset Device Command, which will set the device state to 0,
* set the device address to 0, and disable all the endpoints except the default
@@ -3803,6 +3805,23 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
SLOT_STATE_DISABLED)
return 0;
+ if (xhci->quirks & XHCI_ETRON_HOST) {
+ /*
+ * Obtaining a new device slot to inform the xHCI host that
+ * the USB device has been reset.
+ */
+ ret = xhci_disable_slot(xhci, udev->slot_id);
+ xhci_free_virt_device(xhci, udev->slot_id);
+ if (!ret) {
+ ret = xhci_alloc_dev(hcd, udev);
+ if (ret == 1)
+ ret = 0;
+ else
+ ret = -EINVAL;
+ }
+ return ret;
+ }
+
trace_xhci_discover_or_reset_device(slot_ctx);
xhci_dbg(xhci, "Resetting device with slot ID %u\n", slot_id);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d3b250c736b8..a0204e10486d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1631,6 +1631,7 @@ struct xhci_hcd {
#define XHCI_ZHAOXIN_HOST BIT_ULL(46)
#define XHCI_WRITE_64_HI_LO BIT_ULL(47)
#define XHCI_CDNS_SCTX_QUIRK BIT_ULL(48)
+#define XHCI_ETRON_HOST BIT_ULL(49)
unsigned int num_active_eps;
unsigned int limit_active_eps;
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 19/33] xhci: Fix control transfer error on Etron xHCI host
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (17 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 18/33] xhci: Don't issue Reset Device command to " Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 20/33] xhci: Don't perform Soft Retry for " Mathias Nyman
` (13 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Kuangyi Chiang, stable, Mathias Nyman
From: Kuangyi Chiang <ki.chiang65@gmail.com>
Performing a stability stress test on a USB3.0 2.5G ethernet adapter
results in errors like this:
[ 91.441469] r8152 2-3:1.0 eth3: get_registers -71
[ 91.458659] r8152 2-3:1.0 eth3: get_registers -71
[ 91.475911] r8152 2-3:1.0 eth3: get_registers -71
[ 91.493203] r8152 2-3:1.0 eth3: get_registers -71
[ 91.510421] r8152 2-3:1.0 eth3: get_registers -71
The r8152 driver will periodically issue lots of control-IN requests
to access the status of ethernet adapter hardware registers during
the test.
This happens when the xHCI driver enqueue a control TD (which cross
over the Link TRB between two ring segments, as shown) in the endpoint
zero's transfer ring. Seems the Etron xHCI host can not perform this
TD correctly, causing the USB transfer error occurred, maybe the upper
driver retry that control-IN request can solve problem, but not all
drivers do this.
| |
-------
| TRB | Setup Stage
-------
| TRB | Link
-------
-------
| TRB | Data Stage
-------
| TRB | Status Stage
-------
| |
To work around this, the xHCI driver should enqueue a No Op TRB if
next available TRB is the Link TRB in the ring segment, this can
prevent the Setup and Data Stage TRB to be breaked by the Link TRB.
Check if the XHCI_ETRON_HOST quirk flag is set before invoking the
workaround in xhci_queue_ctrl_tx().
Fixes: d0e96f5a71a0 ("USB: xhci: Control transfer support.")
Cc: <stable@vger.kernel.org>
Signed-off-by: Kuangyi Chiang <ki.chiang65@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f62b243d0fc4..517df97ef496 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3733,6 +3733,20 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
if (!urb->setup_packet)
return -EINVAL;
+ if ((xhci->quirks & XHCI_ETRON_HOST) &&
+ urb->dev->speed >= USB_SPEED_SUPER) {
+ /*
+ * If next available TRB is the Link TRB in the ring segment then
+ * enqueue a No Op TRB, this can prevent the Setup and Data Stage
+ * TRB to be breaked by the Link TRB.
+ */
+ if (trb_is_link(ep_ring->enqueue + 1)) {
+ field = TRB_TYPE(TRB_TR_NOOP) | ep_ring->cycle_state;
+ queue_trb(xhci, ep_ring, false, 0, 0,
+ TRB_INTR_TARGET(0), field);
+ }
+ }
+
/* 1 TRB for setup, 1 for status */
num_trbs = 2;
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 20/33] xhci: Don't perform Soft Retry for Etron xHCI host
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (18 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 19/33] xhci: Fix control transfer error on " Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 21/33] xhci: pci: Use standard pattern for device IDs Mathias Nyman
` (12 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Kuangyi Chiang, stable, Mathias Nyman
From: Kuangyi Chiang <ki.chiang65@gmail.com>
Since commit f8f80be501aa ("xhci: Use soft retry to recover faster from
transaction errors"), unplugging USB device while enumeration results in
errors like this:
[ 364.855321] xhci_hcd 0000:0b:00.0: ERROR Transfer event for disabled endpoint slot 5 ep 2
[ 364.864622] xhci_hcd 0000:0b:00.0: @0000002167656d70 67f03000 00000021 0c000000 05038001
[ 374.934793] xhci_hcd 0000:0b:00.0: Abort failed to stop command ring: -110
[ 374.958793] xhci_hcd 0000:0b:00.0: xHCI host controller not responding, assume dead
[ 374.967590] xhci_hcd 0000:0b:00.0: HC died; cleaning up
[ 374.973984] xhci_hcd 0000:0b:00.0: Timeout while waiting for configure endpoint command
Seems that Etorn xHCI host can not perform Soft Retry correctly, apply
XHCI_NO_SOFT_RETRY quirk to disable Soft Retry and then issue is gone.
This patch depends on commit a4a251f8c235 ("usb: xhci: do not perform
Soft Retry for some xHCI hosts").
Fixes: f8f80be501aa ("xhci: Use soft retry to recover faster from transaction errors")
Cc: <stable@vger.kernel.org>
Signed-off-by: Kuangyi Chiang <ki.chiang65@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 4b8c93e59d6d..0d49d9c390c1 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -399,6 +399,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
xhci->quirks |= XHCI_ETRON_HOST;
xhci->quirks |= XHCI_RESET_ON_RESUME;
xhci->quirks |= XHCI_BROKEN_STREAMS;
+ xhci->quirks |= XHCI_NO_SOFT_RETRY;
}
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 21/33] xhci: pci: Use standard pattern for device IDs
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (19 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 20/33] xhci: Don't perform Soft Retry for " Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 22/33] xhci: pci: Fix indentation in the PCI device ID definitions Mathias Nyman
` (11 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Andy Shevchenko, Mathias Nyman
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
The definitions of vendor IDs follow the pattern PCI_VENDOR_ID_#vendor,
while device IDs — PCI_DEVICE_ID_#vendor_#device.
Update the ETRON device IDs to follow the above mentioned pattern.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 0d49d9c390c1..77f9fe7a7dcd 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -34,9 +34,9 @@
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1100 0x1100
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1400 0x1400
-#define PCI_VENDOR_ID_ETRON 0x1b6f
-#define PCI_DEVICE_ID_EJ168 0x7023
-#define PCI_DEVICE_ID_EJ188 0x7052
+#define PCI_VENDOR_ID_ETRON 0x1b6f
+#define PCI_DEVICE_ID_ETRON_EJ168 0x7023
+#define PCI_DEVICE_ID_ETRON_EJ188 0x7052
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
@@ -394,8 +394,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
- (pdev->device == PCI_DEVICE_ID_EJ168 ||
- pdev->device == PCI_DEVICE_ID_EJ188)) {
+ (pdev->device == PCI_DEVICE_ID_ETRON_EJ168 ||
+ pdev->device == PCI_DEVICE_ID_ETRON_EJ188)) {
xhci->quirks |= XHCI_ETRON_HOST;
xhci->quirks |= XHCI_RESET_ON_RESUME;
xhci->quirks |= XHCI_BROKEN_STREAMS;
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 22/33] xhci: pci: Fix indentation in the PCI device ID definitions
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (20 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 21/33] xhci: pci: Use standard pattern for device IDs Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 23/33] usb: xhci: simplify TDs start and end naming scheme in struct 'xhci_td' Mathias Nyman
` (10 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Andy Shevchenko
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Some of the definitions are missing the one TAB, add it to them.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 77f9fe7a7dcd..b1f4dd3f9eff 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -28,8 +28,8 @@
#define SPARSE_CNTL_ENABLE 0xC12C
/* Device for a quirk */
-#define PCI_VENDOR_ID_FRESCO_LOGIC 0x1b73
-#define PCI_DEVICE_ID_FRESCO_LOGIC_PDK 0x1000
+#define PCI_VENDOR_ID_FRESCO_LOGIC 0x1b73
+#define PCI_DEVICE_ID_FRESCO_LOGIC_PDK 0x1000
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1009 0x1009
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1100 0x1100
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1400 0x1400
@@ -38,8 +38,8 @@
#define PCI_DEVICE_ID_ETRON_EJ168 0x7023
#define PCI_DEVICE_ID_ETRON_EJ188 0x7052
-#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
-#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
+#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
+#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
#define PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_XHCI 0x9cb1
#define PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI 0x22b5
#define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_XHCI 0xa12f
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 23/33] usb: xhci: simplify TDs start and end naming scheme in struct 'xhci_td'
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (21 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 22/33] xhci: pci: Fix indentation in the PCI device ID definitions Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 24/33] usb: xhci: move link TRB quirk to xhci_gen_setup() Mathias Nyman
` (9 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Old names:
* start_seg - last_trb_seg
* first_trb - last_trb
New names:
* start_seg - end_seg
* start_trb - end_trb
A Transfer Descriptor (TD) in the xhci driver is a data structure that
represents a single transaction to be performed by the USB host controller.
This transaction is defined by TRBs from 'start_trb' in 'start_seg' to
'end_trb' in 'end_seg'.
The terms "start" and "end" were chosen over "first" and "last" for ease
of searching within the codebase. The ring structure uses 'first_seg' and
'last_seg', while the TD structure uses 'start_seg' and 'end_seg'.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 68 ++++++++++++++++++------------------
drivers/usb/host/xhci.c | 2 +-
drivers/usb/host/xhci.h | 6 ++--
3 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 517df97ef496..1b4c785c8dad 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -660,8 +660,8 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
/*
* We want to find the pointer, segment and cycle state of the new trb
- * (the one after current TD's last_trb). We know the cycle state at
- * hw_dequeue, so walk the ring until both hw_dequeue and last_trb are
+ * (the one after current TD's end_trb). We know the cycle state at
+ * hw_dequeue, so walk the ring until both hw_dequeue and end_trb are
* found.
*/
do {
@@ -671,7 +671,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
if (td_last_trb_found)
break;
}
- if (new_deq == td->last_trb)
+ if (new_deq == td->end_trb)
td_last_trb_found = true;
if (cycle_found && trb_is_link(new_deq) &&
@@ -744,16 +744,16 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
struct xhci_td *td, bool flip_cycle)
{
struct xhci_segment *seg = td->start_seg;
- union xhci_trb *trb = td->first_trb;
+ union xhci_trb *trb = td->start_trb;
while (1) {
trb_to_noop(trb, TRB_TR_NOOP);
/* flip cycle if asked to */
- if (flip_cycle && trb != td->first_trb && trb != td->last_trb)
+ if (flip_cycle && trb != td->start_trb && trb != td->end_trb)
trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE);
- if (trb == td->last_trb)
+ if (trb == td->end_trb)
break;
next_trb(&seg, &trb);
@@ -978,7 +978,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Removing canceled TD starting at 0x%llx (dma) in stream %u URB %p",
(unsigned long long)xhci_trb_virt_to_dma(
- td->start_seg, td->first_trb),
+ td->start_seg, td->start_trb),
td->urb->stream_id, td->urb);
list_del_init(&td->td_list);
ring = xhci_urb_to_transfer_ring(xhci, td->urb);
@@ -2078,7 +2078,7 @@ struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_td *td, dma_ad
dma_addr_t end_trb_dma;
struct xhci_segment *cur_seg;
- start_dma = xhci_trb_virt_to_dma(td->start_seg, td->first_trb);
+ start_dma = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
cur_seg = td->start_seg;
do {
@@ -2088,7 +2088,7 @@ struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_td *td, dma_ad
end_seg_dma = xhci_trb_virt_to_dma(cur_seg,
&cur_seg->trbs[TRBS_PER_SEGMENT - 1]);
/* 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->last_trb);
+ end_trb_dma = xhci_trb_virt_to_dma(cur_seg, td->end_trb);
if (debug)
xhci_warn(xhci,
@@ -2230,7 +2230,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
!list_empty(&td->cancelled_td_list)) {
xhci_dbg(xhci, "Already resolving halted ep for 0x%llx\n",
(unsigned long long)xhci_trb_virt_to_dma(
- td->start_seg, td->first_trb));
+ td->start_seg, td->start_trb));
return 0;
}
/* endpoint not halted, don't reset it */
@@ -2262,8 +2262,8 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
}
/* Update ring dequeue pointer */
- ep_ring->dequeue = td->last_trb;
- ep_ring->deq_seg = td->last_trb_seg;
+ ep_ring->dequeue = td->end_trb;
+ ep_ring->deq_seg = td->end_seg;
inc_deq(xhci, ep_ring);
return xhci_td_cleanup(xhci, td, ep_ring, td->status);
@@ -2273,7 +2273,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
static u32 sum_trb_lengths(struct xhci_td *td, union xhci_trb *stop_trb)
{
u32 sum;
- union xhci_trb *trb = td->first_trb;
+ union xhci_trb *trb = td->start_trb;
struct xhci_segment *seg = td->start_seg;
for (sum = 0; trb != stop_trb; next_trb(&seg, &trb)) {
@@ -2428,7 +2428,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
fallthrough;
case COMP_ISOCH_BUFFER_OVERRUN:
frame->status = -EOVERFLOW;
- if (ep_trb != td->last_trb)
+ if (ep_trb != td->end_trb)
td->error_mid_td = true;
break;
case COMP_INCOMPATIBLE_DEVICE_ERROR:
@@ -2438,7 +2438,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
case COMP_USB_TRANSACTION_ERROR:
frame->status = -EPROTO;
sum_trbs_for_length = true;
- if (ep_trb != td->last_trb)
+ if (ep_trb != td->end_trb)
td->error_mid_td = true;
break;
case COMP_STOPPED:
@@ -2474,7 +2474,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
finish_td:
/* Don't give back TD yet if we encountered an error mid TD */
- if (td->error_mid_td && ep_trb != td->last_trb) {
+ if (td->error_mid_td && ep_trb != td->end_trb) {
xhci_dbg(xhci, "Error mid isoc TD, wait for final completion event\n");
td->urb_length_set = true;
return 0;
@@ -2501,8 +2501,8 @@ static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
frame->actual_length = 0;
/* Update ring dequeue pointer */
- ep->ring->dequeue = td->last_trb;
- ep->ring->deq_seg = td->last_trb_seg;
+ ep->ring->dequeue = td->end_trb;
+ ep->ring->deq_seg = td->end_seg;
inc_deq(xhci, ep->ring);
return xhci_td_cleanup(xhci, td, ep->ring, status);
@@ -2529,7 +2529,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
case COMP_SUCCESS:
ep->err_count = 0;
/* handle success with untransferred data as short packet */
- if (ep_trb != td->last_trb || remaining) {
+ if (ep_trb != td->end_trb || remaining) {
xhci_warn(xhci, "WARN Successful completion on short TX\n");
xhci_dbg(xhci, "ep %#x - asked for %d bytes, %d bytes untransferred\n",
td->urb->ep->desc.bEndpointAddress,
@@ -2562,7 +2562,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
break;
}
- if (ep_trb == td->last_trb)
+ if (ep_trb == td->end_trb)
td->urb->actual_length = requested - remaining;
else
td->urb->actual_length =
@@ -2795,8 +2795,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
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");
- ep_ring->dequeue = td->last_trb;
- ep_ring->deq_seg = td->last_trb_seg;
+ ep_ring->dequeue = td->end_trb;
+ ep_ring->deq_seg = td->end_seg;
inc_deq(xhci, ep_ring);
xhci_td_cleanup(xhci, td, ep_ring, td->status);
}
@@ -3308,7 +3308,7 @@ static int prepare_transfer(struct xhci_hcd *xhci,
/* Add this TD to the tail of the endpoint ring's TD list */
list_add_tail(&td->td_list, &ep_ring->td_list);
td->start_seg = ep_ring->enq_seg;
- td->first_trb = ep_ring->enqueue;
+ td->start_trb = ep_ring->enqueue;
return 0;
}
@@ -3647,8 +3647,8 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
field &= ~TRB_CHAIN;
field |= TRB_IOC;
more_trbs_coming = false;
- td->last_trb = ring->enqueue;
- td->last_trb_seg = ring->enq_seg;
+ td->end_trb = ring->enqueue;
+ td->end_seg = ring->enq_seg;
if (xhci_urb_suitable_for_idt(urb)) {
memcpy(&send_addr, urb->transfer_buffer,
trb_buff_len);
@@ -3696,8 +3696,8 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
ret = prepare_transfer(xhci, xhci->devs[slot_id],
ep_index, urb->stream_id,
1, urb, 1, mem_flags);
- urb_priv->td[1].last_trb = ring->enqueue;
- urb_priv->td[1].last_trb_seg = ring->enq_seg;
+ urb_priv->td[1].end_trb = ring->enqueue;
+ urb_priv->td[1].end_seg = ring->enq_seg;
field = TRB_TYPE(TRB_NORMAL) | ring->cycle_state | TRB_IOC;
queue_trb(xhci, ring, 0, 0, 0, TRB_INTR_TARGET(0), field);
}
@@ -3835,8 +3835,8 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
}
/* Save the DMA address of the last TRB in the TD */
- td->last_trb = ep_ring->enqueue;
- td->last_trb_seg = ep_ring->enq_seg;
+ td->end_trb = ep_ring->enqueue;
+ td->end_seg = ep_ring->enq_seg;
/* Queue status TRB - see Table 7 and sections 4.11.2.2 and 6.4.1.2.3 */
/* If the device sent data, the status stage is an OUT transfer */
@@ -4121,8 +4121,8 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
field |= TRB_CHAIN;
} else {
more_trbs_coming = false;
- td->last_trb = ep_ring->enqueue;
- td->last_trb_seg = ep_ring->enq_seg;
+ td->end_trb = ep_ring->enqueue;
+ td->end_seg = ep_ring->enq_seg;
field |= TRB_IOC;
if (trb_block_event_intr(xhci, num_tds, i, ir))
field |= TRB_BEI;
@@ -4188,14 +4188,14 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
/* Use the first TD as a temporary variable to turn the TDs we've queued
* into No-ops with a software-owned cycle bit. That way the hardware
* won't accidentally start executing bogus TDs when we partially
- * overwrite them. td->first_trb and td->start_seg are already set.
+ * overwrite them. td->start_trb and td->start_seg are already set.
*/
- urb_priv->td[0].last_trb = ep_ring->enqueue;
+ urb_priv->td[0].end_trb = ep_ring->enqueue;
/* Every TRB except the first & last will have its cycle bit flipped. */
td_to_noop(xhci, ep_ring, &urb_priv->td[0], true);
/* Reset the ring enqueue back to the first TRB and its cycle bit. */
- ep_ring->enqueue = urb_priv->td[0].first_trb;
+ ep_ring->enqueue = urb_priv->td[0].start_trb;
ep_ring->enq_seg = urb_priv->td[0].start_seg;
ep_ring->cycle_state = start_cycle;
usb_hcd_unlink_urb_from_ep(bus_to_hcd(urb->dev->bus), urb);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ae16253b53fb..a5ac1e01f82d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1752,7 +1752,7 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
urb->ep->desc.bEndpointAddress,
(unsigned long long) xhci_trb_virt_to_dma(
urb_priv->td[i].start_seg,
- urb_priv->td[i].first_trb));
+ urb_priv->td[i].start_trb));
for (; i < urb_priv->num_tds; i++) {
td = &urb_priv->td[i];
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index a0204e10486d..a0e992c3db0d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1294,9 +1294,9 @@ struct xhci_td {
enum xhci_cancelled_td_status cancel_status;
struct urb *urb;
struct xhci_segment *start_seg;
- union xhci_trb *first_trb;
- union xhci_trb *last_trb;
- struct xhci_segment *last_trb_seg;
+ union xhci_trb *start_trb;
+ struct xhci_segment *end_seg;
+ union xhci_trb *end_trb;
struct xhci_segment *bounce_seg;
/* actual_length of the URB has already been set */
bool urb_length_set;
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 24/33] usb: xhci: move link TRB quirk to xhci_gen_setup()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (22 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 23/33] usb: xhci: simplify TDs start and end naming scheme in struct 'xhci_td' Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 25/33] usb: xhci: request MSI/-X according to requested amount Mathias Nyman
` (8 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
This quirk is old and seldom seen, as a result the trace is changed
to debug message and only printed when the quirk is set.
Move it into xhci_gen_setup() where the majority of quirks are set.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a5ac1e01f82d..e5719fd45a38 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -473,14 +473,7 @@ static int xhci_init(struct usb_hcd *hcd)
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "xhci_init");
spin_lock_init(&xhci->lock);
- if (xhci->hci_version == 0x95 && link_quirk) {
- xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
- "QUIRK: Not clearing Link TRB chain bits.");
- xhci->quirks |= XHCI_LINK_TRB_QUIRK;
- } else {
- xhci_dbg_trace(xhci, trace_xhci_dbg_init,
- "xHCI doesn't need link TRB QUIRK");
- }
+
retval = xhci_mem_init(xhci, GFP_KERNEL);
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished xhci_init");
@@ -5311,6 +5304,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
if (xhci->hci_version > 0x96)
xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
+ if (xhci->hci_version == 0x95 && link_quirk) {
+ xhci_dbg(xhci, "QUIRK: Not clearing Link TRB chain bits");
+ xhci->quirks |= XHCI_LINK_TRB_QUIRK;
+ }
+
/* Make sure the HC is halted. */
retval = xhci_halt(xhci);
if (retval)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 25/33] usb: xhci: request MSI/-X according to requested amount
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (23 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 24/33] usb: xhci: move link TRB quirk to xhci_gen_setup() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 26/33] usb: xhci: improve xhci_clear_command_ring() Mathias Nyman
` (7 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Variable 'max_interrupters' contains the maximum supported interrupters
or the maximum interrupters the user has requested. Thus, it should be
used instead of HCS_MAX_INTRS().
User set 'max_interrupters' value is validated in xhci_gen_setup(),
otherwise 'max_interrupters' value is 'HCS_MAX_INTRS(xhci->hcs_params1)'.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b1f4dd3f9eff..47c4f70793e4 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -149,14 +149,11 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
hcd->irq = 0;
/*
- * calculate number of MSI-X vectors supported.
- * - HCS_MAX_INTRS: the max number of interrupts the host can handle,
- * with max number of interrupters based on the xhci HCSPARAMS1.
- * - num_online_cpus: maximum MSI-X vectors per CPUs core.
- * Add additional 1 vector to ensure always available interrupt.
+ * Calculate number of MSI/MSI-X vectors supported.
+ * - max_interrupters: the max number of interrupts requested, capped to xhci HCSPARAMS1.
+ * - num_online_cpus: one vector per CPUs core, with at least one overall.
*/
- xhci->nvecs = min(num_online_cpus() + 1,
- HCS_MAX_INTRS(xhci->hcs_params1));
+ xhci->nvecs = min(num_online_cpus() + 1, xhci->max_interrupters);
/* TODO: Check with MSI Soc for sysdev */
xhci->nvecs = pci_alloc_irq_vectors(pdev, 1, xhci->nvecs,
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 26/33] usb: xhci: improve xhci_clear_command_ring()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (24 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 25/33] usb: xhci: request MSI/-X according to requested amount Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 27/33] usb: xhci: remove unused arguments from td_to_noop() Mathias Nyman
` (6 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Remove redundant TRB cycle reset, the TRB cycle is already set to zero by
the preceding memset(), making the explicit reset unnecessary.
Clarify ring loop start point. Change the loop start from the dequeue
segment to the start segment. Both approaches achieve the same result,
but starting from the start segment makes it clearer that the entire ring
is being zeroed out.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e5719fd45a38..bc477cf99805 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -778,10 +778,8 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
struct xhci_segment *seg;
ring = xhci->cmd_ring;
- xhci_for_each_ring_seg(ring->deq_seg, seg) {
+ xhci_for_each_ring_seg(ring->first_seg, seg)
memset(seg->trbs, 0, sizeof(union xhci_trb) * (TRBS_PER_SEGMENT - 1));
- seg->trbs[TRBS_PER_SEGMENT - 1].link.control &= cpu_to_le32(~TRB_CYCLE);
- }
xhci_initialize_ring_info(ring);
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 27/33] usb: xhci: remove unused arguments from td_to_noop()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (25 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 26/33] usb: xhci: improve xhci_clear_command_ring() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 28/33] usb: xhci: refactor xhci_td_cleanup() to return void Mathias Nyman
` (5 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Function td_to_noop() does not utilize arguments 'xhci' and 'ep_ring'.
These unused arguments are removed to clean up the code.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1b4c785c8dad..5fb3d771c429 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -740,8 +740,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
* (The last TRB actually points to the ring enqueue pointer, which is not part
* of this TD.) This is used to remove partially enqueued isoc TDs from a ring.
*/
-static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
- struct xhci_td *td, bool flip_cycle)
+static void td_to_noop(struct xhci_td *td, bool flip_cycle)
{
struct xhci_segment *seg = td->start_seg;
union xhci_trb *trb = td->start_trb;
@@ -1020,16 +1019,16 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
"Found multiple active URBs %p and %p in stream %u?\n",
td->urb, cached_td->urb,
td->urb->stream_id);
- td_to_noop(xhci, ring, cached_td, false);
+ td_to_noop(cached_td, false);
cached_td->cancel_status = TD_CLEARED;
}
- td_to_noop(xhci, ring, td, false);
+ td_to_noop(td, false);
td->cancel_status = TD_CLEARING_CACHE;
cached_td = td;
break;
}
} else {
- td_to_noop(xhci, ring, td, false);
+ td_to_noop(td, false);
td->cancel_status = TD_CLEARED;
}
}
@@ -1054,7 +1053,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
continue;
xhci_warn(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n",
td->urb);
- td_to_noop(xhci, ring, td, false);
+ td_to_noop(td, false);
td->cancel_status = TD_CLEARED;
}
}
@@ -4192,7 +4191,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
*/
urb_priv->td[0].end_trb = ep_ring->enqueue;
/* Every TRB except the first & last will have its cycle bit flipped. */
- td_to_noop(xhci, ep_ring, &urb_priv->td[0], true);
+ td_to_noop(&urb_priv->td[0], true);
/* Reset the ring enqueue back to the first TRB and its cycle bit. */
ep_ring->enqueue = urb_priv->td[0].start_trb;
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 28/33] usb: xhci: refactor xhci_td_cleanup() to return void
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (26 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 27/33] usb: xhci: remove unused arguments from td_to_noop() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 29/33] usb: xhci: add help function xhci_dequeue_td() Mathias Nyman
` (4 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The function is modified to return 'void' instead of an integer since it
invariably returns '0'. Additionally, multiple functions which only
return xhci_td_cleanup() are also refactored to return void.
This change eliminates the need for callers to handle a return value that
does not convey meaningful information and improve code readability, as it
becomes immediately clear that the function does not produce a significant
output.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 59 +++++++++++++++++-------------------
1 file changed, 28 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5fb3d771c429..e48ee58fdb46 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -813,8 +813,8 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
seg->bounce_offs = 0;
}
-static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
- struct xhci_ring *ep_ring, int status)
+static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
+ struct xhci_ring *ep_ring, int status)
{
struct urb *urb = NULL;
@@ -857,8 +857,6 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
status = 0;
xhci_giveback_urb_in_irq(xhci, td, status);
}
-
- return 0;
}
@@ -2187,9 +2185,9 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code)
return 0;
}
-static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
- struct xhci_ring *ep_ring, struct xhci_td *td,
- u32 trb_comp_code)
+static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+ struct xhci_ring *ep_ring, struct xhci_td *td,
+ u32 trb_comp_code)
{
struct xhci_ep_ctx *ep_ctx;
@@ -2204,7 +2202,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
* stopped TDs. A stopped TD may be restarted, so don't update
* the ring dequeue pointer or take this TD off any lists yet.
*/
- return 0;
+ return;
case COMP_USB_TRANSACTION_ERROR:
case COMP_BABBLE_DETECTED_ERROR:
case COMP_SPLIT_TRANSACTION_ERROR:
@@ -2230,7 +2228,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
xhci_dbg(xhci, "Already resolving halted ep for 0x%llx\n",
(unsigned long long)xhci_trb_virt_to_dma(
td->start_seg, td->start_trb));
- return 0;
+ return;
}
/* endpoint not halted, don't reset it */
break;
@@ -2238,7 +2236,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
/* Almost same procedure as for STALL_ERROR below */
xhci_clear_hub_tt_buffer(xhci, td, ep);
xhci_handle_halted_endpoint(xhci, ep, td, EP_HARD_RESET);
- return 0;
+ return;
case COMP_STALL_ERROR:
/*
* xhci internal endpoint state will go to a "halt" state for
@@ -2255,7 +2253,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
xhci_handle_halted_endpoint(xhci, ep, td, EP_HARD_RESET);
- return 0; /* xhci_handle_halted_endpoint marked td cancelled */
+ return; /* xhci_handle_halted_endpoint marked td cancelled */
default:
break;
}
@@ -2265,7 +2263,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
ep_ring->deq_seg = td->end_seg;
inc_deq(xhci, ep_ring);
- return xhci_td_cleanup(xhci, td, ep_ring, td->status);
+ xhci_td_cleanup(xhci, td, ep_ring, td->status);
}
/* sum trb lengths from the first trb up to stop_trb, _excluding_ stop_trb */
@@ -2285,9 +2283,9 @@ static u32 sum_trb_lengths(struct xhci_td *td, union xhci_trb *stop_trb)
/*
* Process control tds, update urb status and actual_length.
*/
-static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
- struct xhci_ring *ep_ring, struct xhci_td *td,
- union xhci_trb *ep_trb, struct xhci_transfer_event *event)
+static void process_ctrl_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+ struct xhci_ring *ep_ring, struct xhci_td *td,
+ union xhci_trb *ep_trb, struct xhci_transfer_event *event)
{
struct xhci_ep_ctx *ep_ctx;
u32 trb_comp_code;
@@ -2366,7 +2364,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->urb_length_set = true;
td->urb->actual_length = requested - remaining;
xhci_dbg(xhci, "Waiting for status stage event\n");
- return 0;
+ return;
}
/* at status stage */
@@ -2374,15 +2372,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->urb->actual_length = requested;
finish_td:
- return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
+ finish_td(xhci, ep, ep_ring, td, trb_comp_code);
}
/*
* Process isochronous tds, update urb packet status and actual_length.
*/
-static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
- struct xhci_ring *ep_ring, struct xhci_td *td,
- union xhci_trb *ep_trb, struct xhci_transfer_event *event)
+static void process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+ struct xhci_ring *ep_ring, struct xhci_td *td,
+ union xhci_trb *ep_trb, struct xhci_transfer_event *event)
{
struct urb_priv *urb_priv;
int idx;
@@ -2476,14 +2474,13 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
if (td->error_mid_td && ep_trb != td->end_trb) {
xhci_dbg(xhci, "Error mid isoc TD, wait for final completion event\n");
td->urb_length_set = true;
- return 0;
+ return;
}
-
- return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
+ finish_td(xhci, ep, ep_ring, td, trb_comp_code);
}
-static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
- struct xhci_virt_ep *ep, int status)
+static void skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
+ struct xhci_virt_ep *ep, int status)
{
struct urb_priv *urb_priv;
struct usb_iso_packet_descriptor *frame;
@@ -2504,15 +2501,15 @@ static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
ep->ring->deq_seg = td->end_seg;
inc_deq(xhci, ep->ring);
- return xhci_td_cleanup(xhci, td, ep->ring, status);
+ xhci_td_cleanup(xhci, td, ep->ring, status);
}
/*
* Process bulk and interrupt tds, update urb status and actual_length.
*/
-static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
- struct xhci_ring *ep_ring, struct xhci_td *td,
- union xhci_trb *ep_trb, struct xhci_transfer_event *event)
+static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
+ struct xhci_ring *ep_ring, struct xhci_td *td,
+ union xhci_trb *ep_trb, struct xhci_transfer_event *event)
{
struct xhci_slot_ctx *slot_ctx;
u32 trb_comp_code;
@@ -2555,7 +2552,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->status = 0;
xhci_handle_halted_endpoint(xhci, ep, td, EP_SOFT_RESET);
- return 0;
+ return;
default:
/* do nothing */
break;
@@ -2574,7 +2571,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->urb->actual_length = 0;
}
- return finish_td(xhci, ep, ep_ring, td, trb_comp_code);
+ finish_td(xhci, ep, ep_ring, td, trb_comp_code);
}
/* Transfer events which don't point to a transfer TRB, see xhci 4.17.4 */
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 29/33] usb: xhci: add help function xhci_dequeue_td()
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (27 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 28/33] usb: xhci: refactor xhci_td_cleanup() to return void Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 30/33] usb: xhci: remove irrelevant comment Mathias Nyman
` (3 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Add xhci_dequeue_td() helper function to reduce code duplication.
Function xhci_dequeue_td() advances the dequeue pointer past the specified
Transfer Descriptor (TD) and releases the TD.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e48ee58fdb46..c9c0c4a7588a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -859,6 +859,16 @@ static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
}
}
+/* Give back previous TD and move on to the next TD. */
+static void xhci_dequeue_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ring *ring,
+ u32 status)
+{
+ ring->dequeue = td->end_trb;
+ ring->deq_seg = td->end_seg;
+ inc_deq(xhci, ring);
+
+ xhci_td_cleanup(xhci, td, ring, status);
+}
/* Complete the cancelled URBs we unlinked from td_list. */
static void xhci_giveback_invalidated_tds(struct xhci_virt_ep *ep)
@@ -2258,12 +2268,7 @@ static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
break;
}
- /* Update ring dequeue pointer */
- ep_ring->dequeue = td->end_trb;
- ep_ring->deq_seg = td->end_seg;
- inc_deq(xhci, ep_ring);
-
- xhci_td_cleanup(xhci, td, ep_ring, td->status);
+ xhci_dequeue_td(xhci, td, ep_ring, td->status);
}
/* sum trb lengths from the first trb up to stop_trb, _excluding_ stop_trb */
@@ -2496,12 +2501,7 @@ static void skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
/* calc actual length */
frame->actual_length = 0;
- /* Update ring dequeue pointer */
- ep->ring->dequeue = td->end_trb;
- ep->ring->deq_seg = td->end_seg;
- inc_deq(xhci, ep->ring);
-
- xhci_td_cleanup(xhci, td, ep->ring, status);
+ xhci_dequeue_td(xhci, td, ep->ring, status);
}
/*
@@ -2791,10 +2791,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
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");
- ep_ring->dequeue = td->end_trb;
- ep_ring->deq_seg = td->end_seg;
- inc_deq(xhci, ep_ring);
- xhci_td_cleanup(xhci, td, ep_ring, td->status);
+ xhci_dequeue_td(xhci, td, ep_ring, td->status);
}
if (list_empty(&ep_ring->td_list)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 30/33] usb: xhci: remove irrelevant comment
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (28 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 29/33] usb: xhci: add help function xhci_dequeue_td() Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 31/33] usb: xhci: Limit Stop Endpoint retries Mathias Nyman
` (2 subsequent siblings)
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The code which it is referencing does not exist in the same function,
or the file for that matter. Since it was added [1], the Interrupter
Moderation Interval can be changed within xhci addon, e.g. PCI
xhci_pci_setup().
[1], commit 0ebbab374223 ("USB: xhci: Ring allocation and initialization.")
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 4295e9a4de50..15db90c54a45 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2512,11 +2512,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
ir->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
- /*
- * XXX: Might need to set the Interrupter Moderation Register to
- * something other than the default (~1ms minimum between interrupts).
- * See section 5.5.1.2.
- */
for (i = 0; i < MAX_HC_SLOTS; i++)
xhci->devs[i] = NULL;
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 31/33] usb: xhci: Limit Stop Endpoint retries
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (29 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 30/33] usb: xhci: remove irrelevant comment Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 32/33] usb: xhci: Fix TD invalidation under pending Set TR Dequeue Mathias Nyman
2024-11-06 10:14 ` [PATCH 33/33] usb: xhci: Avoid queuing redundant Stop Endpoint commands Mathias Nyman
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, stable, Mathias Nyman
From: Michal Pecio <michal.pecio@gmail.com>
Some host controllers fail to atomically transition an endpoint to the
Running state on a doorbell ring and enter a hidden "Restarting" state,
which looks very much like Stopped, with the important difference that
it will spontaneously transition to Running anytime soon.
A Stop Endpoint command queued in the Restarting state typically fails
with Context State Error and the completion handler sees the Endpoint
Context State as either still Stopped or already Running. Even a case
of Halted was observed, when an error occurred right after the restart.
The Halted state is already recovered from by resetting the endpoint.
The Running state is handled by retrying Stop Endpoint.
The Stopped state was recognized as a problem on NEC controllers and
worked around also by retrying, because the endpoint soon restarts and
then stops for good. But there is a risk: the command may fail if the
endpoint is "stopped for good" already, and retries will fail forever.
The possibility of this was not realized at the time, but a number of
cases were discovered later and reproduced. Some proved difficult to
deal with, and it is outright impossible to predict if an endpoint may
fail to ever start at all due to a hardware bug. One such bug (albeit
on ASM3142, not on NEC) was found to be reliably triggered simply by
toggling an AX88179 NIC up/down in a tight loop for a few seconds.
An endless retries storm is quite nasty. Besides putting needless load
on the xHC and CPU, it causes URBs never to be given back, paralyzing
the device and connection/disconnection logic for the whole bus if the
device is unplugged. User processes waiting for URBs become unkillable,
drivers and kworker threads lock up and xhci_hcd cannot be reloaded.
For peace of mind, impose a timeout on Stop Endpoint retries in this
case. If they don't succeed in 100ms, consider the endpoint stopped
permanently for some reason and just give back the unlinked URBs. This
failure case is rare already and work is under way to make it rarer.
Start this work today by also handling one simple case of race with
Reset Endpoint, because it costs just two lines to implement.
Fixes: fd9d55d190c0 ("xhci: retry Stop Endpoint on buggy NEC controllers")
CC: stable@vger.kernel.org
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 | 28 ++++++++++++++++++++++++----
drivers/usb/host/xhci.c | 2 ++
drivers/usb/host/xhci.h | 1 +
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c9c0c4a7588a..dd23596ccd84 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -52,6 +52,7 @@
* endpoint rings; it generates events on the event ring for these.
*/
+#include <linux/jiffies.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/dma-mapping.h>
@@ -1158,16 +1159,35 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
return;
case EP_STATE_STOPPED:
/*
- * NEC uPD720200 sometimes sets this state and fails with
- * Context Error while continuing to process TRBs.
- * Be conservative and trust EP_CTX_STATE on other chips.
+ * Per xHCI 4.6.9, Stop Endpoint command on a Stopped
+ * EP is a Context State Error, and EP stays Stopped.
+ *
+ * But maybe it failed on Halted, and somebody ran Reset
+ * Endpoint later. EP state is now Stopped and EP_HALTED
+ * still set because Reset EP handler will run after us.
+ */
+ if (ep->ep_state & EP_HALTED)
+ break;
+ /*
+ * On some HCs EP state remains Stopped for some tens of
+ * us to a few ms or more after a doorbell ring, and any
+ * new Stop Endpoint fails without aborting the restart.
+ * This handler may run quickly enough to still see this
+ * Stopped state, but it will soon change to Running.
+ *
+ * Assume this bug on unexpected Stop Endpoint failures.
+ * Keep retrying until the EP starts and stops again, on
+ * chips where this is known to help. Wait for 100ms.
*/
if (!(xhci->quirks & XHCI_NEC_HOST))
break;
+ if (time_is_before_jiffies(ep->stop_time + msecs_to_jiffies(100)))
+ break;
fallthrough;
case EP_STATE_RUNNING:
/* Race, HW handled stop ep cmd before ep was running */
- xhci_dbg(xhci, "Stop ep completion ctx error, ep is running\n");
+ xhci_dbg(xhci, "Stop ep completion ctx error, ctx_state %d\n",
+ GET_EP_CTX_STATE(ep_ctx));
command = xhci_alloc_command(xhci, false, GFP_ATOMIC);
if (!command) {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index bc477cf99805..4977ada0a19e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -8,6 +8,7 @@
* Some code borrowed from the Linux EHCI driver.
*/
+#include <linux/jiffies.h>
#include <linux/pci.h>
#include <linux/iommu.h>
#include <linux/iopoll.h>
@@ -1764,6 +1765,7 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
ret = -ENOMEM;
goto done;
}
+ ep->stop_time = jiffies;
ep->ep_state |= EP_STOP_CMD_PENDING;
xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
ep_index, 0);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index a0e992c3db0d..6dd3138b2380 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -691,6 +691,7 @@ struct xhci_virt_ep {
/* Bandwidth checking storage */
struct xhci_bw_info bw_info;
struct list_head bw_endpoint_list;
+ unsigned long stop_time;
/* Isoch Frame ID checking storage */
int next_frame_id;
/* Use new Isoch TRB layout needed for extended TBC support */
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 32/33] usb: xhci: Fix TD invalidation under pending Set TR Dequeue
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (30 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 31/33] usb: xhci: Limit Stop Endpoint retries Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
2024-11-06 10:14 ` [PATCH 33/33] usb: xhci: Avoid queuing redundant Stop Endpoint commands Mathias Nyman
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, stable, Mathias Nyman
From: Michal Pecio <michal.pecio@gmail.com>
xhci_invalidate_cancelled_tds() may not work correctly if the hardware
is modifying endpoint or stream contexts at the same time by executing
a Set TR Dequeue command. And even if it worked, it would be unable to
queue Set TR Dequeue for the next stream, failing to clear xHC cache.
On stream endpoints, a chain of Set TR Dequeue commands may take some
time to execute and we may want to cancel more TDs during this time.
Currently this leads to Stop Endpoint completion handler calling this
function without testing for SET_DEQ_PENDING, which will trigger the
aforementioned problems when it happens.
On all endpoints, a halt condition causes Reset Endpoint to be queued
and an error status given to the class driver, which may unlink more
URBs in response. Stop Endpoint is queued and its handler may execute
concurrently with Set TR Dequeue queued by Reset Endpoint handler.
(Reset Endpoint handler calls this function too, but there seems to
be no possibility of it running concurrently with Set TR Dequeue).
Fix xhci_invalidate_cancelled_tds() to work correctly under a pending
Set TR Dequeue. Bail out of the function when SET_DEQ_PENDING is set,
then make the completion handler call the function again and also call
xhci_giveback_invalidated_tds(), which needs to be called next.
This seems to fix another potential bug, where the handler would call
xhci_invalidate_cancelled_tds(), which may clear some deferred TDs if
a sanity check fails, and the TDs wouldn't be given back promptly.
Said sanity check seems to be wrong and prone to false positives when
the endpoint halts, but fixing it is beyond the scope of this change,
besides ensuring that cleared TDs are given back properly.
Fixes: 5ceac4402f5d ("xhci: Handle TD clearing for multiple streams case")
CC: stable@vger.kernel.org
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 | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index dd23596ccd84..55be03be2374 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -980,6 +980,13 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
unsigned int slot_id = ep->vdev->slot_id;
int err;
+ /*
+ * This is not going to work if the hardware is changing its dequeue
+ * pointers as we look at them. Completion handler will call us later.
+ */
+ if (ep->ep_state & SET_DEQ_PENDING)
+ return 0;
+
xhci = ep->xhci;
list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) {
@@ -1367,7 +1374,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
struct xhci_slot_ctx *slot_ctx;
struct xhci_stream_ctx *stream_ctx;
struct xhci_td *td, *tmp_td;
- bool deferred = false;
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2]));
@@ -1471,8 +1477,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
xhci_dbg(ep->xhci, "%s: Giveback cancelled URB %p TD\n",
__func__, td->urb);
xhci_td_cleanup(ep->xhci, td, ep_ring, td->status);
- } else if (td->cancel_status == TD_CLEARING_CACHE_DEFERRED) {
- deferred = true;
} else {
xhci_dbg(ep->xhci, "%s: Keep cancelled URB %p TD as cancel_status is %d\n",
__func__, td->urb, td->cancel_status);
@@ -1483,11 +1487,15 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
ep->queued_deq_seg = NULL;
ep->queued_deq_ptr = NULL;
- if (deferred) {
- /* We have more streams to clear */
+ /* Check for deferred or newly cancelled TDs */
+ if (!list_empty(&ep->cancelled_td_list)) {
xhci_dbg(ep->xhci, "%s: Pending TDs to clear, continuing with invalidation\n",
__func__);
xhci_invalidate_cancelled_tds(ep);
+ /* Try to restart the endpoint if all is done */
+ ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
+ /* Start giving back any TDs invalidated above */
+ xhci_giveback_invalidated_tds(ep);
} else {
/* Restart any rings with pending URBs */
xhci_dbg(ep->xhci, "%s: All TDs cleared, ring doorbell\n", __func__);
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 33/33] usb: xhci: Avoid queuing redundant Stop Endpoint commands
2024-11-06 10:14 [PATCH 00/33] xhci features and fixes for usb-next Mathias Nyman
` (31 preceding siblings ...)
2024-11-06 10:14 ` [PATCH 32/33] usb: xhci: Fix TD invalidation under pending Set TR Dequeue Mathias Nyman
@ 2024-11-06 10:14 ` Mathias Nyman
32 siblings, 0 replies; 34+ messages in thread
From: Mathias Nyman @ 2024-11-06 10:14 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, stable, Mathias Nyman
From: Michal Pecio <michal.pecio@gmail.com>
Stop Endpoint command on an already stopped endpoint fails and may be
misinterpreted as a known hardware bug by the completion handler. This
results in an unnecessary delay with repeated retries of the command.
Avoid queuing this command when endpoint state flags indicate that it's
stopped or halted and the command will fail. If commands are pending on
the endpoint, their completion handlers will process cancelled TDs so
it's done. In case of waiting for external operations like clearing TT
buffer, the endpoint is stopped and cancelled TDs can be processed now.
This eliminates practically all unnecessary retries because an endpoint
with pending URBs is maintained in Running state by the driver, unless
aforementioned commands or other operations are pending on it. This is
guaranteed by xhci_ring_ep_doorbell() and by the fact that it is called
every time any of those operations completes.
The only known exceptions are hardware bugs (the endpoint never starts
at all) and Stream Protocol errors not associated with any TRB, which
cause an endpoint reset not followed by restart. Sounds like a bug.
Generally, these retries are only expected to happen when the endpoint
fails to start for unknown/no reason, which is a worse problem itself,
and fixing the bug eliminates the retries too.
All cases were tested and found to work as expected. SET_DEQ_PENDING
was produced by patching uvcvideo to unlink URBs in 100us intervals,
which then runs into this case very often. EP_HALTED was produced by
restarting 'cat /dev/ttyUSB0' on a serial dongle with broken cable.
EP_CLEARING_TT by the same, with the dongle on an external hub.
Fixes: fd9d55d190c0 ("xhci: retry Stop Endpoint on buggy NEC controllers")
CC: stable@vger.kernel.org
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 | 13 +++++++++++++
drivers/usb/host/xhci.c | 19 +++++++++++++++----
drivers/usb/host/xhci.h | 1 +
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 55be03be2374..4cf5363875c7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1076,6 +1076,19 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
return 0;
}
+/*
+ * Erase queued TDs from transfer ring(s) and give back those the xHC didn't
+ * stop on. If necessary, queue commands to move the xHC off cancelled TDs it
+ * stopped on. Those will be given back later when the commands complete.
+ *
+ * Call under xhci->lock on a stopped endpoint.
+ */
+void xhci_process_cancelled_tds(struct xhci_virt_ep *ep)
+{
+ xhci_invalidate_cancelled_tds(ep);
+ xhci_giveback_invalidated_tds(ep);
+}
+
/*
* Returns the TD the endpoint ring halted on.
* Only call for non-running rings without streams.
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4977ada0a19e..5ebde8cae4fc 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1756,10 +1756,21 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
}
}
- /* Queue a stop endpoint command, but only if this is
- * the first cancellation to be handled.
- */
- if (!(ep->ep_state & EP_STOP_CMD_PENDING)) {
+ /* These completion handlers will sort out cancelled TDs for us */
+ if (ep->ep_state & (EP_STOP_CMD_PENDING | EP_HALTED | SET_DEQ_PENDING)) {
+ xhci_dbg(xhci, "Not queuing Stop Endpoint on slot %d ep %d in state 0x%x\n",
+ urb->dev->slot_id, ep_index, ep->ep_state);
+ goto done;
+ }
+
+ /* In this case no commands are pending but the endpoint is stopped */
+ if (ep->ep_state & EP_CLEARING_TT) {
+ /* and cancelled TDs can be given back right away */
+ xhci_dbg(xhci, "Invalidating TDs instantly on slot %d ep %d in state 0x%x\n",
+ urb->dev->slot_id, ep_index, ep->ep_state);
+ xhci_process_cancelled_tds(ep);
+ } else {
+ /* Otherwise, queue a new Stop Endpoint command */
command = xhci_alloc_command(xhci, false, GFP_ATOMIC);
if (!command) {
ret = -ENOMEM;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 6dd3138b2380..4914f0a10cff 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1922,6 +1922,7 @@ void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring);
unsigned int count_trbs(u64 addr, u64 len);
int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
int suspend, gfp_t gfp_flags);
+void xhci_process_cancelled_tds(struct xhci_virt_ep *ep);
/* xHCI roothub code */
void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread