| 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
| ||
|
Message-ID: <f5e1d50f-d85e-45a3-a131-f2da603c620c@kernel.org>
Date: Wed, 25 Jun 2025 21:03:58 +0200
From: Hans de Goede <hansg@...nel.org>
To: Mario Limonciello <superm1@...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 v2 3/3] Input: soc_button_array: Only debounce cherryview
and baytrail systems
Hi,
On 25-Jun-25 8:13 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
> }
>
> Many cherryview and baytrail systems don't have accurate values in the
> ASL for debouncing and thus use software debouncing in gpio_keys. The
> value to use is programmed in soc_button_array. Detect Cherry View
> and Baytrail using ACPI HID IDs used for those GPIO controllers and apply
> the 50ms only for those systems.
>
> Cc: Hans de Goede <hansg@...nel.org>
> Fixes: 5c4fa2a6da7fb ("Input: soc_button_array - debounce the buttons")
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
I'm not a fan of this approach, I believe that we need to always debounce
when dealing with mechanical buttons otherwise we will get unreliable /
spurious input events.
My suggestion to deal with the issue where setting up debouncing at
the GPIO controller level is causing issues is to always use software
debouncing (which I suspect is what Windows does).
Let me copy and pasting my reply from the v1 thread with
a bit more detail on my proposal:
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, as that will always be necessary when dealing with
mechanical buttons, but always use software debouncing 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 always use software debouncing for the 50 ms
debounce-period. It was *not* my intention to suggest to solve this
with platform specific quirks/behavior.
Regards,
Hans
> ---
> v2:
> * commit message
> * quirk systems instead of revert
> ---
> drivers/input/misc/soc_button_array.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index b8cad415c62ca..56abc93f23e0c 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -129,6 +129,13 @@ static const struct dmi_system_id dmi_invalid_acpi_index[] = {
> {} /* Terminating entry */
> };
>
> +static const struct acpi_device_id force_software_debounce_ids[] = {
> + { "INT33FF" },
> + { "INT33B2" },
> + { "INT33FC" },
> + { }
> +};
> +
> /*
> * Get the Nth GPIO number from the ACPI object.
> */
> @@ -149,11 +156,17 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index,
> return 0;
> }
>
> +static int soc_button_match_acpi_device_ids(struct device *dev, const void *data)
> +{
> + return acpi_match_device(data, dev) ? 1 : 0;
> +}
> +
> static struct platform_device *
> soc_button_device_create(struct platform_device *pdev,
> const struct soc_button_info *button_info,
> bool autorepeat)
> {
> + struct device *quirkdev __free(put_device) = NULL;
> const struct soc_button_info *info;
> struct platform_device *pd;
> struct gpio_keys_button *gpio_keys;
> @@ -181,6 +194,10 @@ soc_button_device_create(struct platform_device *pdev,
> if (dmi_id)
> invalid_acpi_index = (long)dmi_id->driver_data;
>
> + quirkdev = bus_find_device(&platform_bus_type, NULL,
> + force_software_debounce_ids,
> + soc_button_match_acpi_device_ids);
> +
> for (info = button_info; info->name; info++) {
> if (info->autorepeat != autorepeat)
> continue;
> @@ -220,7 +237,8 @@ soc_button_device_create(struct platform_device *pdev,
> 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;
> + if (quirkdev)
> + gpio_keys[n_buttons].debounce_interval = 50;
> n_buttons++;
> }
>
Powered by blists - more mailing lists