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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <6c0514e5-0ea6-4f66-9bc0-4230d7ef0d4b@amd.com>
Date: Wed, 25 Jun 2025 15:34:55 +0000
From: "Limonciello, Mario" <Mario.Limonciello@....com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: Mario Limonciello <superm1@...nel.org>, Hans de Goede <hansg@...nel.org>,
	Mika Westerberg <westeri@...nel.org>, Linus Walleij
	<linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>, Dmitry
 Torokhov <dmitry.torokhov@...il.com>, "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>
Subject: Re: [PATCH 2/2] Revert "Input: soc_button_array - debounce the
 buttons"

On 6/25/25 10:17 AM, Andy Shevchenko wrote:
> On Wed, Jun 25, 2025 at 03:14:40PM +0000, Limonciello, Mario wrote:
>> On 6/25/25 10:10 AM, Andy Shevchenko wrote:
>>> On Wed, Jun 25, 2025 at 03:02:18PM +0000, Limonciello, Mario wrote:
>>>> On 6/25/25 9:41 AM, Mario Limonciello wrote:
>>>>> On 6/25/25 9:31 AM, Hans de Goede wrote:
>>>>>> On 25-Jun-25 4:09 PM, Mario Limonciello wrote:
>>>>>>> On 6/25/25 4:09 AM, Hans de Goede wrote:
>>>>>>>> On 24-Jun-25 10:22 PM, Mario Limonciello wrote:
> 
> ...
> 
>>>>>> Ok, so specifically the gpiod_set_debounce() call with 50 ms
>>>>>> done by gpio_keys.c is the problem I guess?
>>>>>
>>>>> Yep.
>>>>>
>>>>>> So amd_gpio_set_debounce() does accept the 50 ms debounce
>>>>>> passed to it by gpio_keys.c as a valid value and then setting
>>>>>> that breaks the wake from suspend?
>>>>>
>>>>> That's right.
>>>
>>>>>>> Also comparing the GPIO register in Windows (where things work)
>>>>>>> Windows never programs a debounce.
>>>>>>
>>>>>> So maybe the windows ACPI0011 driver always uses a software-
>>>>>> debounce for the buttons? Windows not debouncing the mechanical
>>>>>> switches at all seems unlikely.
>>>>>>
>>>>>> I think the best way to fix this might be to add a no-hw-debounce
>>>>>> flag to the data passed from soc_button_array.c to gpio_keys.c
>>>>>> and have gpio_keys.c not call gpiod_set_debounce()  when the
>>>>>> no-hw-debounce flag is set.
>>>>>>
>>>>>> I've checked and both on Bay Trail and Cherry Trail devices
>>>>>> where soc_button_array is used a lot hw-debouncing is already
>>>>>> unused. pinctrl-baytrail.c does not accept 50 ms as a valid
>>>>>> value and pinctrl-cherryview.c does not support hw debounce
>>>>>> at all.
>>>>>
>>>>> That sounds a like a generally good direction to me.
>>>
>>> Thinking a bit more of this, perhaps the HW debounce support flag should be
>>> per-GPIO-descriptor thingy. In such cases we don't need to distinguish the
>>> platforms, the GPIO ACPI lib may simply set that flag based on 0 read from
>>> the ACPI tables. It will also give a clue to any driver that uses GPIOs
>>> (not only gpio-keys).
>>
>> But 0 doesn't mean hardware debounce support is there, 0 means that
>> hardware debounce is not required to be programmed for this GPIO.
>>
>> That is - if another system had a non-zero value in the GpioInt entry I
>> would expect this to be translated into the GPIO register.
> 
> Correct. The question is only about 0. So the flow will look like
> 
> 1) if the GPIO is defined with 0 debounce, set the flag;
> 2) if the GPIO is defined with non-zero value, try to apply it;
> 3) if the step 2) fails, warn and set the flag.
> 
> Would it make sense?
> Hans?
> 

But so on these problematic BYT/CYT tablets which "layer" should be 
setting the 50ms debounce?
That should still be a quirk at the soc_button_array layer, right?

Because gpio_keys_setup_key() will already fallback to software 
debounce, and the goal here is that both of those only use the 50ms 
specifically with software debouncing.