xhci: handle port status events for removed USB3 hcd
Subsystems (6)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/6 xhci: Fix leaking USB3 shared_hcd at xhci removal
Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()
The shared_hcd is removed and freed in xhci by first calling usb_remove_hcd(xhci->shared_hcd), and later usb_put_hcd(xhci->shared_hcd)
Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have disconnected their devices.") the shared_hcd was never properly put as xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd) was called.
shared_hcd (USB3) is removed before primary hcd (USB2). While removing the primary hcd we might need to handle xhci interrupts to cleanly remove last USB2 devices, therefore we need to set xhci->shared_hcd to NULL before removing the primary hcd to let xhci interrupt handler know shared_hcd is no longer available.
xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before adding them. so to keep the correct reverse removal order use a temporary shared_hcd variable for them.
For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both HCDs before adding them")
--- drivers/usb/host/xhci-histb.c | 6 ++++-- drivers/usb/host/xhci-mtk.c | 6 ++++-- drivers/usb/host/xhci-pci.c | 1 + drivers/usb/host/xhci-plat.c | 6 ++++-- drivers/usb/host/xhci-tegra.c | 1 + drivers/usb/host/xhci.c | 2 -- 6 files changed, 14 insertions(+), 8 deletions(-)
drivers/usb/host/xhci-histb.c +4 -2
@@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device *dev)struct xhci_hcd_histb *histb =platform_get_drvdata (dev);struct usb_hcd *hcd = histb->hcd;struct xhci_hcd *xhci =hcd_to_xhci (hcd); +struct usb_hcd *shared_hcd = xhci->shared_hcd; xhci->xhc_state |=XHCI_STATE_REMOVING ; -usb_remove_hcd (xhci->shared_hcd); +usb_remove_hcd (shared_hcd); + xhci->shared_hcd =NULL ;device_wakeup_disable (&dev->dev);usb_remove_hcd (hcd); -usb_put_hcd (xhci->shared_hcd); +usb_put_hcd (shared_hcd);xhci_histb_host_disable (histb);usb_put_hcd (hcd);
drivers/usb/host/xhci-mtk.c +4 -2
@@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)struct xhci_hcd_mtk *mtk =platform_get_drvdata (dev);struct usb_hcd *hcd = mtk->hcd;struct xhci_hcd *xhci =hcd_to_xhci (hcd); +struct usb_hcd *shared_hcd = xhci->shared_hcd; -usb_remove_hcd (xhci->shared_hcd); +usb_remove_hcd (shared_hcd); + xhci->shared_hcd =NULL ;device_init_wakeup (&dev->dev, false);usb_remove_hcd (hcd); -usb_put_hcd (xhci->shared_hcd); +usb_put_hcd (shared_hcd);usb_put_hcd (hcd);xhci_mtk_sch_exit (mtk);xhci_mtk_clks_disable (mtk);
drivers/usb/host/xhci-pci.c +1 -0
@@ -380,6 +380,7 @@ static void xhci_pci_remove(struct pci_dev *dev)if (xhci->shared_hcd) {usb_remove_hcd (xhci->shared_hcd);usb_put_hcd (xhci->shared_hcd); + xhci->shared_hcd =NULL ; }/* Workaround for spurious wakeups at shutdown with HSW */
drivers/usb/host/xhci-plat.c +4 -2
@@ -362,14 +362,16 @@ static int xhci_plat_remove(struct platform_device *dev)struct xhci_hcd *xhci =hcd_to_xhci (hcd);struct clk *clk = xhci->clk;struct clk *reg_clk = xhci->reg_clk; +struct usb_hcd *shared_hcd = xhci->shared_hcd; xhci->xhc_state |=XHCI_STATE_REMOVING ; -usb_remove_hcd (xhci->shared_hcd); +usb_remove_hcd (shared_hcd); + xhci->shared_hcd =NULL ;usb_phy_shutdown (hcd->usb_phy);usb_remove_hcd (hcd); -usb_put_hcd (xhci->shared_hcd); +usb_put_hcd (shared_hcd);clk_disable_unprepare (clk);clk_disable_unprepare (reg_clk);
drivers/usb/host/xhci-tegra.c +1 -0
@@ -1303,6 +1303,7 @@ static int tegra_xusb_remove(struct platform_device *pdev)usb_remove_hcd (xhci->shared_hcd);usb_put_hcd (xhci->shared_hcd); + xhci->shared_hcd =NULL ;usb_remove_hcd (tegra->hcd);usb_put_hcd (tegra->hcd);
drivers/usb/host/xhci.c +0 -2
@@ -719,8 +719,6 @@ static void xhci_stop(struct usb_hcd *hcd)/* Only halt host and free memory after both hcds are removed */ if (!usb_hcd_is_primary_hcd (hcd)) { -/* usb core will free this hcd shortly, unset pointer */ - xhci->shared_hcd =NULL ;mutex_unlock (&xhci->mutex);return ; }
Signature · 1 line
2.7.4
PATCH 2/6 xhci: handle port status events for removed USB3 hcd
In reply to a message not in the archiveAt xhci removal the USB3 hcd (shared_hcd) is removed before the primary USB2 hcd. Interrupts for port status changes may still occur for USB3 ports after the shared_hcd is freed, causing NULL pointer dereference.
Check if xhci->shared_hcd is still valid before handing USB3 port events
--- drivers/usb/host/xhci-ring.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/usb/host/xhci-ring.c +7 -0
@@ -1556,6 +1556,13 @@ static void handle_port_status(struct xhci_hcd *xhci, goto cleanup; } +/* We might get interrupts after shared_hcd is removed */ +if (port->rhub == &xhci->usb3_rhub && xhci->shared_hcd ==NULL ) { +xhci_dbg (xhci,"ignore port event for removed USB3 hcd\n" ); + bogus_port_status = true; + goto cleanup; + } + hcd = port->rhub->hcd; bus_state = &xhci->bus_state[hcd_index (hcd)]; hcd_portnum = port->hcd_portnum;
Signature · 1 line
2.7.4
PATCH 3/6 xhci: Add check for invalid byte size error when UAS devices are connected.
In reply to a message not in the archiveFrom: Sandeep Singh <[email protected]>
Observed "TRB completion code (27)" error which corresponds to Stopped - Length Invalid error(xhci spec section 4.17.4) while connecting USB to SATA bridge.
Looks like this case was not considered when the following patch[1] was committed. Hence adding this new check which can prevent
the invalid byte size error.
[1] ade2e3a xhci: handle transfer events without TRB pointer
--- drivers/usb/host/xhci-ring.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/host/xhci-ring.c +1 -0
@@ -2273,6 +2273,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, goto cleanup;case COMP_RING_UNDERRUN :case COMP_RING_OVERRUN : +case COMP_STOPPED_LENGTH_INVALID : goto cleanup;default : xhci_err(xhci, "ERROR Transfer event for unknown stream ring slot %u ep %u\n",
Signature · 1 line
2.7.4
PATCH 4/6 usb: xhci: fix uninitialized completion when USB3 port got wrong status
In reply to a message not in the archiveFrom: Aaron Ma <[email protected]>
Realtek USB3.0 Card Reader [0bda:0328] reports wrong port status on Cannon lake PCH USB3.1 xHCI [8086:a36d] after resume from S3, after clear port reset it works fine.
Since this device is registered on USB3 roothub at boot, when port status reports not superspeed, xhci_get_port_status will call an uninitialized completion in bus_state[0].
Kernel will hang because of NULL pointer.
Restrict the USB2 resume status check in USB2 roothub to fix hang issue.
--- drivers/usb/host/xhci-hub.c | 2 +- drivers/usb/host/xhci-ring.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-hub.c +1 -1
@@ -876,7 +876,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, status |=USB_PORT_STAT_SUSPEND ; }if ((raw_port_status &PORT_PLS_MASK ) ==XDEV_RESUME && - !DEV_SUPERSPEED_ANY (raw_port_status)) { + !DEV_SUPERSPEED_ANY (raw_port_status) && hcd->speed <HCD_USB3 ) {if ((raw_port_status &PORT_RESET ) || !(raw_port_status &PORT_PE ))return 0xffffffff ;
drivers/usb/host/xhci-ring.c +1 -1
@@ -1646,7 +1646,7 @@ static void handle_port_status(struct xhci_hcd *xhci,* RExit to a disconnect state). If so, let the the driver know it's * out of the RExit state. */ -if (!DEV_SUPERSPEED_ANY (portsc) && +if (!DEV_SUPERSPEED_ANY (portsc) && hcd->speed <HCD_USB3 &&test_and_clear_bit (hcd_portnum, &bus_state->rexit_ports)) {complete (&bus_state->rexit_done[hcd_portnum]);
Signature · 1 line
2.7.4
PATCH 5/6 usb: xhci: fix timeout for transition from RExit to U0
In reply to a message not in the archiveFrom: Aaron Ma <[email protected]>
This definition is used by msecs_to_jiffies in milliseconds. According to the comments, max rexit timeout should be 20ms. Align with the comments to properly calculate the delay.
Verified on Sunrise Point-LP and Cannon Lake.
--- drivers/usb/host/xhci-hub.c | 4 ++-- drivers/usb/host/xhci.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/usb/host/xhci-hub.c +2 -2
@@ -921,7 +921,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, time_left =wait_for_completion_timeout ( &bus_state->rexit_done[wIndex],msecs_to_jiffies ( -XHCI_MAX_REXIT_TIMEOUT )); +XHCI_MAX_REXIT_TIMEOUT_MS ));spin_lock_irqsave (&xhci->lock, flags);if (time_left) {
@@ -935,7 +935,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, }else {int port_status =readl (port->addr);xhci_warn (xhci,"Port resume took longer than %i msec, port status = 0x%x\n" , -XHCI_MAX_REXIT_TIMEOUT , +XHCI_MAX_REXIT_TIMEOUT_MS , port_status); status |=USB_PORT_STAT_SUSPEND ;clear_bit (wIndex, &bus_state->rexit_ports);
drivers/usb/host/xhci.h +1 -1
@@ -1680,7 +1680,7 @@ struct xhci_bus_state {* It can take up to 20 ms to transition from RExit to U0 on the * Intel Lynx Point LP xHCI host. */ -#define XHCI_MAX_REXIT_TIMEOUT (20 * 1000) +#define XHCI_MAX_REXIT_TIMEOUT_MS 20static inline unsigned int hcd_index (struct usb_hcd *hcd) {
Signature · 1 line
2.7.4
PATCH 6/6 xhci: Add quirk to workaround the errata seen on Cavium Thunder-X2 Soc
In reply to a message not in the archiveFrom: "Cherian, George" <[email protected]>
Implement workaround for ThunderX2 Errata-129 (documented in CN99XX Known Issues" available at Cavium support site).
As per ThunderX2errata-129, USB 2 device may come up as USB 1 if a connection to a USB 1 device is followed by another connection to a USB 2 device, the link will come up as USB 1 for the USB 2 device.
Resolution: Reset the PHY after the USB 1 device is disconnected. The PHY reset sequence is done using private registers in XHCI register space. After the PHY is reset we check for the PLL lock status and retry the operation if it fails. From our tests, retrying 4 times is sufficient.
Add a new quirk flag XHCI_RESET_PLL_ON_DISCONNECT to invoke the workaround in handle_xhci_port_status().
--- drivers/usb/host/xhci-pci.c | 5 +++++ drivers/usb/host/xhci-ring.c | 35 ++++++++++++++++++++++++++++++++++- drivers/usb/host/xhci.h | 1 + 3 files changed, 40 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +5 -0
@@ -248,6 +248,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)if (pdev->vendor ==PCI_VENDOR_ID_TI && pdev->device ==0x8241 ) xhci->quirks |=XHCI_LIMIT_ENDPOINT_INTERVAL_7 ; +if ((pdev->vendor ==PCI_VENDOR_ID_BROADCOM || + pdev->vendor ==PCI_VENDOR_ID_CAVIUM ) && + pdev->device ==0x9026 ) + xhci->quirks |=XHCI_RESET_PLL_ON_DISCONNECT ; +if (xhci->quirks &XHCI_RESET_ON_RESUME )xhci_dbg_trace (xhci, trace_xhci_dbg_quirks,"QUIRK: Resetting on resume" );
drivers/usb/host/xhci-ring.c +34 -1
@@ -1521,6 +1521,35 @@ static void handle_device_notification(struct xhci_hcd *xhci,usb_wakeup_notification (udev->parent, udev->portnum); } +/* +* Quirk hanlder for errata seen on Cavium ThunderX2 processor XHCI +* Controller. +* As per ThunderX2errata-129 USB 2 device may come up as USB 1 +* If a connection to a USB 1 device is followed by another connection +* to a USB 2 device. +* +* Reset the PHY after the USB device is disconnected if device speed +* is less than HCD_USB3. +* Retry the reset sequence max of 4 times checking the PLL lock status. +* +*/ +static void xhci_cavium_reset_phy_quirk (struct xhci_hcd *xhci) +{ +struct usb_hcd *hcd =xhci_to_hcd (xhci); +u32 pll_lock_check; +u32 retry_count =4 ; + +do { +/* Assert PHY reset */ +writel (0x6F , hcd->regs +0x1048 ); +udelay (10 ); +/* De-assert the PHY reset */ +writel (0x7F , hcd->regs +0x1048 ); +udelay (200 ); + pll_lock_check =readl (hcd->regs +0x1070 ); + }while (!(pll_lock_check &0x1 ) && --retry_count); +} +static void handle_port_status (struct xhci_hcd *xhci,union xhci_trb *event) {
@@ -1654,8 +1683,12 @@ static void handle_port_status(struct xhci_hcd *xhci, goto cleanup; } -if (hcd->speed <HCD_USB3 ) +if (hcd->speed <HCD_USB3 ) {xhci_test_and_clear_bit (xhci, port,PORT_PLC ); +if ((xhci->quirks &XHCI_RESET_PLL_ON_DISCONNECT ) && + (portsc &PORT_CSC ) && !(portsc &PORT_CONNECT )) +xhci_cavium_reset_phy_quirk (xhci); + } cleanup:/* Update event ring dequeue pointer before dropping the lock */
drivers/usb/host/xhci.h +1 -0
@@ -1849,6 +1849,7 @@ struct xhci_hcd {#define XHCI_INTEL_USB_ROLE_SW BIT_ULL(31)#define XHCI_ZERO_64B_REGS BIT_ULL(32)#define XHCI_DEFAULT_PM_RUNTIME_ALLOW BIT_ULL(33) +#define XHCI_RESET_PLL_ON_DISCONNECT BIT_ULL(34)unsigned int num_active_eps;unsigned int limit_active_eps;
Signature · 1 line
2.7.4