[PATCH v1 0/3] ACPI: button: Fixes related to GPE handling and probe error path

[PATCH v1 0/3] ACPI: button: Fixes related to GPE handling and probe error path

[Rafael J. Wysocki]

Hi All,

This series fix three issues related to the ACPI button driver,
one introduced during the 6.15 development cycle, one from the 7.0
cycle, and one that has been there for a long time, but has been
omitted by mistake by a recent commit.

Thanks!

[PATCH v1 1/3] ACPI: button: Fix ACPI GPE handler leak during removal

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit a7e23ec17fee ("ACPI: button: Install notifier for system events
as well") changed the ACPI notify handler type for ACPI buttons to
ACPI_ALL_NOTIFY, but it forgot to update acpi_button_remove() to reflect
that change.  This leads to leaking the notify handler past driver
removal, which may cause a kernel crash to occur if ACPI notify on
the given device is triggered after removing the driver, and causes a
subsequent probe of the given device with the same driver to fail.

Address this by updating the acpi_remove_notify_handler() call in
acpi_button_remove() as appropriate.

Fixes: a7e23ec17fee ("ACPI: button: Install notifier for system events as well")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: 6.15+ <stable@vger.kernel.org> # 6.15+
---
 drivers/acpi/button.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -689,7 +689,7 @@ static void acpi_button_remove(struct pl
 						acpi_button_event);
 		break;
 	default:
-		acpi_remove_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
+		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
 					   button->type == ACPI_BUTTON_TYPE_LID ?
 						acpi_lid_notify :
 						acpi_button_notify);

Re: [PATCH v1 1/3] ACPI: button: Fix ACPI GPE handler leak during removal

[Mario Limonciello]

On 5/21/26 08:32, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Commit a7e23ec17fee ("ACPI: button: Install notifier for system events
> as well") changed the ACPI notify handler type for ACPI buttons to
> ACPI_ALL_NOTIFY, but it forgot to update acpi_button_remove() to reflect
> that change.  This leads to leaking the notify handler past driver
> removal, which may cause a kernel crash to occur if ACPI notify on
> the given device is triggered after removing the driver, and causes a
> subsequent probe of the given device with the same driver to fail.
> 
> Address this by updating the acpi_remove_notify_handler() call in
> acpi_button_remove() as appropriate.
> 
> Fixes: a7e23ec17fee ("ACPI: button: Install notifier for system events as well")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: 6.15+ <stable@vger.kernel.org> # 6.15+

Good catch, thanks.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

> ---
>   drivers/acpi/button.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -689,7 +689,7 @@ static void acpi_button_remove(struct pl
>   						acpi_button_event);
>   		break;
>   	default:
> -		acpi_remove_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
> +		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
>   					   button->type == ACPI_BUTTON_TYPE_LID ?
>   						acpi_lid_notify :
>   						acpi_button_notify);
> 
> 
> 

[PATCH v1 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Prior to commit 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake()
for buttons"), ACPI button wakeup GPEs having handler methods remained
enabled after acpi_wakeup_gpe_init(), but currently they are not enabled
because acpi_setup_gpe_for_wake() disables them.

That causes function keys to stop working on some systems [1] and there
may be other related issues elsewhere.

To address that, make the ACPI button driver enable wakeup GPEs for ACPI
buttons so long as they have handler methods.  While this does not
restore the old behavior exactly (the ACPI button driver needs to be
bound to the button devices for the GPEs to be enabled), it should be
sufficient to restore the missing functionality.

For this purpose, introduce acpi_enable_gpe_cond() that enables
a GPE if its dispatch type matches the given type mask and modify
acpi_button_probe() to use it for enabling the GPEs in question.

Link: https://lore.kernel.org/linux-acpi/E2OXET.4X5GTP37VTNC3@kousu.ca/ [1]
Fixes: 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake() for buttons")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: 7.0+ <stable@vger.kernel.org> # 7.0+
---

@Nick, I have reproduced the change of behavior introduced by commit
57c31e6d620f132dc on one of my systems, but not the function keys
regression because they are handled differently here.  This patch
should restore the missing functionality on your machine, but I would
appreciate some testing feedback.

@Saket, This includes some ACPICA changes, but they are Linux-specific
and will not affect any host OSes other than Linux.  I'll submit a
corresponding ACPICA PR when this patch goes to mainline Linux.

Thanks!

---
 drivers/acpi/acpica/evxfgpe.c |   33 +++++++++++++++++++++++++++------
 drivers/acpi/button.c         |   22 ++++++++++++++++++++++
 include/acpi/acpixf.h         |    5 +++++
 3 files changed, 54 insertions(+), 6 deletions(-)

--- a/drivers/acpi/acpica/evxfgpe.c
+++ b/drivers/acpi/acpica/evxfgpe.c
@@ -78,18 +78,21 @@ ACPI_EXPORT_SYMBOL(acpi_update_all_gpes)
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_enable_gpe
+ * FUNCTION:    acpi_enable_gpe_cond
  *
  * PARAMETERS:  gpe_device          - Parent GPE Device. NULL for GPE0/GPE1
  *              gpe_number          - GPE level within the GPE block
+ *              dispatch_type_mask  - GPE dispatch types to match
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Add a reference to a GPE. On the first reference, the GPE is
- *              hardware-enabled.
+ * DESCRIPTION: Add a reference to a GPE so long as the dispatch type of the GPE
+ *              matches the supplied GPE dispatch type mask. On the first
+ *              reference, the GPE is hardware-enabled.
  *
  ******************************************************************************/
-acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
+acpi_status acpi_enable_gpe_cond(acpi_handle gpe_device, u32 gpe_number,
+				 u8 dispatch_type_mask)
 {
 	acpi_status status = AE_BAD_PARAMETER;
 	struct acpi_gpe_event_info *gpe_event_info;
@@ -106,8 +109,7 @@ acpi_status acpi_enable_gpe(acpi_handle
 	 */
 	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
 	if (gpe_event_info) {
-		if (ACPI_GPE_DISPATCH_TYPE(gpe_event_info->flags) !=
-		    ACPI_GPE_DISPATCH_NONE) {
+		if (ACPI_GPE_DISPATCH_TYPE(gpe_event_info->flags) & dispatch_type_mask) {
 			status = acpi_ev_add_gpe_reference(gpe_event_info, TRUE);
 			if (ACPI_SUCCESS(status) &&
 			    ACPI_GPE_IS_POLLING_NEEDED(gpe_event_info)) {
@@ -128,6 +130,25 @@ acpi_status acpi_enable_gpe(acpi_handle
 	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
+ACPI_EXPORT_SYMBOL(acpi_enable_gpe_cond)
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_enable_gpe
+ *
+ * PARAMETERS:  gpe_device          - Parent GPE Device. NULL for GPE0/GPE1
+ *              gpe_number          - GPE level within the GPE block
+ *
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Add a reference to a GPE. On the first reference, the GPE is
+ *              hardware-enabled.
+ *
+ ******************************************************************************/
+acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
+{
+	return acpi_enable_gpe_cond(gpe_device, gpe_number, ACPI_GPE_DISPATCH_MASK);
+}
 ACPI_EXPORT_SYMBOL(acpi_enable_gpe)
 
 /*******************************************************************************
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -179,6 +179,7 @@ struct acpi_button {
 	ktime_t last_time;
 	bool suspended;
 	bool lid_state_initialized;
+	bool gpe_enabled;
 };
 
 static struct acpi_device *lid_device;
@@ -646,6 +647,21 @@ static int acpi_button_probe(struct plat
 		status = acpi_install_notify_handler(device->handle,
 						     ACPI_ALL_NOTIFY, handler,
 						     button);
+		if (ACPI_SUCCESS(status) && device->wakeup.flags.valid) {
+			acpi_status st;
+
+			/*
+			 * If the wakeup GPE has a handler method, enable it in
+			 * case it is also used for signaling runtime events.
+			 */
+			st = acpi_enable_gpe_cond(device->wakeup.gpe_device,
+						   device->wakeup.gpe_number,
+						   ACPI_GPE_DISPATCH_METHOD);
+			button->gpe_enabled = ACPI_SUCCESS(st);
+			if (button->gpe_enabled)
+				dev_dbg(button->dev, "Enabled ACPI GPE%02llx\n",
+					device->wakeup.gpe_number);
+		}
 		break;
 	}
 	if (ACPI_FAILURE(status)) {
@@ -689,6 +705,12 @@ static void acpi_button_remove(struct pl
 						acpi_button_event);
 		break;
 	default:
+		if (button->gpe_enabled) {
+			dev_dbg(button->dev, "Disabling ACPI GPE%02llx\n",
+				adev->wakeup.gpe_number);
+			acpi_disable_gpe(adev->wakeup.gpe_device,
+					 adev->wakeup.gpe_number);
+		}
 		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
 					   button->type == ACPI_BUTTON_TYPE_LID ?
 						acpi_lid_notify :
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -726,6 +726,11 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_sta
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_update_all_gpes(void))
 
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
+				acpi_enable_gpe_cond(acpi_handle gpe_device,
+						     u32 gpe_number,
+						     u8 dispatch_type_mask))
+
+ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
 				acpi_enable_gpe(acpi_handle gpe_device,
 						u32 gpe_number))
 

Re: [PATCH v1 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time

[Nick]

Hello Rafael,


On Thu, May 21 2026 at 03:38:50 PM +02:00:00, Rafael J. Wysocki 
<rafael@kernel.org> wrote:
> 
> Link: 
> https://lore.kernel.org/linux-acpi/E2OXET.4X5GTP37VTNC3@kousu.ca/ [1]
> Fixes: 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake() for 
> buttons")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: 7.0+ <stable@vger.kernel.org> # 7.0+
> ---
> 
> @Nick, I have reproduced the change of behavior introduced by commit
> 57c31e6d620f132dc on one of my systems, but not the function keys
> regression because they are handled differently here.  This patch
> should restore the missing functionality on your machine, but I would
> appreciate some testing feedback.

I have applied this patch series on top of today's mainline 
(8bc67e4db64aa727). It worked! My brightness keys and mute and 
lidswitch are back. That's great. Thank you! Learning how to bisect was 
a steep curve for me but everything after that has been a really easy 
experience. Thank you for knowing this subsystem deeply.

- Nick

Re: [PATCH v1 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time

[Rafael J. Wysocki]

On Fri, May 22, 2026 at 3:48 AM Nick <nick@kousu.ca> wrote:
>
> Hello Rafael,
>
>
> On Thu, May 21 2026 at 03:38:50 PM +02:00:00, Rafael J. Wysocki
> <rafael@kernel.org> wrote:
> >
> > Link:
> > https://lore.kernel.org/linux-acpi/E2OXET.4X5GTP37VTNC3@kousu.ca/ [1]
> > Fixes: 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake() for
> > buttons")
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: 7.0+ <stable@vger.kernel.org> # 7.0+
> > ---
> >
> > @Nick, I have reproduced the change of behavior introduced by commit
> > 57c31e6d620f132dc on one of my systems, but not the function keys
> > regression because they are handled differently here.  This patch
> > should restore the missing functionality on your machine, but I would
> > appreciate some testing feedback.
>
> I have applied this patch series on top of today's mainline
> (8bc67e4db64aa727). It worked! My brightness keys and mute and
> lidswitch are back. That's great. Thank you! Learning how to bisect was
> a steep curve for me but everything after that has been a really easy
> experience. Thank you for knowing this subsystem deeply.
>
> - Nick
>
>
>
>
>
>
>

Re: [PATCH v1 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time

[Rafael J. Wysocki]

On Fri, May 22, 2026 at 3:48 AM Nick <nick@kousu.ca> wrote:
>
> Hello Rafael,
>
>
> On Thu, May 21 2026 at 03:38:50 PM +02:00:00, Rafael J. Wysocki
> <rafael@kernel.org> wrote:
> >
> > Link:
> > https://lore.kernel.org/linux-acpi/E2OXET.4X5GTP37VTNC3@kousu.ca/ [1]
> > Fixes: 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake() for
> > buttons")
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: 7.0+ <stable@vger.kernel.org> # 7.0+
> > ---
> >
> > @Nick, I have reproduced the change of behavior introduced by commit
> > 57c31e6d620f132dc on one of my systems, but not the function keys
> > regression because they are handled differently here.  This patch
> > should restore the missing functionality on your machine, but I would
> > appreciate some testing feedback.
>
> I have applied this patch series on top of today's mainline
> (8bc67e4db64aa727). It worked! My brightness keys and mute and
> lidswitch are back. That's great. Thank you! Learning how to bisect was
> a steep curve for me but everything after that has been a really easy
> experience. Thank you for knowing this subsystem deeply.

You're welcome!

(and sorry for an empty reply message I've just sent)

[PATCH v1 3/3] ACPI: button: Add missing device class clearing on probe failures

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit e18947038bf4 ("ACPI: driver: Do not set acpi_device_class()
unnecessarily") modified acpi_button_remove() to clear the device class
field in struct acpi_device on driver removal, but it should also have
updated the rollback path in acpi_button_probe(), which it didn't do,
so do it now.

Fixes: e18947038bf4 ("ACPI: driver: Do not set acpi_device_class() unnecessarily")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -687,6 +687,7 @@ err_remove_fs:
 	acpi_button_remove_fs(button);
 err_free_button:
 	kfree(button);
+	memset(acpi_device_class(device), 0, sizeof(acpi_device_class));
 	return error;
 }
 

Re: [PATCH v1 3/3] ACPI: button: Add missing device class clearing on probe failures

[Mario Limonciello]

On 5/21/26 08:39, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Commit e18947038bf4 ("ACPI: driver: Do not set acpi_device_class()
> unnecessarily") modified acpi_button_remove() to clear the device class
> field in struct acpi_device on driver removal, but it should also have
> updated the rollback path in acpi_button_probe(), which it didn't do,
> so do it now.
> 
> Fixes: e18947038bf4 ("ACPI: driver: Do not set acpi_device_class() unnecessarily")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

> ---
>   drivers/acpi/button.c |    1 +
>   1 file changed, 1 insertion(+)
> 
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -687,6 +687,7 @@ err_remove_fs:
>   	acpi_button_remove_fs(button);
>   err_free_button:
>   	kfree(button);
> +	memset(acpi_device_class(device), 0, sizeof(acpi_device_class));
>   	return error;
>   }
>   
> 
> 
>