Skip to content

Commit

Permalink
input: gpio_keys: fix suspend race condition
Browse files Browse the repository at this point in the history
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 <[email protected]>
(cherry picked from commit d0a8c41)
  • Loading branch information
fabiobaltieri authored and henrikbrixandersen committed Mar 18, 2024
1 parent 78aaf1d commit 83466c2
Showing 1 changed file with 41 additions and 33 deletions.
74 changes: 41 additions & 33 deletions drivers/input/input_gpio_keys.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
case PM_DEVICE_ACTION_RESUME:
gpio_flags = GPIO_INPUT;
int_flags = GPIO_INT_EDGE_BOTH;
atomic_set(&data->suspended, 0);
break;
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;
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;
}
}

if (cfg->polling_mode) {
continue;
}
return 0;
case PM_DEVICE_ACTION_RESUME:
atomic_set(&data->suspended, 0);

ret = gpio_pin_interrupt_configure_dt(gpio, int_flags);
if (ret < 0) {
LOG_ERR("interrupt configuration failed: %d", ret);
return ret;
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;
}
}
}
}

if (action == PM_DEVICE_ACTION_RESUME && cfg->polling_mode) {
k_work_reschedule(&pin_data[0].work,
K_MSEC(cfg->debounce_interval_ms));
return 0;
default:
return -ENOTSUP;
}

return 0;
}
#endif

Expand Down

0 comments on commit 83466c2

Please sign in to comment.