[RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Laura Abbott]

We received several reports of systems rebooting and powering on
after an attempted shutdown. Testing showed that setting
XHCI_SPURIOUS_WAKEUP quirk in addition to the XHCI_SPURIOUS_REBOOT
quirk allowed the system to shutdown as expected for Lynxpoint
xHCI controllers. Set the qurik.

Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
Bugzilla for reference:

https://bugzilla.redhat.com/show_bug.cgi?id=1257131
https://bugzilla.redhat.com/show_bug.cgi?id=1189107

There was some discussion if this is actually needed across all
chipsets or if it's just some revision. Not sure how to narrow
that down.
---
 drivers/usb/host/xhci-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 5590eac..e6ed595 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -147,6 +147,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
 		pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
 		xhci->quirks |= XHCI_SPURIOUS_REBOOT;
+		xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
 	}
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
 		(pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI ||
-- 
2.4.3

Re: [RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Mathias Nyman]

On 10.09.2015 20:27, Laura Abbott wrote:
>
> We received several reports of systems rebooting and powering on
> after an attempted shutdown. Testing showed that setting
> XHCI_SPURIOUS_WAKEUP quirk in addition to the XHCI_SPURIOUS_REBOOT
> quirk allowed the system to shutdown as expected for Lynxpoint
> xHCI controllers. Set the qurik.
>
> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
> ---

We used to have the XHCI_SPURIOUS_WAKEUP flag set for lynxpoint controllers,
but it was removed in commit:

commit b45abacde3d551c6696c6738bef4a1805d0bf27a
     xhci: no switching back on non-ULT Haswell
     
     The switch back is limited to ULT even on HP. The contrary
     finding arose by bad luck in BIOS versions for testing.
     This fixes spontaneous resume from S3 on some HP laptops.

Adding the SPURIOUS_WAKEUP flag back looks reasonable to me,
but I don't want to break suspend.
I don't understand how it could have caused spontaneous resume in HP laptops
in the first place, it really shouldn't do anything before shutdown.

Better ask Oliver,
Do you still have access to the HP laptop?
Any chance you could see if the flag still causes spontaneous resume?

-Mathias

Re: [RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Oliver Neukum]

On Fri, 2015-09-18 at 13:18 +0300, Mathias Nyman wrote:
> Better ask Oliver,
> Do you still have access to the HP laptop?

No I am sorry, we no longer have those laptops.

	Regards
		Oliver

Re: [RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Laura Abbott]

On 09/18/2015 03:18 AM, Mathias Nyman wrote:
> On 10.09.2015 20:27, Laura Abbott wrote:
>>
>> We received several reports of systems rebooting and powering on
>> after an attempted shutdown. Testing showed that setting
>> XHCI_SPURIOUS_WAKEUP quirk in addition to the XHCI_SPURIOUS_REBOOT
>> quirk allowed the system to shutdown as expected for Lynxpoint
>> xHCI controllers. Set the qurik.
>>
>> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
>> ---
>
> We used to have the XHCI_SPURIOUS_WAKEUP flag set for lynxpoint controllers,
> but it was removed in commit:
>
> commit b45abacde3d551c6696c6738bef4a1805d0bf27a
>      xhci: no switching back on non-ULT Haswell
>      The switch back is limited to ULT even on HP. The contrary
>      finding arose by bad luck in BIOS versions for testing.
>      This fixes spontaneous resume from S3 on some HP laptops.
>
> Adding the SPURIOUS_WAKEUP flag back looks reasonable to me,
> but I don't want to break suspend.
> I don't understand how it could have caused spontaneous resume in HP laptops
> in the first place, it really shouldn't do anything before shutdown.
>
> Better ask Oliver,
> Do you still have access to the HP laptop?
> Any chance you could see if the flag still causes spontaneous resume?
>
> -Mathias

Would you rather see a revert of the patch you gave rather than a new
one re-introducing the flag?

Thanks,
Laura

Re: [RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Oliver Neukum]

On Fri, 2015-09-18 at 09:56 -0700, Laura Abbott wrote:
> Would you rather see a revert of the patch you gave rather than a new
> one re-introducing the flag?

We need a big fat comment here saying that different tests should
different results and the quirk is needed for LynxPoint. That
suggests a new patch.

	Regards
		Oliver

Re: [RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Mathias Nyman]

On 18.09.2015 23:25, Oliver Neukum wrote:
> On Fri, 2015-09-18 at 09:56 -0700, Laura Abbott wrote:
>> Would you rather see a revert of the patch you gave rather than a new
>> one re-introducing the flag?
>
> We need a big fat comment here saying that different tests should
> different results and the quirk is needed for LynxPoint. That
> suggests a new patch.
>

I'll take the patch from Laura and add the big fat warning to it myself

-Mathias