From d0a8c4158c841f865c6d8360efaa0c92a2d29374 Mon Sep 17 00:00:00 2001 From: Fabio Baltieri Date: Tue, 12 Mar 2024 20:26:06 +0000 Subject: [PATCH] input: gpio_keys: fix suspend race condition Change the suspend/resume code to ensure that the interrupt are disabled before changing the pin configuration. The current sequence has been reported to cause spurious readouts on some platforms, this takes the existing code and duplicates for the suspend and resume case, but swaps the interrupt disable and configure for the suspend case. Signed-off-by: Fabio Baltieri --- drivers/input/input_gpio_keys.c | 78 ++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/drivers/input/input_gpio_keys.c b/drivers/input/input_gpio_keys.c index cd277c19fd..7fca24a55e 100644 --- a/drivers/input/input_gpio_keys.c +++ b/drivers/input/input_gpio_keys.c @@ -207,51 +207,59 @@ static int gpio_keys_pm_action(const struct device *dev, const struct gpio_keys_config *cfg = dev->config; struct gpio_keys_data *data = dev->data; struct gpio_keys_pin_data *pin_data = cfg->pin_data; - gpio_flags_t gpio_flags; - gpio_flags_t int_flags; int ret; switch (action) { case PM_DEVICE_ACTION_SUSPEND: - gpio_flags = GPIO_DISCONNECTED; - int_flags = GPIO_INT_DISABLE; atomic_set(&data->suspended, 1); - break; + + for (int i = 0; i < cfg->num_keys; i++) { + const struct gpio_dt_spec *gpio = &cfg->pin_cfg[i].spec; + + if (!cfg->polling_mode) { + ret = gpio_pin_interrupt_configure_dt(gpio, GPIO_INT_DISABLE); + if (ret < 0) { + LOG_ERR("interrupt configuration failed: %d", ret); + return ret; + } + } + + ret = gpio_pin_configure_dt(gpio, GPIO_DISCONNECTED); + if (ret != 0) { + LOG_ERR("Pin %d configuration failed: %d", i, ret); + return ret; + } + } + + return 0; case PM_DEVICE_ACTION_RESUME: - gpio_flags = GPIO_INPUT; - int_flags = GPIO_INT_EDGE_BOTH; atomic_set(&data->suspended, 0); - break; + + for (int i = 0; i < cfg->num_keys; i++) { + const struct gpio_dt_spec *gpio = &cfg->pin_cfg[i].spec; + + ret = gpio_pin_configure_dt(gpio, GPIO_INPUT); + if (ret != 0) { + LOG_ERR("Pin %d configuration failed: %d", i, ret); + return ret; + } + + if (cfg->polling_mode) { + k_work_reschedule(&pin_data[0].work, + K_MSEC(cfg->debounce_interval_ms)); + } else { + ret = gpio_pin_interrupt_configure_dt(gpio, GPIO_INT_EDGE_BOTH); + if (ret < 0) { + LOG_ERR("interrupt configuration failed: %d", ret); + return ret; + } + } + } + + return 0; default: return -ENOTSUP; } - - for (int i = 0; i < cfg->num_keys; i++) { - const struct gpio_dt_spec *gpio = &cfg->pin_cfg[i].spec; - - ret = gpio_pin_configure_dt(gpio, gpio_flags); - if (ret != 0) { - LOG_ERR("Pin %d configuration failed: %d", i, ret); - return ret; - } - - if (cfg->polling_mode) { - continue; - } - - ret = gpio_pin_interrupt_configure_dt(gpio, int_flags); - if (ret < 0) { - LOG_ERR("interrupt configuration failed: %d", ret); - return ret; - } - } - - if (action == PM_DEVICE_ACTION_RESUME && cfg->polling_mode) { - k_work_reschedule(&pin_data[0].work, - K_MSEC(cfg->debounce_interval_ms)); - } - - return 0; } #endif