Re: [PATCH 2/2] Revert "Input: soc_button_array - debounce the buttons"

[Hans de Goede <hansg@kernel.org>]

On 25-Jun-25 4:41 PM, Mario Limonciello wrote:
> On 6/25/25 9:31 AM, Hans de Goede wrote:

<snip>

>> 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.
> 
> I think I would still like to see the ASL values translated into the hardware even if the ASL has a "0" value.
> So I would keep patch 1 but adjust for the warning you guys both called out.
> 
> As you have this hardware would you be able to work out that quirk?

I think we've a bit of miscommunication going on here.

My proposal is to add a "no_hw_debounce" flag to 
struct gpio_keys_platform_data and make the soc_button_array
driver set that regardless of which platform it is running on.

And then in gpio_keys.c do something like this:

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index f9db86da0818..2788d1e5782c 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -552,8 +552,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		bool active_low = gpiod_is_active_low(bdata->gpiod);
 
 		if (button->debounce_interval) {
-			error = gpiod_set_debounce(bdata->gpiod,
-					button->debounce_interval * 1000);
+			if (ddata->pdata->no_hw_debounce)
+				error = -EINVAL;
+			else
+				error = gpiod_set_debounce(bdata->gpiod,
+						button->debounce_interval * 1000);
 			/* use timer if gpiolib doesn't provide debounce */
 			if (error < 0)
 				bdata->software_debounce =

So keep debouncing, which I believe will always be necessary when
dealing with mechanical buttons, but always use software debouncing
(which I suspect is what Windows does) to avoid issues like the issue
you are seeing.

My mention of the BYT/CHT behavior in my previous email was to point
out that those already do use software debouncing for the 50 ms
debounce-period. It was *not* my intention to suggest to solve this
with platform specific quirks/behavior.

<semi offtopic>
Hmm, I did found one interesting thing looking at further DSDTs
the Dell Venue 10 Pro 5056 DSDT actually specifies a non 0
debounce time in the ACPI0011 device's GPIO descriptors
it uses a value of 30 ms. This device being one of the few
actually specifying a debounce time in the ACPI is ironic
since it uses drivers/pinctrl/intel/pinctrl-cherryview.c
which does not support PIN_CONFIG_INPUT_DEBOUNCE...
</semi offtopic>

Regards,

Hans





> 
> Or if you want me to do it, I'll need something to go on how to how to effectively detect BYT and CYT hardware.
> 
>>
>>> 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?
>>
>> I'm afraid I don't have Windows installed on any of these.
>>
>> But based on your testing + the DSDT specifying no debounce
>> for the GPIO I guess Windows just follows the DSDt when it
>> comes to setting up the hw debounce-settings and then uses
>> sw-debouncing on top to actually avoid very quick
>> press-release-press event cycles caused by the bouncing.
>>
> 
> Yeah that sounds like a plausible hypothesis.
> 
>