Hi Sandeep,
> On Nov 22, 2018, at 12:23 PM, Singh, Sandeep <sandeep.si...@amd.com> wrote:
>
> From: Sandeep Singh <sandeep.si...@amd.com>
>
> Occasionally AMD SNPS 3.0 xHC does not respond to
> CSS when set, also it does not flag anything on SRE and HCE
> to point the internal xHC errors on USBSTS register. This stalls
> the entire system wide suspend and there is no point in stalling
> just because of xHC CSS is not responding.
>
> To work around this problem, if the xHC does not flag
> anything on SRE and HCE, we can skip the CSS
> timeout and allow the system to continue the suspend. Once the
> system resume happens we can internally reset the controller
> using XHCI_RESET_ON_RESUME quirk
>
> Signed-off-by: Shyam Sundar S K <shyam-sundar....@amd.com>
> Signed-off-by: Sandeep Singh <sandeep.si...@amd.com>
> cc: Nehal Shah <nehal-bakulchandra.s...@amd.com>
> ---
> Changes since v1:
>
> -> New Variable based decision making when SNPS issue happens hence
> quirk interdependency removed.
> -> Removed STS conditional check in suspend function.
>
> drivers/usb/host/xhci-pci.c | 4 ++++
> drivers/usb/host/xhci.c | 26 ++++++++++++++++++++++----
> drivers/usb/host/xhci.h | 3 +++
> 3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 01c5705..72493c4 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -139,6 +139,10 @@ static void xhci_pci_quirks(struct device *dev, struct
> xhci_hcd *xhci)
> pdev->device == 0x43bb))
> xhci->quirks |= XHCI_SUSPEND_DELAY;
>
> + if (pdev->vendor == PCI_VENDOR_ID_AMD &&
> + (pdev->device == 0x15e0 || pdev->device == 0x15e1))
> + xhci->quirks |= XHCI_SNPS_BROKEN_SUSPEND;
> +
> if (pdev->vendor == PCI_VENDOR_ID_AMD)
> xhci->quirks |= XHCI_TRUST_TX_LENGTH;
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 0420eef..808677d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -970,6 +970,7 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
> unsigned int delay = XHCI_MAX_HALT_USEC;
> struct usb_hcd *hcd = xhci_to_hcd(xhci);
> u32 command;
> + u32 res;
>
> if (!hcd->state)
> return 0;
> @@ -1023,11 +1024,28 @@ int xhci_suspend(struct xhci_hcd *xhci, bool
> do_wakeup)
> command = readl(&xhci->op_regs->command);
> command |= CMD_CSS;
> writel(command, &xhci->op_regs->command);
> + xhci->broken_suspend = 0;
> if (xhci_handshake(&xhci->op_regs->status,
> STS_SAVE, 0, 10 * 1000)) {
> - xhci_warn(xhci, "WARN: xHC save state timeout\n");
> - spin_unlock_irq(&xhci->lock);
> - return -ETIMEDOUT;
> + /*
> + * AMD SNPS xHC 3.0 occasionally does not clear the
> + * SSS bit of USBSTS and when driver tries to poll
> + * to see if the xHC clears BIT(8) which never happens
> + * and driver assumes that controller is not responding
> + * and times out. To workaround this, its good to check
> + * if SRE and HCE bits are not set (as per xhci
> + * Section 5.4.2) and bypass the timeout.
> + */
> + res = readl(&xhci->op_regs->status);
> + if ((xhci->quirks & XHCI_SNPS_BROKEN_SUSPEND) &&
> + (((res & STS_SRE) == 0) &&
> + ((res & STS_HCE) == 0))) {
> + xhci->broken_suspend = 1;
> + } else {
> + xhci_warn(xhci, "WARN: xHC save state timeout\n");
> + spin_unlock_irq(&xhci->lock);
> + return -ETIMEDOUT;
> + }
> }
> spin_unlock_irq(&xhci->lock);
>
> @@ -1080,7 +1098,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
>
> spin_lock_irq(&xhci->lock);
> - if (xhci->quirks & XHCI_RESET_ON_RESUME)
> + if ((xhci->quirks & XHCI_RESET_ON_RESUME) || xhci->broken_suspend)
> hibernated = true;
>
> if (!hibernated) {
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index bf0b369..d5d19b2 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -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_SNPS_BROKEN_SUSPEND BIT_ULL(34)
This bit is already in use by a another patch, so please update its value.
Kai-Heng
>
> unsigned int num_active_eps;
> unsigned int limit_active_eps;
> @@ -1878,6 +1879,8 @@ struct xhci_hcd {
> void *dbc;
> /* platform-specific data -- must come last */
> unsigned long priv[0] __aligned(sizeof(s64));
> + /* Broken Suspend flag for SNPS Suspend resume issue */
> + u8 broken_suspend;
> };
>
> /* Platform specific overrides to generic XHCI hc_driver ops */
> --
> 2.7.4
>
