Re: [PATCH v4 2/2] Input: gpio-keys - support wakeup pinctrl state on suspend
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-05 20:58:45
Also in:
linux-devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] input: spurious key press on resume due to LIFO violation in pinctrl teardown -- commit 77748a917235cbadd6477449603ad11920308ecf Author: Kendall Willis [off-list ref] Input: gpio-keys - support wakeup pinctrl state on suspend This commit adds support for selecting a wakeup pinctrl state on suspend and restoring the default state on resume for GPIO devices. This enables wakeup support on certain platforms where the GPIO controller is powered down during suspend.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index e196174856796..b8ecaaded6a44 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1022,6 +1042,9 @@ gpio_keys_enable_wakeup(struct gpio_keys_drvdata *ddata) return 0; err_out: + if (!IS_ERR_OR_NULL(ddata->pinctrl_wakeup)) + pinctrl_pm_select_default_state(ddata->input->dev.parent); +
[Severity: Medium] Could this error path in gpio_keys_enable_wakeup() in drivers/input/keyboard/gpio_keys.c trigger a spurious IRQ? The default pinctrl state is restored before the successfully configured wakeup IRQs are disabled. If switching the pinctrl state causes a glitch on the GPIO line, an IRQ might fire prematurely.
quoted hunk ↗ jump to hunk
while (i--) { bdata = &ddata->data[i]; if (bdata->button->wakeup)@@ -1038,6 +1061,9 @@ gpio_keys_disable_wakeup(struct gpio_keys_drvdata *ddata) struct gpio_button_data *bdata; int i; + if (!IS_ERR_OR_NULL(ddata->pinctrl_wakeup)) + pinctrl_pm_select_default_state(ddata->input->dev.parent); +
[Severity: Medium] Can this ordering in gpio_keys_disable_wakeup() in drivers/input/keyboard/gpio_keys.c inject a spurious key press on system resume? On resume, gpio_keys_disable_wakeup() restores the default pinctrl state before disabling the wakeup IRQs and clearing bdata->suspended. If the pinctrl state transition causes a line glitch, the IRQ fires while bdata->suspended is still true. The IRQ handler then sees bdata->suspended is true and explicitly simulates a key press to handle transient wakeup events. If this key is mapped to a power or sleep button, it might cause the system to immediately re-suspend or power off. Should the default pinctrl state be restored after disabling the IRQs and clearing the suspended state to maintain a strict LIFO teardown order?
for (i = 0; i < ddata->pdata->nbuttons; i++) {
bdata = &ddata->data[i];
bdata->suspended = false;-- Sashiko AI review · https://sashiko.dev/#/patchset/20261005-upstream-gpio-wakeup-v4-1-96f092de4690@ti.com?part=2