| Message ID | 20250303212719.4153485-1-superm1@kernel.org (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | ACPI: button: Install notifier for system events as well | expand |
Internal Use - Confidential > -----Original Message----- > From: Mario Limonciello <superm1@kernel.org> > Sent: Tuesday, March 4, 2025 5:27 AM > To: Limonciello, Mario <mario.limonciello@amd.com>; rafael@kernel.org > Cc: Shen, Yijun <Yijun_Shen@Dell.com>; Richard.Gong > <Richard.Gong@amd.com>; linux-acpi@vger.kernel.org > Subject: [PATCH] ACPI: button: Install notifier for system events as well > > > [EXTERNAL EMAIL] > > From: Mario Limonciello <mario.limonciello@amd.com> > > On some systems when the system is put to sleep pressing the ACPI power > button will cause the EC SCI to try to wake the system by a Notify(DEV, 0x2) > with an intention to wake the system up from suspend. > > This behavior matches the ACPI specification in ACPI 6.4 section > 4.8.3.1.1.2 which describes that the AML handler would generate a Notify() > with a code of 0x2 to indicate it was responsible for waking the system. > > This currently doesn't work because acpi_button_add() only configured > `ACPI_DEVICE_NOTIFY` which means that device handler notifications > 0x80 through 0xFF are handled. > > To fix the wakeups on such systems, adjust the ACPI button handler to use > `ACPI_ALL_NOTIFY` which will handle all events 0x00 through 0x7F. > > Reported-by: Yijun Shen <Yijun.Shen@dell.com> > Tested-by: Richard Gong <Richard.Gong@amd.com> > Link: > https://urldefense.com/v3/__https://uefi.org/htmlspecs/ACPI_Spec_6_4_html > /04_ACPI_Hardware_Specification/ACPI_Hardware_Specification.html?highlig > ht=0x2*control-method-power-button__;Iw!!LpKI!m8bmT5JUyck9Q0BMUA- > LmC5MXTEXFeJ1nmcNGIhJ4AWCQ7XMUR_UqxaxBor674mhMk53MkwkXqT1a > cTF$ [uefi[.]org] > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> Verified the patch on the issued system, the issue is gone. Tested-by: Yijun Shen <Yijun_Shen@Dell.com> > --- > 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 > 7773e6b860e73..61701c646e92f 100644 > --- a/drivers/acpi/button.c > +++ b/drivers/acpi/button.c > @@ -24,6 +24,7 @@ > #define ACPI_BUTTON_CLASS "button" > #define ACPI_BUTTON_FILE_STATE "state" > #define ACPI_BUTTON_TYPE_UNKNOWN 0x00 > +#define ACPI_BUTTON_NOTIFY_WAKE 0x02 > #define ACPI_BUTTON_NOTIFY_STATUS 0x80 > > #define ACPI_BUTTON_SUBCLASS_POWER "power" > @@ -443,11 +444,16 @@ static void acpi_button_notify(acpi_handle handle, > u32 event, void *data) > struct input_dev *input; > int keycode; > > - if (event != ACPI_BUTTON_NOTIFY_STATUS) { > + switch (event) { > + case ACPI_BUTTON_NOTIFY_STATUS: > + break; > + case ACPI_BUTTON_NOTIFY_WAKE: > + break; > + default: > acpi_handle_debug(device->handle, "Unsupported event > [0x%x]\n", > event); > return; > - } > + }; > > acpi_pm_wakeup_event(&device->dev); > > @@ -629,7 +635,7 @@ static int acpi_button_add(struct acpi_device *device) > break; > default: > status = acpi_install_notify_handler(device->handle, > - ACPI_DEVICE_NOTIFY, > handler, > + ACPI_ALL_NOTIFY, handler, > device); > break; > } > -- > 2.43.0
On Tue, Mar 4, 2025 at 3:45 PM Shen, Yijun <Yijun.Shen@dell.com> wrote: > > > Internal Use - Confidential > > -----Original Message----- > > From: Mario Limonciello <superm1@kernel.org> > > Sent: Tuesday, March 4, 2025 5:27 AM > > To: Limonciello, Mario <mario.limonciello@amd.com>; rafael@kernel.org > > Cc: Shen, Yijun <Yijun_Shen@Dell.com>; Richard.Gong > > <Richard.Gong@amd.com>; linux-acpi@vger.kernel.org > > Subject: [PATCH] ACPI: button: Install notifier for system events as well > > > > > > [EXTERNAL EMAIL] > > > > From: Mario Limonciello <mario.limonciello@amd.com> > > > > On some systems when the system is put to sleep pressing the ACPI power > > button will cause the EC SCI to try to wake the system by a Notify(DEV, 0x2) > > with an intention to wake the system up from suspend. > > > > This behavior matches the ACPI specification in ACPI 6.4 section > > 4.8.3.1.1.2 which describes that the AML handler would generate a Notify() > > with a code of 0x2 to indicate it was responsible for waking the system. > > > > This currently doesn't work because acpi_button_add() only configured > > `ACPI_DEVICE_NOTIFY` which means that device handler notifications > > 0x80 through 0xFF are handled. > > > > To fix the wakeups on such systems, adjust the ACPI button handler to use > > `ACPI_ALL_NOTIFY` which will handle all events 0x00 through 0x7F. > > > > Reported-by: Yijun Shen <Yijun.Shen@dell.com> > > Tested-by: Richard Gong <Richard.Gong@amd.com> > > Link: > > https://urldefense.com/v3/__https://uefi.org/htmlspecs/ACPI_Spec_6_4_html > > /04_ACPI_Hardware_Specification/ACPI_Hardware_Specification.html?highlig > > ht=0x2*control-method-power-button__;Iw!!LpKI!m8bmT5JUyck9Q0BMUA- > > LmC5MXTEXFeJ1nmcNGIhJ4AWCQ7XMUR_UqxaxBor674mhMk53MkwkXqT1a > > cTF$ [uefi[.]org] > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > Verified the patch on the issued system, the issue is gone. > > Tested-by: Yijun Shen <Yijun_Shen@Dell.com> Applied as 6.15 material, thanks!
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 7773e6b860e73..61701c646e92f 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -24,6 +24,7 @@ #define ACPI_BUTTON_CLASS "button" #define ACPI_BUTTON_FILE_STATE "state" #define ACPI_BUTTON_TYPE_UNKNOWN 0x00 +#define ACPI_BUTTON_NOTIFY_WAKE 0x02 #define ACPI_BUTTON_NOTIFY_STATUS 0x80 #define ACPI_BUTTON_SUBCLASS_POWER "power" @@ -443,11 +444,16 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data) struct input_dev *input; int keycode; - if (event != ACPI_BUTTON_NOTIFY_STATUS) { + switch (event) { + case ACPI_BUTTON_NOTIFY_STATUS: + break; + case ACPI_BUTTON_NOTIFY_WAKE: + break; + default: acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n", event); return; - } + }; acpi_pm_wakeup_event(&device->dev); @@ -629,7 +635,7 @@ static int acpi_button_add(struct acpi_device *device) break; default: status = acpi_install_notify_handler(device->handle, - ACPI_DEVICE_NOTIFY, handler, + ACPI_ALL_NOTIFY, handler, device); break; }