Re: [PATCH v3 4/4] Input: Don't send fake button presses to wake system
From: Mario Limonciello
Date: Thu Jun 26 2025 - 14:55:41 EST
On 6/26/2025 1:48 PM, Dmitry Torokhov wrote:
On Thu, Jun 26, 2025 at 01:20:54PM -0500, Mario Limonciello wrote:
On 6/26/2025 1:07 PM, Dmitry Torokhov wrote:
On Thu, Jun 26, 2025 at 12:53:02PM -0500, Mario Limonciello wrote:
On 6/26/25 12:44 PM, Dmitry Torokhov wrote:
Hi Mario,
On Thu, Jun 26, 2025 at 06:33:08AM -0500, Mario Limonciello wrote:
On 6/26/25 3:35 AM, Hans de Goede wrote:
Hi Mario,
On 25-Jun-25 23:58, Mario Limonciello wrote:
From: Mario Limonciello <mario.limonciello@xxxxxxx>
Sending an input event to wake a system does wake it, but userspace picks
up the keypress and processes it. This isn't the intended behavior as it
causes a suspended system to wake up and then potentially turn off if
userspace is configured to turn off on power button presses.
Instead send a PM wakeup event for the PM core to handle waking the system.
Cc: Hans de Goede <hansg@xxxxxxxxxx>
Fixes: 0f107573da417 ("Input: gpio_keys - handle the missing key press event in resume phase")
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/input/keyboard/gpio_keys.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 773aa5294d269..4c6876b099c43 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -420,12 +420,7 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
pm_stay_awake(bdata->input->dev.parent);
if (bdata->suspended &&
(button->type == 0 || button->type == EV_KEY)) {
- /*
- * Simulate wakeup key press in case the key has
- * already released by the time we got interrupt
- * handler to run.
- */
- input_report_key(bdata->input, button->code, 1);
+ pm_wakeup_event(bdata->input->dev.parent, 0);
There is already pm_stay_awake() above.
But that doesn't help with the fact that userspace gets KEY_POWER from this
and reacts to it.
}
}
Hmm, we have the same problem on many Bay Trail / Cherry Trail
windows 8 / win10 tablets, so this has been discussed before and e.g.
Android userspace actually needs the button-press (evdev) event to not
immediately go back to sleep, so a similar patch has been nacked in
the past.
At least for GNOME this has been fixed in userspace by ignoring
power-button events the first few seconds after a resume from suspend.
The default behavior for logind is:
HandlePowerKey=poweroff
Can you share more about what version of GNOME has a workaround?
This was actually GNOME (on Ubuntu 24.04) that I found this issue.
Nonetheless if this is dependent on an Android userspace problem could we
perhaps conditionalize it on CONFIG_ANDROID_BINDER_DEVICES?
No it is not only Android, other userspace may want to distinguish
between normal and "dark" resume based on keyboard or other user
activity.
Thanks.
In this specific case does the key passed up to satisfy this userspace
requirement and keep it awake need to specifically be a fabricated
KEY_POWER?
Or could we find a key that doesn't require some userspace to ignore
KEY_POWER?
Maybe something like KEY_RESERVED, KEY_FN, or KEY_POWER2?
The code makes no distinction between KEY_POWER and KEY_A or KEY_B, etc.
It simply passes event to userspace for processing.
Right. I don't expect a problem with most keys, but my proposal is to
special case KEY_POWER while suspended. If a key press event must be sent
to keep Android and other userspace happy I suggest sending something
different just for that situation.
I do not know if userspace specifically looks for KEY_POWER or if it
looks for user input in general, and I'd rather be on safe side and not
mangle user input.
As Hans mentioned, at least some userspace already prepared to deal with
this issue. And again, this only works if by the time ISR/debounce
runs the key is already released. What if it is still pressed? You still
going to observe KEY_POWER and need to suppress turning off the screen.
Like this:
diff --git a/drivers/input/keyboard/gpio_keys.c
b/drivers/input/keyboard/gpio_keys.c
index 773aa5294d269..66e788d381956 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -425,7 +425,10 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void
*dev_id)
* already released by the time we got interrupt
* handler to run.
*/
- input_report_key(bdata->input, button->code, 1);
+ if (button->code == KEY_POWER)
+ input_report_key(bdata->input, KEY_WAKEUP,
1);
Just FYI: Here your KEY_WAKEUP is stuck forever.
Thanks.
+ else
+ input_report_key(bdata->input, button->code,
1);
}
}
You need to fix your userspace. Even with your tweak it is possible for
userspace to get a normal key event "too early" and turn off the screen
again, so you still need to handle this situation.
Thanks.
I want to note this driver works quite differently than how ACPI power
button does.
You can see in acpi_button_notify() that the "keypress" is only forwarded
when not suspended [1]. Otherwise it's just wakeup event (which is what my
patch was modeling).
https://github.com/torvalds/linux/blob/v6.16-rc3/drivers/acpi/button.c#L461
[1]
If you check acpi_button_resume() you will see that the events are sent
from there. Except that for some reason they chose to use KEY_WAKEUP and
not KEY_POWER, oh well. Unlike acpi button driver gpio_keys is used on
multiple other platforms.
Thanks.
Well that would explain the difference, and git blame gives the history [1].
It's from enablement for Android with ACPI power button. That commit
also mentions that Android can handle both POWER and WAKEUP from input
device to wakeup the system. Non-Android userspace doesn't do anything
with KEY_WAKEUP today.
So this has me thinking the proposal I had above to special case
KEY_POWER and translate to KEY_WAKEUP is the right way forward, just
making sure to release the key as you rightfully pointed out.
https://github.com/torvalds/linux/commit/16f70feaabe9fde0af703f2991d44a7589f0b6e3
[1]