From: Rafael J. Wysocki <[email protected]>
If a PCI device is enabled to generate wakeup signals (PME) when put
into a low-power state by runtime PM, it will be still enabled to
generate those signals after the system shutdown, unless its driver's
.shutdown() callback takes care of the wakeup signals generation
setting. Moreover, there are devices that are not enabled to wake
up the system and that are configured by runtime PM to generate
wakeup signals so that (runtime) remote wakeup works with them.
Those devices should be reconfigured during system shutdown so that
they don't generate wakeup signals, but at least some drivers don't
do that. However, that very well may be done by the PCI core so
that drivers don't have to worry about it. For this reason, modify
pci_device_shutdown() to disable the generation of wakeup events for
devices not supposed to wake up the system.
References: https://bugzilla.kernel.org/show_bug.cgi?id=37952
Reported-and-tested-by: Kamil Iskra <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/pci-driver.c | 10 ++++++++++
1 file changed, 10 insertions(+)
Index: linux/drivers/pci/pci-driver.c
===================================================================
--- linux.orig/drivers/pci/pci-driver.c
+++ linux/drivers/pci/pci-driver.c
@@ -430,6 +430,16 @@ static void pci_device_shutdown(struct d
drv->shutdown(pci_dev);
pci_msi_shutdown(pci_dev);
pci_msix_shutdown(pci_dev);
+
+ /*
+ * Devices may be enabled to wake up by runtime PM, but they need not
+ * be supposed to wake up the system from its "power off" state (e.g.
+ * ACPI S5). Therefore disable wakeup for all devices that aren't
+ * supposed to wake up the system at this point. The state argument
+ * will be ignored by pci_enable_wake().
+ */
+ if (!device_may_wakeup(dev))
+ pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
}
#ifdef CONFIG_PM
On Tue, 7 Feb 2012 00:50:35 +0100
"Rafael J. Wysocki" <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> If a PCI device is enabled to generate wakeup signals (PME) when put
> into a low-power state by runtime PM, it will be still enabled to
> generate those signals after the system shutdown, unless its driver's
> .shutdown() callback takes care of the wakeup signals generation
> setting. Moreover, there are devices that are not enabled to wake
> up the system and that are configured by runtime PM to generate
> wakeup signals so that (runtime) remote wakeup works with them.
> Those devices should be reconfigured during system shutdown so that
> they don't generate wakeup signals, but at least some drivers don't
> do that. However, that very well may be done by the PCI core so
> that drivers don't have to worry about it. For this reason, modify
> pci_device_shutdown() to disable the generation of wakeup events for
> devices not supposed to wake up the system.
>
> References: https://bugzilla.kernel.org/show_bug.cgi?id=37952
> Reported-and-tested-by: Kamil Iskra <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/pci/pci-driver.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> Index: linux/drivers/pci/pci-driver.c
> ===================================================================
> --- linux.orig/drivers/pci/pci-driver.c
> +++ linux/drivers/pci/pci-driver.c
> @@ -430,6 +430,16 @@ static void pci_device_shutdown(struct d
> drv->shutdown(pci_dev);
> pci_msi_shutdown(pci_dev);
> pci_msix_shutdown(pci_dev);
> +
> + /*
> + * Devices may be enabled to wake up by runtime PM, but they need not
> + * be supposed to wake up the system from its "power off" state (e.g.
> + * ACPI S5). Therefore disable wakeup for all devices that aren't
> + * supposed to wake up the system at this point. The state argument
> + * will be ignored by pci_enable_wake().
> + */
> + if (!device_may_wakeup(dev))
> + pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
> }
>
> #ifdef CONFIG_PM
>
So where do we end up setting the right wakeup state for devices we
actually *do* want to wake the system from S5? Presumably later in the
shutdown sequence?
--
Jesse Barnes, Intel Open Source Technology Center
On Friday, February 10, 2012, Jesse Barnes wrote:
> On Tue, 7 Feb 2012 00:50:35 +0100
> "Rafael J. Wysocki" <[email protected]> wrote:
>
> > From: Rafael J. Wysocki <[email protected]>
> >
> > If a PCI device is enabled to generate wakeup signals (PME) when put
> > into a low-power state by runtime PM, it will be still enabled to
> > generate those signals after the system shutdown, unless its driver's
> > .shutdown() callback takes care of the wakeup signals generation
> > setting. Moreover, there are devices that are not enabled to wake
> > up the system and that are configured by runtime PM to generate
> > wakeup signals so that (runtime) remote wakeup works with them.
> > Those devices should be reconfigured during system shutdown so that
> > they don't generate wakeup signals, but at least some drivers don't
> > do that. However, that very well may be done by the PCI core so
> > that drivers don't have to worry about it. For this reason, modify
> > pci_device_shutdown() to disable the generation of wakeup events for
> > devices not supposed to wake up the system.
> >
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=37952
> > Reported-and-tested-by: Kamil Iskra <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/pci/pci-driver.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > Index: linux/drivers/pci/pci-driver.c
> > ===================================================================
> > --- linux.orig/drivers/pci/pci-driver.c
> > +++ linux/drivers/pci/pci-driver.c
> > @@ -430,6 +430,16 @@ static void pci_device_shutdown(struct d
> > drv->shutdown(pci_dev);
> > pci_msi_shutdown(pci_dev);
> > pci_msix_shutdown(pci_dev);
> > +
> > + /*
> > + * Devices may be enabled to wake up by runtime PM, but they need not
> > + * be supposed to wake up the system from its "power off" state (e.g.
> > + * ACPI S5). Therefore disable wakeup for all devices that aren't
> > + * supposed to wake up the system at this point. The state argument
> > + * will be ignored by pci_enable_wake().
> > + */
> > + if (!device_may_wakeup(dev))
> > + pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
> > }
> >
> > #ifdef CONFIG_PM
> >
>
> So where do we end up setting the right wakeup state for devices we
> actually *do* want to wake the system from S5? Presumably later in the
> shutdown sequence?
Yes, in general. However, device_may_wakeup(dev) has to return true for those
devices, so they aren't affected by this change.
Thanks,
Rafael
On Fri, 10 Feb 2012 23:40:29 +0100
"Rafael J. Wysocki" <[email protected]> wrote:
> On Friday, February 10, 2012, Jesse Barnes wrote:
> > On Tue, 7 Feb 2012 00:50:35 +0100
> > "Rafael J. Wysocki" <[email protected]> wrote:
> >
> > > From: Rafael J. Wysocki <[email protected]>
> > >
> > > If a PCI device is enabled to generate wakeup signals (PME) when put
> > > into a low-power state by runtime PM, it will be still enabled to
> > > generate those signals after the system shutdown, unless its driver's
> > > .shutdown() callback takes care of the wakeup signals generation
> > > setting. Moreover, there are devices that are not enabled to wake
> > > up the system and that are configured by runtime PM to generate
> > > wakeup signals so that (runtime) remote wakeup works with them.
> > > Those devices should be reconfigured during system shutdown so that
> > > they don't generate wakeup signals, but at least some drivers don't
> > > do that. However, that very well may be done by the PCI core so
> > > that drivers don't have to worry about it. For this reason, modify
> > > pci_device_shutdown() to disable the generation of wakeup events for
> > > devices not supposed to wake up the system.
> > >
> > > References: https://bugzilla.kernel.org/show_bug.cgi?id=37952
> > > Reported-and-tested-by: Kamil Iskra <[email protected]>
> > > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > > ---
> > > drivers/pci/pci-driver.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > Index: linux/drivers/pci/pci-driver.c
> > > ===================================================================
> > > --- linux.orig/drivers/pci/pci-driver.c
> > > +++ linux/drivers/pci/pci-driver.c
> > > @@ -430,6 +430,16 @@ static void pci_device_shutdown(struct d
> > > drv->shutdown(pci_dev);
> > > pci_msi_shutdown(pci_dev);
> > > pci_msix_shutdown(pci_dev);
> > > +
> > > + /*
> > > + * Devices may be enabled to wake up by runtime PM, but they need not
> > > + * be supposed to wake up the system from its "power off" state (e.g.
> > > + * ACPI S5). Therefore disable wakeup for all devices that aren't
> > > + * supposed to wake up the system at this point. The state argument
> > > + * will be ignored by pci_enable_wake().
> > > + */
> > > + if (!device_may_wakeup(dev))
> > > + pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
> > > }
> > >
> > > #ifdef CONFIG_PM
> > >
> >
> > So where do we end up setting the right wakeup state for devices we
> > actually *do* want to wake the system from S5? Presumably later in the
> > shutdown sequence?
>
> Yes, in general. However, device_may_wakeup(dev) has to return true for those
> devices, so they aren't affected by this change.
Ok thanks Rafael, applied. Now to do a little testing...
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center