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?
Also, you've got version numbers in the subject line, and version info
in the patch body. I was just going to clean that up until I noticed
the pci_ids.h issue.
Sarah Sharp
On Wed, Sep 11, 2013 at 02:43:24PM +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_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.
>
> v1->v2: forgot to refresh the patch before submission
> v2->v3: reduce the workaround, just call xhci_reset(); also use D3hot
>
> Cc: Oliver Neukum <oneukum@xxxxxxx>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> ---
> drivers/usb/host/xhci-pci.c | 14 ++++++++++++++
> drivers/usb/host/xhci.c | 7 +++++++
> drivers/usb/host/xhci.h | 1 +
> include/linux/pci_ids.h | 2 ++
> 4 files changed, 24 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index c2d4950..52fa71e 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_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 */
> 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
