Linux ACPI
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Len Brown <lenb@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	linux-pm@lists.linux-foundation.org,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Len Brown <len.brown@intel.com>,
	kernel list <linux-kernel@vger.kernel.org>,
	Michal Hocko <mhocko@suse.cz>
Subject: [PATCH] ACPI / PM: Do not enable GPEs for system wakeup in advance
Date: Thu, 17 Jun 2010 17:40:57 +0200	[thread overview]
Message-ID: <201006171740.57315.rjw@sisk.pl> (raw)

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

After commit 9630bdd9b15d2f489c646d8bc04b60e53eb5ec78
(ACPI: Use GPE reference counting to support shared GPEs) the wakeup
enable mask bits of GPEs are set as soon as the GPEs are enabled to
wake up the system.  Unfortunately, this leads to a regression
reported by Michal Hocko, where a system is woken up from ACPI S5 by
a device that is not supposed to do that, because the wakeup enable
mask bit of this device's GPE is always set when
acpi_enter_sleep_state() calls acpi_hw_enable_all_wakeup_gpes(),
although it should only be set if the device is supposed to wake up
the system from the target state.

To work around this issue, rework the ACPI power management code so
that GPEs are not enabled to wake up the system upfront, but only
during a system state transition when the target state of the system
is known.  [Of course, this means that the reference counting of
"wakeup" GPEs doesn't really make sense and it is sufficient to
set/unset the wakeup mask bits for them during system sleep
transitions.  This will allow us to simplify the GPE handling code
quite a bit, but that change is too intrusive for 2.6.35.]

Fixes https://bugzilla.kernel.org/show_bug.cgi?id=15951

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reported-and-tested-by: Michal Hocko <mhocko@suse.cz>
---
Hi Len,

This is on top of the GPE fixes I send before that are in your tree.

Please apply.

Rafael
---
 drivers/acpi/button.c |    4 ++--
 drivers/acpi/wakeup.c |   20 +++++++-------------
 2 files changed, 9 insertions(+), 15 deletions(-)

Index: linux-2.6/drivers/acpi/button.c
===================================================================
--- linux-2.6.orig/drivers/acpi/button.c
+++ linux-2.6/drivers/acpi/button.c
@@ -425,7 +425,7 @@ static int acpi_button_add(struct acpi_d
 		/* Button's GPE is run-wake GPE */
 		acpi_enable_gpe(device->wakeup.gpe_device,
 				device->wakeup.gpe_number,
-				ACPI_GPE_TYPE_WAKE_RUN);
+				ACPI_GPE_TYPE_RUNTIME);
 		device->wakeup.run_wake_count++;
 		device->wakeup.state.enabled = 1;
 	}
@@ -449,7 +449,7 @@ static int acpi_button_remove(struct acp
 	if (device->wakeup.flags.valid) {
 		acpi_disable_gpe(device->wakeup.gpe_device,
 				device->wakeup.gpe_number,
-				ACPI_GPE_TYPE_WAKE_RUN);
+				ACPI_GPE_TYPE_RUNTIME);
 		device->wakeup.run_wake_count--;
 		device->wakeup.state.enabled = 0;
 	}
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
--- linux-2.6.orig/drivers/acpi/wakeup.c
+++ linux-2.6/drivers/acpi/wakeup.c
@@ -64,16 +64,13 @@ void acpi_enable_wakeup_device(u8 sleep_
 		struct acpi_device *dev =
 			container_of(node, struct acpi_device, wakeup_list);
 
-		if (!dev->wakeup.flags.valid)
-			continue;
-
-		if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
+		if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled
 		    || sleep_state > (u32) dev->wakeup.sleep_state)
 			continue;
 
 		/* The wake-up power should have been enabled already. */
-		acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
-				ACPI_GPE_ENABLE);
+		acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
+				ACPI_GPE_TYPE_WAKE);
 	}
 }
 
@@ -96,6 +93,8 @@ void acpi_disable_wakeup_device(u8 sleep
 		    || (sleep_state > (u32) dev->wakeup.sleep_state))
 			continue;
 
+		acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
+				ACPI_GPE_TYPE_WAKE);
 		acpi_disable_wakeup_device_power(dev);
 	}
 }
@@ -109,13 +108,8 @@ int __init acpi_wakeup_device_init(void)
 		struct acpi_device *dev = container_of(node,
 						       struct acpi_device,
 						       wakeup_list);
-		/* In case user doesn't load button driver */
-		if (!dev->wakeup.flags.always_enabled ||
-		    dev->wakeup.state.enabled)
-			continue;
- 		acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
- 				ACPI_GPE_TYPE_WAKE);
-		dev->wakeup.state.enabled = 1;
+		if (dev->wakeup.flags.always_enabled)
+			dev->wakeup.state.enabled = 1;
 	}
 	mutex_unlock(&acpi_device_lock);
 	return 0;

             reply	other threads:[~2010-06-17 15:43 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-17 15:40 Rafael J. Wysocki [this message]
2010-06-17 16:18 ` [PATCH] ACPI / PM: Do not enable GPEs for system wakeup in advance Len Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201006171740.57315.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=akpm@linux-foundation.org \
    --cc=len.brown@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=mhocko@suse.cz \
    --cc=mjg59@srcf.ucam.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox