[PATCH] ACPI: button: Install notifier for system events as well

[PATCH] ACPI: button: Install notifier for system events as well

[Mario Limonciello]

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://uefi.org/htmlspecs/ACPI_Spec_6_4_html/04_ACPI_Hardware_Specification/ACPI_Hardware_Specification.html?highlight=0x2#control-method-power-button
Signed-off-by: Mario Limonciello <mario.limonciello@amd.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

RE: [PATCH] ACPI: button: Install notifier for system events as well

[Shen, Yijun]

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

Re: [PATCH] ACPI: button: Install notifier for system events as well

[Rafael J. Wysocki]

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!