Re: [RESEND patch 2.6.25] ACPI uses device_may_wakeup() policy inputs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2008-04-18 at 21:18 -0700, David Brownell wrote:
> This imports the driver model device.power.may_wakeup flags to ACPI,
> using it to *REPLACE* the /proc/acpi/wakeup flags for some devices.
> It depends on the previous patch making device.power.can_wakeup behave.
> It does that by:
> 
>  - Implementing platform_enable_wakeup(), which is currently invoked only
>    by pci_enable_wake().  When that's called -- probably in the driver
>    suspend() call -- it updates acpi_device.wakeup.state.enabled flag in
>    the same way writing to /proc/acpi/wakeup updates it.
>    
>  - Updating the usage of the corresponding ACPI flags when turning on
>    wakeup power domains and GPEs.
> 
> THIS PATCH NEEDS MORE ATTENTION because of the way the ACPI method
> invocations have been changing, e.g. the 1.0 vs 2.0 sequencing.
> 
> Right now it's not clear to me whether the GPEs are always enabled at
> the right time, and for that matter whether the rules haven't changed
> so that drivers can no longer effectively control those settings from
> suspend() unless acpi_new_pts_ordering is in effect.
Agree with what Rui said. The patch doesn't change the time when GPEs
are enabled/disabled.

But after the patch is applied, some PCI device(the ACPI device with the
_PRW object) can wake the sleeping system by default. And it is totally
opposite to the current flowchart. 

> ACPI systems see behavioral changes as follows:
> 
>    * Wakeup-aware PCI drivers no longer need to have someone override the
>      initial system config by writing to /proc/acpi/wakeup; existing calls
>      to pci_enable_wake() suffice.  For wakeup-unaware drivers, it's still
>      not clearly safe to enable wakeup in /proc/acpi/wakeup.
> 
>    * Non-PCI frameworks (as for PS2 serial ports) could call the platform
>      hook like PCI does, supporting wakeup-aware drivers.
> 
>    * The /sys/devices/.../power/wakeup flags are a different kind of manual
>      override.  They _disable_ wakeup for broken hardware or drivers, rather
>      than _enabling_ it in the hope that unmodified drivers won't break when
>      their hardware triggers wakeup events.
> 
> NOT YET SIGNED-OFF ... primarily because of the confusion about
> the order in which ACPI methods get called during entry to suspend
> states.  Presumably one of the "new style" PM methods calls will
> now always work for drivers wanting to enable wakeup methods...
> 
> ---
>  drivers/acpi/sleep/main.c   |    2 +-
>  drivers/acpi/sleep/wakeup.c |   44 ++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 
> --- g26.orig/drivers/acpi/sleep/main.c	2008-02-24 02:07:57.000000000 -0800
> +++ g26/drivers/acpi/sleep/main.c	2008-02-24 02:08:04.000000000 -0800
> @@ -470,7 +470,7 @@ int acpi_pm_device_sleep_state(struct de
>  	 * can wake the system.  _S0W may be valid, too.
>  	 */
>  	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> -	    (device_may_wakeup(dev) && adev->wakeup.state.enabled &&
> +	    (device_may_wakeup(dev) &&
>  	     adev->wakeup.sleep_state <= acpi_target_sleep_state)) {
>  		acpi_status status;
>  
> --- g26.orig/drivers/acpi/sleep/wakeup.c	2008-02-24 02:03:27.000000000 -0800
> +++ g26/drivers/acpi/sleep/wakeup.c	2008-02-24 02:08:04.000000000 -0800
> @@ -35,12 +35,22 @@ void acpi_enable_wakeup_device_prep(u8 s
>  		struct acpi_device *dev = container_of(node,
>  						       struct acpi_device,
>  						       wakeup_list);
> +		struct device *ldev;
>  
>  		if (!dev->wakeup.flags.valid ||
>  		    !dev->wakeup.state.enabled ||
>  		    (sleep_state > (u32) dev->wakeup.sleep_state))
>  			continue;
>  
> +		ldev = acpi_get_physical_device(dev->handle);
> +		if (ldev) {
> +			int flag = device_may_wakeup(ldev);
> +
> +			put_device(ldev);
> +			if (!flag)
> +				continue;
> +		}
> +
>  		spin_unlock(&acpi_device_lock);
>  		acpi_enable_wakeup_device_power(dev);
>  		spin_lock(&acpi_device_lock);
> @@ -57,8 +67,8 @@ void acpi_enable_wakeup_device(u8 sleep_
>  {
>  	struct list_head *node, *next;
>  
> -	/* 
> -	 * Caution: this routine must be invoked when interrupt is disabled 
> +	/*
> +	 * Caution: this routine must be invoked when interrupt is disabled
>  	 * Refer ACPI2.0: P212
>  	 */
>  	ACPI_FUNCTION_TRACE("acpi_enable_wakeup_device");
> @@ -107,6 +117,7 @@ void acpi_disable_wakeup_device(u8 sleep
>  	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
>  		struct acpi_device *dev =
>  			container_of(node, struct acpi_device, wakeup_list);
> +		struct device *ldev;
>  
>  		if (!dev->wakeup.flags.valid)
>  			continue;
> @@ -125,6 +136,15 @@ void acpi_disable_wakeup_device(u8 sleep
>  			continue;
>  		}
>  
> +		ldev = acpi_get_physical_device(dev->handle);
> +		if (ldev) {
> +			int flag = device_may_wakeup(ldev);
> +
> +			put_device(ldev);
> +			if (!flag)
> +				continue;
> +		}
> +
>  		spin_unlock(&acpi_device_lock);
>  		acpi_disable_wakeup_device_power(dev);
>  		/* Never disable run-wake GPE */
> @@ -139,6 +159,24 @@ void acpi_disable_wakeup_device(u8 sleep
>  	spin_unlock(&acpi_device_lock);
>  }
>  
> +static int acpi_platform_enable_wakeup(struct device *dev, int is_on)
> +{
> +	struct acpi_device	*adev;
> +	int			status;
> +
> +	if (!device_can_wakeup(dev))
> +		return -EINVAL;
> +	if (is_on && !device_may_wakeup(dev))
> +		return -EINVAL;
> +
> +	status = acpi_bus_get_device(DEVICE_ACPI_HANDLE(dev), &adev);
> +	if (status < 0)
> +		return status;
> +
> +	adev->wakeup.state.enabled = !!is_on;
> +	return 0;
> +}
> +
>  static int __init acpi_wakeup_device_init(void)
>  {
>  	struct list_head *node, *next;
> @@ -146,6 +184,8 @@ static int __init acpi_wakeup_device_ini
>  	if (acpi_disabled)
>  		return 0;
>  
> +	platform_enable_wakeup = acpi_platform_enable_wakeup;
> +
>  	spin_lock(&acpi_device_lock);
>  	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
>  		struct acpi_device *dev = container_of(node,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux