linux-kernel - Re: [PATCH 2/2] Revert "Input: soc_button_array

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1f8c0262-b376-43cb-b2c5-5b60e8cbf678@kernel.org>
Date: Wed, 25 Jun 2025 09:09:34 -0500
From: Mario Limonciello <superm1@...nel.org>
To: Hans de Goede <hansg@...nel.org>, Mika Westerberg <westeri@...nel.org>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: "open list:GPIO ACPI SUPPORT" <linux-gpio@...r.kernel.org>,
 "open list:GPIO ACPI SUPPORT" <linux-acpi@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>,
 "open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)..."
 <linux-input@...r.kernel.org>, Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH 2/2] Revert "Input: soc_button_array - debounce the
 buttons"

On 6/25/25 4:09 AM, Hans de Goede wrote:
> Hi Mario,
> 
> On 24-Jun-25 10:22 PM, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@....com>
>>
>> commit 5c4fa2a6da7fb ("Input: soc_button_array - debounce the buttons")
>> hardcoded all soc-button-array devices to use a 50ms debounce timeout
>> but this doesn't work on all hardware.  The hardware I have on hand
>> actually prescribes in the ASL that the timeout should be 0:
>>
>> GpioInt (Edge, ActiveBoth, Exclusive, PullUp, 0x0000,
>>           "\\_SB.GPIO", 0x00, ResourceConsumer, ,)
>> {   // Pin list
>>      0x0000
>> }
>>
>> Let the GPIO core program the debounce instead of hardcoding it into a
>> driver.
>>
>> This reverts commit 5c4fa2a6da7fbc76290d1cb54a7e35633517a522.
> 
> This is going to cause problems I'm afraid I just checked and
> based on randomly checking a few DSDTs of the tablets this driver
> is used on, it seems the DSDT always specifies a debounce timeout
> of 0 like your example above. And on many many devices using
> the soc_button_array driver debouncing is actually necessary.

That's unfortunate to hear.

> 
> May I ask what problem you are seeing with the 50ms debounce timeout /
> what problem you are exactly trying to fix here ?

The power button doesn't work to wake from suspend.  I bisected it down 
to your commit and then later traced that debounce from the ASL never 
gets set (pinctrl-amd's amd_gpio_set_debounce() is never called).

Also comparing the GPIO register in Windows (where things work) Windows 
never programs a debounce.

So that's where both patches in this series came from.

> 
> drivers/input/keyboard/gpio_keys.c first will call gpiod_set_debounce()
> it self with the 50 ms provided by soc_button_array and if that does
> not work it will fall back to software debouncing. So I don't see how
> the 50 ms debounce can cause problems, other then maybe making
> really really (impossible?) fast double-clicks register as a single
> click .
> 
> These buttons (e.g. volume up/down) are almost always simply mechanical
> switches and these definitely will need debouncing, the 0 value from
> the DSDT is plainly just wrong. There is no such thing as a not bouncing
> mechanical switch.

On one of these tablets can you check the GPIO in Windows to see if it's 
using any debounce?

> 
> Regards,
> 
> Hans
> 
> 
> 
>>
>> Cc: Hans de Goede <hansg@...nel.org>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>>   drivers/input/misc/soc_button_array.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
>> index b8cad415c62ca..99490df42b6f2 100644
>> --- a/drivers/input/misc/soc_button_array.c
>> +++ b/drivers/input/misc/soc_button_array.c
>> @@ -219,8 +219,6 @@ soc_button_device_create(struct platform_device *pdev,
>>   		gpio_keys[n_buttons].active_low = info->active_low;
>>   		gpio_keys[n_buttons].desc = info->name;
>>   		gpio_keys[n_buttons].wakeup = info->wakeup;
>> -		/* These devices often use cheap buttons, use 50 ms debounce */
>> -		gpio_keys[n_buttons].debounce_interval = 50;
>>   		n_buttons++;
>>   	}
>>   
>