[PATCH v3 1/4] Input: gpio_keys - store a pointer to driver_data in button_data

[PATCH v3 1/4] Input: gpio_keys - store a pointer to driver_data in button_data

[Hans de Goede]

Instead of a pointer to the input_dev, store a pointer the
gpio_keys_drvdata struct in struct gpio_button_data, so that per button
ISRs can access the entire driver-data struct and we don't need to
store a copy of global state (e.g. the suspended flag) for each button.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-This is a new patch in v3 of this patch-set
---
 drivers/input/keyboard/gpio_keys.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 87e613dc33b8..36ab7daba957 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -31,9 +31,11 @@
 #include <linux/of_irq.h>
 #include <linux/spinlock.h>
 
+struct gpio_keys_drvdata;
+
 struct gpio_button_data {
 	const struct gpio_keys_button *button;
-	struct input_dev *input;
+	struct gpio_keys_drvdata *ddata;
 	struct gpio_desc *gpiod;
 
 	unsigned short *code;
@@ -360,7 +362,7 @@ static const struct attribute_group gpio_keys_attr_group = {
 static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 {
 	const struct gpio_keys_button *button = bdata->button;
-	struct input_dev *input = bdata->input;
+	struct input_dev *input = bdata->ddata->input;
 	unsigned int type = button->type ?: EV_KEY;
 	int state;
 
@@ -388,19 +390,20 @@ static void gpio_keys_gpio_work_func(struct work_struct *work)
 	gpio_keys_gpio_report_event(bdata);
 
 	if (bdata->button->wakeup)
-		pm_relax(bdata->input->dev.parent);
+		pm_relax(bdata->ddata->input->dev.parent);
 }
 
 static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
 {
 	struct gpio_button_data *bdata = dev_id;
+	struct input_dev *input = bdata->ddata->input;
 
 	BUG_ON(irq != bdata->irq);
 
 	if (bdata->button->wakeup) {
 		const struct gpio_keys_button *button = bdata->button;
 
-		pm_stay_awake(bdata->input->dev.parent);
+		pm_stay_awake(input->dev.parent);
 		if (bdata->suspended  &&
 		    (button->type == 0 || button->type == EV_KEY)) {
 			/*
@@ -408,7 +411,7 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
 			 * already released by the time we got interrupt
 			 * handler to run.
 			 */
-			input_report_key(bdata->input, button->code, 1);
+			input_report_key(input, button->code, 1);
 		}
 	}
 
@@ -422,7 +425,7 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
 static void gpio_keys_irq_timer(struct timer_list *t)
 {
 	struct gpio_button_data *bdata = from_timer(bdata, t, release_timer);
-	struct input_dev *input = bdata->input;
+	struct input_dev *input = bdata->ddata->input;
 	unsigned long flags;
 
 	spin_lock_irqsave(&bdata->lock, flags);
@@ -437,7 +440,7 @@ static void gpio_keys_irq_timer(struct timer_list *t)
 static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
 {
 	struct gpio_button_data *bdata = dev_id;
-	struct input_dev *input = bdata->input;
+	struct input_dev *input = bdata->ddata->input;
 	unsigned long flags;
 
 	BUG_ON(irq != bdata->irq);
@@ -446,7 +449,7 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
 
 	if (!bdata->key_pressed) {
 		if (bdata->button->wakeup)
-			pm_wakeup_event(bdata->input->dev.parent, 0);
+			pm_wakeup_event(input->dev.parent, 0);
 
 		input_event(input, EV_KEY, *bdata->code, 1);
 		input_sync(input);
@@ -493,7 +496,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	int irq;
 	int error;
 
-	bdata->input = input;
+	bdata->ddata = ddata;
 	bdata->button = button;
 	spin_lock_init(&bdata->lock);
 
-- 
2.14.2

[PATCH v3 2/4] Input: gpio_keys - Use a single suspended flag per device

[Hans de Goede]

There is no need to have a suspended flag per button, this also
allows gpio_keys_resume to set suspended to false after calling
gpio_keys_report_state, which will allow testing the suspended flag
in gpio_keys_report_state in a follow-up patch.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-This is a new patch in v3 of this patch-set
---
 drivers/input/keyboard/gpio_keys.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 36ab7daba957..9b51aa56cc83 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -50,7 +50,6 @@ struct gpio_button_data {
 	spinlock_t lock;
 	bool disabled;
 	bool key_pressed;
-	bool suspended;
 };
 
 struct gpio_keys_drvdata {
@@ -58,6 +57,7 @@ struct gpio_keys_drvdata {
 	struct input_dev *input;
 	struct mutex disable_lock;
 	unsigned short *keymap;
+	bool suspended;
 	struct gpio_button_data data[0];
 };
 
@@ -404,7 +404,7 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
 		const struct gpio_keys_button *button = bdata->button;
 
 		pm_stay_awake(input->dev.parent);
-		if (bdata->suspended  &&
+		if (bdata->ddata->suspended  &&
 		    (button->type == 0 || button->type == EV_KEY)) {
 			/*
 			 * Simulate wakeup key press in case the key has
@@ -859,7 +859,6 @@ static int __maybe_unused gpio_keys_suspend(struct device *dev)
 			struct gpio_button_data *bdata = &ddata->data[i];
 			if (bdata->button->wakeup)
 				enable_irq_wake(bdata->irq);
-			bdata->suspended = true;
 		}
 	} else {
 		mutex_lock(&input->mutex);
@@ -868,6 +867,7 @@ static int __maybe_unused gpio_keys_suspend(struct device *dev)
 		mutex_unlock(&input->mutex);
 	}
 
+	ddata->suspended = true;
 	return 0;
 }
 
@@ -883,7 +883,6 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
 			struct gpio_button_data *bdata = &ddata->data[i];
 			if (bdata->button->wakeup)
 				disable_irq_wake(bdata->irq);
-			bdata->suspended = false;
 		}
 	} else {
 		mutex_lock(&input->mutex);
@@ -896,6 +895,7 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
 		return error;
 
 	gpio_keys_report_state(ddata);
+	ddata->suspended = false;
 	return 0;
 }
 
-- 
2.14.2

[PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Hans de Goede]

In some cases it is undesirable for a wakeup button to send input events
to userspace if pressed to wakeup the system (if pressed during suspend).

A typical example of this is the power-button on laptops / tablets,
sending a KEY_POWER event to userspace when woken up with the power-button
will cause userspace to immediately suspend the system again which is
undesirable.

For power-buttons attached to a PMIC, or handled by e.g. ACPI, not sending
an input event in this case is take care of by the PMIC / ACPI hardware /
code. But in the case of a GPIO button we need to explicitly suppress the
sending of the input event.

This commit supports this by adding a suppress_evdev_events_on_wakeup bool
to struct gpio_keys_button, which platform code can set to suppress the
input events for presses of wakeup keys during suspend.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-This is a rewrite if my "Input: gpio_keys - Do not report wake button
 presses as evdev events" patch.
-Instead of unconditionally ignoring presses of all wake-up buttons during
 suspend, this rewrite makes this configurable per button
-This version uses a timer to delay clearing the suspended flag for software
 debouncing, rather then jiffy compare magic

Changes in v3:
-Get rid of the need to have a timer all-together
-Rename the flag from no_wakeup_events to suppress_evdev_events_on_wakeup
---
 drivers/input/keyboard/gpio_keys.c | 13 +++++++++++++
 include/linux/gpio_keys.h          |  4 ++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 9b51aa56cc83..3beff91f1a4c 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -373,6 +373,10 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 		return;
 	}
 
+	if (bdata->button->suppress_evdev_events_on_wakeup &&
+	    bdata->ddata->suspended && state)
+		return;
+
 	if (type == EV_ABS) {
 		if (state)
 			input_event(input, type, button->code, button->value);
@@ -400,6 +404,10 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
 
 	BUG_ON(irq != bdata->irq);
 
+	if (bdata->button->suppress_evdev_events_on_wakeup &&
+	    bdata->ddata->suspended)
+		return IRQ_HANDLED;
+
 	if (bdata->button->wakeup) {
 		const struct gpio_keys_button *button = bdata->button;
 
@@ -445,6 +453,10 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
 
 	BUG_ON(irq != bdata->irq);
 
+	if (bdata->button->suppress_evdev_events_on_wakeup &&
+	    bdata->ddata->suspended)
+		return IRQ_HANDLED;
+
 	spin_lock_irqsave(&bdata->lock, flags);
 
 	if (!bdata->key_pressed) {
@@ -894,6 +906,7 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
 	if (error)
 		return error;
 
+	/* Sync state before clearing suspended, so it knows this is a resume */
 	gpio_keys_report_state(ddata);
 	ddata->suspended = false;
 	return 0;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 0b71024c082c..e05fe0902358 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -15,6 +15,9 @@ struct device;
  * @debounce_interval:	debounce ticks interval in msecs
  * @can_disable:	%true indicates that userspace is allowed to
  *			disable button via sysfs
+ * @suppress_evdev_events_on_wakeup:  For wake-up source buttons only, if %true
+ *			then no evdev-events will be generated if pressed while
+ *			suspended
  * @value:		axis value for %EV_ABS
  * @irq:		Irq number in case of interrupt keys
  */
@@ -27,6 +30,7 @@ struct gpio_keys_button {
 	int wakeup;
 	int debounce_interval;
 	bool can_disable;
+	bool suppress_evdev_events_on_wakeup;
 	int value;
 	unsigned int irq;
 };
-- 
2.14.2

[PATCH v3 4/4] Input: soc_button_array - Suppress power button presses during suspend

[Hans de Goede]

If the power-button is pressed to wakeup the laptop/tablet from suspend
and we report a KEY_POWER event to userspace when woken up this will cause
userspace to immediately suspend the system again which is undesirable.

This commit sets the new suppress_evdev_events_on_wakeup flag in the
gpio_keys_button struct for the power-button suppressing the undesirable
KEY_POWER input events on wake-up.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-Adjust for the renaming of the flag to suppress_evdev_events_on_wakeup
-Explicitly init the flag for all members of the soc_button_PNP0C40 array
---
 drivers/input/misc/soc_button_array.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 23520df7650f..39f06979961f 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -27,6 +27,7 @@ struct soc_button_info {
 	unsigned int event_code;
 	bool autorepeat;
 	bool wakeup;
+	bool suppress_evdev_events_on_wakeup;
 };
 
 /*
@@ -100,6 +101,8 @@ soc_button_device_create(struct platform_device *pdev,
 		gpio_keys[n_buttons].active_low = 1;
 		gpio_keys[n_buttons].desc = info->name;
 		gpio_keys[n_buttons].wakeup = info->wakeup;
+		gpio_keys[n_buttons].suppress_evdev_events_on_wakeup =
+					info->suppress_evdev_events_on_wakeup;
 		/* These devices often use cheap buttons, use 50 ms debounce */
 		gpio_keys[n_buttons].debounce_interval = 50;
 		n_buttons++;
@@ -185,6 +188,7 @@ static int soc_button_parse_btn_desc(struct device *dev,
 		info->name = "power";
 		info->event_code = KEY_POWER;
 		info->wakeup = true;
+		info->suppress_evdev_events_on_wakeup = true;
 	} else if (upage == 0x07 && usage == 0xe3) {
 		info->name = "home";
 		info->event_code = KEY_LEFTMETA;
@@ -369,11 +373,11 @@ static int soc_button_probe(struct platform_device *pdev)
  * Platforms"
  */
 static struct soc_button_info soc_button_PNP0C40[] = {
-	{ "power", 0, EV_KEY, KEY_POWER, false, true },
-	{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true },
-	{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
-	{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false },
-	{ "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false },
+	{ "power", 0, EV_KEY, KEY_POWER, false, true, true },
+	{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true, false },
+	{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false, false },
+	{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false, false },
+	{ "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false, false },
 	{ }
 };
 
-- 
2.14.2

Re: [PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Dmitry Torokhov]

On Mon, Oct 30, 2017 at 06:40:49PM +0100, Hans de Goede wrote:
> In some cases it is undesirable for a wakeup button to send input events
> to userspace if pressed to wakeup the system (if pressed during suspend).
> 
> A typical example of this is the power-button on laptops / tablets,
> sending a KEY_POWER event to userspace when woken up with the power-button
> will cause userspace to immediately suspend the system again which is
> undesirable.
> 
> For power-buttons attached to a PMIC, or handled by e.g. ACPI, not sending
> an input event in this case is take care of by the PMIC / ACPI hardware /
> code. But in the case of a GPIO button we need to explicitly suppress the
> sending of the input event.
> 
> This commit supports this by adding a suppress_evdev_events_on_wakeup bool
> to struct gpio_keys_button, which platform code can set to suppress the
> input events for presses of wakeup keys during suspend.

I think this is [your] userspace issue. What if I press the button
rapidly several times? I know Android actually _wants_ to see KEY_POWER
at resume, or its opportunistic suspend will kick in right away. I think
ChromeOS is OK with getting KEY_POWER on resume as well.

I'd say you need to have a small timeout before you start suspending
again.

Thanks.

-- 
Dmitry

Re: [PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Hans de Goede]

Hi,

On 30-10-17 19:17, Dmitry Torokhov wrote:
> On Mon, Oct 30, 2017 at 06:40:49PM +0100, Hans de Goede wrote:
>> In some cases it is undesirable for a wakeup button to send input events
>> to userspace if pressed to wakeup the system (if pressed during suspend).
>>
>> A typical example of this is the power-button on laptops / tablets,
>> sending a KEY_POWER event to userspace when woken up with the power-button
>> will cause userspace to immediately suspend the system again which is
>> undesirable.
>>
>> For power-buttons attached to a PMIC, or handled by e.g. ACPI, not sending
>> an input event in this case is take care of by the PMIC / ACPI hardware /
>> code. But in the case of a GPIO button we need to explicitly suppress the
>> sending of the input event.
>>
>> This commit supports this by adding a suppress_evdev_events_on_wakeup bool
>> to struct gpio_keys_button, which platform code can set to suppress the
>> input events for presses of wakeup keys during suspend.
> 
> I think this is [your] userspace issue.

It would have been nice if you started discussing this when I posted v1
quite some time ago.

> What if I press the button
> rapidly several times?

Why would anyone do that ?  But to answer the question it depends on
the timing, if you keep pressing it long enough for the resume to complete
then the first press after that will cause it to suspend again.

> I know Android actually _wants_ to see KEY_POWER
> at resume, or its opportunistic suspend will kick in right away.

Hmm, that is weird, because I believe that android x86 does work
on regular PCs and they do not do that.

> I think
> ChromeOS is OK with getting KEY_POWER on resume as well.
> 
> I'd say you need to have a small timeout before you start suspending
> again.

The problem with that is that it is going to be inherently racy.

More in general Bay Trail / Cherry Trail devices using gpio_key
for their power-button are the only "PC" devices sending a
KEY_POWER after wake-up, Bay / Cherry Trail devices which
have their power button hooked-up differently such as the Asus
Transformer series do not do this.

So both for consistency and because a timeout is racy I believe
that having the power button not send KEY_POWER after resume is
the proper solution here.

Note that I've made this configurable and that soc_button_array
is the only driver getting this enabled, making the power-button
behavior on PC like devices consistent, without impacting any
other devices.

Regards,

Hans

Re: [PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Dmitry Torokhov]

On Mon, Oct 30, 2017 at 09:08:21PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 30-10-17 19:17, Dmitry Torokhov wrote:
> > On Mon, Oct 30, 2017 at 06:40:49PM +0100, Hans de Goede wrote:
> > > In some cases it is undesirable for a wakeup button to send input events
> > > to userspace if pressed to wakeup the system (if pressed during suspend).
> > > 
> > > A typical example of this is the power-button on laptops / tablets,
> > > sending a KEY_POWER event to userspace when woken up with the power-button
> > > will cause userspace to immediately suspend the system again which is
> > > undesirable.
> > > 
> > > For power-buttons attached to a PMIC, or handled by e.g. ACPI, not sending
> > > an input event in this case is take care of by the PMIC / ACPI hardware /
> > > code. But in the case of a GPIO button we need to explicitly suppress the
> > > sending of the input event.
> > > 
> > > This commit supports this by adding a suppress_evdev_events_on_wakeup bool
> > > to struct gpio_keys_button, which platform code can set to suppress the
> > > input events for presses of wakeup keys during suspend.
> > 
> > I think this is [your] userspace issue.
> 
> It would have been nice if you started discussing this when I posted v1
> quite some time ago.

The original commits IIRC were about poor quality of buttons on these
x86 tablets and the apparent need for debounce on resume path. Or did I
mix them up with something else.

> 
> > What if I press the button
> > rapidly several times?
> 
> Why would anyone do that ?  But to answer the question it depends on

Because they can?

> the timing, if you keep pressing it long enough for the resume to complete
> then the first press after that will cause it to suspend again.
> 
> > I know Android actually _wants_ to see KEY_POWER
> > at resume, or its opportunistic suspend will kick in right away.
> 
> Hmm, that is weird, because I believe that android x86 does work
> on regular PCs and they do not do that.

The commit 0f107573da417c7f5c6d3a0160ebacc3adb019c4 was done
specifically so that Pixel C would resume if power button was pressed
and immediately released. I.e. before there was inconsistency with
driver behavior, depending on how long the button was pressed.

I am not sure about android x86 frankly.

> 
> > I think
> > ChromeOS is OK with getting KEY_POWER on resume as well.
> > 
> > I'd say you need to have a small timeout before you start suspending
> > again.
> 
> The problem with that is that it is going to be inherently racy.

Racy in what way?

> 
> More in general Bay Trail / Cherry Trail devices using gpio_key
> for their power-button are the only "PC" devices sending a
> KEY_POWER after wake-up, Bay / Cherry Trail devices which
> have their power button hooked-up differently such as the Asus
> Transformer series do not do this.
> 
> So both for consistency and because a timeout is racy I believe
> that having the power button not send KEY_POWER after resume is
> the proper solution here.
> 
> Note that I've made this configurable and that soc_button_array
> is the only driver getting this enabled, making the power-button
> behavior on PC like devices consistent, without impacting any
> other devices.

I'd rather we did not make assumption in the kernel about behavior of
userspace we happen to run on. The "PC like" devices change all the
time, and one could even say that failure to deliver events by ACPI
drivers on resume is a bug.

Please teach userspace how to handle events coming during resume phase
and what to do about them, and be done with it. One option is to say (in
whatever implements your power policy) "we want to ignore next KEY_POWER
event for the next NNN msec". You will need it ianyway if you decide to
put your userspace onto, let's say, ASUS Flip (an ARM device using
gpio-keys for power button and volume up/down buttons on the side of the
case).

Thanks.

-- 
Dmitry

Re: [PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Hans de Goede]

Hi,

On 30-10-17 21:48, Dmitry Torokhov wrote:

<snip>

>> So both for consistency and because a timeout is racy I believe
>> that having the power button not send KEY_POWER after resume is
>> the proper solution here.
>>
>> Note that I've made this configurable and that soc_button_array
>> is the only driver getting this enabled, making the power-button
>> behavior on PC like devices consistent, without impacting any
>> other devices.
> 
> I'd rather we did not make assumption in the kernel about behavior of
> userspace we happen to run on. The "PC like" devices change all the
> time, and one could even say that failure to deliver events by ACPI
> drivers on resume is a bug.
> 
> Please teach userspace how to handle events coming during resume phase
> and what to do about them, and be done with it. One option is to say (in
> whatever implements your power policy) "we want to ignore next KEY_POWER
> event for the next NNN msec". You will need it ianyway if you decide to
> put your userspace onto, let's say, ASUS Flip (an ARM device using
> gpio-keys for power button and volume up/down buttons on the side of the
> case).

Ok, I've written and submitted patches for this for gnome:
https://bugzilla.gnome.org/show_bug.cgi?id=789771

That still leaves the same issue for KDE, XFCE, mate, etc. though,
note I've no intention of fixing those.

Regards,

Hans

Re: [PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Benjamin Tissoires]

On Nov 01 2017 or thereabouts, Hans de Goede wrote:
> Hi,
> 
> On 30-10-17 21:48, Dmitry Torokhov wrote:
> 
> <snip>
> 
> > > So both for consistency and because a timeout is racy I believe
> > > that having the power button not send KEY_POWER after resume is
> > > the proper solution here.
> > > 
> > > Note that I've made this configurable and that soc_button_array
> > > is the only driver getting this enabled, making the power-button
> > > behavior on PC like devices consistent, without impacting any
> > > other devices.
> > 
> > I'd rather we did not make assumption in the kernel about behavior of
> > userspace we happen to run on. The "PC like" devices change all the
> > time, and one could even say that failure to deliver events by ACPI
> > drivers on resume is a bug.
> > 
> > Please teach userspace how to handle events coming during resume phase
> > and what to do about them, and be done with it. One option is to say (in
> > whatever implements your power policy) "we want to ignore next KEY_POWER
> > event for the next NNN msec". You will need it ianyway if you decide to
> > put your userspace onto, let's say, ASUS Flip (an ARM device using
> > gpio-keys for power button and volume up/down buttons on the side of the
> > case).
> 
> Ok, I've written and submitted patches for this for gnome:
> https://bugzilla.gnome.org/show_bug.cgi?id=789771
> 
> That still leaves the same issue for KDE, XFCE, mate, etc. though,
> note I've no intention of fixing those.
> 

Hi Hans,

I roughly had a look at the series, and I am wondering, can't we at
least take the cleanup patches (1/4 2/4 as far as i can tell)?

Cheers,
Benjamin

> Regards,
> 
> Hans

Re: [PATCH v3 3/4] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Hans de Goede]

Hi,

On 06-11-17 10:19, Benjamin Tissoires wrote:
> On Nov 01 2017 or thereabouts, Hans de Goede wrote:
>> Hi,
>>
>> On 30-10-17 21:48, Dmitry Torokhov wrote:
>>
>> <snip>
>>
>>>> So both for consistency and because a timeout is racy I believe
>>>> that having the power button not send KEY_POWER after resume is
>>>> the proper solution here.
>>>>
>>>> Note that I've made this configurable and that soc_button_array
>>>> is the only driver getting this enabled, making the power-button
>>>> behavior on PC like devices consistent, without impacting any
>>>> other devices.
>>>
>>> I'd rather we did not make assumption in the kernel about behavior of
>>> userspace we happen to run on. The "PC like" devices change all the
>>> time, and one could even say that failure to deliver events by ACPI
>>> drivers on resume is a bug.
>>>
>>> Please teach userspace how to handle events coming during resume phase
>>> and what to do about them, and be done with it. One option is to say (in
>>> whatever implements your power policy) "we want to ignore next KEY_POWER
>>> event for the next NNN msec". You will need it ianyway if you decide to
>>> put your userspace onto, let's say, ASUS Flip (an ARM device using
>>> gpio-keys for power button and volume up/down buttons on the side of the
>>> case).
>>
>> Ok, I've written and submitted patches for this for gnome:
>> https://bugzilla.gnome.org/show_bug.cgi?id=789771
>>
>> That still leaves the same issue for KDE, XFCE, mate, etc. though,
>> note I've no intention of fixing those.
>>
> 
> Hi Hans,
> 
> I roughly had a look at the series, and I am wondering, can't we at
> least take the cleanup patches (1/4 2/4 as far as i can tell)?

You could take those, but I'm not sure they are worth the churn,
without the need for the interrupt handlers to be able to access
(more) device-level (rather then button-level) data.

Regards,

Hans