All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	"russianneuromancer @ ya . ru" <russianneuromancer@ya.ru>,
	Gregor Riepl <onitake@gmail.com>,
	linux-input@vger.kernel.org
Subject: [PATCH 1/2] Input: gpio_keys - Do not report wake button presses as evdev events
Date: Mon,  9 Jan 2017 18:56:59 +0100	[thread overview]
Message-ID: <20170109175700.12811-1-hdegoede@redhat.com> (raw)

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 | 50 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 2909365..ffede98 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -32,6 +32,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;
@@ -42,10 +44,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 {
@@ -353,6 +359,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;
@@ -366,6 +393,10 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 	}
 
 	state = (state ? 1 : 0) ^ button->active_low;
+
+	if (state && gpio_keys_ignore_wakeup_button_press(bdata))
+		return;
+
 	if (type == EV_ABS) {
 		if (state)
 			input_event(input, type, button->code, button->value);
@@ -426,6 +457,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) {
@@ -819,13 +853,18 @@ static int 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);
@@ -841,14 +880,21 @@ static int 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);
-- 
2.9.3


             reply	other threads:[~2017-01-09 17:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-09 17:56 Hans de Goede [this message]
2017-01-09 17:57 ` [PATCH 2/2] Input: gpio-keys - Add support for setkeycode Hans de Goede
2017-01-21 19:49   ` Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170109175700.12811-1-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=onitake@gmail.com \
    --cc=russianneuromancer@ya.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.