xhci features for usb-next
Subsystems (2)
cover xhci features for usb-next
Hi Greg
A few small xhci changes and features for usb-next, including support for PCI based xhci hosts with only USB2 ports, and align xhci trace and dynamic debug log port numbering
Michal Pecio (2):
Revert "usb: xhci: Avoid Stop Endpoint retry loop if the endpoint
seems Running"
usb: xhci: Update a comment about Stop Endpoint retries
Niklas Neronin (4):
usb: xhci-pci: add support for hosts with zero USB3 ports
usb: xhci: improve TR Dequeue Pointer mask
usb: xhci: correct indentation for PORTSC tracing function
usb: xhci: align PORTSC trace with one-based port numbering
drivers/usb/host/xhci-pci.c | 42 ++++++++++++++++++++--------------- drivers/usb/host/xhci-ring.c | 24 +++++++++----------- drivers/usb/host/xhci-trace.h | 34 ++++++++++++++-------------- drivers/usb/host/xhci.h | 3 ++- 4 files changed, 54 insertions(+), 49 deletions(-)
Signature · 1 line
2.43.0
PATCH 1/6 Revert "usb: xhci: Avoid Stop Endpoint retry loop if the endpoint seems Running"
From: Michal Pecio <[email protected]>
This reverts commit 28a76fcc4c85dd39633fb96edb643c91820133e3.
No actual HW bugs are known where Endpoint Context shows Running state but Stop Endpoint fails repeatedly with Context State Error and leaves the endpoint state unchanged. Stop Endpoint retries on Running EPs have been performed since early 2021 with no such issues reported so far.
Trying to handle this hypothetical case brings a more realistic danger:
if Stop Endpoint fails on an endpoint which hasn't yet started after a doorbell ring and enough latency occurs before this completion event is handled, the driver may time out and begin removing cancelled TDs from a running endpoint, even though one more retry would stop it reliably.
Such high latency is rare but not impossible, and removing TDs from a running endpoint can cause more damage than not giving back a cancelled URB (which wasn't happening anyway). So err on the side of caution and revert to the old policy of always retrying if the EP appears running.
[Remove stable tag as we are dealing with theoretical cases -Mathias]
--- drivers/usb/host/xhci-ring.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/usb/host/xhci-ring.c +4 -7
@@ -1262,19 +1262,16 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,* 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. +* Keep retrying until the EP starts and stops again, on +* chips where this is known to help. Wait for 100ms. */ +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, ctx_state %d\n" ,GET_EP_CTX_STATE (ep_ctx)); -/* -* Don't retry forever if we guessed wrong or a defective HC never starts -* the EP or says 'Running' but fails the command. We must give back TDs. -*/ -if (time_is_before_jiffies (ep->stop_time +msecs_to_jiffies (100 ))) -break ; command =xhci_alloc_command (xhci, false,GFP_ATOMIC );if (!command) {
Signature · 1 line
2.43.0
PATCH 2/6 usb: xhci: Update a comment about Stop Endpoint retries
In reply to Mathias NymanFrom: Michal Pecio <[email protected]>
Retries are no longer gated by a quirk, so remove that part. Add a brief explanation of the timeout.
--- drivers/usb/host/xhci-ring.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-ring.c +3 -2
@@ -1262,8 +1262,9 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,* 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. +* Keep retrying until the EP starts and stops again or +* up to a timeout (a defective HC may never start, or a +* driver bug may cause stopping an already stopped EP). */ if (time_is_before_jiffies (ep->stop_time +msecs_to_jiffies (100 )))break ;
Signature · 1 line
2.43.0
PATCH 3/6 usb: xhci-pci: add support for hosts with zero USB3 ports
In reply to Mathias NymanFrom: Niklas Neronin <[email protected]>
Add xhci support for PCI hosts that have zero USB3 ports. Avoid creating a shared Host Controller Driver (HCD) when there is only one root hub. Additionally, all references to 'xhci->shared_hcd' are now checked before use.
Only xhci-pci.c requires modification to accommodate this change, as the xhci core already supports configurations with zero USB3 ports. This capability was introduced when xHCI Platform and MediaTek added support for zero USB3 ports.
--- drivers/usb/host/xhci-pci.c | 42 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-)
drivers/usb/host/xhci-pci.c +24 -18
@@ -610,7 +610,7 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id) {int retval;struct xhci_hcd *xhci; -struct usb_hcd *hcd; +struct usb_hcd *hcd, *usb3_hcd;struct reset_control *reset; reset =devm_reset_control_get_optional_exclusive (&dev->dev,NULL );
@@ -636,26 +636,32 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id) hcd =dev_get_drvdata (&dev->dev); xhci =hcd_to_xhci (hcd); xhci->reset = reset; - xhci->shared_hcd =usb_create_shared_hcd (&xhci_pci_hc_driver, &dev->dev, -pci_name (dev), hcd); -if (!xhci->shared_hcd) { - retval = -ENOMEM ; - goto dealloc_usb2_hcd; - } - retval =xhci_ext_cap_init (xhci); -if (retval) - goto put_usb3_hcd; + xhci->allow_single_roothub =1 ; +if (!xhci_has_one_roothub (xhci)) { + xhci->shared_hcd =usb_create_shared_hcd (&xhci_pci_hc_driver, &dev->dev, +pci_name (dev), hcd); +if (!xhci->shared_hcd) { + retval = -ENOMEM ; + goto dealloc_usb2_hcd; + } - retval =usb_add_hcd (xhci->shared_hcd, dev->irq, -IRQF_SHARED ); -if (retval) - goto put_usb3_hcd; -/* Roothub already marked as USB 3.0 speed */ + retval =xhci_ext_cap_init (xhci); +if (retval) + goto put_usb3_hcd; + + retval =usb_add_hcd (xhci->shared_hcd, dev->irq,IRQF_SHARED ); +if (retval) + goto put_usb3_hcd; + }else { + retval =xhci_ext_cap_init (xhci); +if (retval) + goto dealloc_usb2_hcd; + } -if (!(xhci->quirks &XHCI_BROKEN_STREAMS ) && -HCC_MAX_PSA (xhci->hcc_params) >=4 ) - xhci->shared_hcd->can_do_streams =1 ; + usb3_hcd =xhci_get_usb3_hcd (xhci); +if (usb3_hcd && !(xhci->quirks &XHCI_BROKEN_STREAMS ) &&HCC_MAX_PSA (xhci->hcc_params) >=4 ) + usb3_hcd->can_do_streams =1 ;/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */ pm_runtime_put_noidle (&dev->dev);
Signature · 1 line
2.43.0
Re: [PATCH 3/6] usb: xhci-pci: add support for hosts with zero USB3 ports
Quoted 12 lines · Mathias Nyman
From: Niklas Neronin <[email protected]>
Add xhci support for PCI hosts that have zero USB3 ports. Avoid creating a shared Host Controller Driver (HCD) when there is only one root hub. Additionally, all references to '
xhci->shared_hcd' are now checked before use.Only
xhci-pci.crequires modification to accommodate this change, as the xhci core already supports configurations with zero USB3 ports. This capability was introduced when xHCI Platform and MediaTek added support for zero USB3 ports.
Quoted 5 lines
Tested-by Nick Nielsen <[email protected]>Tested-by grm1 <[email protected]>Signed-off-by Niklas Neronin <[email protected]>Signed-off-by Mathias Nyman <[email protected]>
I noticed that on my notebook (Lenovo ThinkPad L14 Gen 5, model 21L5CTO1WW) the integrated camera is no longer detected.
Bisecting this problem (in Linus' tree) led me to this commit.
I'm unsure what further information might be useful, please let me know what else is needed.
Regards,
Dirk
Re: [PATCH 3/6] usb: xhci-pci: add support for hosts with zero USB3 ports
Quoted 4 lines · Dirk Gouders
I noticed that on my notebook (Lenovo ThinkPad L14 Gen 5, model 21L5CTO1WW) the integrated camera is no longer detected.
Bisecting this problem (in Linus' tree) led me to this commit.
Hi,
Probably not only the camera but if you run 'lsusb' you will see that the whole bus is missing.
That's a known bug, fixed by 8607edcd1748 in Greg's USB repository.
Regards,
Michal
Re: [PATCH 3/6] usb: xhci-pci: add support for hosts with zero USB3 ports
Quoted 4 lines · Michal Pecio
Quoted 3 lines · Dirk Gouders
I noticed that on my notebook (Lenovo ThinkPad L14 Gen 5, model 21L5CTO1WW) the integrated camera is no longer detected.
Quoted 8 lines
Quoted 1 line
Bisecting this problem (in Linus' tree) led me to this commit.
Hi,
Probably not only the camera but if you run 'lsusb' you will see that the whole bus is missing.
That's a known bug, fixed by
8607edcd1748in Greg's USB repository.
Thanks for that information and sorry for the noise.
Regards,
Dirk
PATCH 4/6 usb: xhci: improve TR Dequeue Pointer mask
In reply to Mathias NymanFrom: Niklas Neronin <[email protected]>
Address the naming and usage of the TR Dequeue Pointer mask in the xhci driver. The Endpoint Context Field at offset 0x08 is defined as follows:
Bit 0 Dequeue Cycle State (DCS)
Bits 3:1 RsvdZ (Reserved and Zero)
Bits 63:4 TR Dequeue Pointer
When extracting the TR Dequeue Pointer for an Endpoint without Streams, in xhci_handle_cmd_set_deq(), the inverted Dequeue Cycle State mask (~EP_CTX_CYCLE_MASK) is used, inadvertently including the Reserved bits. Although bits 3:1 are typically zero, using the incorrect mask could cause issues.
The existing mask, named "SCTX_DEQ_MASK," is misleading because "SCTX" implies exclusivity to Stream Contexts, whereas the TR Dequeue Pointer is applicable to both Stream and non-Stream Contexts.
Rename the mask to "TR_DEQ_PTR_MASK", utilize GENMASK_ULL() macro and use the mask when handling the TR Dequeue Pointer field.
Function xhci_get_hw_deq() returns the Endpoint Context Field 0x08, either directly from the Endpoint context or a Stream.
--- drivers/usb/host/xhci-ring.c | 12 ++++++------ drivers/usb/host/xhci.h | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-)
drivers/usb/host/xhci-ring.c +6 -6
@@ -711,7 +711,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,return -ENODEV ; } - hw_dequeue =xhci_get_hw_deq (xhci, dev, ep_index, stream_id); + hw_dequeue =xhci_get_hw_deq (xhci, dev, ep_index, stream_id) &TR_DEQ_PTR_MASK ; new_seg = ep_ring->deq_seg; new_deq = ep_ring->dequeue; new_cycle =le32_to_cpu (td->end_trb->generic.field[3 ]) &TRB_CYCLE ;
@@ -723,7 +723,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,*/ do { if (!hw_dequeue_found && xhci_trb_virt_to_dma(new_seg, new_deq) - == (dma_addr_t)(hw_dequeue & ~0xf)) { + == (dma_addr_t)hw_dequeue) { hw_dequeue_found = true; if (td_last_trb_found) break;
@@ -1066,7 +1066,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)*/ hw_deq =xhci_get_hw_deq (xhci, ep->vdev, ep->ep_index, td->urb->stream_id); - hw_deq &= ~0xf ; + hw_deq &=TR_DEQ_PTR_MASK ;if (td->cancel_status ==TD_HALTED ||trb_in_td (td, hw_deq)) {switch (td->cancel_status) {
@@ -1156,7 +1156,7 @@ static struct xhci_td *find_halted_td(struct xhci_virt_ep *ep)if (!list_empty (&ep->ring->td_list)) {/* Not streams compatible */ hw_deq =xhci_get_hw_deq (ep->xhci, ep->vdev, ep->ep_index,0 ); - hw_deq &= ~0xf ; + hw_deq &=TR_DEQ_PTR_MASK ; td = list_first_entry(&ep->ring->td_list, struct xhci_td, td_list);if (trb_in_td (td, hw_deq))return td;
@@ -1479,7 +1479,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 ) { - deq =le64_to_cpu (stream_ctx->stream_ring) &SCTX_DEQ_MASK ; + deq =le64_to_cpu (stream_ctx->stream_ring) &TR_DEQ_PTR_MASK ;/* * Cadence xHCI controllers store some endpoint state
@@ -1495,7 +1495,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, stream_ctx->reserved[1 ] =0 ; } }else { - deq =le64_to_cpu (ep_ctx->deq) & ~EP_CTX_CYCLE_MASK ; + deq =le64_to_cpu (ep_ctx->deq) &TR_DEQ_PTR_MASK ; }xhci_dbg_trace (xhci, trace_xhci_dbg_cancel_urb,"Successful Set TR Deq Ptr cmd, deq = @%08llx" , deq);
drivers/usb/host/xhci.h +2 -1
@@ -500,7 +500,8 @@ struct xhci_ep_ctx {/* deq bitmasks */ #define EP_CTX_CYCLE_MASK (1 << 0) -#define SCTX_DEQ_MASK (~0xfL) +/* bits 63:4 - TR Dequeue Pointer */ +#define TR_DEQ_PTR_MASK GENMASK_ULL(63, 4)/**
Signature · 1 line
2.43.0
PATCH 5/6 usb: xhci: correct indentation for PORTSC tracing function
In reply to Mathias NymanFrom: Niklas Neronin <[email protected]>
Correct the indentation in USB Port Register Set (PORTSC) tracing.
--- drivers/usb/host/xhci-trace.h | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/usb/host/xhci-trace.h +17 -17
@@ -541,23 +541,23 @@ DEFINE_EVENT(xhci_log_ring, xhci_inc_deq, );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-%d: %s" , - __entry->busnum, - __entry->portnum, -xhci_decode_portsc (__get_buf (XHCI_MSG_MAX ), __entry->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-%d: %s" , + __entry->busnum, + __entry->portnum, +xhci_decode_portsc (__get_buf (XHCI_MSG_MAX ), __entry->portsc) + ) ); DEFINE_EVENT(xhci_log_portsc, xhci_handle_port_status,
Signature · 1 line
2.43.0
PATCH 6/6 usb: xhci: align PORTSC trace with one-based port numbering
In reply to Mathias NymanFrom: Niklas Neronin <[email protected]>
In the xHCI driver, port numbers are typically described using a one-based index. However, tracing currently uses a zero-based index. To ensure consistency between tracing and dynamic debugging, update the trace port number to use a one-based index.
--- drivers/usb/host/xhci-trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci-trace.h +1 -1
@@ -550,7 +550,7 @@ DECLARE_EVENT_CLASS(xhci_log_portsc, ), TP_fast_assign( __entry->busnum = port->rhub->hcd->self.busnum; - __entry->portnum = port->hcd_portnum; + __entry->portnum = port->hcd_portnum +1 ; __entry->portsc = portsc; ), TP_printk("port %d-%d: %s",
Signature · 1 line
2.43.0