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

[Mario Limonciello <superm1@kernel.org>]

On 6/27/2025 2:18 PM, Dmitry Torokhov wrote:
> On Fri, Jun 27, 2025 at 01:56:53PM -0500, Mario Limonciello wrote:
>> On 6/27/2025 1:36 PM, Dmitry Torokhov wrote:
>>> On Fri, Jun 27, 2025 at 05:56:05PM +0200, Hans de Goede wrote:
> 
> [ ... trim ... ]
> 
>>>
>>> 2. There is a patch from Mario (a8605b0ed187) suppressing sending
>>> KEY_POWER as part of "normal" wakeup handling, pretty much the same as
>>> what he and you are proposing to do in gpio-keys (and eventually in
>>> every driver keyboard or button driver in the kernel). This means we no
>>> longer can tell if wakeup is done by power button or sleep button (on
>>> systems with dual-button models, see ACPI 4.8.3.1).
>>
>> Actually a8605b0ed187 was about a runtime regression not a suspend
>> regression.  I didn't change anything with sending KEY_POWER during wakeup
>> handling.
> 
> Ah, right, ignorng events for "suspended" buttons was done in
> e71eeb2a6bcc ("ACPI / button: Do not propagate wakeup-from-suspend
> events"). Again trying to add heuristic to the kernel instead of
> enlightening userspace.
> 
> I am curious why the system is sending "Notify Wake" events when not
> sleeping though?

I wondered the same thing.  My guess is this is a BIOS bug.

> 
> [ .. skip .. ]
> 
>>
>> FTR I did test Hans suggestion and it does work effectively (code below).
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c
>> b/drivers/input/keyboard/gpio_keys.c
>> index f9db86da0818b..3bc8c95e9943b 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -425,7 +425,8 @@ 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);
>> +                       input_report_key(bdata->input, *bdata->code, 1);
>> +                       input_sync(bdata->input);
> 
> I start wondering if we should keep the fake press given that we do not
> know for sure if wakeup truly happened because of this button press...
> 
> Can we track back to the wakeup source and determine this? It will not
> help your problem, but I still believe userspace is where policy should
> live.
> 
> Thanks.
>