Hi all,
This patch series contains two interrupt-related fixes for the gpio-keys
DT bindings and driver, and a small clean-up.
The first two patches can be applied independently.
The third patch, which is marked RFC, depends on the second.
Thanks for your comments!
Geert Uytterhoeven (3):
dt-bindings: input: gpio-keys: Fix interrupts in example
Input: gpio-keys - Use input_report_key()
[WIP] [RFC] Input: gpio-keys - Fix ghost events with both-edge irqs
.../devicetree/bindings/input/gpio-keys.yaml | 2 +-
drivers/input/keyboard/gpio_keys.c | 38 ++++++++++++-------
2 files changed, 25 insertions(+), 15 deletions(-)
--
2.25.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
When using interrupts instead of GPIOs, the driver auto-generates
key-release events, either immediately or after a delay. This works
fine with rising-edge or falling-edge interrupts, but causes ghost
events with both-edge interrupts. Indeed, the driver will generate a
pair of key press/release events when pressing the key, and another pair
when releasing the key.
Fix this by not auto-generating key-release events for both-edge
interrupts. Rename release_delay to auto_release_delay to clarify its
use.
Note that unlike with GPIOs, the driver cannot know the state of the key
at initialization time, or after resume. Hence the driver assumes the
key is not pressed at initialization time, and does not reconfigure the
trigger type for wakeup.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Tested on rskrza1.
Are these limitations acceptable? Or should users only use rising-edge
or falling-edge interrupts?
There are several existing users of IRQ_TYPE_EDGE_BOTH.
---
drivers/input/keyboard/gpio_keys.c | 34 +++++++++++++++++++-----------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index ab077dfb90a76ac3..dfcbedec226cb4cf 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -38,7 +38,8 @@ struct gpio_button_data {
unsigned short *code;
struct hrtimer release_timer;
- unsigned int release_delay; /* in msecs, for IRQ-only buttons */
+ int auto_release_delay; /* in msecs, for IRQ-only buttons */
+ /* a negative value means no auto-release */
struct delayed_work work;
struct hrtimer debounce_timer;
@@ -474,25 +475,25 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
spin_lock_irqsave(&bdata->lock, flags);
- if (!bdata->key_pressed) {
+ if (!bdata->key_pressed || bdata->auto_release_delay < 0) {
if (bdata->button->wakeup)
pm_wakeup_event(bdata->input->dev.parent, 0);
- input_report_key(input, *bdata->code, 1);
+ input_report_key(input, *bdata->code, !bdata->key_pressed);
input_sync(input);
- if (!bdata->release_delay) {
+ if (!bdata->auto_release_delay) {
input_report_key(input, *bdata->code, 0);
input_sync(input);
goto out;
}
- bdata->key_pressed = true;
+ bdata->key_pressed = !bdata->key_pressed;
}
- if (bdata->release_delay)
+ if (bdata->auto_release_delay > 0)
hrtimer_start(&bdata->release_timer,
- ms_to_ktime(bdata->release_delay),
+ ms_to_ktime(bdata->auto_release_delay),
HRTIMER_MODE_REL_HARD);
out:
spin_unlock_irqrestore(&bdata->lock, flags);
@@ -630,7 +631,6 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
return -EINVAL;
}
- bdata->release_delay = button->debounce_interval;
hrtimer_init(&bdata->release_timer,
CLOCK_REALTIME, HRTIMER_MODE_REL_HARD);
bdata->release_timer.function = gpio_keys_irq_timer;
@@ -638,10 +638,20 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
isr = gpio_keys_irq_isr;
irqflags = 0;
- /*
- * For IRQ buttons, there is no interrupt for release.
- * So we don't need to reconfigure the trigger type for wakeup.
- */
+ if (irq_get_trigger_type(bdata->irq) == IRQ_TYPE_EDGE_BOTH) {
+ bdata->auto_release_delay = -1;
+ /*
+ * Unlike with GPIOs, we do not know what the state of
+ * the key is at initialization time, or after resume.
+ * So we don't reconfigure the trigger type for wakeup.
+ */
+ } else {
+ bdata->auto_release_delay = button->debounce_interval;
+ /*
+ * There is no interrupt for release. So we don't
+ * need to reconfigure the trigger type for wakeup.
+ */
+ }
}
bdata->code = &ddata->keymap[idx];
--
2.25.1
Use the input_report_key() helper instead of open-coding the same
operation.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/input/keyboard/gpio_keys.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index fc706918d7b103cb..ab077dfb90a76ac3 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -456,7 +456,7 @@ static enum hrtimer_restart gpio_keys_irq_timer(struct hrtimer *t)
struct input_dev *input = bdata->input;
if (bdata->key_pressed) {
- input_event(input, EV_KEY, *bdata->code, 0);
+ input_report_key(input, *bdata->code, 0);
input_sync(input);
bdata->key_pressed = false;
}
@@ -478,11 +478,11 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
if (bdata->button->wakeup)
pm_wakeup_event(bdata->input->dev.parent, 0);
- input_event(input, EV_KEY, *bdata->code, 1);
+ input_report_key(input, *bdata->code, 1);
input_sync(input);
if (!bdata->release_delay) {
- input_event(input, EV_KEY, *bdata->code, 0);
+ input_report_key(input, *bdata->code, 0);
input_sync(input);
goto out;
}
--
2.25.1