Re: [RFC][PATCH 4/4] PCI/ACPI PM: Propagate wake-up enable for devices w/o ACPI support

[ykzhao <yakui.zhao@intel.com>]

On Fri, 2009-09-04 at 06:07 +0800, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Some PCI devices (not PCI Express), like PCI add-on cards, can
> generate PME#, but they don't have any special platform wake-up
> support.  For this reason, even if they generate PME# to wake up the
> system from a sleep state, wake-up events are not generated by the
> platform.
> 
> It turns out that, at least on some systems, PCI bridges and the PCI
> host bridge have ACPI GPEs associated with them that, if enabled to
> generate wake-up events, allow the system to wake up if one of the
> add-on devices asserts PME# while the system is in a sleep state.
> Following this observation, if a PCI device without direct ACPI
> wake-up support is prepared to wake up the system during a transition
> into a sleep state (eg. suspend to RAM), configure all the bridges on
> the path from the device to the root bridge (including the root
> bridge itself) to wake-up the system.
>From the description it seems that it will propagate the wake-up
function to its parent device. 
Is it enough to stop the propagation when we find one device that can
generate wake-up events in its parent device tree?

thanks.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/power.c    |   52 ++++++++++++++++++++++++++++++++----------------
>  drivers/acpi/scan.c     |    1 
>  drivers/acpi/sleep.c    |    2 -
>  drivers/acpi/wakeup.c   |    4 +--
>  drivers/pci/pci-acpi.c  |   25 +++++++++++++++++++++--
>  include/acpi/acpi_bus.h |    2 -
>  6 files changed, 63 insertions(+), 23 deletions(-)
> 
> Index: linux-2.6/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci-acpi.c
> +++ linux-2.6/drivers/pci/pci-acpi.c
> @@ -109,10 +109,31 @@ static bool acpi_pci_can_wakeup(struct p
>  	return handle ? acpi_bus_can_wakeup(handle) : false;
>  }
>  
> +static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus, bool enable)
> +{
> +	while (bus->parent) {
> +		struct pci_dev *bridge = bus->self;
> +
> +		acpi_pm_device_sleep_wake(&bridge->dev, enable);
> +		if (bridge->is_pcie)
> +			return;
> +		bus = bus->parent;
> +	}
> +
> +	/* We have reached the root bus. */
> +	if (bus->bridge)
> +		acpi_pm_device_sleep_wake(bus->bridge, enable);
> +}
> +
>  static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
>  {
> -	return acpi_pci_can_wakeup(dev) ?
> -		acpi_pm_device_sleep_wake(&dev->dev, enable) : 0;
> +	if (acpi_pci_can_wakeup(dev))
> +		return acpi_pm_device_sleep_wake(&dev->dev, enable);
> +
> +	if (!dev->is_pcie)
> +		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
> +
> +	return 0;
>  }
>  
>  static struct pci_platform_pm_ops acpi_pci_platform_pm = {
> Index: linux-2.6/drivers/acpi/sleep.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/sleep.c
> +++ linux-2.6/drivers/acpi/sleep.c
> @@ -691,7 +691,7 @@ int acpi_pm_device_sleep_wake(struct dev
>  	struct acpi_device *adev;
>  	int error;
>  
> -	if (!device_may_wakeup(dev))
> +	if (!device_can_wakeup(dev))
>  		return -EINVAL;
>  
>  	handle = DEVICE_ACPI_HANDLE(dev);
> Index: linux-2.6/drivers/acpi/scan.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/scan.c
> +++ linux-2.6/drivers/acpi/scan.c
> @@ -782,6 +782,7 @@ static int acpi_bus_get_wakeup_device_fl
>  	kfree(buffer.pointer);
>  
>  	device->wakeup.flags.valid = 1;
> +	device->wakeup.prepare_count = 0;
>  	/* Call _PSW/_DSW object to disable its ability to wake the sleeping
>  	 * system for the ACPI device with the _PRW object.
>  	 * The _PSW object is depreciated in ACPI 3.0 and is replaced by _DSW.
> Index: linux-2.6/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpi_bus.h
> +++ linux-2.6/include/acpi/acpi_bus.h
> @@ -248,7 +248,6 @@ struct acpi_device_perf {
>  /* Wakeup Management */
>  struct acpi_device_wakeup_flags {
>  	u8 valid:1;		/* Can successfully enable wakeup? */
> -	u8 prepared:1;		/* Has the wake-up capability been enabled? */
>  	u8 run_wake:1;		/* Run-Wake GPE devices */
>  };
>  
> @@ -263,6 +262,7 @@ struct acpi_device_wakeup {
>  	struct acpi_handle_list resources;
>  	struct acpi_device_wakeup_state state;
>  	struct acpi_device_wakeup_flags flags;
> +	int prepare_count;
>  };
>  
>  /* Device */
> Index: linux-2.6/drivers/acpi/power.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/power.c
> +++ linux-2.6/drivers/acpi/power.c
> @@ -44,6 +44,8 @@
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
>  
> +#include "sleep.h"
> +
>  #define _COMPONENT			ACPI_POWER_COMPONENT
>  ACPI_MODULE_NAME("power");
>  #define ACPI_POWER_CLASS		"power_resource"
> @@ -361,17 +363,19 @@ int acpi_device_sleep_wake(struct acpi_d
>   */
>  int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
>  {
> -	int i, err;
> +	int i, err = 0;
>  
>  	if (!dev || !dev->wakeup.flags.valid)
>  		return -EINVAL;
>  
> +	mutex_lock(&acpi_device_lock);
> +
>  	/*
>  	 * Do not execute the code below twice in a row without calling
>  	 * acpi_disable_wakeup_device_power() in between for the same device
>  	 */
> -	if (dev->wakeup.flags.prepared)
> -		return 0;
> +	if (dev->wakeup.prepare_count++)
> +		goto out;
>  
>  	/* Open power resource */
>  	for (i = 0; i < dev->wakeup.resources.count; i++) {
> @@ -379,7 +383,8 @@ int acpi_enable_wakeup_device_power(stru
>  		if (ret) {
>  			printk(KERN_ERR PREFIX "Transition power state\n");
>  			dev->wakeup.flags.valid = 0;
> -			return -ENODEV;
> +			err = -ENODEV;
> +			goto err_out;
>  		}
>  	}
>  
> @@ -388,9 +393,13 @@ int acpi_enable_wakeup_device_power(stru
>  	 * in arbitrary power state afterwards.
>  	 */
>  	err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
> -	if (!err)
> -		dev->wakeup.flags.prepared = 1;
>  
> + err_out:
> +	if (err)
> +		dev->wakeup.prepare_count = 0;
> +
> + out:
> +	mutex_unlock(&acpi_device_lock);
>  	return err;
>  }
>  
> @@ -402,35 +411,44 @@ int acpi_enable_wakeup_device_power(stru
>   */
>  int acpi_disable_wakeup_device_power(struct acpi_device *dev)
>  {
> -	int i, ret;
> +	int i, err = 0;
>  
>  	if (!dev || !dev->wakeup.flags.valid)
>  		return -EINVAL;
>  
> +	mutex_lock(&acpi_device_lock);
> +
>  	/*
>  	 * Do not execute the code below twice in a row without calling
>  	 * acpi_enable_wakeup_device_power() in between for the same device
>  	 */
> -	if (!dev->wakeup.flags.prepared)
> -		return 0;
> -
> -	dev->wakeup.flags.prepared = 0;
> +	if (--dev->wakeup.prepare_count) {
> +		if (dev->wakeup.prepare_count < 0) {
> +			dev_warn(&dev->dev, "unbalanced %s!\n", __func__);
> +			dev->wakeup.prepare_count = 0;
> +		}
> +		goto out;
> +	}
>  
> -	ret = acpi_device_sleep_wake(dev, 0, 0, 0);
> -	if (ret)
> -		return ret;
> +	err = acpi_device_sleep_wake(dev, 0, 0, 0);
> +	if (err)
> +		goto out;
>  
>  	/* Close power resource */
>  	for (i = 0; i < dev->wakeup.resources.count; i++) {
> -		ret = acpi_power_off_device(dev->wakeup.resources.handles[i], dev);
> +		int ret = acpi_power_off_device(
> +				dev->wakeup.resources.handles[i], dev);
>  		if (ret) {
>  			printk(KERN_ERR PREFIX "Transition power state\n");
>  			dev->wakeup.flags.valid = 0;
> -			return -ENODEV;
> +			err = -ENODEV;
> +			goto out;
>  		}
>  	}
>  
> -	return ret;
> + out:
> +	mutex_unlock(&acpi_device_lock);
> +	return err;
>  }
>  
>  /* --------------------------------------------------------------------------
> Index: linux-2.6/drivers/acpi/wakeup.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/wakeup.c
> +++ linux-2.6/drivers/acpi/wakeup.c
> @@ -68,7 +68,7 @@ void acpi_enable_wakeup_device(u8 sleep_
>  		/* If users want to disable run-wake GPE,
>  		 * we only disable it for wake and leave it for runtime
>  		 */
> -		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> +		if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
>  		    || sleep_state > (u32) dev->wakeup.sleep_state) {
>  			if (dev->wakeup.flags.run_wake) {
>  				/* set_gpe_type will disable GPE, leave it like that */
> @@ -100,7 +100,7 @@ void acpi_disable_wakeup_device(u8 sleep
>  		if (!dev->wakeup.flags.valid)
>  			continue;
>  
> -		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> +		if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
>  		    || sleep_state > (u32) dev->wakeup.sleep_state) {
>  			if (dev->wakeup.flags.run_wake) {
>  				acpi_set_gpe_type(dev->wakeup.gpe_device,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html