Patchwork [1/1] ACPI / PM: Make acpi_pm_device_sleep_state() follow the specification

login
register
mail settings
Submitter Colin King
Date July 11, 2012, 3:24 p.m.
Message ID <1342020250-5149-2-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/170483/
State New
Headers show

Comments

Colin King - July 11, 2012, 3:24 p.m.
From: "Rafael J. Wysocki" <rjw@sisk.pl>

BugLink: http://bugs.launchpad.net/bugs/904261

The comparison between the system sleep state being entered
and the lowest system sleep state the given device may wake up
from in acpi_pm_device_sleep_state() is reversed, because the
specification (ACPI 5.0) says that for wakeup to work:

"The sleeping state being entered must be less than or equal to the
 power state declared in element 1 of the _PRW object."

In other words, the state returned by _PRW is the deepest
(lowest-power) system sleep state the device is capable of waking up
the system from.

Moreover, acpi_pm_device_sleep_state() also should check if the
wakeup capability is supported through ACPI, because in principle it
may be done via native PCIe PME, for example, in which case _SxW
should not be evaluated.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/acpi/sleep.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Tim Gardner - July 11, 2012, 4:25 p.m.
Upstream commit dbe9a2edd17d843d80faf2b99f20a691c1853418

rtg
Seth Forshee - July 11, 2012, 4:44 p.m.
On Wed, Jul 11, 2012 at 04:24:10PM +0100, Colin King wrote:
> From: "Rafael J. Wysocki" <rjw@sisk.pl>
> 
> BugLink: http://bugs.launchpad.net/bugs/904261
> 
> The comparison between the system sleep state being entered
> and the lowest system sleep state the given device may wake up
> from in acpi_pm_device_sleep_state() is reversed, because the
> specification (ACPI 5.0) says that for wakeup to work:
> 
> "The sleeping state being entered must be less than or equal to the
>  power state declared in element 1 of the _PRW object."
> 
> In other words, the state returned by _PRW is the deepest
> (lowest-power) system sleep state the device is capable of waking up
> the system from.
> 
> Moreover, acpi_pm_device_sleep_state() also should check if the
> wakeup capability is supported through ACPI, because in principle it
> may be done via native PCIe PME, for example, in which case _SxW
> should not be evaluated.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/acpi/sleep.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index ca191ff..ed6bc52 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -702,8 +702,8 @@ int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p)
>  	 * can wake the system.  _S0W may be valid, too.
>  	 */
>  	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> -	    (device_may_wakeup(dev) &&
> -	     adev->wakeup.sleep_state <= acpi_target_sleep_state)) {
> +	    (device_may_wakeup(dev) && adev->wakeup.flags.valid &&
> +	     adev->wakeup.sleep_state >= acpi_target_sleep_state)) {
>  		acpi_status status;
>  
>  		acpi_method[3] = 'W';
> -- 
> 1.7.9.5
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Patch

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index ca191ff..ed6bc52 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -702,8 +702,8 @@  int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p)
 	 * can wake the system.  _S0W may be valid, too.
 	 */
 	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
-	    (device_may_wakeup(dev) &&
-	     adev->wakeup.sleep_state <= acpi_target_sleep_state)) {
+	    (device_may_wakeup(dev) && adev->wakeup.flags.valid &&
+	     adev->wakeup.sleep_state >= acpi_target_sleep_state)) {
 		acpi_status status;
 
 		acpi_method[3] = 'W';