* [PATCH 01/19] xhci: pass port structure to tracing instead of port number
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 02/19] xhci: Add busnumber to port tracing Mathias Nyman
` (17 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
We want to trace other port structure members than just port number
so pass entire port structure as parameter instead of just port number.
Dig the port number from the port structure.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 4 ++--
drivers/usb/host/xhci-ring.c | 2 +-
drivers/usb/host/xhci-trace.h | 18 +++++++++---------
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0df5d807a77e..0980ade2a234 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1262,7 +1262,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
retval = -ENODEV;
break;
}
- trace_xhci_get_port_status(wIndex, temp);
+ trace_xhci_get_port_status(port, temp);
status = xhci_get_port_status(hcd, bus_state, wIndex, temp,
&flags);
if (status == 0xffffffff)
@@ -1687,7 +1687,7 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
retval = -ENODEV;
break;
}
- trace_xhci_hub_status_data(i, temp);
+ trace_xhci_hub_status_data(ports[i], temp);
if ((temp & mask) != 0 ||
(bus_state->port_c_suspend & 1 << i) ||
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3e5dc0723a8f..48daeb4b4a46 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1906,7 +1906,7 @@ static void handle_port_status(struct xhci_hcd *xhci,
xhci_dbg(xhci, "Port change event, %d-%d, id %d, portsc: 0x%x\n",
hcd->self.busnum, hcd_portnum + 1, port_id, portsc);
- trace_xhci_handle_port_status(hcd_portnum, portsc);
+ trace_xhci_handle_port_status(port, portsc);
if (hcd->state == HC_STATE_SUSPENDED) {
xhci_dbg(xhci, "resume root hub\n");
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index d6b32f2ad90e..2208eda1ff27 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -509,14 +509,14 @@ DEFINE_EVENT(xhci_log_ring, xhci_inc_deq,
);
DECLARE_EVENT_CLASS(xhci_log_portsc,
- TP_PROTO(u32 portnum, u32 portsc),
- TP_ARGS(portnum, portsc),
+ TP_PROTO(struct xhci_port *port, u32 portsc),
+ TP_ARGS(port, portsc),
TP_STRUCT__entry(
__field(u32, portnum)
__field(u32, portsc)
),
TP_fast_assign(
- __entry->portnum = portnum;
+ __entry->portnum = port->hcd_portnum;
__entry->portsc = portsc;
),
TP_printk("port-%d: %s",
@@ -526,18 +526,18 @@ DECLARE_EVENT_CLASS(xhci_log_portsc,
);
DEFINE_EVENT(xhci_log_portsc, xhci_handle_port_status,
- TP_PROTO(u32 portnum, u32 portsc),
- TP_ARGS(portnum, portsc)
+ TP_PROTO(struct xhci_port *port, u32 portsc),
+ TP_ARGS(port, portsc)
);
DEFINE_EVENT(xhci_log_portsc, xhci_get_port_status,
- TP_PROTO(u32 portnum, u32 portsc),
- TP_ARGS(portnum, portsc)
+ TP_PROTO(struct xhci_port *port, u32 portsc),
+ TP_ARGS(port, portsc)
);
DEFINE_EVENT(xhci_log_portsc, xhci_hub_status_data,
- TP_PROTO(u32 portnum, u32 portsc),
- TP_ARGS(portnum, portsc)
+ TP_PROTO(struct xhci_port *port, u32 portsc),
+ TP_ARGS(port, portsc)
);
DECLARE_EVENT_CLASS(xhci_log_doorbell,
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/19] xhci: Add busnumber to port tracing
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
2023-10-19 10:29 ` [PATCH 01/19] xhci: pass port structure to tracing instead of port number Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 03/19] xhci: expand next_trb() helper to support more ring types Mathias Nyman
` (16 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
With several xhci controllers active at the same time its hard to
keep track of ports without knowing bus number
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-trace.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 2208eda1ff27..ac47b1c0544a 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -512,14 +512,17 @@ DECLARE_EVENT_CLASS(xhci_log_portsc,
TP_PROTO(struct xhci_port *port, u32 portsc),
TP_ARGS(port, portsc),
TP_STRUCT__entry(
+ __field(u32, busnum)
__field(u32, portnum)
__field(u32, portsc)
),
TP_fast_assign(
+ __entry->busnum = port->rhub->hcd->self.busnum;
__entry->portnum = port->hcd_portnum;
__entry->portsc = portsc;
),
- TP_printk("port-%d: %s",
+ TP_printk("port %d-%d: %s",
+ __entry->busnum,
__entry->portnum,
xhci_decode_portsc(__get_buf(XHCI_MSG_MAX), __entry->portsc)
)
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/19] xhci: expand next_trb() helper to support more ring types
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
2023-10-19 10:29 ` [PATCH 01/19] xhci: pass port structure to tracing instead of port number Mathias Nyman
2023-10-19 10:29 ` [PATCH 02/19] xhci: Add busnumber to port tracing Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 04/19] xhci: Set DESI bits in ERDP register correctly Mathias Nyman
` (15 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
The next_trb() helper relies on a link TRB at the end of a ring segment
to know a segment ends. This works well with transfer rings that use
link trbs, but not with event rings.
Event rings segments are always filled by host to segment size
before moving to next segment. It does not use link TRBs
Check for both link trb and full segment in next_trb() helper to
support event rings.
Useful if several interrupters with several event rings are supported.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 48daeb4b4a46..17e37c2a14aa 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -144,7 +144,7 @@ static void next_trb(struct xhci_hcd *xhci,
struct xhci_segment **seg,
union xhci_trb **trb)
{
- if (trb_is_link(*trb)) {
+ if (trb_is_link(*trb) || last_trb_on_seg(*seg, *trb)) {
*seg = (*seg)->next;
*trb = ((*seg)->trbs);
} else {
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/19] xhci: Set DESI bits in ERDP register correctly
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (2 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 03/19] xhci: expand next_trb() helper to support more ring types Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 05/19] xhci: Use more than one Event Ring segment Mathias Nyman
` (14 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
When using more than one Event Ring segment (ERSTSZ > 1), software shall
set the DESI bits in the ERDP register to the number of the segment to
which the upper ERDP bits are pointing. The xHC may use the DESI bits
as a shortcut to determine whether it needs to check for an Event Ring
Full condition: If it's enqueueing events in a different segment, it
need not compare its internal Enqueue Pointer with the Dequeue Pointer
in the upper bits of the ERDP register (sec 5.5.2.3.3).
Not setting the DESI bits correctly can result in the xHC enqueueing
events past the Dequeue Pointer. On Renesas uPD720201 host controllers,
incorrect DESI bits cause an interrupt storm. For comparison, VIA VL805
host controllers do not exhibit such problems. Perhaps they do not take
advantage of the optimization afforded by the DESI bits.
To fix the issue, assign the segment number to each struct xhci_segment
in xhci_segment_alloc(). When advancing the Dequeue Pointer in
xhci_update_erst_dequeue(), write the segment number to the DESI bits.
On driver probe, set the DESI bits to zero in xhci_set_hc_event_deq() as
processing starts in segment 0. Likewise on driver teardown, clear the
DESI bits to zero in xhci_free_interrupter() when clearing the upper
bits of the ERDP register. Previously those functions (incorrectly)
treated the DESI bits as if they're declared RsvdP.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 25 +++++++++++--------------
drivers/usb/host/xhci-ring.c | 2 +-
drivers/usb/host/xhci.h | 1 +
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 0a37f0d511cf..17c9942010c8 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -29,6 +29,7 @@
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;
@@ -60,6 +61,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
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;
@@ -324,6 +326,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
{
struct xhci_segment *prev;
+ unsigned int num = 0;
bool chain_links;
/* Set chain bit for 0.95 hosts, and for isoc rings on AMD 0.96 host */
@@ -331,16 +334,17 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
(type == TYPE_ISOC &&
(xhci->quirks & XHCI_AMD_0x96_HOST)));
- prev = xhci_segment_alloc(xhci, cycle_state, max_packet, flags);
+ prev = xhci_segment_alloc(xhci, cycle_state, max_packet, num, flags);
if (!prev)
return -ENOMEM;
- num_segs--;
+ num++;
*first = prev;
- while (num_segs > 0) {
+ while (num < num_segs) {
struct xhci_segment *next;
- next = xhci_segment_alloc(xhci, cycle_state, max_packet, flags);
+ next = xhci_segment_alloc(xhci, cycle_state, max_packet, num,
+ flags);
if (!next) {
prev = *first;
while (prev) {
@@ -353,7 +357,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
xhci_link_segments(prev, next, type, chain_links);
prev = next;
- num_segs--;
+ num++;
}
xhci_link_segments(prev, *first, type, chain_links);
*last = prev;
@@ -1801,7 +1805,6 @@ xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
{
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
size_t erst_size;
- u64 tmp64;
u32 tmp;
if (!ir)
@@ -1824,9 +1827,7 @@ xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
tmp &= ERST_SIZE_MASK;
writel(tmp, &ir->ir_set->erst_size);
- tmp64 = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
- tmp64 &= (u64) ERST_PTR_MASK;
- xhci_write_64(xhci, tmp64, &ir->ir_set->erst_dequeue);
+ xhci_write_64(xhci, ERST_EHB, &ir->ir_set->erst_dequeue);
}
/* free interrrupter event ring */
@@ -1933,7 +1934,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
static void xhci_set_hc_event_deq(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
{
- u64 temp;
dma_addr_t deq;
deq = xhci_trb_virt_to_dma(ir->event_ring->deq_seg,
@@ -1941,15 +1941,12 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci, struct xhci_interrupter
if (!deq)
xhci_warn(xhci, "WARN something wrong with SW event ring dequeue ptr.\n");
/* Update HC event ring dequeue pointer */
- temp = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
- temp &= ERST_PTR_MASK;
/* Don't clear the EHB bit (which is RW1C) because
* there might be more events to service.
*/
- temp &= ~ERST_EHB;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"// Write event ring dequeue pointer, preserving EHB bit");
- xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK) | temp,
+ xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK),
&ir->ir_set->erst_dequeue);
}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 17e37c2a14aa..173c2068eb64 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3018,7 +3018,7 @@ static void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
return;
/* Update HC event ring dequeue pointer */
- temp_64 &= ERST_DESI_MASK;
+ temp_64 = ir->event_ring->deq_seg->num & ERST_DESI_MASK;
temp_64 |= ((u64) deq & (u64) ~ERST_PTR_MASK);
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8342076cb309..5ddca9280ec3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1518,6 +1518,7 @@ struct xhci_segment {
union xhci_trb *trbs;
/* private to HCD */
struct xhci_segment *next;
+ unsigned int num;
dma_addr_t dma;
/* Max packet sized bounce buffer for td-fragmant alignment */
dma_addr_t bounce_dma;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/19] xhci: Use more than one Event Ring segment
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (3 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 04/19] xhci: Set DESI bits in ERDP register correctly Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 06/19] xhci: Adjust segment numbers after ring expansion Mathias Nyman
` (13 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Jonathan Bell, Lukas Wunner, Mathias Nyman
From: Jonathan Bell <jonathan@raspberrypi.com>
Users have reported log spam created by "Event Ring Full" xHC event
TRBs. These are caused by interrupt latency in conjunction with a very
busy set of devices on the bus. The errors are benign, but throughput
will suffer as the xHC will pause processing of transfers until the
Event Ring is drained by the kernel.
Commit dc0ffbea5729 ("usb: host: xhci: update event ring dequeue pointer
on purpose") mitigated the issue by advancing the Event Ring Dequeue
Pointer already after half a segment has been processed. Nevertheless,
providing a larger Event Ring would be useful to cope with load peaks.
Expand the number of event TRB slots available by increasing the number
of Event Ring segments in the ERST.
Controllers have a hardware-defined limit as to the number of ERST
entries they can process, but with up to 32k it can be excessively high
(sec 5.3.4). So cap the actual number at 2 (configurable through the
ERST_MAX_SEGS macro), which seems like a reasonable quantity. It is
supported by any xHC because the limit in the HCSPARAMS2 register is
defined as a power of 2. Renesas uPD720201 and VIA VL805 controllers
do not support more than 2 ERST entries.
An alternative to increasing the number of Event Ring segments would be
an increase of the segment size. But that requires allocating multiple
contiguous pages, which may be impossible if memory is fragmented.
Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 10 +++++++---
drivers/usb/host/xhci.h | 5 +++--
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 17c9942010c8..a8a8fc2cc4a5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2235,14 +2235,18 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, gfp_t flags)
{
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
struct xhci_interrupter *ir;
+ unsigned int num_segs;
int ret;
ir = kzalloc_node(sizeof(*ir), flags, dev_to_node(dev));
if (!ir)
return NULL;
- ir->event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, 1, TYPE_EVENT,
- 0, flags);
+ num_segs = min_t(unsigned int, 1 << HCS_ERST_MAX(xhci->hcs_params2),
+ ERST_MAX_SEGS);
+
+ ir->event_ring = xhci_ring_alloc(xhci, num_segs, 1, TYPE_EVENT, 0,
+ flags);
if (!ir->event_ring) {
xhci_warn(xhci, "Failed to allocate interrupter event ring\n");
kfree(ir);
@@ -2278,7 +2282,7 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
/* set ERST count with the number of entries in the segment table */
erst_size = readl(&ir->ir_set->erst_size);
erst_size &= ERST_SIZE_MASK;
- erst_size |= ERST_NUM_SEGS;
+ erst_size |= ir->event_ring->num_segs;
writel(erst_size, &ir->ir_set->erst_size);
erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 5ddca9280ec3..41820fd97c00 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1647,8 +1647,9 @@ struct urb_priv {
* Each segment table entry is 4*32bits long. 1K seems like an ok size:
* (1K bytes * 8bytes/bit) / (4*32 bits) = 64 segment entries in the table,
* meaning 64 ring segments.
- * Initial allocated size of the ERST, in number of entries */
-#define ERST_NUM_SEGS 1
+ * Reasonable limit for number of Event Ring segments (spec allows 32k)
+ */
+#define ERST_MAX_SEGS 2
/* Poll every 60 seconds */
#define POLL_TIMEOUT 60
/* Stop endpoint command timeout (secs) for URB cancellation watchdog timer */
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/19] xhci: Adjust segment numbers after ring expansion
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (4 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 05/19] xhci: Use more than one Event Ring segment Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 07/19] xhci: Update last segment pointer after Event Ring expansion Mathias Nyman
` (12 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
Initial xhci_ring allocation has just been amended to assign a
monotonically increasing number to each ring segment.
However rings may be expanded after initial allocation.
So number newly inserted segments starting from the preceding segment in
the ring and renumber all segments succeeding the newly inserted ones.
This is not a fix because ring expansion currently isn't done on the
Event Ring and that's the only ring type using the segment number.
It's just in preparation for when either Event Ring expansion is added
or when other ring types start making use of the segment number.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index a8a8fc2cc4a5..1c0f5263cf81 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -130,7 +130,7 @@ 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)
{
- struct xhci_segment *next;
+ struct xhci_segment *next, *seg;
bool chain_links;
if (!ring || !first || !last)
@@ -153,6 +153,9 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring,
|= cpu_to_le32(LINK_TOGGLE);
ring->last_seg = last;
}
+
+ for (seg = last; seg != ring->last_seg; seg = seg->next)
+ seg->next->num = seg->num + 1;
}
/*
@@ -322,11 +325,11 @@ void xhci_initialize_ring_info(struct xhci_ring *ring,
/* 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, unsigned int cycle_state,
- enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
+ unsigned int num_segs, unsigned int num,
+ unsigned int cycle_state, enum xhci_ring_type type,
+ unsigned int max_packet, gfp_t flags)
{
struct xhci_segment *prev;
- unsigned int num = 0;
bool chain_links;
/* Set chain bit for 0.95 hosts, and for isoc rings on AMD 0.96 host */
@@ -392,7 +395,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,
+ &ring->last_seg, num_segs, 0, cycle_state, type,
max_packet, flags);
if (ret)
goto fail;
@@ -432,7 +435,8 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
int ret;
ret = xhci_alloc_segments_for_ring(xhci, &first, &last,
- num_new_segs, ring->cycle_state, ring->type,
+ num_new_segs, ring->enq_seg->num + 1,
+ ring->cycle_state, ring->type,
ring->bounce_buf_len, flags);
if (ret)
return -ENOMEM;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/19] xhci: Update last segment pointer after Event Ring expansion
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (5 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 06/19] xhci: Adjust segment numbers after ring expansion Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 08/19] xhci: Expose segment numbers in debugfs Mathias Nyman
` (11 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
When expanding a ring at its "end", ring->last_seg needs to be updated
for Event Rings as well, not just for all the other ring types.
This is not a fix because ring expansion currently isn't done on the
Event Ring. It's just in preparation for when it's added.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1c0f5263cf81..d4123e6f2549 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -146,11 +146,13 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring,
xhci_link_segments(last, next, ring->type, chain_links);
ring->num_segs += num_segs;
- if (ring->type != TYPE_EVENT && ring->enq_seg == ring->last_seg) {
- ring->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
- &= ~cpu_to_le32(LINK_TOGGLE);
- last->trbs[TRBS_PER_SEGMENT-1].link.control
- |= cpu_to_le32(LINK_TOGGLE);
+ if (ring->enq_seg == ring->last_seg) {
+ if (ring->type != TYPE_EVENT) {
+ ring->last_seg->trbs[TRBS_PER_SEGMENT-1].link.control
+ &= ~cpu_to_le32(LINK_TOGGLE);
+ last->trbs[TRBS_PER_SEGMENT-1].link.control
+ |= cpu_to_le32(LINK_TOGGLE);
+ }
ring->last_seg = last;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 08/19] xhci: Expose segment numbers in debugfs
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (6 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 07/19] xhci: Update last segment pointer after Event Ring expansion Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 09/19] xhci: Clean up ERST_PTR_MASK inversion Mathias Nyman
` (10 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
Ring segments have just been amended with a monotonically increasing
number.
To allow developers to inspect the segment numbers and ensure
correctness in particular after ring expansion, expose them in each
ring's "trbs" file in debugfs.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 99baa60ef50f..6d142cd61bd6 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -204,7 +204,7 @@ static void xhci_ring_dump_segment(struct seq_file *s,
for (i = 0; i < TRBS_PER_SEGMENT; i++) {
trb = &seg->trbs[i];
dma = seg->dma + i * sizeof(*trb);
- seq_printf(s, "%pad: %s\n", &dma,
+ seq_printf(s, "%2u %pad: %s\n", seg->num, &dma,
xhci_decode_trb(str, XHCI_MSG_MAX, le32_to_cpu(trb->generic.field[0]),
le32_to_cpu(trb->generic.field[1]),
le32_to_cpu(trb->generic.field[2]),
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/19] xhci: Clean up ERST_PTR_MASK inversion
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (7 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 08/19] xhci: Expose segment numbers in debugfs Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 10/19] xhci: Clean up stale comment on ERST_SIZE macro Mathias Nyman
` (9 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
Mathias notes that the ERST_PTR_MASK macro is named as if it's masking
the Event Ring Dequeue Pointer in the ERDP register, but in actuality
it's masking the inverse.
Invert the macro's value for clarity.
Migrate it to the modern GENMASK_ULL() syntax to avoid u64 casts.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 3 +--
drivers/usb/host/xhci-ring.c | 5 ++---
drivers/usb/host/xhci.c | 2 +-
drivers/usb/host/xhci.h | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d4123e6f2549..b133817ad180 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1952,8 +1952,7 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci, struct xhci_interrupter
*/
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"// Write event ring dequeue pointer, preserving EHB bit");
- xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK),
- &ir->ir_set->erst_dequeue);
+ xhci_write_64(xhci, deq & ERST_PTR_MASK, &ir->ir_set->erst_dequeue);
}
static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 173c2068eb64..17404b14d1bf 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3013,13 +3013,12 @@ static void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
* Per 4.9.4, Software writes to the ERDP register shall
* always advance the Event Ring Dequeue Pointer value.
*/
- if ((temp_64 & (u64) ~ERST_PTR_MASK) ==
- ((u64) deq & (u64) ~ERST_PTR_MASK))
+ if ((temp_64 & ERST_PTR_MASK) == (deq & ERST_PTR_MASK))
return;
/* Update HC event ring dequeue pointer */
temp_64 = ir->event_ring->deq_seg->num & ERST_DESI_MASK;
- temp_64 |= ((u64) deq & (u64) ~ERST_PTR_MASK);
+ temp_64 |= deq & ERST_PTR_MASK;
}
/* Clear the event handler busy flag (RW1C) */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e1b1b64a0723..68920cb96044 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -520,7 +520,7 @@ int xhci_run(struct usb_hcd *hcd)
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "xhci_run");
temp_64 = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
- temp_64 &= ~ERST_PTR_MASK;
+ temp_64 &= ERST_PTR_MASK;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"ERST deq = 64'h%0lx", (long unsigned int) temp_64);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 41820fd97c00..f97896740c3f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -525,7 +525,7 @@ struct xhci_intr_reg {
* a work queue (or delayed service routine)?
*/
#define ERST_EHB (1 << 3)
-#define ERST_PTR_MASK (0xf)
+#define ERST_PTR_MASK (GENMASK_ULL(63, 4))
/**
* struct xhci_run_regs
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/19] xhci: Clean up stale comment on ERST_SIZE macro
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (8 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 09/19] xhci: Clean up ERST_PTR_MASK inversion Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 11/19] xhci: Clean up xhci_{alloc,free}_erst() declarations Mathias Nyman
` (8 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
Commit ebd88cf50729 ("xhci: Remove unused defines for ERST_SIZE and
ERST_ENTRIES") removed the ERST_SIZE macro but retained a code comment
explaining the quantity chosen in the macro.
Remove the code comment as well.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f97896740c3f..1453fcab33d9 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1643,12 +1643,7 @@ struct urb_priv {
struct xhci_td td[];
};
-/*
- * Each segment table entry is 4*32bits long. 1K seems like an ok size:
- * (1K bytes * 8bytes/bit) / (4*32 bits) = 64 segment entries in the table,
- * meaning 64 ring segments.
- * Reasonable limit for number of Event Ring segments (spec allows 32k)
- */
+/* Reasonable limit for number of Event Ring segments (spec allows 32k) */
#define ERST_MAX_SEGS 2
/* Poll every 60 seconds */
#define POLL_TIMEOUT 60
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/19] xhci: Clean up xhci_{alloc,free}_erst() declarations
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (9 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 10/19] xhci: Clean up stale comment on ERST_SIZE macro Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 12/19] xhci: simplify event ring dequeue tracking for transfer events Mathias Nyman
` (7 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Lukas Wunner, Mathias Nyman
From: Lukas Wunner <lukas@wunner.de>
xhci_alloc_erst() has global scope even though it's only used in
xhci-mem.c. Declare it static.
xhci_free_erst() was removed by commit b17a57f89f69 ("xhci: Refactor
interrupter code for initial multi interrupter support."), but a
declaration in xhci.h still remains. Drop it.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 2 +-
drivers/usb/host/xhci.h | 5 -----
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b133817ad180..4d0b1c0e61a8 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1776,7 +1776,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
kfree(command);
}
-int xhci_alloc_erst(struct xhci_hcd *xhci,
+static int xhci_alloc_erst(struct xhci_hcd *xhci,
struct xhci_ring *evt_ring,
struct xhci_erst *erst,
gfp_t flags)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 1453fcab33d9..813d55468c00 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2048,13 +2048,8 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
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);
-int xhci_alloc_erst(struct xhci_hcd *xhci,
- struct xhci_ring *evt_ring,
- struct xhci_erst *erst,
- gfp_t flags);
void xhci_initialize_ring_info(struct xhci_ring *ring,
unsigned int cycle_state);
-void xhci_free_erst(struct xhci_hcd *xhci, struct xhci_erst *erst);
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] 20+ messages in thread* [PATCH 12/19] xhci: simplify event ring dequeue tracking for transfer events
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (10 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 11/19] xhci: Clean up xhci_{alloc,free}_erst() declarations Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 13/19] xhci: Simplify event ring dequeue pointer update for port change events Mathias Nyman
` (6 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
No matter what type of event we receive we want to increase the event
ring dequeue pointer one step for every event that is handled.
For unknown reasons the event ring dequeue increase is done inside the
transfer event handler and port event handler.
As the transfer event handler got more complex and can now loop through
several transfer TRBs on a transfer ring, there were additinal checks
added to avoid increasing event ring dequeue more than one step.
No need for elaborate checks to avoid increasing event ring dequeue
in case the transfer event handler goes through a loop.
Just increasing the event ring dequeue outside the transfer event
handler.
End goal is to increase event ring dequeue in just one place.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 17404b14d1bf..7aa6f132835b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2884,13 +2884,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
trb_comp_code != COMP_MISSED_SERVICE_ERROR &&
trb_comp_code != COMP_NO_PING_RESPONSE_ERROR;
- /*
- * Do not update event ring dequeue pointer if we're in a loop
- * processing missed tds.
- */
- if (!handling_skipped_tds)
- inc_deq(xhci, ir->event_ring);
-
/*
* If ep->skip is set, it means there are missed tds on the
* endpoint ring need to take care of.
@@ -2924,7 +2917,6 @@ static int xhci_handle_event(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
union xhci_trb *event;
int update_ptrs = 1;
u32 trb_type;
- int ret;
/* Event ring hasn't been allocated yet. */
if (!ir || !ir->event_ring || !ir->event_ring->dequeue) {
@@ -2957,9 +2949,7 @@ static int xhci_handle_event(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
update_ptrs = 0;
break;
case TRB_TRANSFER:
- ret = handle_tx_event(xhci, ir, &event->trans_event);
- if (ret >= 0)
- update_ptrs = 0;
+ handle_tx_event(xhci, ir, &event->trans_event);
break;
case TRB_DEV_NOTE:
handle_device_notification(xhci, event);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 13/19] xhci: Simplify event ring dequeue pointer update for port change events
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (11 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 12/19] xhci: simplify event ring dequeue tracking for transfer events Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 14/19] xhci: Loosen RPM as default policy to cover for AMD xHC 1.1 Mathias Nyman
` (5 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
Increase the event ring dequeue pointer for port change events in the
same way as other event types. No need to handle it separately.
This only touches the driver side tracking of event ring dequeue.
Note: this does move forward the event ring dequeue increase for port
change events a bit.
Previously the dequeue was increased before temporarily dropping
the xhci lock while kicking roothub polling.
Now dequeue is increased after re-aquiring the lock.
This should not matter as event ring dequeue is not touched at all by
hub thread. It's only touched in xhci interrupt handler.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7aa6f132835b..73057f01e4aa 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1879,7 +1879,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
if ((port_id <= 0) || (port_id > max_ports)) {
xhci_warn(xhci, "Port change event with invalid port ID %d\n",
port_id);
- inc_deq(xhci, ir->event_ring);
return;
}
@@ -2007,8 +2006,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
}
cleanup:
- /* Update event ring dequeue pointer before dropping the lock */
- inc_deq(xhci, ir->event_ring);
/* Don't make the USB core poll the roothub if we got a bad port status
* change event. Besides, at that point we can't tell which roothub
@@ -2915,7 +2912,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
static int xhci_handle_event(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
{
union xhci_trb *event;
- int update_ptrs = 1;
u32 trb_type;
/* Event ring hasn't been allocated yet. */
@@ -2946,7 +2942,6 @@ static int xhci_handle_event(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
break;
case TRB_PORT_STATUS:
handle_port_status(xhci, ir, event);
- update_ptrs = 0;
break;
case TRB_TRANSFER:
handle_tx_event(xhci, ir, &event->trans_event);
@@ -2969,9 +2964,8 @@ static int xhci_handle_event(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
return 0;
}
- if (update_ptrs)
- /* Update SW event ring dequeue pointer */
- inc_deq(xhci, ir->event_ring);
+ /* Update SW event ring dequeue pointer */
+ inc_deq(xhci, ir->event_ring);
/* Are there more items on the event ring? Caller will call us again to
* check.
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/19] xhci: Loosen RPM as default policy to cover for AMD xHC 1.1
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (12 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 13/19] xhci: Simplify event ring dequeue pointer update for port change events Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 15/19] xhci: Enable RPM on controllers that support low-power states Mathias Nyman
` (4 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Basavaraj Natikar, Mario Limonciello, Mathias Nyman
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
The AMD USB host controller (1022:43f7) isn't going into PCI D3 by default
without anything connected. This is because the policy that was introduced
by commit a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all
xHC 1.2 or later devices") only covered 1.2 or later.
The 1.1 specification also has the same requirement as the 1.2
specification for D3 support. So expand the runtime PM as default policy
to all AMD 1.1 devices as well.
Fixes: a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all xHC 1.2 or later devices")
Link: https://composter.com.ua/documents/xHCI_Specification_for_USB.pdf
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b9ae5c2a2527..bde43cef8846 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -535,6 +535,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
/* xHC spec requires PCI devices to support D3hot and D3cold */
if (xhci->hci_version >= 0x120)
xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
+ else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
+ xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
if (xhci->quirks & XHCI_RESET_ON_RESUME)
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 15/19] xhci: Enable RPM on controllers that support low-power states
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (13 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 14/19] xhci: Loosen RPM as default policy to cover for AMD xHC 1.1 Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 16/19] xhci: split free interrupter into separate remove and free parts Mathias Nyman
` (3 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Basavaraj Natikar, Mario Limonciello, Mathias Nyman
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Use the low-power states of the underlying platform to enable runtime PM.
If the platform doesn't support runtime D3, then enabling default RPM will
result in the controller malfunctioning, as in the case of hotplug devices
not being detected because of a failed interrupt generation.
Cc: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index bde43cef8846..95ed9404f6f8 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -695,7 +695,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
pm_runtime_put_noidle(&dev->dev);
- if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
+ if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
+ pm_runtime_forbid(&dev->dev);
+ else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
pm_runtime_allow(&dev->dev);
dma_set_max_seg_size(&dev->dev, UINT_MAX);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 16/19] xhci: split free interrupter into separate remove and free parts
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (14 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 15/19] xhci: Enable RPM on controllers that support low-power states Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 17/19] usb: xhci: Implement xhci_handshake_check_state() helper Mathias Nyman
` (2 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
The current function that both removes and frees an interrupter isn't
optimal when using several interrupters. The array of interrupters need
to be protected with a lock while removing interrupters, but the default
xhci spin lock can't be used while freeing the interrupters event ring
segment table as dma_free_coherent() should be called with IRQs enabled.
There is no need to free the interrupter under the lock, so split this
code into separate unlocked free part, and a lock protected remove part.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 4d0b1c0e61a8..62116586848b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1807,22 +1807,13 @@ static int xhci_alloc_erst(struct xhci_hcd *xhci,
}
static void
-xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
+xhci_remove_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
{
- struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
- size_t erst_size;
u32 tmp;
if (!ir)
return;
- erst_size = sizeof(struct xhci_erst_entry) * ir->erst.num_entries;
- if (ir->erst.entries)
- dma_free_coherent(dev, erst_size,
- ir->erst.entries,
- ir->erst.erst_dma_addr);
- ir->erst.entries = NULL;
-
/*
* Clean out interrupter registers except ERSTBA. Clearing either the
* low or high 32 bits of ERSTBA immediately causes the controller to
@@ -1835,10 +1826,28 @@ xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
xhci_write_64(xhci, ERST_EHB, &ir->ir_set->erst_dequeue);
}
+}
+
+static void
+xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
+{
+ struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+ size_t erst_size;
+
+ if (!ir)
+ return;
+
+ erst_size = sizeof(struct xhci_erst_entry) * ir->erst.num_entries;
+ if (ir->erst.entries)
+ dma_free_coherent(dev, erst_size,
+ ir->erst.entries,
+ ir->erst.erst_dma_addr);
+ ir->erst.entries = NULL;
- /* free interrrupter event ring */
+ /* free interrupter event ring */
if (ir->event_ring)
xhci_ring_free(xhci, ir->event_ring);
+
ir->event_ring = NULL;
kfree(ir);
@@ -1851,6 +1860,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
cancel_delayed_work_sync(&xhci->cmd_timer);
+ xhci_remove_interrupter(xhci, xhci->interrupter);
xhci_free_interrupter(xhci, xhci->interrupter);
xhci->interrupter = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event ring");
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 17/19] usb: xhci: Implement xhci_handshake_check_state() helper
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (15 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 16/19] xhci: split free interrupter into separate remove and free parts Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 18/19] usb: host: xhci-plat: fix possible kernel oops while resuming Mathias Nyman
2023-10-19 10:29 ` [PATCH 19/19] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present Mathias Nyman
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Udipto Goswami, Mathias Nyman
From: Udipto Goswami <quic_ugoswami@quicinc.com>
In some situations where xhci removal happens parallel to xhci_handshake,
we encounter a scenario where the xhci_handshake can't succeed, and it
polls until timeout.
If xhci_handshake runs until timeout it can on some platforms result in
a long wait which might lead to a watchdog timeout.
Add a helper that checks xhci status during the handshake, and exits if
set state is entered. Use this helper in places where xhci_handshake is
called unlocked and has a long timeout. For example xhci command timeout
and xhci reset.
[commit message and code comment rewording -Mathias]
Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 5 +++--
drivers/usb/host/xhci.c | 26 +++++++++++++++++++++++++-
drivers/usb/host/xhci.h | 2 ++
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 73057f01e4aa..f3b5e6345858 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -450,8 +450,9 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
* In the future we should distinguish between -ENODEV and -ETIMEDOUT
* and try to recover a -ETIMEDOUT with a host controller reset.
*/
- ret = xhci_handshake(&xhci->op_regs->cmd_ring,
- CMD_RING_RUNNING, 0, 5 * 1000 * 1000);
+ ret = xhci_handshake_check_state(xhci, &xhci->op_regs->cmd_ring,
+ CMD_RING_RUNNING, 0, 5 * 1000 * 1000,
+ XHCI_STATE_REMOVING);
if (ret < 0) {
xhci_err(xhci, "Abort failed to stop command ring: %d\n", ret);
xhci_halt(xhci);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 68920cb96044..119cbe2a3d65 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -81,6 +81,29 @@ int xhci_handshake(void __iomem *ptr, u32 mask, u32 done, u64 timeout_us)
return ret;
}
+/*
+ * xhci_handshake_check_state - same as xhci_handshake but takes an additional
+ * exit_state parameter, and bails out with an error immediately when xhc_state
+ * has exit_state flag set.
+ */
+int xhci_handshake_check_state(struct xhci_hcd *xhci, void __iomem *ptr,
+ u32 mask, u32 done, int usec, unsigned int exit_state)
+{
+ u32 result;
+ int ret;
+
+ ret = readl_poll_timeout_atomic(ptr, result,
+ (result & mask) == done ||
+ result == U32_MAX ||
+ xhci->xhc_state & exit_state,
+ 1, usec);
+
+ if (result == U32_MAX || xhci->xhc_state & exit_state)
+ return -ENODEV;
+
+ return ret;
+}
+
/*
* Disable interrupts and begin the xHCI halting process.
*/
@@ -201,7 +224,8 @@ int xhci_reset(struct xhci_hcd *xhci, u64 timeout_us)
if (xhci->quirks & XHCI_INTEL_HOST)
udelay(1000);
- ret = xhci_handshake(&xhci->op_regs->command, CMD_RESET, 0, timeout_us);
+ ret = xhci_handshake_check_state(xhci, &xhci->op_regs->command,
+ CMD_RESET, 0, timeout_us, XHCI_STATE_REMOVING);
if (ret)
return ret;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 813d55468c00..5768ce804caa 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2084,6 +2084,8 @@ void xhci_free_container_ctx(struct xhci_hcd *xhci,
/* xHCI host controller glue */
typedef void (*xhci_get_quirks_t)(struct device *, struct xhci_hcd *);
int xhci_handshake(void __iomem *ptr, u32 mask, u32 done, u64 timeout_us);
+int xhci_handshake_check_state(struct xhci_hcd *xhci, void __iomem *ptr,
+ u32 mask, u32 done, int usec, unsigned int exit_state);
void xhci_quiesce(struct xhci_hcd *xhci);
int xhci_halt(struct xhci_hcd *xhci);
int xhci_start(struct xhci_hcd *xhci);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 18/19] usb: host: xhci-plat: fix possible kernel oops while resuming
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (16 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 17/19] usb: xhci: Implement xhci_handshake_check_state() helper Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
2023-10-19 10:29 ` [PATCH 19/19] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present Mathias Nyman
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Sergey Shtylyov, Mathias Nyman
From: Sergey Shtylyov <s.shtylyov@omp.ru>
If this driver enables the xHC clocks while resuming from sleep, it calls
clk_prepare_enable() without checking for errors and blithely goes on to
read/write the xHC's registers -- which, with the xHC not being clocked,
at least on ARM32 usually causes an imprecise external abort exceptions
which cause kernel oops. Currently, the chips for which the driver does
the clock dance on suspend/resume seem to be the Broadcom STB SoCs, based
on ARM32 CPUs, as it seems...
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Fixes: 8bd954c56197 ("usb: host: xhci-plat: suspend and resume clocks")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-plat.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 28218c8f1837..b93161374293 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -458,23 +458,38 @@ static int __maybe_unused xhci_plat_resume(struct device *dev)
int ret;
if (!device_may_wakeup(dev) && (xhci->quirks & XHCI_SUSPEND_RESUME_CLKS)) {
- clk_prepare_enable(xhci->clk);
- clk_prepare_enable(xhci->reg_clk);
+ ret = clk_prepare_enable(xhci->clk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(xhci->reg_clk);
+ if (ret) {
+ clk_disable_unprepare(xhci->clk);
+ return ret;
+ }
}
ret = xhci_priv_resume_quirk(hcd);
if (ret)
- return ret;
+ goto disable_clks;
ret = xhci_resume(xhci, PMSG_RESUME);
if (ret)
- return ret;
+ goto disable_clks;
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
return 0;
+
+disable_clks:
+ if (!device_may_wakeup(dev) && (xhci->quirks & XHCI_SUSPEND_RESUME_CLKS)) {
+ clk_disable_unprepare(xhci->clk);
+ clk_disable_unprepare(xhci->reg_clk);
+ }
+
+ return ret;
}
static int __maybe_unused xhci_plat_runtime_suspend(struct device *dev)
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 19/19] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present
2023-10-19 10:29 [PATCH 00/19] xhci features for usb-next Mathias Nyman
` (17 preceding siblings ...)
2023-10-19 10:29 ` [PATCH 18/19] usb: host: xhci-plat: fix possible kernel oops while resuming Mathias Nyman
@ 2023-10-19 10:29 ` Mathias Nyman
18 siblings, 0 replies; 20+ messages in thread
From: Mathias Nyman @ 2023-10-19 10:29 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Wesley Cheng, Mathias Nyman
From: Wesley Cheng <quic_wcheng@quicinc.com>
There is a 120ms delay implemented for allowing the XHCI host controller to
detect a U3 wakeup pulse. The intention is to wait for the device to retry
the wakeup event if the USB3 PORTSC doesn't reflect the RESUME link status
by the time it is checked. As per the USB3 specification:
tU3WakeupRetryDelay ("Table 7-12. LTSSM State Transition Timeouts")
This would allow the XHCI resume sequence to determine if the root hub
needs to be also resumed. However, in case there is no device connected,
or if there is only a HSUSB device connected, this delay would still affect
the overall resume timing.
Since this delay is solely for detecting U3 wake events (USB3 specific)
then ignore this delay for the disconnected case and the HSUSB connected
only case.
[skip helper function, rename usb3_connected variable -Mathias ]
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 119cbe2a3d65..884b0898d9c9 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -992,6 +992,7 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
int retval = 0;
bool comp_timer_running = false;
bool pending_portevent = false;
+ bool suspended_usb3_devs = false;
bool reinit_xhc = false;
if (!hcd->state)
@@ -1139,10 +1140,17 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
/*
* Resume roothubs only if there are pending events.
* USB 3 devices resend U3 LFPS wake after a 100ms delay if
- * the first wake signalling failed, give it that chance.
+ * the first wake signalling failed, give it that chance if
+ * there are suspended USB 3 devices.
*/
+ if (xhci->usb3_rhub.bus_state.suspended_ports ||
+ xhci->usb3_rhub.bus_state.bus_suspended)
+ suspended_usb3_devs = true;
+
pending_portevent = xhci_pending_portevent(xhci);
- if (!pending_portevent && msg.event == PM_EVENT_AUTO_RESUME) {
+
+ if (suspended_usb3_devs && !pending_portevent &&
+ msg.event == PM_EVENT_AUTO_RESUME) {
msleep(120);
pending_portevent = xhci_pending_portevent(xhci);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread