[PATCH v2 1/1] ACPI: button: Report wakeup key only for power button wakeups

Baorui.Liu <[email protected]>
Newsgroups gmane.linux.acpi.devel,gmane.linux.kernel
Message-ID <[email protected]>
The ACPI button driver reports KEY_WAKEUP from the Power Button input
device to let userspace know that the system was resumed by a power
button wakeup.

However, reporting KEY_WAKEUP from generic system resume paths can make
userspace observe a Power Button wakeup even when the system was resumed
by a different wake source.

This is reproducible on an AMD Android 15 Xen guest. With a kernel
without this fix, a non-power-button S3 resume:

  echo mem > /sys/power/state
  xl trigger android s3resume

makes the Power Button input device report KEY_WAKEUP. The same test on
a kernel with this fix no longer reports KEY_WAKEUP from the Power Button
input device.

Track whether a power button event/notify is observed while the ACPI
button device is suspended, and report KEY_WAKEUP on resume only in that
case. Do not synthesize a Power Button input event from generic ACPI
sleep resume code.

Signed-off-by: Baorui.Liu <[email protected]>
---
 drivers/acpi/button.c | 38 ++++++++++++++++-------------
 drivers/acpi/sleep.c  | 56 +++----------------------------------------
 include/acpi/button.h |  5 ----
 3 files changed, 24 insertions(+), 75 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 8d2843bece29..f9b6b7fff5bd 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -175,6 +175,7 @@ struct acpi_button {
 	int last_state;
 	ktime_t last_time;
 	bool suspended;
+	bool wakeup_pending;
 	bool lid_state_initialized;
 };
 
@@ -452,6 +453,9 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 	acpi_pm_wakeup_event(&device->dev);
 
 	button = acpi_driver_data(device);
+	if (button->type == ACPI_BUTTON_TYPE_POWER && button->suspended)
+		button->wakeup_pending = true;
+
 	if (button->suspended)
 		return;
 
@@ -468,23 +472,6 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 					event, ++button->pushed);
 }
 
-void acpi_power_button_wakeup(struct acpi_device *device)
-{
-       struct acpi_button *button = acpi_driver_data(device);
-       struct input_dev *input;
-
-       if (button->type == ACPI_BUTTON_TYPE_POWER) {
-               input = button->input;
-               input_report_key(input, KEY_WAKEUP, 1);
-               input_sync(input);
-               input_report_key(input, KEY_WAKEUP, 0);
-               input_sync(input);
-       }
-
-       return;
-}
-EXPORT_SYMBOL(acpi_power_button_wakeup);
-
 static void acpi_button_notify_run(void *data)
 {
 	acpi_button_notify(NULL, ACPI_BUTTON_NOTIFY_STATUS, data);
@@ -492,6 +479,12 @@ static void acpi_button_notify_run(void *data)
 
 static u32 acpi_button_event(void *data)
 {
+	struct acpi_device *device = data;
+	struct acpi_button *button = acpi_driver_data(device);
+
+	if (button->type == ACPI_BUTTON_TYPE_POWER && button->suspended)
+		button->wakeup_pending = true;
+
 	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_button_notify_run, data);
 	return ACPI_INTERRUPT_HANDLED;
 }
@@ -503,11 +496,13 @@ static int acpi_button_suspend(struct device *dev)
 	struct acpi_button *button = acpi_driver_data(device);
 
 	button->suspended = true;
+	button->wakeup_pending = false;
 	return 0;
 }
 
 static int acpi_button_resume(struct device *dev)
 {
+	struct input_dev *input;
 	struct acpi_device *device = to_acpi_device(dev);
 	struct acpi_button *button = acpi_driver_data(device);
 
@@ -518,6 +513,15 @@ static int acpi_button_resume(struct device *dev)
 		acpi_lid_initialize_state(device);
 	}
 
+	if (button->type == ACPI_BUTTON_TYPE_POWER && button->wakeup_pending) {
+		input = button->input;
+		input_report_key(input, KEY_WAKEUP, 1);
+		input_sync(input);
+		input_report_key(input, KEY_WAKEUP, 0);
+		input_sync(input);
+		button->wakeup_pending = false;
+	}
+
 	return 0;
 }
 #endif
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 0adfcd0fb55e..d655248c20d4 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -22,7 +22,6 @@
 #include <linux/syscore_ops.h>
 #include <asm/io.h>
 #include <trace/events/power.h>
-#include <acpi/button.h>
 
 #include "internal.h"
 #include "sleep.h"
@@ -105,8 +104,6 @@ u32 acpi_target_system_state(void)
 }
 EXPORT_SYMBOL_GPL(acpi_target_system_state);
 
-static bool pwr_btn_event_pending;
-
 /*
  * The ACPI specification wants us to save NVS memory regions during hibernation
  * and to restore them during the subsequent resume.  Windows does that also for
@@ -463,7 +460,6 @@ static int acpi_pm_prepare(void)
  */
 static void acpi_pm_finish(void)
 {
-	struct acpi_device *pwr_btn_adev;
 	u32 acpi_state = acpi_target_sleep_state;
 
 	acpi_ec_unblock_transactions();
@@ -482,24 +478,6 @@ static void acpi_pm_finish(void)
 	acpi_target_sleep_state = ACPI_STATE_S0;
 
 	acpi_resume_power_resources();
-
-	/* If we were woken with the fixed power button, provide a small
-	 * hint to userspace in the form of a wakeup event on the fixed power
-	 * button device (if it can be found).
-	 *
-	 * We delay the event generation til now, as the PM layer requires
-	 * timekeeping to be running before we generate events. */
-	if (!pwr_btn_event_pending)
-		return;
-
-	pwr_btn_event_pending = false;
-	pwr_btn_adev = acpi_dev_get_first_match_dev(ACPI_BUTTON_HID_POWERF,
-						    NULL, -1);
-	if (pwr_btn_adev) {
-		pm_wakeup_event(&pwr_btn_adev->dev, 0);
-		acpi_power_button_wakeup(pwr_btn_adev);
-		acpi_dev_put(pwr_btn_adev);
-	}
 }
 
 /**
@@ -604,27 +582,6 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
 	/* Reprogram control registers */
 	acpi_leave_sleep_state_prep(acpi_state);
 
-	/* ACPI 3.0 specs (P62) says that it's the responsibility
-	 * of the OSPM to clear the status bit [ implying that the
-	 * POWER_BUTTON event should not reach userspace ]
-	 *
-	 * However, we do generate a small hint for userspace in the form of
-	 * a wakeup event. We flag this condition for now and generate the
-	 * event later, as we're currently too early in resume to be able to
-	 * generate wakeup events.
-	 */
-	if (ACPI_SUCCESS(status) && (acpi_state == ACPI_STATE_S3)) {
-		acpi_event_status pwr_btn_status = ACPI_EVENT_FLAG_DISABLED;
-
-		acpi_get_event_status(ACPI_EVENT_POWER_BUTTON, &pwr_btn_status);
-
-		if (pwr_btn_status & ACPI_EVENT_FLAG_STATUS_SET) {
-			acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
-			/* Flag for later */
-			pwr_btn_event_pending = true;
-		}
-	}
-
 	/*
 	 * Disable all GPE and clear their status bits before interrupts are
 	 * enabled. Some GPEs (like wakeup GPEs) have no handlers and this can
@@ -797,19 +754,12 @@ bool acpi_s2idle_wake(void)
 
 void acpi_s2idle_restore_check_powerkey(void)
 {
-	struct acpi_device *pwr_btn_adev;
 	acpi_event_status pwr_btn_status = ACPI_EVENT_FLAG_DISABLED;
+
 	acpi_get_event_status(ACPI_EVENT_POWER_BUTTON, &pwr_btn_status);
 
-	if (pwr_btn_status & ACPI_EVENT_FLAG_STATUS_SET) {
-		pwr_btn_adev = acpi_dev_get_first_match_dev(ACPI_BUTTON_HID_POWERF,
-			NULL, -1);
-		if (pwr_btn_adev) {
-			pm_wakeup_event(&pwr_btn_adev->dev, 0);
-			acpi_power_button_wakeup(pwr_btn_adev);
-			acpi_dev_put(pwr_btn_adev);
-		}
-	}
+	if (pwr_btn_status & ACPI_EVENT_FLAG_STATUS_SET)
+		acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
 }
 
 void acpi_s2idle_restore(void)
diff --git a/include/acpi/button.h b/include/acpi/button.h
index be02d3ff2896..af2fce5d2ee3 100644
--- a/include/acpi/button.h
+++ b/include/acpi/button.h
@@ -8,16 +8,11 @@
 
 #if IS_ENABLED(CONFIG_ACPI_BUTTON)
 extern int acpi_lid_open(void);
-extern void acpi_power_button_wakeup(struct acpi_device *device);
 #else
 static inline int acpi_lid_open(void)
 {
 	return 1;
 }
-static inline void acpi_power_button_wakeup(struct acpi_device *device)
-{
-       return;
-}
 #endif /* IS_ENABLED(CONFIG_ACPI_BUTTON) */
 
 #endif /* ACPI_BUTTON_H */
-- 
2.34.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.