mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] ACPI: button: Fixes related to GPE handling and probe error path
@ 2026-05-22 14:48 Rafael J. Wysocki
  2026-05-22 14:49 ` [PATCH v2 1/3] ACPI: button: Fix ACPI GPE handler leak during removal Rafael J. Wysocki
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2026-05-22 14:48 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Nick, Saket Dumbre, Pawel Chmielewski, Mario Limonciello

Hi All,

This is an update of

https://lore.kernel.org/linux-acpi/5996185.DvuYhMxLoT@rafael.j.wysocki/

sent to address sashiko.dev feedback on patch [2/3]:

> > @@ -106,8 +109,7 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device,
> > u32 gpe_number)> > 
> >  	 */
> >  	
> >  	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)
> > {
> 
> Does this incorrectly treat sequential enumeration values as bit flags?
> ACPI_GPE_DISPATCH_METHOD is defined as 1, and ACPI_GPE_DISPATCH_NOTIFY is 3.
> Since dispatch_type_mask is passed as ACPI_GPE_DISPATCH_METHOD (1) from
> acpi_button_probe(), this bitwise AND evaluates to true for both METHOD
> (1 & 1 == 1) and NOTIFY (3 & 1 == 1).
> Could this unintentionally enable GPEs for NOTIFY handlers?

The original series description below still applies.

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!




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] ACPI: button: Fix ACPI GPE handler leak during removal
  2026-05-22 14:48 [PATCH v2 0/3] ACPI: button: Fixes related to GPE handling and probe error path Rafael J. Wysocki
@ 2026-05-22 14:49 ` Rafael J. Wysocki
  2026-05-22 14:52 ` [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time Rafael J. Wysocki
  2026-05-22 14:53 ` [PATCH v2 3/3] ACPI: button: Add missing device class clearing on probe failures Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2026-05-22 14:49 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Nick, Saket Dumbre, Pawel Chmielewski, Mario Limonciello

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>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Cc: 6.15+ <stable@vger.kernel.org> # 6.15+
---

v1 -> v2:
   * Add R-by from Mario
   * No other changes

---
 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);




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time
  2026-05-22 14:48 [PATCH v2 0/3] ACPI: button: Fixes related to GPE handling and probe error path Rafael J. Wysocki
  2026-05-22 14:49 ` [PATCH v2 1/3] ACPI: button: Fix ACPI GPE handler leak during removal Rafael J. Wysocki
@ 2026-05-22 14:52 ` Rafael J. Wysocki
  2026-05-23  2:15   ` Nick
  2026-05-22 14:53 ` [PATCH v2 3/3] ACPI: button: Add missing device class clearing on probe failures Rafael J. Wysocki
  2 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2026-05-22 14:52 UTC (permalink / raw)
  To: Linux ACPI, Nick; +Cc: LKML, Saket Dumbre, Pawel Chmielewski, Mario Limonciello

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 supplied one and modify
acpi_button_probe() to use that function for enabling the GPEs in
question.

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

v1 -> v2:
   * Change the acpi_enable_gpe_cond() third parameter to a dispatch type (or
     "any type" value) and modify checks in it accordingly to address a
     problem pointed out by sashiko.dev
   * Adjust comments in acpi_enable_gpe_cond() and acpi_enable_gpe()
   * Update the changelog

@Nick, It would be good to test this version of the patch on your system
because it is a bit different from the previous one (which was not entirely
correct).  I'm not expecting it to behave differently, but additional
testing would be welcome.

Thanks!

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

--- a/drivers/acpi/acpica/evxfgpe.c
+++ b/drivers/acpi/acpica/evxfgpe.c
@@ -78,18 +78,22 @@ 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       - GPE dispatch type 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 its dispatch type matches
+ *              the supplied one, or it is different from ACPI_GPE_DISPATCH_NONE
+ *              if the supplied one is ACPI_GPE_DISPATCH_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)
 {
 	acpi_status status = AE_BAD_PARAMETER;
 	struct acpi_gpe_event_info *gpe_event_info;
@@ -100,14 +104,18 @@ acpi_status acpi_enable_gpe(acpi_handle
 	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
 
 	/*
-	 * Ensure that we have a valid GPE number and that there is some way
-	 * of handling the GPE (handler or a GPE method). In other words, we
-	 * won't allow a valid GPE to be enabled if there is no way to handle it.
+	 * Ensure that we have a valid GPE number and that the dispatch type of
+	 * the GPE matches the supplied one (or it is not ACPI_GPE_DISPATCH_NONE
+	 * if the supplied one is ACPI_GPE_DISPATCH_MASK).
 	 */
 	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 (dispatch_type == ACPI_GPE_DISPATCH_MASK)
+			dispatch_type = ACPI_GPE_DISPATCH_TYPE(gpe_event_info->flags);
+		else if (dispatch_type != ACPI_GPE_DISPATCH_TYPE(gpe_event_info->flags))
+			dispatch_type = ACPI_GPE_DISPATCH_NONE;
+
+		if (dispatch_type != ACPI_GPE_DISPATCH_NONE) {
 			status = acpi_ev_add_gpe_reference(gpe_event_info, TRUE);
 			if (ACPI_SUCCESS(status) &&
 			    ACPI_GPE_IS_POLLING_NEEDED(gpe_event_info)) {
@@ -128,6 +136,30 @@ 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)
+{
+	/*
+	 * Ensure that there is some way of handling the GPE (handler or a GPE
+	 * method). In other words, we won't allow a valid GPE to be enabled if
+	 * there is no way to handle it.
+	 */
+	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))
+
+ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
 				acpi_enable_gpe(acpi_handle gpe_device,
 						u32 gpe_number))
 




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] ACPI: button: Add missing device class clearing on probe failures
  2026-05-22 14:48 [PATCH v2 0/3] ACPI: button: Fixes related to GPE handling and probe error path Rafael J. Wysocki
  2026-05-22 14:49 ` [PATCH v2 1/3] ACPI: button: Fix ACPI GPE handler leak during removal Rafael J. Wysocki
  2026-05-22 14:52 ` [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time Rafael J. Wysocki
@ 2026-05-22 14:53 ` Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2026-05-22 14:53 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Nick, Saket Dumbre, Pawel Chmielewski, Mario Limonciello

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>
---

v1 -> v2:
   * Add R-by from Mario
   * No other changes

---
 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;
 }
 




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time
  2026-05-22 14:52 ` [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time Rafael J. Wysocki
@ 2026-05-23  2:15   ` Nick
  2026-05-23 17:29     ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Nick @ 2026-05-23  2:15 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: LKML, Saket Dumbre, Pawel Chmielewski, Mario Limonciello



On May 22, 2026 2:52:10 p.m. UTC, "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>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 supplied one and modify
>acpi_button_probe() to use that function for enabling the GPEs in
>question.
>
>Fixes: 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake() for buttons")
>Reported-by: Nick <nick@kousu.ca>
>Closes: https://lore.kernel.org/linux-acpi/E2OXET.4X5GTP37VTNC3@kousu.ca/ [1]
>Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>Cc: 7.0+ <stable@vger.kernel.org> # 7.0+
>---
>
>v1 -> v2:
>   * Change the acpi_enable_gpe_cond() third parameter to a dispatch type (or
>     "any type" value) and modify checks in it accordingly to address a
>     problem pointed out by sashiko.dev
>   * Adjust comments in acpi_enable_gpe_cond() and acpi_enable_gpe()
>   * Update the changelog
>
>@Nick, It would be good to test this version of the patch on your system
>because it is a bit different from the previous one (which was not entirely
>correct).  I'm not expecting it to behave differently, but additional
>testing would be welcome.
>


I tested this on top of 45255ea1ca096. The keys are still working as expected! I compared it against 6.19.14 too and all the keys seem to behave as before.

I noticed one difference, but it might not be directly related: KEY_SLEEP takes significantly longer to trigger suspend on 7.1-rc4 than on 6.19.14. On the older version it suspends in about a second; on 7.1-rc4 it takes about a minute before it fully suspends. The lid switch on the other hand suspends in about a second, and so does the suspend button in my DE, on both versions. I guess they're doing different kinds of suspend. That's probably related to the other ACPI changes that landed in 7.0.

Here's a log of me pressing the keys again: (we don't see KEY_SUSPEND here because it triggers hiberation which I don't have set up so it just shuts down and I lose the log)

(Apologies if you got this twice, I managed to reset.my mail client to HTML defaults while testing somehow)


$ uname -r
6.19.14-arch1-1
$ sudo libinput debug-events --show-keycodes
-event2   DEVICE_ADDED                 Power Button                      seat0 default group1  cap:k
-event4   DEVICE_ADDED                 Video Bus                         seat0 default group2  cap:k
-event1   DEVICE_ADDED                 Power Button                      seat0 default group3  cap:k
-event0   DEVICE_ADDED                 Lid Switch                        seat0 default group4  cap:S
-event5   DEVICE_ADDED                 Toshiba input device              seat0 default group5  cap:k
-event12  DEVICE_ADDED                 Synaptics TM3322-002              seat0 default group6  cap:pg  size 95x53mm tap (dl off) left scroll-nat scroll-2fg-edge click-buttonareas-clickfinger dwt-on dwtp-on
-event3   DEVICE_ADDED                 AT Translated Set 2 keyboard      seat0 default group7  cap:k
-event5   KEYBOARD_KEY                 +0.000s	KEY_MUTE (113) pressed
 event5   KEYBOARD_KEY                 +0.000s	KEY_MUTE (113) released
 event5   KEYBOARD_KEY                 +1.054s	KEY_MUTE (113) pressed
 event5   KEYBOARD_KEY                 +1.054s	KEY_MUTE (113) released
 event5   KEYBOARD_KEY                 +2.979s	KEY_COFFEE (152) pressed
 event5   KEYBOARD_KEY                 +2.979s	KEY_COFFEE (152) released
 event5   KEYBOARD_KEY                 +3.810s	KEY_COFFEE (152) pressed
 event5   KEYBOARD_KEY                 +3.810s	KEY_COFFEE (152) released
 event5   KEYBOARD_KEY                 +5.552s	KEY_BATTERY (236) pressed
 event5   KEYBOARD_KEY                 +5.552s	KEY_BATTERY (236) released
 event5   KEYBOARD_KEY                 +6.180s	KEY_BATTERY (236) pressed
 event5   KEYBOARD_KEY                 +6.180s	KEY_BATTERY (236) released
 event5   KEYBOARD_KEY                 +7.964s	KEY_SLEEP (142) pressed
 event5   KEYBOARD_KEY                 +7.964s	KEY_SLEEP (142) released
 event5   KEYBOARD_KEY                 +10.545s	KEY_UNKNOWN (240) pressed
 event5   KEYBOARD_KEY                 +10.545s	KEY_UNKNOWN (240) released
-event1   KEYBOARD_KEY                 +10.598s	KEY_WAKEUP (143) pressed
 event1   KEYBOARD_KEY                 +10.598s	KEY_WAKEUP (143) released
-event2   KEYBOARD_KEY                 +10.600s	KEY_WAKEUP (143) pressed
 event2   KEYBOARD_KEY                 +10.600s	KEY_WAKEUP (143) released
-event5   KEYBOARD_KEY                 +20.312s	KEY_SWITCHVIDEOMODE (227) pressed
 event5   KEYBOARD_KEY                 +20.312s	KEY_SWITCHVIDEOMODE (227) released
 event5   KEYBOARD_KEY                 +20.981s	KEY_SWITCHVIDEOMODE (227) pressed
 event5   KEYBOARD_KEY                 +20.981s	KEY_SWITCHVIDEOMODE (227) released
 event5   KEYBOARD_KEY                 +21.467s	KEY_BRIGHTNESSDOWN (224) pressed
 event5   KEYBOARD_KEY                 +21.467s	KEY_BRIGHTNESSDOWN (224) released
 event5   KEYBOARD_KEY                 +22.034s	KEY_BRIGHTNESSDOWN (224) pressed
 event5   KEYBOARD_KEY                 +22.034s	KEY_BRIGHTNESSDOWN (224) released
 event5   KEYBOARD_KEY                 +22.420s	KEY_BRIGHTNESSUP (225) pressed
 event5   KEYBOARD_KEY                 +22.420s	KEY_BRIGHTNESSUP (225) released
 event5   KEYBOARD_KEY                 +22.845s	KEY_BRIGHTNESSUP (225) pressed
 event5   KEYBOARD_KEY                 +22.845s	KEY_BRIGHTNESSUP (225) released
 event5   KEYBOARD_KEY                 +23.757s	KEY_WLAN (238) pressed
 event5   KEYBOARD_KEY                 +23.757s	KEY_WLAN (238) released
 event5   KEYBOARD_KEY                 +24.953s	KEY_WLAN (238) pressed
 event5   KEYBOARD_KEY                 +24.953s	KEY_WLAN (238) released


$ uname -r
7.1.0-rc4-1-git-00393-g39551c4ef087
$ sudo libinput debug-events --show-keycodes
-event5   DEVICE_ADDED                 Toshiba input device              seat0 default group1  cap:k
-event12  DEVICE_ADDED                 Synaptics TM3322-002              seat0 default group2  cap:pg  size 95x53mm tap (dl off) left scroll-nat scroll-2fg-edge click-buttonareas-clickfinger dwt-on dwtp-on
-event4   DEVICE_ADDED                 Video Bus                         seat0 default group3  cap:k
-event2   DEVICE_ADDED                 Power Button                      seat0 default group4  cap:k
-event1   DEVICE_ADDED                 Power Button                      seat0 default group5  cap:k
-event0   DEVICE_ADDED                 Lid Switch                        seat0 default group6  cap:S
-event3   DEVICE_ADDED                 AT Translated Set 2 keyboard      seat0 default group7  cap:k
 event3   KEYBOARD_KEY                 +1.467s	KEY_LEFTCTRL (29) pressed
 event3   KEYBOARD_KEY                 +1.469s	KEY_LEFTMETA (125) pressed
 event3   KEYBOARD_KEY                 +1.470s	KEY_F24 (194) pressed
 event3   KEYBOARD_KEY                 +1.608s	KEY_F24 (194) released
 event3   KEYBOARD_KEY                 +1.611s	KEY_LEFTMETA (125) released
 event3   KEYBOARD_KEY                 +1.612s	KEY_LEFTCTRL (29) released
-event5   KEYBOARD_KEY                 +4.041s	KEY_WLAN (238) pressed
 event5   KEYBOARD_KEY                 +4.041s	KEY_WLAN (238) released
 event5   KEYBOARD_KEY                 +7.001s	KEY_BRIGHTNESSUP (225) pressed
 event5   KEYBOARD_KEY                 +7.001s	KEY_BRIGHTNESSUP (225) released
 event5   KEYBOARD_KEY                 +7.568s	KEY_BRIGHTNESSDOWN (224) pressed
 event5   KEYBOARD_KEY                 +7.568s	KEY_BRIGHTNESSDOWN (224) released
 event5   KEYBOARD_KEY                 +12.350s	KEY_SWITCHVIDEOMODE (227) pressed
 event5   KEYBOARD_KEY                 +12.350s	KEY_SWITCHVIDEOMODE (227) released
 event5   KEYBOARD_KEY                 +13.726s	KEY_SWITCHVIDEOMODE (227) pressed
 event5   KEYBOARD_KEY                 +13.726s	KEY_SWITCHVIDEOMODE (227) released
 event5   KEYBOARD_KEY                 +19.846s	KEY_SLEEP (142) pressed
 event5   KEYBOARD_KEY                 +19.846s	KEY_SLEEP (142) released
-event3   KEYBOARD_KEY                 +50.998s	KEY_LEFTCTRL (29) pressed
 event3   KEYBOARD_KEY                 +51.140s	KEY_LEFTCTRL (29) released
-event5   KEYBOARD_KEY                 +96.713s	KEY_UNKNOWN (240) pressed
 event5   KEYBOARD_KEY                 +96.716s	KEY_UNKNOWN (240) released
-event1   KEYBOARD_KEY                 +96.740s	KEY_WAKEUP (143) pressed
 event1   KEYBOARD_KEY                 +96.740s	KEY_WAKEUP (143) released
-event2   KEYBOARD_KEY                 +96.743s	KEY_WAKEUP (143) pressed
 event2   KEYBOARD_KEY                 +96.743s	KEY_WAKEUP (143) released
-event5   KEYBOARD_KEY                 +111.647s	KEY_BATTERY (236) pressed
 event5   KEYBOARD_KEY                 +111.647s	KEY_BATTERY (236) released
 event5   KEYBOARD_KEY                 +113.308s	KEY_BATTERY (236) pressed
 event5   KEYBOARD_KEY                 +113.308s	KEY_BATTERY (236) released
 event5   KEYBOARD_KEY                 +119.950s	KEY_COFFEE (152) pressed
 event5   KEYBOARD_KEY                 +119.950s	KEY_COFFEE (152) released
 event5   KEYBOARD_KEY                 +121.349s	KEY_COFFEE (152) pressed
 event5   KEYBOARD_KEY                 +121.349s	KEY_COFFEE (152) released
 event5   KEYBOARD_KEY                 +124.933s	KEY_MUTE (113) pressed
 event5   KEYBOARD_KEY                 +124.933s	KEY_MUTE (113) released
 event5   KEYBOARD_KEY                 +125.420s	KEY_MUTE (113) pressed
 event5   KEYBOARD_KEY                 +125.420s	KEY_MUTE (113) released
-event0   SWITCH_TOGGLE                +133.939s	switch lid state 1
-event5   KEYBOARD_KEY                 +136.839s	KEY_UNKNOWN (240) pressed
 event5   KEYBOARD_KEY                 +136.850s	KEY_UNKNOWN (240) released
-event0   SWITCH_TOGGLE                +136.873s	switch lid state 0
-event1   KEYBOARD_KEY                 +136.873s	KEY_WAKEUP (143) pressed
 event1   KEYBOARD_KEY                 +136.873s	KEY_WAKEUP (143) released
-event2   KEYBOARD_KEY                 +136.874s	KEY_WAKEUP (143) pressed
 event2   KEYBOARD_KEY                 +136.874s	KEY_WAKEUP (143) released

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time
  2026-05-23  2:15   ` Nick
@ 2026-05-23 17:29     ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2026-05-23 17:29 UTC (permalink / raw)
  To: Nick
  Cc: Rafael J. Wysocki, Linux ACPI, LKML, Saket Dumbre,
	Pawel Chmielewski, Mario Limonciello

On Sat, May 23, 2026 at 4:15 AM Nick <nick@kousu.ca> wrote:
>
>
>
> On May 22, 2026 2:52:10 p.m. UTC, "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> >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 supplied one and modify
> >acpi_button_probe() to use that function for enabling the GPEs in
> >question.
> >
> >Fixes: 57c31e6d620f ("ACPI: scan: Use acpi_setup_gpe_for_wake() for buttons")
> >Reported-by: Nick <nick@kousu.ca>
> >Closes: https://lore.kernel.org/linux-acpi/E2OXET.4X5GTP37VTNC3@kousu.ca/ [1]
> >Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >Cc: 7.0+ <stable@vger.kernel.org> # 7.0+
> >---
> >
> >v1 -> v2:
> >   * Change the acpi_enable_gpe_cond() third parameter to a dispatch type (or
> >     "any type" value) and modify checks in it accordingly to address a
> >     problem pointed out by sashiko.dev
> >   * Adjust comments in acpi_enable_gpe_cond() and acpi_enable_gpe()
> >   * Update the changelog
> >
> >@Nick, It would be good to test this version of the patch on your system
> >because it is a bit different from the previous one (which was not entirely
> >correct).  I'm not expecting it to behave differently, but additional
> >testing would be welcome.
> >
>
>
> I tested this on top of 45255ea1ca096. The keys are still working as expected! I compared it against 6.19.14 too and all the keys seem to behave as before.

Thanks for the confirmation!

I'll add a Tested-by tag for you to the patch when applying it unless
you don't want me to do that.

> I noticed one difference, but it might not be directly related: KEY_SLEEP takes significantly longer
> to trigger suspend on 7.1-rc4 than on 6.19.14. On the older version it suspends in about a second;
> on 7.1-rc4 it takes about a minute before it fully suspends. The lid switch on the other hand suspends
> in about a second, and so does the suspend button in my DE, on both versions.

This is unlikely to be related to the issue addressed by the $subject patch.

> I guess they're doing different kinds of suspend. That's probably related to the other ACPI changes
> that landed in 7.0.

One thing to take into consideration here is that the kernel doesn't
trigger suspend by itself.  The
key press event first goes to user space which then decides what to do
with it and may (or may
not) trigger a system suspend in response.  It may be doing different
things for KEY_SLEEP and
the lid switch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-23 17:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-22 14:48 [PATCH v2 0/3] ACPI: button: Fixes related to GPE handling and probe error path Rafael J. Wysocki
2026-05-22 14:49 ` [PATCH v2 1/3] ACPI: button: Fix ACPI GPE handler leak during removal Rafael J. Wysocki
2026-05-22 14:52 ` [PATCH v2 2/3] ACPI: button: Enable wakeup GPEs for ACPI buttons at probe time Rafael J. Wysocki
2026-05-23  2:15   ` Nick
2026-05-23 17:29     ` Rafael J. Wysocki
2026-05-22 14:53 ` [PATCH v2 3/3] ACPI: button: Add missing device class clearing on probe failures Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®