[PATCH] PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

[PATCH] PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

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 <kamil.54002@iskra.name>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 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

Re: [PATCH] PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]

On Tue, 7 Feb 2012 00:50:35 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> 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 <kamil.54002@iskra.name>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

[Rafael J. Wysocki]

On Friday, February 10, 2012, Jesse Barnes wrote:
> On Tue, 7 Feb 2012 00:50:35 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > 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 <kamil.54002@iskra.name>
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  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

Re: [PATCH] PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]

On Fri, 10 Feb 2012 23:40:29 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Friday, February 10, 2012, Jesse Barnes wrote:
> > On Tue, 7 Feb 2012 00:50:35 +0100
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > 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 <kamil.54002@iskra.name>
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > ---
> > >  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]