[SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1764684

[Impact]
Some PCI device are not power managed by system firmware get runtime
suspended incorrectly. In this case it's an xHC device, which can't be
woken up by plugging USB device.

[Test]
With the patch, the xHC no longer get put to suspend incorrectly. USB
devices can work on the xHC now.

[Fix]
We need to check if the device is power managed by system firmware, also
if it supports PME from D3hot.

Now the patch is in upstream, we can backport it as what we did for
linux-oem:
https://lists.ubuntu.com/archives/kernel-team/2018-April/091665.html

[Regression Potential]
Low.
It fixes a regression caused by the PM core refactor.

Kai Heng Feng (1):
  PCI / PM: Always check PME wakeup capability for runtime wakeup
    support

 drivers/pci/pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [A/B] [PATCH 1/1] PCI / PM: Always check PME wakeup capability for runtime wakeup support

Kai-Heng Feng
From: Kai Heng Feng <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1764684

USB controller ASM1042 stops working after commit de3ef1eb1cd0 (PM /
core: Drop run_wake flag from struct dev_pm_info).

The device in question is not power managed by platform firmware,
furthermore, it only supports PME# from D3cold:
Capabilities: [78] Power Management version 3
       Flags: PMEClk- DSI- D1- D2- AuxCurrent=55mA PME(D0-,D1-,D2-,D3hot-,D3cold+)
       Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-

Before commit de3ef1eb1cd0, the device never gets runtime suspended.
After that commit, the device gets runtime suspended to D3hot, which can
not generate any PME#.

usb_hcd_pci_probe() unconditionally calls device_wakeup_enable(), hence
device_can_wakeup() in pci_dev_run_wake() always returns true.

So pci_dev_run_wake() needs to check PME wakeup capability as its first
condition.

In addition, change wakeup flag passed to pci_target_state() from false
to true, because we want to find the deepest state different from D3cold
that the device can still generate PME#. In this case, it's D0 for the
device in question.

Fixes: de3ef1eb1cd0 (PM / core: Drop run_wake flag from struct dev_pm_info)
Signed-off-by: Kai-Heng Feng <[hidden email]>
Cc: 4.13+ <[hidden email]> # 4.13+
Acked-by: Bjorn Helgaas <[hidden email]>
Signed-off-by: Rafael J. Wysocki <[hidden email]>
(cherry picked from commit 8feaec33b9868582654cd3d5355225dcb79aeca6)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/pci/pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8eac876b50af..6467af7cd5bd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2121,16 +2121,16 @@ bool pci_dev_run_wake(struct pci_dev *dev)
 {
  struct pci_bus *bus = dev->bus;
 
- if (device_can_wakeup(&dev->dev))
- return true;
-
  if (!dev->pme_support)
  return false;
 
  /* PME-capable in principle, but not from the target power state */
- if (!pci_pme_capable(dev, pci_target_state(dev, false)))
+ if (!pci_pme_capable(dev, pci_target_state(dev, true)))
  return false;
 
+ if (device_can_wakeup(&dev->dev))
+ return true;
+
  while (bus->parent) {
  struct pci_dev *bridge = bus->self;
 
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Po-Hsu Lin (Sam)
In reply to this post by Kai-Heng Feng
The "backport" mentioned in the cover-letter should be "cherry-picked" instead.
As this patch can be cherry-picked cleanly for these two kernels (and this is
what it says in the patch).

Acked-by: Po-Hsu Lin <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: ACK/cmnt: [SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Po-Hsu Lin (Sam)
One more thing,
This should be targeted to the Cosmic as well. (Can be cherry-picked too)

Didn't catch that at the very beginning, sorry.

On Mon, May 14, 2018 at 3:34 PM, Po-Hsu Lin <[hidden email]> wrote:
> The "backport" mentioned in the cover-letter should be "cherry-picked" instead.
> As this patch can be cherry-picked cleanly for these two kernels (and this is
> what it says in the patch).
>
> Acked-by: Po-Hsu Lin <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: ACK/cmnt: [SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Kai-Heng Feng
>
> On May 14, 2018, at 3:43 PM, Po-Hsu Lin <[hidden email]> wrote:
>
> One more thing,
> This should be targeted to the Cosmic as well. (Can be cherry-picked too)

Thanks for your finding.

Please also apply this to Cosmic.

Kai-Heng

>
> Didn't catch that at the very beginning, sorry.
>
> On Mon, May 14, 2018 at 3:34 PM, Po-Hsu Lin <[hidden email]>  
> wrote:
>> The "backport" mentioned in the cover-letter should be "cherry-picked"  
>> instead.
>> As this patch can be cherry-picked cleanly for these two kernels (and  
>> this is
>> what it says in the patch).
>>
>> Acked-by: Po-Hsu Lin <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

AceLan Kao-3
In reply to this post by Kai-Heng Feng
Reply | Threaded
Open this post in threaded view
|

[Acked] [SRU] [A/B] [PATCH 1/1] PCI / PM: Always check PME wakeup capability for runtime wakeup support

Andy Whitcroft-3
In reply to this post by Kai-Heng Feng
On Mon, May 14, 2018 at 03:07:45PM +0800, Kai-Heng Feng wrote:

> From: Kai Heng Feng <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1764684
>
> USB controller ASM1042 stops working after commit de3ef1eb1cd0 (PM /
> core: Drop run_wake flag from struct dev_pm_info).
>
> The device in question is not power managed by platform firmware,
> furthermore, it only supports PME# from D3cold:
> Capabilities: [78] Power Management version 3
>        Flags: PMEClk- DSI- D1- D2- AuxCurrent=55mA PME(D0-,D1-,D2-,D3hot-,D3cold+)
>        Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
>
> Before commit de3ef1eb1cd0, the device never gets runtime suspended.
> After that commit, the device gets runtime suspended to D3hot, which can
> not generate any PME#.
>
> usb_hcd_pci_probe() unconditionally calls device_wakeup_enable(), hence
> device_can_wakeup() in pci_dev_run_wake() always returns true.
>
> So pci_dev_run_wake() needs to check PME wakeup capability as its first
> condition.
>
> In addition, change wakeup flag passed to pci_target_state() from false
> to true, because we want to find the deepest state different from D3cold
> that the device can still generate PME#. In this case, it's D0 for the
> device in question.
>
> Fixes: de3ef1eb1cd0 (PM / core: Drop run_wake flag from struct dev_pm_info)
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> Cc: 4.13+ <[hidden email]> # 4.13+
> Acked-by: Bjorn Helgaas <[hidden email]>
> Signed-off-by: Rafael J. Wysocki <[hidden email]>
> (cherry picked from commit 8feaec33b9868582654cd3d5355225dcb79aeca6)
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> ---
>  drivers/pci/pci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8eac876b50af..6467af7cd5bd 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2121,16 +2121,16 @@ bool pci_dev_run_wake(struct pci_dev *dev)
>  {
>   struct pci_bus *bus = dev->bus;
>  
> - if (device_can_wakeup(&dev->dev))
> - return true;
> -
>   if (!dev->pme_support)
>   return false;
>  
>   /* PME-capable in principle, but not from the target power state */
> - if (!pci_pme_capable(dev, pci_target_state(dev, false)))
> + if (!pci_pme_capable(dev, pci_target_state(dev, true)))
>   return false;
>  
> + if (device_can_wakeup(&dev->dev))
> + return true;
> +
>   while (bus->parent) {
>   struct pci_dev *bridge = bus->self;
>  
> --
> 2.17.0
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Simple cherry-pick, looks to do what is claimed.

Acked-by: Andy Whitcroft <[hidden email]>

-apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Kleber Souza
In reply to this post by Kai-Heng Feng
On 05/14/18 09:07, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1764684
>
> [Impact]
> Some PCI device are not power managed by system firmware get runtime
> suspended incorrectly. In this case it's an xHC device, which can't be
> woken up by plugging USB device.
>
> [Test]
> With the patch, the xHC no longer get put to suspend incorrectly. USB
> devices can work on the xHC now.
>
> [Fix]
> We need to check if the device is power managed by system firmware, also
> if it supports PME from D3hot.
>
> Now the patch is in upstream, we can backport it as what we did for
> linux-oem:
> https://lists.ubuntu.com/archives/kernel-team/2018-April/091665.html
>
> [Regression Potential]
> Low.
> It fixes a regression caused by the PM core refactor.
>
> Kai Heng Feng (1):
>   PCI / PM: Always check PME wakeup capability for runtime wakeup
>     support
>
>  drivers/pci/pci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>

Applied to artful/master-next and bionic/master-next branches.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED: [SRU] [A/B] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Po-Hsu Lin (Sam)
This needs to be applied to Cosmic as well, as per sender's request in
the following ACK/cmnt discussions.
https://lists.ubuntu.com/archives/kernel-team/2018-May/092372.html

On Wed, May 16, 2018 at 12:22 AM, Kleber Souza
<[hidden email]> wrote:

> On 05/14/18 09:07, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1764684
>>
>> [Impact]
>> Some PCI device are not power managed by system firmware get runtime
>> suspended incorrectly. In this case it's an xHC device, which can't be
>> woken up by plugging USB device.
>>
>> [Test]
>> With the patch, the xHC no longer get put to suspend incorrectly. USB
>> devices can work on the xHC now.
>>
>> [Fix]
>> We need to check if the device is power managed by system firmware, also
>> if it supports PME from D3hot.
>>
>> Now the patch is in upstream, we can backport it as what we did for
>> linux-oem:
>> https://lists.ubuntu.com/archives/kernel-team/2018-April/091665.html
>>
>> [Regression Potential]
>> Low.
>> It fixes a regression caused by the PM core refactor.
>>
>> Kai Heng Feng (1):
>>   PCI / PM: Always check PME wakeup capability for runtime wakeup
>>     support
>>
>>  drivers/pci/pci.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>
> Applied to artful/master-next and bionic/master-next branches.
>
> Thanks,
> Kleber
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED: [SRU] [A/B][Cosmic] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Kleber Souza
On 05/15/18 18:55, Po-Hsu Lin wrote:
> This needs to be applied to Cosmic as well, as per sender's request in
> the following ACK/cmnt discussions.
> https://lists.ubuntu.com/archives/kernel-team/2018-May/092372.html

Cosmic is handled by the development team, I have added a Cosmic tag on
the subject to get their attention.


Thanks,
Kleber

>
> On Wed, May 16, 2018 at 12:22 AM, Kleber Souza
> <[hidden email]> wrote:
>> On 05/14/18 09:07, Kai-Heng Feng wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1764684
>>>
>>> [Impact]
>>> Some PCI device are not power managed by system firmware get runtime
>>> suspended incorrectly. In this case it's an xHC device, which can't be
>>> woken up by plugging USB device.
>>>
>>> [Test]
>>> With the patch, the xHC no longer get put to suspend incorrectly. USB
>>> devices can work on the xHC now.
>>>
>>> [Fix]
>>> We need to check if the device is power managed by system firmware, also
>>> if it supports PME from D3hot.
>>>
>>> Now the patch is in upstream, we can backport it as what we did for
>>> linux-oem:
>>> https://lists.ubuntu.com/archives/kernel-team/2018-April/091665.html
>>>
>>> [Regression Potential]
>>> Low.
>>> It fixes a regression caused by the PM core refactor.
>>>
>>> Kai Heng Feng (1):
>>>   PCI / PM: Always check PME wakeup capability for runtime wakeup
>>>     support
>>>
>>>  drivers/pci/pci.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>
>> Applied to artful/master-next and bionic/master-next branches.
>>
>> Thanks,
>> Kleber
>>
>> --
>> kernel-team mailing list
>> [hidden email]
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED: [SRU] [A/B][Cosmic] [PATCH 0/1] Fix an issue that some PCI devices get incorrectly suspended

Seth Forshee
On Wed, May 16, 2018 at 09:17:44AM +0200, Kleber Souza wrote:
> On 05/15/18 18:55, Po-Hsu Lin wrote:
> > This needs to be applied to Cosmic as well, as per sender's request in
> > the following ACK/cmnt discussions.
> > https://lists.ubuntu.com/archives/kernel-team/2018-May/092372.html
>
> Cosmic is handled by the development team, I have added a Cosmic tag on
> the subject to get their attention.

Kind of depends on what is meant by "cosmic" here ... currently the
kernel in cosmic-release is copied forward from bionic. We are working
on the unstable kernel which will become cosmic's kernel later on, but
this is an upstream cherry pick and should already be in unstable or
picked up on the next rebase.

In any case, we do monitor for patches sent for bionic and apply them to
unstable as needed.

Seth

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team