Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation

[Muhammad Usama Anjum <usama.anjum@collabora.com>]

On 11/24/25 11:42 PM, Rafael J. Wysocki wrote:
> On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
>> called when power button is pressed. The system is worken up from s2idle
>> in this case by setting hard parameter to pm_wakeup_dev_event().
> 
> Right, so presumably you want to set it for hibernation too.
> 
>> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
>> is in progress.
> 
> Well, this is not what the code does after the change.
> 
>> Set the hard parameter such that pm_system_wakeup()
>> gets called which increments pm_abort_suspend counter. The explicit call
>> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
>> wakeup source. Hence call to input_report_key() with input device
>> doesn't call pm_system_wakeup() as it doesn't have wakeup source
>> registered.
>>
>> Hence hibernation would be cancelled as in hibernation path, this counter
>> is checked if it should be aborted.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since RFC:
>> - Use pm_sleep_transition_in_progress()
>> - Update descriptin why explicit call to acpi_pm_wakeup_event() is
>>   necessary
>> ---
>>  drivers/acpi/button.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
>> index 3c6dd9b4ba0ad..e4be5c763edaf 100644
>> --- a/drivers/acpi/button.c
>> +++ b/drivers/acpi/button.c
>> @@ -20,6 +20,7 @@
>>  #include <linux/acpi.h>
>>  #include <linux/dmi.h>
>>  #include <acpi/button.h>
>> +#include <linux/suspend.h>
>>
>>  #define ACPI_BUTTON_CLASS              "button"
>>  #define ACPI_BUTTON_FILE_STATE         "state"
>> @@ -458,11 +459,16 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
>>         acpi_pm_wakeup_event(&device->dev);
> 
> The above is what you want to change, as this already reports the
> event.  Reporting it twice is unnecessary and potentially confusing.
Thanks for mentioning. This will be fixed easily by adding newer
version of acpi_pm_wakeup_event() which takes hard parameter.

> 
>>         button = acpi_driver_data(device);
>> -       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
>> -               return;
>> -
>>         input = button->input;
>>         keycode = test_bit(KEY_SLEEP, input->keybit) ? KEY_SLEEP : KEY_POWER;
>> +       if (event == ACPI_BUTTON_NOTIFY_STATUS && keycode == KEY_POWER &&
>> +           pm_sleep_transition_in_progress()) {
>> +               pm_wakeup_dev_event(&device->dev, 0, true);
>> +               return;
>> +       }
> 
> First, this will affect suspend too.
I'll fix terminologies in entire series.

> 
> Second, this reports an already reported wakeup event.
As mentioned above, I'll update to only 1 call to pm_wakeup_dev_event().> 
> Next, why KEY_POWER only?  Is KEY_SLEEP not expected to wake up?
Yes, this is true. This will be fixed in next version.

> 
> And why event == ACPI_BUTTON_NOTIFY_STATUS?  Isn't this what
> ACPI_BUTTON_NOTIFY_WAKE is for?
While device is hibernating, at that if power button is pressed, 
ACPI_BUTTON_NOTIFY_STATUS event is received. 

ACPI_BUTTON_NOTIFY_WAKE is received when we turn on the device from sleeping state
(suspend) or turned off state (hibernated).

> 
>> +
>> +       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
>> +               return;
>>
>>         input_report_key(input, keycode, 1);
>>         input_sync(input);
>> --


-- 
---
Thanks,
Usama