On Tue, 4 Oct 2016, Mathias Nyman wrote: > On 03.10.2016 23:54, Pierre de Villemereuil wrote: > > Hi Mathias, > > > > Just to know: does that mean the firmware from Asus is faulty in here? Do > > you > > think I should contact Asus about this? > > > > Probably, _S0W, _DSM and XFLT code for xHCI are useless in that ACPI DSDT > firmware version. > > But we still want the driver to handle cases like this. > Just wrote the patch. > Adding Alan Stern to CC for PM sanity feedback on it: > > --- > > Author: Mathias Nyman <mathias.ny...@linux.intel.com> > Date: Tue Oct 4 13:07:59 2016 +0300 > > xhci: Prevent a non-responsive xhci suspend state. > > ACPI may limit the lowest possible runtime suspend PCI D-state to D0. > If xHC is not capable of generating PME# events in D0 we end > up with a non-responsive runtime suspended host without PME# capability > and with interrupts disabled, unable to detect or wake up when a > new usb device is connected. > > This was triggered on a ASUS TP301UA machine. > > Reported-by: Pierre de Villemereuil <fl...@mailoo.org> > Signed-off-by: Mathias Nyman <mathias.ny...@linux.intel.com> > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index d7b0f97..08b021c 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -396,6 +396,11 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool > do_wakeup) > if (xhci->quirks & XHCI_SSIC_PORT_UNUSED) > xhci_ssic_port_unused_quirk(hcd, true); > > + /* Prevent a non-responsive PCI D0 state without PME# wake capability > */ > + if (pci_choose_state(pdev, PMSG_AUTO_SUSPEND) == PCI_D0) > + if (!pci_pme_capable(pdev, PCI_D0)) > + return -EBUSY; > + > ret = xhci_suspend(xhci, do_wakeup); > if (ret && (xhci->quirks & XHCI_SSIC_PORT_UNUSED)) > xhci_ssic_port_unused_quirk(hcd, false);
I've got three comments about this proposal. First, this logic should not apply during system suspend, only during runtime suspend. You don't want to abort putting a laptop to sleep just because the xHCI controller can't generate wakeup signals. Second, this really has nothing to do with the D0 state. The same logic should apply to whatever state is chosen for runtime suspend: If the controller can't generate PME# wakeup signals in that state then the suspend should be aborted. Third, the same reasoning applies to every PCI device that relies on PME#, not just to xHCI controllers. Therefore the new code should be added to drivers/pci/pci-driver.c:pci_pm_runtime_suspend(), not to xhci-pci.c. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
