xhci-pci: don't enable runtime PM for Alpine Ridge on Maximus VIII Impact

xhci-pci: don't enable runtime PM for Alpine Ridge on Maximus VIII Impact

[Mika Westerberg]

On Wed, Jan 02, 2019 at 08:00:41PM -0800, Philip Langdale wrote:
> > 0e157e5 PCI/PME: Implement runtime PM callbacks
> 
> Yes, if I revert this change it also works. And I am reminded that when
> I tested the hash from the usb branch that was merged for 4.20, but on
> the original branch, it didn't fail either. So I guess it's the
> interaction between the two changes that causes the breakage.  

Right, if only the xHCI is runtime suspended but not the PCIe switch it
is part of, things should work. We definitely want to make sure the
whole hierarhcy goes into D3cold whenever possible (to save energy) so
xHCI runtime PM should be enabled. The above commit tries to fix another
issue but it seems it is not correct and causes issues like what you are
seeing and another one reported here:

  https://bugzilla.kernel.org/show_bug.cgi?id=202103

I'm currently investigating it.

xhci-pci: don't enable runtime PM for Alpine Ridge on Maximus VIII Impact

[Philip Langdale]

On Wed, 2 Jan 2019 13:23:51 +0200
Mathias Nyman <mathias.nyman@linux.intel.com> wrote:

> On 30.12.2018 18:34, Philip Langdale wrote:
> > This fixes a regression since:
> > 2815ef7fe4d43072b9eda448d04fbc184f2aa513
> > 
> > This motherboard is a strange beast, with an Alpine Ridge
> > thunderbolt controller configured to only do USB with no actual
> > thunderbolt capabilities (Apparently due to the wiring not being
> > thunderbolt compliant).
> > 
> > When runtime PM is enabled in this case, plugging in a USB device
> > results in nothing being detected. Everything works fine when
> > runtime PM is not enabled.
> > 
> > Signed-off-by: Philip Langdale <philipl@overt.org>
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202095
> > ---
> >   drivers/usb/host/xhci-pci.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/xhci-pci.c
> > b/drivers/usb/host/xhci-pci.c index a9ec7051f286..147ae893f055
> > 100644 --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -211,7 +211,9 @@ static void xhci_pci_quirks(struct device *dev,
> > struct xhci_hcd *xhci) pdev->device ==
> > PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI || pdev->device ==
> > PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI || pdev->device ==
> > PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI ||
> > -	     pdev->device ==
> > PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI))
> > +	     pdev->device ==
> > PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI) &&
> > +	    pdev->subsystem_vendor != 0x2222 &&
> > +	    pdev->subsystem_device != 0x1111)
> >   		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> >   
> >   	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
> >   
> 
> Before doing this there are a couple other possible reasons why this
> is not working.
> 
> Mika (cc) suspects it might be related to a PCI PM patch, can you try
> reverting it?:
> 
> 0e157e5 PCI/PME: Implement runtime PM callbacks

Yes, if I revert this change it also works. And I am reminded that when
I tested the hash from the usb branch that was merged for 4.20, but on
the original branch, it didn't fail either. So I guess it's the
interaction between the two changes that causes the breakage.  

> Another possibility is that xHC is stopped (runtime suspended), but
> still in PCI D0 state, which would prevent PME from waking it up.
> 
> can you check the D state of the Alpine Ridge xHCI controller when it
> doesn't react: cat /sys/bus/pci/devices/<your Alpine Ridge xHCI PCI
> bus address>/firmware_node/power_state
> 
> Don't use lspci for checking D state, it might wake up the PCI
> devices.

The alpine ridge xhci controller doesn't have a firmware node. Perhaps
that is, itself, significant. None of the alpine ridge bridge devices
have firmware nodes either.

Thanks,

--phil

xhci-pci: don't enable runtime PM for Alpine Ridge on Maximus VIII Impact

[Mathias Nyman]

On 30.12.2018 18:34, Philip Langdale wrote:
> This fixes a regression since: 2815ef7fe4d43072b9eda448d04fbc184f2aa513
> 
> This motherboard is a strange beast, with an Alpine Ridge thunderbolt
> controller configured to only do USB with no actual thunderbolt
> capabilities (Apparently due to the wiring not being thunderbolt
> compliant).
> 
> When runtime PM is enabled in this case, plugging in a USB device results
> in nothing being detected. Everything works fine when runtime PM is not
> enabled.
> 
> Signed-off-by: Philip Langdale <philipl@overt.org>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202095
> ---
>   drivers/usb/host/xhci-pci.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index a9ec7051f286..147ae893f055 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -211,7 +211,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>   	     pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI ||
>   	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI ||
>   	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI ||
> -	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI))
> +	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI) &&
> +	    pdev->subsystem_vendor != 0x2222 &&
> +	    pdev->subsystem_device != 0x1111)
>   		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
>   
>   	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
> 

Before doing this there are a couple other possible reasons why this is not working.

Mika (cc) suspects it might be related to a PCI PM patch, can you try reverting it?:

0e157e5 PCI/PME: Implement runtime PM callbacks

Another possibility is that xHC is stopped (runtime suspended), but still in PCI D0 state,
which would prevent PME from waking it up.

can you check the D state of the Alpine Ridge xHCI controller when it doesn't react:
cat /sys/bus/pci/devices/<your Alpine Ridge xHCI PCI bus address>/firmware_node/power_state

Don't use lspci for checking D state, it might wake up the PCI devices.

-Mathias

xhci-pci: don't enable runtime PM for Alpine Ridge on Maximus VIII Impact

[Philip Langdale]

This fixes a regression since: 2815ef7fe4d43072b9eda448d04fbc184f2aa513

This motherboard is a strange beast, with an Alpine Ridge thunderbolt
controller configured to only do USB with no actual thunderbolt
capabilities (Apparently due to the wiring not being thunderbolt
compliant).

When runtime PM is enabled in this case, plugging in a USB device results
in nothing being detected. Everything works fine when runtime PM is not
enabled.

Signed-off-by: Philip Langdale <philipl@overt.org>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=202095
---
 drivers/usb/host/xhci-pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a9ec7051f286..147ae893f055 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -211,7 +211,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	     pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI ||
 	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI ||
 	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI ||
-	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI))
+	     pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI) &&
+	    pdev->subsystem_vendor != 0x2222 &&
+	    pdev->subsystem_device != 0x1111)
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&