Re: [PATCH v3 4/4] Input: Don't send fake button presses to wake system

On Fri, Jun 27, 2025 at 07:59:56PM +0200, Hans de Goede wrote:

Hi,

On 27-Jun-25 6:12 PM, Andy Shevchenko wrote:

quoted

On Fri, Jun 27, 2025 at 05:56:05PM +0200, Hans de Goede wrote:

quoted

On 27-Jun-25 4:44 PM, Dmitry Torokhov wrote:

quoted

On Fri, Jun 27, 2025 at 04:14:38PM +0200, Hans de Goede wrote:

quoted

On 27-Jun-25 4:06 PM, Mario Limonciello wrote:

quoted

On 6/26/2025 11:56 PM, Dmitry Torokhov wrote:

...

quoted

quoted

quoted

quoted

Hans, you have a lot of experience in the GNOME community.  Your thoughts?

I guess it would be good to fix this in the kernel, sending
KEY_WAKEUP from gpio_key when the event is KEY_POWER and
we are going through the special wakeup path in gpio_keys.

When this was discussed quite a while ago the ACPI button
driver simply did not send any event at all on wkaeup
by ACPI power-button. Know that it does send an event
it would be good to mimic this, at least when the gpio_key
devices where instantiated by soc_button_array.

So maybe add a new field to struct gpio_keys_button
called wakeup_code and when that is not 0 use that
instead of the plain "code" member on wakeups ?

That would keep the gpio_keys code generic while
allowing to mimic the ACPI button behavior.

And then set wakeup_code to KEY_WAKEUP for
the power-button in soc_button_array.

To me this sounds better then trying to fix all userspace
code which does something on KEY_POWER of which there
is quite a lot.

The special GNOME power-button handling was always
a workaround because last time a kernel fix was
nacked. But now with the KEY_WAKEUP done by the ACPI
button code it looks like we do have a good way
to fix this in the kernel, so that would be better
IMHO.

Dmitry, what do you think of adding a wakeup_code
field to struct gpio_keys_button and let the code
creating the gpio_keys_button decide if a different
code should be used on wakeup or not ?

And what is the plan on dealing with all other drivers that emit
KEY_POWER?

There actually aren't that many that I'm aware of.

Note that this gpio_keys KEY_POWER evdev event generation
on resume issue goes way back until the last time we had
this conversation and it still has not really been fixed.

And I've not seen any bug-reports about the same problem
with any other drivers.

quoted

What about acpi button behavior when using S0ix?

AFAIK it is the same as with S3, at least it is not
causing any issues. I've never seen the ACPI button code
cause re-suspend immediately on wakeup by what for all
intends and purposes is a spurious KEY_POWER event.

Last time we discussed this I wasn't really happy with
the outcome of the discussion but I just went for it
because of Android's reliance on the event and we
lacked a better plan.

Now that we've a fix for this in the form of KEY_WAKEUP
it is time to properly fix this instead of doing userspace
kludges.

quoted

What about
holding power button for too long so that normal reporting "catches" the
pressed state?

The key-down event is send as KEY_WAKEUP instead,
so userspace sees KEY_WAKEUP pressed not KEY_POWER.

quoted

Kernel reports hardware events, interpreting them and applying certain
policies is task for userspace.

And atm it is actually doing a shitty job of reporting
hwevents because there is no way for userspace to be able
to differentiate between:

1. User pressed power-button to wakeup system
2. User pressed power-button after resume to do
   power-button-action (e.g. suspend system)

Even though *the kernel* does *know* the difference.

So the suggested change actually makes the kernel
do its job of reporting hw-events better by making
the reporting more accurate.

ATM if I resume say a tablet with GNOME and then
change my mind and press the power button within
3 seconds of resume to suspend it again the second
power-button press will outright be ignored

The current userspace workaround is racy like this,
again the whole workaround in GNOME is just an ugly
kludge which I did back then because we couldn't
agree on a better way to deal with this in the kernel /
because just suppressing sending KEY_POWER would break
Android.

The suggested use of KEY_WAKEUP is lightyears better
then doing ignore KEY_POWER events for xx seconds
after resume which is simply always going to be racy
and always was just an ugly hack / never was
a great solution.

My take away from this discussion that in a sleep state the power button
(or actually any wakeup source) should tell userspace "hey, we want to wake
up". It doesn't tell "hey, we want to wake to power off".
This sounds good to me as a user. Yes, if laptop is sleeping we need to wake
it up to continue, the power off is just a next step of this flow.

Exactly.

These are really 2 different events and the problem
is that atm the kernel reports this as one and
the same event type. It really is as simple as this.

Note that I'm not proposing on making this change
something which all gpio_keys drivers will get
automatically.

My idea of adding a wakeup_code field to
struct gpio_keys_button allows individual gpio_keys
users to opt-in to the behavior of
sending KEY_SOMETHINGELSE on wakeup-by-gpio-button-press.

Since soc_button_array is only used on x86/ACPI platforms
and since by far the most x86/ACPI platforms use
the ACPI button code for the power-button, which already
behaves this way I do not expect any userspace problems
from doing such a change in soc_button_array because
if that were a problem then we would already have bug
reports because of the ACPI button code's behavior.

ACPI/x86 system does not necessarily mean something running generic
Linux distribution, and it may have different set of devices and
drivers. Or it may even use soc_button_array and still not want this
behavior.

quoted

The  expected hot topic here is the longer presses of power button, but I think
if we have a timer and tell after say 3 second that the K_WUP was up and K_PW
is down is not a solution, it will be always flaky.

If users do a long power-button press on x86 they are
*always* trying to do a forced power-off and after 4s
(or longer in some special cases) the hw itself will
do such a forced poweroff. So I do not believe that
we need to worry about long presses since those
have a very specific meaning on x86/ACPI platforms.

You only consider one environment: Gnome. There are other alternatives.
For example on Chrome OS pressing power button for longish time (but
shorter that forced power off) will result in orderly system shutdown.

Also if there is a long press userspace will simply
see KEY_WAKEUP never getting released, which is
actually an accurate representation of things,
the user woke up the system through the power-button
and never released ir.

Yeah, it will break everyone who is monitoring user activity.

Thanks.

-- 
Dmitry