diff mbox

[resend,4/4] Input: gpio_keys - Do not report wake button presses as evdev events

Message ID 20170308085413.5185-5-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede March 8, 2017, 8:54 a.m. UTC
If a button is a wake button, it may still be bouncing from the press
to wakeup the device by the time the gpio interrupts get enabled again
and / or the gpio_keys_report_state call from gpio_keys_resume may
find the button still pressed and report this as a new press.

This is undesirable, esp. since the powerbutton on tablets is typically
a wakeup source and uses the gpio_keys driver on some tablets, leading
to userspace immediately re-suspending the tablet after the powerbutton
is pressed, due to it seeing a powerbutton press.

This commit ignores wakeup button presses for the first 1 second after
resume (and while resumed, as the workqueue may run before the resume
function runs), avoiding this problem.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note: maybe we should make WAKE_DEBOUNCE part of gpio_keys_button and
only do this when drivers / platform-data set this to a non-zero value ?
---
 drivers/input/keyboard/gpio_keys.c | 49 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)
diff mbox

Patch

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index da3d362..e1488b5 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -31,6 +31,8 @@ 
 #include <linux/of_irq.h>
 #include <linux/spinlock.h>
 
+#define WAKE_DEBOUNCE msecs_to_jiffies(1000)
+
 struct gpio_button_data {
 	const struct gpio_keys_button *button;
 	struct input_dev *input;
@@ -44,10 +46,14 @@  struct gpio_button_data {
 	struct delayed_work work;
 	unsigned int software_debounce;	/* in msecs, for GPIO-driven buttons */
 
+	unsigned long resume_time;	/* in jiffies, for wakeup buttons */
+
 	unsigned int irq;
 	spinlock_t lock;
 	bool disabled;
 	bool key_pressed;
+	bool suspended;
+	bool resume_time_valid;
 };
 
 struct gpio_keys_drvdata {
@@ -356,6 +362,27 @@  static struct attribute_group gpio_keys_attr_group = {
 	.attrs = gpio_keys_attrs,
 };
 
+static bool gpio_keys_ignore_wakeup_button_press(struct gpio_button_data *bdata)
+{
+	unsigned long flags;
+	bool ret = false;
+
+	if (!bdata->button->wakeup)
+		return ret;
+
+	spin_lock_irqsave(&bdata->lock, flags);
+
+	if (bdata->suspended)
+		ret = true; /* Our resume method did not run yet */
+	else if (bdata->resume_time_valid &&
+		 time_before(jiffies, bdata->resume_time + WAKE_DEBOUNCE))
+		ret = true; /* Assume this is a wakeup press and ignore */
+
+	spin_unlock_irqrestore(&bdata->lock, flags);
+
+	return ret;
+}
+
 static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 {
 	const struct gpio_keys_button *button = bdata->button;
@@ -370,6 +397,9 @@  static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 		return;
 	}
 
+	if (state && gpio_keys_ignore_wakeup_button_press(bdata))
+		return;
+
 	if (type == EV_ABS) {
 		if (state)
 			input_event(input, type, button->code, button->value);
@@ -429,6 +459,9 @@  static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
 
 	BUG_ON(irq != bdata->irq);
 
+	if (gpio_keys_ignore_wakeup_button_press(bdata))
+		return IRQ_HANDLED;
+
 	spin_lock_irqsave(&bdata->lock, flags);
 
 	if (!bdata->key_pressed) {
@@ -848,13 +881,18 @@  static int __maybe_unused gpio_keys_suspend(struct device *dev)
 {
 	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
 	struct input_dev *input = ddata->input;
+	unsigned long flags;
 	int i;
 
 	if (device_may_wakeup(dev)) {
 		for (i = 0; i < ddata->pdata->nbuttons; i++) {
 			struct gpio_button_data *bdata = &ddata->data[i];
-			if (bdata->button->wakeup)
+			if (bdata->button->wakeup) {
+				spin_lock_irqsave(&bdata->lock, flags);
+				bdata->suspended = true;
+				spin_unlock_irqrestore(&bdata->lock, flags);
 				enable_irq_wake(bdata->irq);
+			}
 		}
 	} else {
 		mutex_lock(&input->mutex);
@@ -870,14 +908,21 @@  static int __maybe_unused gpio_keys_resume(struct device *dev)
 {
 	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
 	struct input_dev *input = ddata->input;
+	unsigned long flags;
 	int error = 0;
 	int i;
 
 	if (device_may_wakeup(dev)) {
 		for (i = 0; i < ddata->pdata->nbuttons; i++) {
 			struct gpio_button_data *bdata = &ddata->data[i];
-			if (bdata->button->wakeup)
+			if (bdata->button->wakeup) {
 				disable_irq_wake(bdata->irq);
+				spin_lock_irqsave(&bdata->lock, flags);
+				bdata->resume_time = jiffies;
+				bdata->resume_time_valid = true;
+				bdata->suspended = false;
+				spin_unlock_irqrestore(&bdata->lock, flags);
+			}
 		}
 	} else {
 		mutex_lock(&input->mutex);