At Tue, 10 Sep 2013 08:38:50 -0700,
Sarah Sharp wrote:
>
> On Tue, Sep 10, 2013 at 08:24:51AM +0200, Takashi Iwai wrote:
> > Haswell LynxPoint and LynxPoint-LP with the recent Intel BIOS show
> > mysterious wakeups after shutdown occasionally. After discussing with
> > BIOS engineers, they explained that the new BIOS expects that the
> > wakeup sources are cleared and set to D3 for all wakeup devices when
> > the system is going to sleep or power off, but the current xhci driver
> > doesn't do this properly (partly intentionally).
> >
> > This patch introduces a new quirk, XHCI_HSW_SPURIOUS_WAKEUP, for
> > fixing the spurious wakeups at S5 by calling xhci_stop() instead of
> > its stripped version in the xhci shutdown ops, and setting the device
> > to PCI D3 at shutdown and remove ops.
>
> Which part of xhci_stop() causes the wakeups to cease? Is it the reset
> of the host, or the disabling of the event ring interrupts, or both?
> I'd like to understand exactly what caused the BIOS to wake up the
> system.
I'd love to understand what the heck it is, too :)
Currently we're in rush for band-aiding this problem, so had no time
for the detailed analysis. We'll check it soon later.
> Comments below.
>
> > The PCI D3 call is based on the initial fix patch by Oliver Neukum.
> >
> > Cc: Oliver Neukum <oneukum@xxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > v1->v2: forgot to refresh the patch before submission, sorry.
> >
> > drivers/usb/host/xhci-pci.c | 14 ++++++++++++++
> > drivers/usb/host/xhci.c | 18 +++++++++++++-----
> > drivers/usb/host/xhci.h | 1 +
> > include/linux/pci_ids.h | 2 ++
> > 4 files changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index c2d4950..bfed043 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -110,6 +110,15 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> > xhci->quirks |= XHCI_SPURIOUS_REBOOT;
> > xhci->quirks |= XHCI_AVOID_BEI;
> > }
> > + if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + (pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI ||
> > + pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI)) {
> > + /* Workaround for occasional spurious wakeups from S5 (or
> > + * any other sleep) on Haswell machines with LPT and LPT-LP
> > + * with the new Intel BIOS
> > + */
> > + xhci->quirks |= XHCI_HSW_SPURIOUS_WAKEUP;
> > + }
> > if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
> > pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
> > xhci->quirks |= XHCI_RESET_ON_RESUME;
> > @@ -217,6 +226,11 @@ static void xhci_pci_remove(struct pci_dev *dev)
> > usb_put_hcd(xhci->shared_hcd);
> > }
> > usb_hcd_pci_remove(dev);
> > +
> > + /* Workaround for spurious wakeups at shutdown with HSW */
> > + if (xhci->quirks & XHCI_HSW_SPURIOUS_WAKEUP)
> > + pci_set_power_state(dev, PCI_D3cold);
> > +
>
> As far as I know, the LPT and LPT-LP hosts don't support D3cold. Is
> there a particular reason you set the power state to D3cold here?
It can be D3hot, too, as far as I've tested.
The only requirement is the ACPI _PS3 call, as it seems.
thanks,
Takashi
>
> Sarah Sharp
>
> > kfree(xhci);
> > }
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 49b6edb..c6027ba 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -728,11 +728,19 @@ void xhci_shutdown(struct usb_hcd *hcd)
> > if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
> > usb_disable_xhci_ports(to_pci_dev(hcd->self.controller));
> >
> > - spin_lock_irq(&xhci->lock);
> > - xhci_halt(xhci);
> > - spin_unlock_irq(&xhci->lock);
> > -
> > - xhci_cleanup_msix(xhci);
> > + if (xhci->quirks & XHCI_HSW_SPURIOUS_WAKEUP) {
> > + /* Workaround for spurious wakeups at shutdown with HSW:
> > + * we must call xhci_stop() for clearing all wakeups, and
> > + * set the device to D3.
> > + */
> > + xhci_stop(hcd);
> > + pci_set_power_state(to_pci_dev(hcd->self.controller), PCI_D3cold);
> > + } else {
> > + spin_lock_irq(&xhci->lock);
> > + xhci_halt(xhci);
> > + spin_unlock_irq(&xhci->lock);
> > + xhci_cleanup_msix(xhci);
> > + }
> >
> > xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> > "xhci_shutdown completed - status = %x",
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > index 46aa148..d71386b 100644
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1538,6 +1538,7 @@ struct xhci_hcd {
> > #define XHCI_COMP_MODE_QUIRK (1 << 14)
> > #define XHCI_AVOID_BEI (1 << 15)
> > #define XHCI_PLAT (1 << 16)
> > +#define XHCI_HSW_SPURIOUS_WAKEUP (1 << 17)
> > unsigned int num_active_eps;
> > unsigned int limit_active_eps;
> > /* There are two roothubs to keep track of bus suspend info for */
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index bc95b2b..1b3ab7d 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2860,7 +2860,9 @@
> > #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb
> > #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea
> > #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500
> > +#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
> > #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004
> > +#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
> > #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152
> >
> > #define PCI_VENDOR_ID_SCALEMP 0x8686
> > --
> > 1.8.4
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
