Re: [PATCH v5] ACPI: button: Report wakeup key only for power button wakeups

[Rafael J. Wysocki (Intel)]

On Mon, Oct 12, 2026 at 8:09 AM Baorui.Liu <baorliu@xxxxxxx> wrote:
>
> 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.
>
> When the fixed power-button status bit is set during early S3 resume,
> clear it to avoid a deferred KEY_POWER delivery to userspace, and mark
> wakeup_pending so button resume can still report KEY_WAKEUP. Also clear
> wakeup_pending before setting suspended to avoid a race with a power
> button SCI during suspend entry.
>
> Declare acpi_button_power_wakeup_pending() with IS_REACHABLE so that
> CONFIG_ACPI_BUTTON=m does not leave an unresolved reference in built-in
> sleep.c.
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Closes: https://lore.kernel.org/r/202609290454.LJrhMCb3-lkp@xxxxxxxxx/
> Signed-off-by: Baorui.Liu <baorliu@xxxxxxx>
> ---
> Changes since v4:
> - Fix LKP build ERROR when CONFIG_ACPI_BUTTON=m by declaring
>   acpi_button_power_wakeup_pending() with IS_REACHABLE().

Please see the Sashiko feedback on this patch:

https://sashiko.dev/#/patchset/20261012060847.486-1-baorliu%40amd.com

and let me know what you think about it.

Thanks!

>  drivers/acpi/button.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
>  drivers/acpi/sleep.c  | 27 +++++++++++--------------
>  include/acpi/button.h | 13 ++++++++++++
>  3 files changed, 70 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index cdbb1023a8ee..c07de417df5d 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -191,6 +191,7 @@ struct acpi_button {
>         bool last_state;
>         ktime_t last_time;
>         bool suspended;
> +       bool wakeup_pending;
>         bool lid_state_initialized;
>         bool gpe_enabled;
>  };
> @@ -397,6 +398,34 @@ static void acpi_lid_forget(struct acpi_device *adev)
>  }
>
>  /* Driver Interface */
> +/**
> + * acpi_button_power_wakeup_pending - Mark fixed power button wakeup pending.
> + *
> + * Used when the fixed power-button status bit is observed and cleared during
> + * early S3 resume, so KEY_WAKEUP can still be reported from button resume
> + * without delivering a deferred KEY_POWER to userspace.
> + */
> +void acpi_button_power_wakeup_pending(void)
> +{
> +       struct acpi_device *adev;
> +       struct device *phys_dev;
> +       struct acpi_button *button;
> +
> +       adev = acpi_dev_get_first_match_dev(ACPI_BUTTON_HID_POWERF, NULL, -1);
> +       if (!adev)
> +               return;
> +
> +       phys_dev = acpi_get_first_physical_node(adev);
> +       if (phys_dev) {
> +               button = dev_get_drvdata(phys_dev);
> +               if (button && button->type == ACPI_BUTTON_TYPE_POWER)
> +                       button->wakeup_pending = true;
> +       }
> +
> +       acpi_dev_put(adev);
> +}
> +EXPORT_SYMBOL_GPL(acpi_button_power_wakeup_pending);
> +
>  int acpi_lid_open(void)
>  {
>         guard(mutex)(&acpi_lid_lock);
> @@ -476,6 +505,9 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
>
>         acpi_pm_wakeup_event(button->dev);
>
> +       if (button->type == ACPI_BUTTON_TYPE_POWER && button->suspended)
> +               button->wakeup_pending = true;
> +
>         if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
>                 return;
>
> @@ -498,6 +530,11 @@ static void acpi_button_notify_run(void *data)
>
>  static u32 acpi_button_event(void *data)
>  {
> +       struct acpi_button *button = data;
> +
> +       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;
>  }
> @@ -507,6 +544,12 @@ static int acpi_button_suspend(struct device *dev)
>  {
>         struct acpi_button *button = dev_get_drvdata(dev);
>
> +       /*
> +        * Clear wakeup_pending before marking suspended. Otherwise a power
> +        * button SCI between the two stores could set wakeup_pending and then
> +        * be overwritten here, losing a real wakeup.
> +        */
> +       button->wakeup_pending = false;
>         button->suspended = true;
>         return 0;
>  }
> @@ -523,12 +566,13 @@ static int acpi_button_resume(struct device *dev)
>                 acpi_lid_initialize_state(button);
>         }
>
> -       if (button->type == ACPI_BUTTON_TYPE_POWER) {
> +       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;
>  }
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 132a9df98471..30e268987e37 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -22,6 +22,7 @@
>  #include <linux/syscore_ops.h>
>  #include <asm/io.h>
>  #include <trace/events/power.h>
> +#include <acpi/button.h>
>
>  #include "internal.h"
>  #include "sleep.h"
> @@ -504,12 +505,11 @@ static void acpi_pm_finish(void)
>
>         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 woken by the fixed power button, provide a wakeup event on that
> +        * device. KEY_WAKEUP input reporting is handled by the button driver
> +        * via wakeup_pending; do not synthesize KEY_POWER here.
> +        */
>         if (!pwr_btn_event_pending)
>                 return;
>
> @@ -626,14 +626,11 @@ 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.
> +       /*
> +        * ACPI 3.0 (P62): OSPM should clear the fixed power-button status bit
> +        * so the event does not reach userspace as KEY_POWER. Remember that a
> +        * power-button wake occurred so button resume can report KEY_WAKEUP
> +        * and acpi_pm_finish() can emit a PM wakeup event.
>          */
>         if (ACPI_SUCCESS(status) && (acpi_state == ACPI_STATE_S3)) {
>                 acpi_event_status pwr_btn_status = ACPI_EVENT_FLAG_DISABLED;
> @@ -642,8 +639,8 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
>
>                 if (pwr_btn_status & ACPI_EVENT_FLAG_STATUS_SET) {
>                         acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
> -                       /* Flag for later */
>                         pwr_btn_event_pending = true;
> +                       acpi_button_power_wakeup_pending();
>                 }
>         }
>
> diff --git a/include/acpi/button.h b/include/acpi/button.h
> index af2fce5d2ee3..c909c00664c7 100644
> --- a/include/acpi/button.h
> +++ b/include/acpi/button.h
> @@ -15,4 +15,17 @@ static inline int acpi_lid_open(void)
>  }
>  #endif /* IS_ENABLED(CONFIG_ACPI_BUTTON) */
>
> +/*
> + * sleep.c is built-in and calls this helper. Use IS_REACHABLE so that when
> + * CONFIG_ACPI_BUTTON=m the call site gets an inline stub instead of an
> + * unresolved vmlinux link reference (0day/LKP ERROR).
> + */
> +#if IS_REACHABLE(CONFIG_ACPI_BUTTON)
> +extern void acpi_button_power_wakeup_pending(void);
> +#else
> +static inline void acpi_button_power_wakeup_pending(void)
> +{
> +}
> +#endif /* IS_REACHABLE(CONFIG_ACPI_BUTTON) */
> +
>  #endif /* ACPI_BUTTON_H */
>
> base-commit: 4a819ee5f2834330656d6ac168c4c8cf27fdeec2
> --
> 2.34.1
>