[PATCH v5] usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir

[PATCH v5] usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir

[Mario Limonciello]

The XHCI controller is required to enter D3hot rather than D3cold for AMD
s2idle on this hardware generation.

Otherwise, the 'Controller Not Ready' (CNR) bit is not being cleared by
host in resume and eventually this results in xhci resume failures during
the s2idle wakeup.

Suggested-by: Prike Liang <Prike.Liang@amd.com>
Link: https://lore.kernel.org/linux-usb/1612527609-7053-1-git-send-email-Prike.Liang@amd.com/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/usb/host/xhci-pci.c | 7 ++++++-
 drivers/usb/host/xhci.h     | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

v1 -> v2: drop the XHCI_COMP_MODE_QUIRK quirk and create a new one for handling
XHCI D3cold.

v2 -> v3: correct the quirk name typo XHCI_AMD_S2IDL_SUPPORT_QUIRK -> XHCI_AMD_S2IDLE_SUPPORT_QUIRK

v3 -> v4: Fix commit message to clarify and reference HW
          Rename quirk to describe problem, not hardware
          Add definition for the hardware to source
v4 -> v5: Correct a typographical error

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7bc18cf8042c..18c2bbddf080 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -59,6 +59,7 @@
 #define PCI_DEVICE_ID_INTEL_MAPLE_RIDGE_XHCI		0x1138
 #define PCI_DEVICE_ID_INTEL_ALDER_LAKE_XHCI		0x461e
 
+#define PCI_DEVICE_ID_AMD_RENOIR_XHCI			0x1639
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_4			0x43b9
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_3			0x43ba
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_2			0x43bb
@@ -182,6 +183,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 		(pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_1)))
 		xhci->quirks |= XHCI_U2_DISABLE_WAKE;
 
+	if (pdev->vendor == PCI_VENDOR_ID_AMD &&
+		pdev->device == PCI_DEVICE_ID_AMD_RENOIR_XHCI)
+		xhci->quirks |= XHCI_BROKEN_D3COLD;
+
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
 		xhci->quirks |= XHCI_LPM_SUPPORT;
 		xhci->quirks |= XHCI_INTEL_HOST;
@@ -539,7 +544,7 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
 	 * Systems with the TI redriver that loses port status change events
 	 * need to have the registers polled during D3, so avoid D3cold.
 	 */
-	if (xhci->quirks & XHCI_COMP_MODE_QUIRK)
+	if (xhci->quirks & (XHCI_COMP_MODE_QUIRK | XHCI_BROKEN_D3COLD))
 		pci_d3cold_disable(pdev);
 
 	if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2595a8f057c4..e417f5ce13d1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1892,6 +1892,7 @@ struct xhci_hcd {
 #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)
+#define XHCI_BROKEN_D3COLD	BIT_ULL(41)
 
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
-- 
2.25.1

Re: [PATCH v5] usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir

[Greg Kroah-Hartman]

On Thu, May 27, 2021 at 10:45:34AM -0500, Mario Limonciello wrote:
> The XHCI controller is required to enter D3hot rather than D3cold for AMD
> s2idle on this hardware generation.
> 
> Otherwise, the 'Controller Not Ready' (CNR) bit is not being cleared by
> host in resume and eventually this results in xhci resume failures during
> the s2idle wakeup.
> 
> Suggested-by: Prike Liang <Prike.Liang@amd.com>
> Link: https://lore.kernel.org/linux-usb/1612527609-7053-1-git-send-email-Prike.Liang@amd.com/
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/usb/host/xhci-pci.c | 7 ++++++-
>  drivers/usb/host/xhci.h     | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)

Should it go to stable kernels, and if so, how far back?

thanks,

greg k-h

Re: [PATCH v5] usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir

[Limonciello, Mario]

On 6/2/2021 10:03, Greg Kroah-Hartman wrote:
> On Thu, May 27, 2021 at 10:45:34AM -0500, Mario Limonciello wrote:
>> The XHCI controller is required to enter D3hot rather than D3cold for AMD
>> s2idle on this hardware generation.
>>
>> Otherwise, the 'Controller Not Ready' (CNR) bit is not being cleared by
>> host in resume and eventually this results in xhci resume failures during
>> the s2idle wakeup.
>>
>> Suggested-by: Prike Liang <Prike.Liang@amd.com>
>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-usb%2F1612527609-7053-1-git-send-email-Prike.Liang%40amd.com%2F&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C07d80c804533439da6ad08d925d798a8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637582430177777264%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=uV%2FMr7rXtBVzNmEBg%2FrIoXVKfLAjdQOO4rkR8DrdhBw%3D&amp;reserved=0
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/usb/host/xhci-pci.c | 7 ++++++-
>>   drivers/usb/host/xhci.h     | 1 +
>>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> Should it go to stable kernels, and if so, how far back?
> 
> thanks,
> 
> greg k-h
> 

Sure, as far back as 5.11 makes sense to me.