At Wed, 11 Sep 2013 16:42:09 -0700,
Sarah Sharp wrote:
>
> Hi Takashi,
>
> You've added the PCI device IDs of the xHCI LPT and LPT-LP to pci_ids.h.
> AFAIK, the xHCI driver is the only driver to use it, and I thought the
> rule was that we would only add the device IDs to pci_ids.h if more than
> one driver used it.
>
> Wouldn't it make sense to avoid polluting pci_ids.h and move the device
> IDs to xhci-pci.c?
I don't know of any strict rule, but indeed many drivers usually keep
the PCI ID definitions local. I took this approach just to make it
similar as PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI case.
> Also, you've got version numbers in the subject line, and version info
> in the patch body.
This is really a matter of taste :)
> I was just going to clean that up until I noticed
> the pci_ids.h issue.
OK, below is the v4 patch with these cleanups.
thanks,
Takashi
---
From: Takashi Iwai <tiwai@xxxxxxx>
Subject: [PATCH v4] xhci: Fix spurious wakeups after S5 on Haswell
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_reset() in the xhci
shutdown ops as done in xhci_stop(), and setting the device to PCI D3
at shutdown and remove ops.
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
v2->v3: reduce the workaround, just call xhci_reset(); also use D3hot
v3->v4: move LPT pci ids to xhci-pci.c, removed version# from subject
drivers/usb/host/xhci-pci.c | 17 +++++++++++++++++
drivers/usb/host/xhci.c | 7 +++++++
drivers/usb/host/xhci.h | 1 +
3 files changed, 25 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c2d4950..344edde 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -35,6 +35,9 @@
#define PCI_VENDOR_ID_ETRON 0x1b6f
#define PCI_DEVICE_ID_ASROCK_P67 0x7023
+#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
+#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
+
static const char hcd_name[] = "xhci_hcd";
/* called after powerup, by probe or system-pm "wakeup" */
@@ -110,6 +113,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 +229,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_D3hot);
+
kfree(xhci);
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 49b6edb..d6f953c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -730,6 +730,9 @@ void xhci_shutdown(struct usb_hcd *hcd)
spin_lock_irq(&xhci->lock);
xhci_halt(xhci);
+ /* Workaround for spurious wakeups at shutdown with HSW */
+ if (xhci->quirks & XHCI_HSW_SPURIOUS_WAKEUP)
+ xhci_reset(xhci);
spin_unlock_irq(&xhci->lock);
xhci_cleanup_msix(xhci);
@@ -737,6 +740,10 @@ void xhci_shutdown(struct usb_hcd *hcd)
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"xhci_shutdown completed - status = %x",
xhci_readl(xhci, &xhci->op_regs->status));
+
+ /* Yet another workaround for spurious wakeups at shutdown with HSW */
+ if (xhci->quirks & XHCI_HSW_SPURIOUS_WAKEUP)
+ pci_set_power_state(to_pci_dev(hcd->self.controller), PCI_D3hot);
}
#ifdef CONFIG_PM
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 */
--
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
