[PATCH] x86/PCI: Remove D0 PME capability on AMD FCH xHCI

[PATCH] x86/PCI: Remove D0 PME capability on AMD FCH xHCI

[Kai-Heng Feng]

There's an xHCI device that doesn't wake when a USB 2.0 device gets
plugged to its USB 3.0 port. The driver's own runtime suspend callback
was called, PME# signaling was enabled, but it stays at PCI D0:

00:10.0 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller [1022:7914] (rev 20) (prog-if 30 [XHCI])
        Subsystem: Dell FCH USB XHCI Controller [1028:087e]
        Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Interrupt: pin A routed to IRQ 18
        Region 0: Memory at f0b68000 (64-bit, non-prefetchable) [size=8K]
        Capabilities: [50] Power Management version 3
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
                Status: D0 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME-

A PCI device can be runtime suspended while still stays at D0 when it
supports D0 PME# and its ACPI _S0W method reports D0. Though plugging
USB 3.0 devices can wakeup the xHCI, it doesn't respond to USB 2.0
devices.

So let's disable D0 PME capability on this device to avoid the issue.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203673
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 arch/x86/pci/fixup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 527e69b12002..0851a05d092f 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -588,6 +588,17 @@ static void pci_fixup_amd_ehci_pme(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7808, pci_fixup_amd_ehci_pme);
 
+/*
+ * Device [1022:7914]
+ * D0 PME# doesn't get asserted when plugging USB 2.0 device.
+ */
+static void pci_fixup_amd_fch_xhci_pme(struct pci_dev *dev)
+{
+	dev_info(&dev->dev, "PME# does not work under D0, disabling it\n");
+	dev->pme_support &= ~(PCI_PM_CAP_PME_D0 >> PCI_PM_CAP_PME_SHIFT);
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7914, pci_fixup_amd_fch_xhci_pme);
+
 /*
  * Apple MacBook Pro: Avoid [mem 0x7fa00000-0x7fbfffff]
  *
-- 
2.17.1

Re: [PATCH] x86/PCI: Remove D0 PME capability on AMD FCH xHCI

[Bjorn Helgaas]

[+cc Alan, Mathias, Rafael, Lukas]

On Mon, Sep 02, 2019 at 10:52:52PM +0800, Kai-Heng Feng wrote:
> There's an xHCI device that doesn't wake when a USB 2.0 device gets
> plugged to its USB 3.0 port. The driver's own runtime suspend callback
> was called, PME# signaling was enabled, but it stays at PCI D0:
> 
> 00:10.0 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller [1022:7914] (rev 20) (prog-if 30 [XHCI])
>         Subsystem: Dell FCH USB XHCI Controller [1028:087e]
>         Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Interrupt: pin A routed to IRQ 18
>         Region 0: Memory at f0b68000 (64-bit, non-prefetchable) [size=8K]
>         Capabilities: [50] Power Management version 3
>                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
>                 Status: D0 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME-
> 
> A PCI device can be runtime suspended while still stays at D0 when it
> supports D0 PME# and its ACPI _S0W method reports D0. Though plugging
> USB 3.0 devices can wakeup the xHCI, it doesn't respond to USB 2.0
> devices.

I don't think _S0W and runtime suspend are relevant here.  What *is*
relevant is that the device advertises that it can generate PME from
D0, and it apparently does not do so.

Table 10 in the xHCI spec r1.0, sec 4.15.2.3, says the xHC should
assert PME# if enabled and the port's WCE bit is set.  Did you ever
confirm that WCE is set?

I assume WCE *is* set because plugging in a USB3 device *does*
generate a PME#, and I don't see anything in Table 10 that says it
would work for USB3 but not USB2.

> So let's disable D0 PME capability on this device to avoid the issue.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203673
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  arch/x86/pci/fixup.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 527e69b12002..0851a05d092f 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -588,6 +588,17 @@ static void pci_fixup_amd_ehci_pme(struct pci_dev *dev)
>  }
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7808, pci_fixup_amd_ehci_pme);
>  
> +/*
> + * Device [1022:7914]
> + * D0 PME# doesn't get asserted when plugging USB 2.0 device.
> + */
> +static void pci_fixup_amd_fch_xhci_pme(struct pci_dev *dev)
> +{
> +	dev_info(&dev->dev, "PME# does not work under D0, disabling it\n");

Use pci_info() as in the rest of the file.

> +	dev->pme_support &= ~(PCI_PM_CAP_PME_D0 >> PCI_PM_CAP_PME_SHIFT);
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7914, pci_fixup_amd_fch_xhci_pme);
> +
>  /*
>   * Apple MacBook Pro: Avoid [mem 0x7fa00000-0x7fbfffff]
>   *
> -- 
> 2.17.1
> 

Re: [PATCH] x86/PCI: Remove D0 PME capability on AMD FCH xHCI

[Kai-Heng Feng]

> On Oct 2, 2019, at 08:07, Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> [+cc Alan, Mathias, Rafael, Lukas]
> 
> On Mon, Sep 02, 2019 at 10:52:52PM +0800, Kai-Heng Feng wrote:
>> There's an xHCI device that doesn't wake when a USB 2.0 device gets
>> plugged to its USB 3.0 port. The driver's own runtime suspend callback
>> was called, PME# signaling was enabled, but it stays at PCI D0:
>> 
>> 00:10.0 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller [1022:7914] (rev 20) (prog-if 30 [XHCI])
>>        Subsystem: Dell FCH USB XHCI Controller [1028:087e]
>>        Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>>        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Interrupt: pin A routed to IRQ 18
>>        Region 0: Memory at f0b68000 (64-bit, non-prefetchable) [size=8K]
>>        Capabilities: [50] Power Management version 3
>>                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
>>                Status: D0 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME-
>> 
>> A PCI device can be runtime suspended while still stays at D0 when it
>> supports D0 PME# and its ACPI _S0W method reports D0. Though plugging
>> USB 3.0 devices can wakeup the xHCI, it doesn't respond to USB 2.0
>> devices.
> 
> I don't think _S0W and runtime suspend are relevant here.  What *is*
> relevant is that the device advertises that it can generate PME from
> D0, and it apparently does not do so.

Yes that's the case. It doesn't generate PME when USB2.0 or USB1.1 device gets plugged.

> 
> Table 10 in the xHCI spec r1.0, sec 4.15.2.3, says the xHC should
> assert PME# if enabled and the port's WCE bit is set.  Did you ever
> confirm that WCE is set?

How do I check WCE when xHCI is suspended?
If I want to read WCE then I have the resume the device, but after resuming all USB devices get enumerated, and checking WCE doesn't matter anymore.

> 
> I assume WCE *is* set because plugging in a USB3 device *does*
> generate a PME#, and I don't see anything in Table 10 that says it
> would work for USB3 but not USB2.

It should work on all USB speeds, but it didn't.
That's why the OEM/ODM use the _S0W workaround on Windows.

Kai-Heng

> 
>> So let's disable D0 PME capability on this device to avoid the issue.
>> 
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203673
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> arch/x86/pci/fixup.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>> 
>> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
>> index 527e69b12002..0851a05d092f 100644
>> --- a/arch/x86/pci/fixup.c
>> +++ b/arch/x86/pci/fixup.c
>> @@ -588,6 +588,17 @@ static void pci_fixup_amd_ehci_pme(struct pci_dev *dev)
>> }
>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7808, pci_fixup_amd_ehci_pme);
>> 
>> +/*
>> + * Device [1022:7914]
>> + * D0 PME# doesn't get asserted when plugging USB 2.0 device.
>> + */
>> +static void pci_fixup_amd_fch_xhci_pme(struct pci_dev *dev)
>> +{
>> +	dev_info(&dev->dev, "PME# does not work under D0, disabling it\n");
> 
> Use pci_info() as in the rest of the file.
> 
>> +	dev->pme_support &= ~(PCI_PM_CAP_PME_D0 >> PCI_PM_CAP_PME_SHIFT);
>> +}
>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7914, pci_fixup_amd_fch_xhci_pme);
>> +
>> /*
>>  * Apple MacBook Pro: Avoid [mem 0x7fa00000-0x7fbfffff]
>>  *
>> -- 
>> 2.17.1

Re: [PATCH] x86/PCI: Remove D0 PME capability on AMD FCH xHCI

[Bjorn Helgaas]

On Wed, Oct 02, 2019 at 01:32:07PM +0800, Kai-Heng Feng wrote:
> On Oct 2, 2019, at 08:07, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Sep 02, 2019 at 10:52:52PM +0800, Kai-Heng Feng wrote:
> >> There's an xHCI device that doesn't wake when a USB 2.0 device gets
> >> plugged to its USB 3.0 port. The driver's own runtime suspend callback
> >> was called, PME# signaling was enabled, but it stays at PCI D0:
> >> 
> >> 00:10.0 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller [1022:7914] (rev 20) (prog-if 30 [XHCI])
> >>        Subsystem: Dell FCH USB XHCI Controller [1028:087e]
> >>        Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
> >>        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> >>        Interrupt: pin A routed to IRQ 18
> >>        Region 0: Memory at f0b68000 (64-bit, non-prefetchable) [size=8K]
> >>        Capabilities: [50] Power Management version 3
> >>                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
> >>                Status: D0 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME-
> >> 
> >> A PCI device can be runtime suspended while still stays at D0 when it
> >> supports D0 PME# and its ACPI _S0W method reports D0. Though plugging
> >> USB 3.0 devices can wakeup the xHCI, it doesn't respond to USB 2.0
> >> devices.
> > 
> > I don't think _S0W and runtime suspend are relevant here.  What *is*
> > relevant is that the device advertises that it can generate PME from
> > D0, and it apparently does not do so.
> 
> Yes that's the case. It doesn't generate PME when USB2.0 or USB1.1
> device gets plugged.

OK, thanks.  I added a stable tag and applied this to pci/misc for v5.5.

> > Table 10 in the xHCI spec r1.0, sec 4.15.2.3, says the xHC should
> > assert PME# if enabled and the port's WCE bit is set.  Did you ever
> > confirm that WCE is set?
> 
> How do I check WCE when xHCI is suspended?  If I want to read WCE
> then I have the resume the device, but after resuming all USB
> devices get enumerated, and checking WCE doesn't matter anymore.

Yeah, that's a problem.  I guess you'd have to inspect or instrument
the driver to ensure WCE is set in that path.  But if you get PME# for
USB3 devices, it seems safe to assume WCE is set.

> > I assume WCE *is* set because plugging in a USB3 device *does*
> > generate a PME#, and I don't see anything in Table 10 that says it
> > would work for USB3 but not USB2.
> 
> It should work on all USB speeds, but it didn't.
> That's why the OEM/ODM use the _S0W workaround on Windows.
> 
> Kai-Heng
> 
> > 
> >> So let's disable D0 PME capability on this device to avoid the issue.
> >> 
> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203673
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> >> ---
> >> arch/x86/pci/fixup.c | 11 +++++++++++
> >> 1 file changed, 11 insertions(+)
> >> 
> >> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> >> index 527e69b12002..0851a05d092f 100644
> >> --- a/arch/x86/pci/fixup.c
> >> +++ b/arch/x86/pci/fixup.c
> >> @@ -588,6 +588,17 @@ static void pci_fixup_amd_ehci_pme(struct pci_dev *dev)
> >> }
> >> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7808, pci_fixup_amd_ehci_pme);
> >> 
> >> +/*
> >> + * Device [1022:7914]
> >> + * D0 PME# doesn't get asserted when plugging USB 2.0 device.
> >> + */
> >> +static void pci_fixup_amd_fch_xhci_pme(struct pci_dev *dev)
> >> +{
> >> +	dev_info(&dev->dev, "PME# does not work under D0, disabling it\n");
> > 
> > Use pci_info() as in the rest of the file.

Sorry, this was just wrong.  I was assuming this was for
drivers/pci/quirks.c, where dev_info() has been replaced by
pci_info().  But that's not the case for this file.

> >> +	dev->pme_support &= ~(PCI_PM_CAP_PME_D0 >> PCI_PM_CAP_PME_SHIFT);
> >> +}
> >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x7914, pci_fixup_amd_fch_xhci_pme);
> >> +
> >> /*
> >>  * Apple MacBook Pro: Avoid [mem 0x7fa00000-0x7fbfffff]
> >>  *
> >> -- 
> >> 2.17.1
>