xhci fixes for usb-linus
Subsystems (3)
cover xhci fixes for usb-linus
Hi Greg
Fix a couple power management related xHCI issues, and a couple vendor specific workarounds.
-Mathias
Forest Crossman (1):
usb: xhci: Fix ASMedia ASM1042A and ASM3242 DMA addressing
Mathias Nyman (2):
xhci: Improve detection of device initiated wake signal.
xhci: Fix repeated xhci wake after suspend due to uncleared internal
wake state
Stanislaw Gruszka (1):
usb: xhci: do not perform Soft Retry for some xHCI hosts
drivers/usb/host/xhci-pci.c | 13 +++++- drivers/usb/host/xhci-ring.c | 3 +- drivers/usb/host/xhci.c | 78 ++++++++++++++++++++---------------- drivers/usb/host/xhci.h | 1 + 4 files changed, 57 insertions(+), 38 deletions(-)
Signature · 1 line
2.25.1
PATCH 1/4 usb: xhci: do not perform Soft Retry for some xHCI hosts
From: Stanislaw Gruszka <[email protected]>
On some systems rt2800usb and mt7601u devices are unable to operate since commit f8f80be501aa ("xhci: Use soft retry to recover faster from transaction errors")
Seems that some xHCI controllers can not perform Soft Retry correctly, affecting those devices.
To avoid the problem add xhci->quirks flag that restore pre soft retry xhci behaviour for affected xHCI controllers. Currently those are AMD_PROMONTORYA_4 and AMD_PROMONTORYA_2, since it was confirmed by the users: on those xHCI hosts issue happen and is gone after disabling Soft Retry.
[minor commit message rewording for checkpatch -Mathias]
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202541
--- drivers/usb/host/xhci-pci.c | 5 +++++ drivers/usb/host/xhci-ring.c | 3 ++- drivers/usb/host/xhci.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +5 -0
@@ -295,6 +295,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) pdev->device == 0x9026) xhci->quirks |=XHCI_RESET_PLL_ON_DISCONNECT ; +if (pdev->vendor ==PCI_VENDOR_ID_AMD && + (pdev->device ==PCI_DEVICE_ID_AMD_PROMONTORYA_2 || + pdev->device ==PCI_DEVICE_ID_AMD_PROMONTORYA_4 )) + xhci->quirks |=XHCI_NO_SOFT_RETRY ; +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 +2 -1
@@ -2484,7 +2484,8 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, remaining =0 ;break ;case COMP_USB_TRANSACTION_ERROR : -if ((ep_ring->err_count++ >MAX_SOFT_RETRY ) || +if (xhci->quirks &XHCI_NO_SOFT_RETRY || + (ep_ring->err_count++ >MAX_SOFT_RETRY ) ||le32_to_cpu (slot_ctx->tt_info) &TT_SLOT )break ;
drivers/usb/host/xhci.h +1 -0
@@ -1891,6 +1891,7 @@ struct xhci_hcd {#define XHCI_SKIP_PHY_INIT BIT_ULL(37)#define XHCI_DISABLE_SPARSE BIT_ULL(38)#define XHCI_SG_TRB_CACHE_SIZE_QUIRK BIT_ULL(39) +#define XHCI_NO_SOFT_RETRY BIT_ULL(40)unsigned int num_active_eps;unsigned int limit_active_eps;
Signature · 1 line
2.25.1
PATCH 2/4 xhci: Improve detection of device initiated wake signal.
In reply to Mathias NymanA xHC USB 3 port might miss the first wake signal from a USB 3 device if the port LFPS reveiver isn't enabled fast enough after xHC resume.
xHC host will anyway be resumed by a PME# signal, but will go back to suspend if no port activity is seen.
The device resends the U3 LFPS wake signal after a 100ms delay, but by then host is already suspended, starting all over from the beginning of this issue.
USB 3 specs say U3 wake LFPS signal is sent for max 10ms, then device needs to delay 100ms before resending the wake.
Don't suspend immediately if port activity isn't detected in resume. Instead add a retry. If there is no port activity then delay for 120ms, and re-check for port activity.
--- drivers/usb/host/xhci.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
drivers/usb/host/xhci.c +13 -3
@@ -1088,6 +1088,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)struct usb_hcd *secondary_hcd;int retval =0 ;bool comp_timer_running = false; +bool pending_portevent = false;if (!hcd->state)return 0 ;
@@ -1226,13 +1227,22 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) done:if (retval ==0 ) { -/* Resume root hubs only when have pending events. */ -if (xhci_pending_portevent (xhci)) { +/* +* 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. +*/ + pending_portevent =xhci_pending_portevent (xhci); +if (!pending_portevent) { +msleep (120 ); + pending_portevent =xhci_pending_portevent (xhci); + } + +if (pending_portevent) {usb_hcd_resume_root_hub (xhci->shared_hcd);usb_hcd_resume_root_hub (hcd); } } -/* * If system is subject to the Quirk, Compliance Mode Timer needs to * be re-initialized Always after a system resume. Ports are subject
Signature · 1 line
2.25.1
Re: [PATCH 2/4] xhci: Improve detection of device initiated wake signal.
Hi Mathias,
Quoted 9 lines · Mathias Nyman
A xHC USB 3 port might miss the first wake signal from a USB 3 device if the port LFPS reveiver isn't enabled fast enough after xHC resume.
xHC host will anyway be resumed by a PME# signal, but will go back to suspend if no port activity is seen.
The device resends the U3 LFPS wake signal after a 100ms delay, but by then host is already suspended, starting all over from the beginning of this issue.
Quoted 6 lines
USB 3 specs say U3 wake LFPS signal is sent for max 10ms, then device needs to delay 100ms before resending the wake.
Don't suspend immediately if port activity isn't detected in resume. Instead add a retry. If there is no port activity then delay for 120ms, and re-check for port activity.
We have a use case with which this change is causing unnecessary delay. Consider a USB2* device is attached and host is initiating the resume. Since this is not a device initiated wakeup there wouldn't be any pending event seen on the PORTSC registers, yet this adds an additional 120ms delay to re-check the PORTSC before returning and allowing the USB core to perform resume signaling.
Is there a way to avoid this delay in that case? Perhaps could we distinguish whether we arrive here at xhci_resume() due to a host-initiated resume vs. a device remote wakeup?
* I think it should be similar for attached USB3 devices as well, since the host-initiated exit from U3 wouldn't happen until usb_port_resume().
Thanks,
Jack
Quoted 40 lines
Cc <[email protected]>Signed-off-by Mathias Nyman <[email protected]>--- drivers/usb/host/xhci.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)drivers/usb/host/xhci.c +13
-2@@ -1088,6 +1088,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)struct usb_hcd *secondary_hcd;int retval =0 ;bool comp_timer_running = false; +bool pending_portevent = false;if (!hcd->state)return 0 ;@@ -1226,13 +1227,22 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) done:if (retval ==0 ) { -/* Resume root hubs only when have pending events. */ -if (xhci_pending_portevent (xhci)) { +/* +* 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. +*/ + pending_portevent =xhci_pending_portevent (xhci); +if (!pending_portevent) { +msleep (120 ); + pending_portevent =xhci_pending_portevent (xhci); + } + +if (pending_portevent) {usb_hcd_resume_root_hub (xhci->shared_hcd);usb_hcd_resume_root_hub (hcd); } }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Greg KH
maintainerRe: [PATCH 2/4] xhci: Improve detection of device initiated wake signal.
Quoted 26 lines · Jack Pham
Hi Mathias,
Quoted 9 lines · Mathias Nyman
A xHC USB 3 port might miss the first wake signal from a USB 3 device if the port LFPS reveiver isn't enabled fast enough after xHC resume.
xHC host will anyway be resumed by a PME# signal, but will go back to suspend if no port activity is seen.
The device resends the U3 LFPS wake signal after a 100ms delay, but by then host is already suspended, starting all over from the beginning of this issue.Quoted 6 lines
USB 3 specs say U3 wake LFPS signal is sent for max 10ms, then device needs to delay 100ms before resending the wake.
Don't suspend immediately if port activity isn't detected in resume. Instead add a retry. If there is no port activity then delay for 120ms, and re-check for port activity.
We have a use case with which this change is causing unnecessary delay. Consider a USB2* device is attached and host is initiating the resume. Since this is not a device initiated wakeup there wouldn't be any pending event seen on the PORTSC registers, yet this adds an additional 120ms delay to re-check the PORTSC before returning and allowing the USB core to perform resume signaling.
Quoted 3 lines
Is there a way to avoid this delay in that case? Perhaps could we distinguish whether we arrive here at
xhci_resume()due to a host-initiated resume vs. a device remote wakeup?
Do you have a proposed patch that would do such a thing? Given that you are seeing the problem it should be easy for you to test :)
Quoted 2 lines
* I think it should be similar for attached USB3 devices as well, since the host-initiated exit from U3 wouldn't happen until
usb_port_resume().
Can you reliably detect "attached" devices?
thanks,
greg k-h
PATCH 3/4 usb: xhci: Fix ASMedia ASM1042A and ASM3242 DMA addressing
In reply to Mathias NymanFrom: Forest Crossman <[email protected]>
I've confirmed that both the ASMedia ASM1042A and ASM3242 have the same problem as the ASM1142 and ASM2142/ASM3142, where they lose some of the upper bits of 64-bit DMA addresses. As with the other chips, this can cause problems on systems where the upper bits matter, and adding the XHCI_NO_64BIT_SUPPORT quirk completely fixes the issue.
--- drivers/usb/host/xhci-pci.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-pci.c +6 -2
@@ -66,6 +66,7 @@#define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI 0x1142#define PCI_DEVICE_ID_ASMEDIA_1142_XHCI 0x1242#define PCI_DEVICE_ID_ASMEDIA_2142_XHCI 0x2142 +#define PCI_DEVICE_ID_ASMEDIA_3242_XHCI 0x3242static const char hcd_name[] ="xhci_hcd" ;
@@ -276,11 +277,14 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) pdev->device == PCI_DEVICE_ID_ASMEDIA_1042_XHCI) xhci->quirks |= XHCI_BROKEN_STREAMS;if (pdev->vendor ==PCI_VENDOR_ID_ASMEDIA && - pdev->device ==PCI_DEVICE_ID_ASMEDIA_1042A_XHCI ) + pdev->device ==PCI_DEVICE_ID_ASMEDIA_1042A_XHCI ) { xhci->quirks |=XHCI_TRUST_TX_LENGTH ; + xhci->quirks |=XHCI_NO_64BIT_SUPPORT ; + }if (pdev->vendor ==PCI_VENDOR_ID_ASMEDIA && (pdev->device ==PCI_DEVICE_ID_ASMEDIA_1142_XHCI || - pdev->device ==PCI_DEVICE_ID_ASMEDIA_2142_XHCI )) + pdev->device ==PCI_DEVICE_ID_ASMEDIA_2142_XHCI || + pdev->device ==PCI_DEVICE_ID_ASMEDIA_3242_XHCI )) xhci->quirks |=XHCI_NO_64BIT_SUPPORT ; if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
Signature · 1 line
2.25.1
PATCH 4/4 xhci: Fix repeated xhci wake after suspend due to uncleared internal wake state
In reply to Mathias NymanIf port terminations are detected in suspend, but link never reaches U0 then xHCI may have an internal uncleared wake state that will cause an immediate wake after suspend.
This wake state is normally cleared when driver clears the PORT_CSC bit, which is set after a device is enabled and in U0.
Write 1 to clear PORT_CSC for ports that don't have anything connected when suspending. This makes sure any pending internal wake states in xHCI are cleared.
--- drivers/usb/host/xhci.c | 62 ++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 32 deletions(-)
drivers/usb/host/xhci.c +30 -32
@@ -883,44 +883,42 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)xhci_set_cmd_ring_deq (xhci); } -static void xhci_disable_port_wake_on_bits (struct xhci_hcd *xhci) +/* +* Disable port wake bits if do_wakeup is not set. +* +* Also clear a possible internal port wake state left hanging for ports that +* detected termination but never successfully enumerated (trained to 0U). +* Internal wake causes immediate xHCI wake after suspend. PORT_CSC write done +* at enumeration clears this wake, force one here as well for unconnected ports +*/ + +static void xhci_disable_hub_port_wake (struct xhci_hcd *xhci, +struct xhci_hub *rhub, +bool do_wakeup) { -struct xhci_port **ports; -int port_index;unsigned long flags;u32 t1, t2, portsc; +int i;spin_lock_irqsave (&xhci->lock, flags); -/* disable usb3 ports Wake bits */ - port_index = xhci->usb3_rhub.num_ports; - ports = xhci->usb3_rhub.ports; -while (port_index--) { - t1 =readl (ports[port_index]->addr); - portsc = t1; - t1 =xhci_port_state_to_neutral (t1); - t2 = t1 & ~PORT_WAKE_BITS ; -if (t1 != t2) { -writel (t2, ports[port_index]->addr); -xhci_dbg (xhci,"disable wake bits port %d-%d, portsc: 0x%x, write: 0x%x\n" , - xhci->usb3_rhub.hcd->self.busnum, - port_index +1 , portsc, t2); - } - } +for (i =0 ; i < rhub->num_ports; i++) { + portsc =readl (rhub->ports[i]->addr); + t1 =xhci_port_state_to_neutral (portsc); + t2 = t1; + +/* clear wake bits if do_wake is not set */ +if (!do_wakeup) + t2 &= ~PORT_WAKE_BITS ; + +/* Don't touch csc bit if connected or connect change is set */ +if (!(portsc & (PORT_CSC |PORT_CONNECT ))) + t2 |=PORT_CSC ; -/* disable usb2 ports Wake bits */ - port_index = xhci->usb2_rhub.num_ports; - ports = xhci->usb2_rhub.ports; -while (port_index--) { - t1 =readl (ports[port_index]->addr); - portsc = t1; - t1 =xhci_port_state_to_neutral (t1); - t2 = t1 & ~PORT_WAKE_BITS ;if (t1 != t2) { -writel (t2, ports[port_index]->addr); -xhci_dbg (xhci,"disable wake bits port %d-%d, portsc: 0x%x, write: 0x%x\n" , - xhci->usb2_rhub.hcd->self.busnum, - port_index +1 , portsc, t2); +writel (t2, rhub->ports[i]->addr); +xhci_dbg (xhci,"config port %d-%d wake bits, portsc: 0x%x, write: 0x%x\n" , + rhub->hcd->self.busnum, i +1 , portsc, t2); } }spin_unlock_irqrestore (&xhci->lock, flags);
@@ -983,8 +981,8 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)return -EINVAL ;/* Clear root port wake on bits if wakeup not allowed. */ -if (!do_wakeup) -xhci_disable_port_wake_on_bits (xhci); +xhci_disable_hub_port_wake (xhci, &xhci->usb3_rhub, do_wakeup); +xhci_disable_hub_port_wake (xhci, &xhci->usb2_rhub, do_wakeup);if (!HCD_HW_ACCESSIBLE (hcd))return 0 ;
Signature · 1 line
2.25.1