[PATCH resend v2 1/2] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[PATCH resend v2 1/2] 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 adds support for this by adding a no_wakeup_events 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
---
 drivers/input/keyboard/gpio_keys.c | 33 +++++++++++++++++++++++++++++++--
 include/linux/gpio_keys.h          |  3 +++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index e9f0ebf3267a..611b47261df4 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -38,6 +38,7 @@ struct gpio_button_data {
 
 	unsigned short *code;
 
+	struct timer_list unsuspend_timer;
 	struct timer_list release_timer;
 	unsigned int release_delay;	/* in msecs, for IRQ-only buttons */
 
@@ -371,6 +372,9 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 		return;
 	}
 
+	if (state && bdata->button->no_wakeup_events && bdata->suspended)
+		return;
+
 	if (type == EV_ABS) {
 		if (state)
 			input_event(input, type, button->code, button->value);
@@ -400,6 +404,9 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
 	if (bdata->button->wakeup) {
 		const struct gpio_keys_button *button = bdata->button;
 
+		if (bdata->button->no_wakeup_events && bdata->suspended)
+			return IRQ_HANDLED;
+
 		pm_stay_awake(bdata->input->dev.parent);
 		if (bdata->suspended  &&
 		    (button->type == 0 || button->type == EV_KEY)) {
@@ -445,9 +452,13 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
 	spin_lock_irqsave(&bdata->lock, flags);
 
 	if (!bdata->key_pressed) {
-		if (bdata->button->wakeup)
+		if (bdata->button->wakeup) {
 			pm_wakeup_event(bdata->input->dev.parent, 0);
 
+			if (bdata->button->no_wakeup_events && bdata->suspended)
+				goto out;
+		}
+
 		input_event(input, EV_KEY, *bdata->code, 1);
 		input_sync(input);
 
@@ -468,6 +479,13 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void gpio_keys_unsuspend_timer(unsigned long _data)
+{
+	struct gpio_button_data *bdata = (struct gpio_button_data *)_data;
+
+	bdata->suspended = false;
+}
+
 static void gpio_keys_quiesce_key(void *data)
 {
 	struct gpio_button_data *bdata = data;
@@ -476,6 +494,8 @@ static void gpio_keys_quiesce_key(void *data)
 		cancel_delayed_work_sync(&bdata->work);
 	else
 		del_timer_sync(&bdata->release_timer);
+
+	del_timer_sync(&bdata->unsuspend_timer);
 }
 
 static int gpio_keys_setup_key(struct platform_device *pdev,
@@ -496,6 +516,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	bdata->input = input;
 	bdata->button = button;
 	spin_lock_init(&bdata->lock);
+	setup_timer(&bdata->unsuspend_timer, gpio_keys_unsuspend_timer,
+		    (unsigned long)bdata);
 
 	if (child) {
 		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
@@ -857,6 +879,7 @@ 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);
+			del_timer_sync(&bdata->unsuspend_timer);
 			bdata->suspended = true;
 		}
 	} else {
@@ -881,7 +904,13 @@ 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;
+			if (bdata->button->no_wakeup_events) {
+				mod_timer(&bdata->unsuspend_timer, jiffies +
+					  msecs_to_jiffies(
+						    bdata->software_debounce));
+			} else {
+				bdata->suspended = false;
+			}
 		}
 	} else {
 		mutex_lock(&input->mutex);
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 0b71024c082c..d8a85e52b6bb 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -15,6 +15,8 @@ struct device;
  * @debounce_interval:	debounce ticks interval in msecs
  * @can_disable:	%true indicates that userspace is allowed to
  *			disable button via sysfs
+ * @no_wakeup_events:	For wake-up source buttons only, if %true then no input
+ *			events will be generated if pressed while suspended
  * @value:		axis value for %EV_ABS
  * @irq:		Irq number in case of interrupt keys
  */
@@ -27,6 +29,7 @@ struct gpio_keys_button {
 	int wakeup;
 	int debounce_interval;
 	bool can_disable;
+	bool no_wakeup_events;
 	int value;
 	unsigned int irq;
 };
-- 
2.14.2

[PATCH resend v2 2/2] 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 no_wakeup_events 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 v2:
-New patch in v2 of this patch-set
---
 drivers/input/misc/soc_button_array.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 23520df7650f..0f7cce70f748 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 no_wakeup_events;
 };
 
 /*
@@ -100,6 +101,7 @@ 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].no_wakeup_events = info->no_wakeup_events;
 		/* These devices often use cheap buttons, use 50 ms debounce */
 		gpio_keys[n_buttons].debounce_interval = 50;
 		n_buttons++;
@@ -185,6 +187,7 @@ static int soc_button_parse_btn_desc(struct device *dev,
 		info->name = "power";
 		info->event_code = KEY_POWER;
 		info->wakeup = true;
+		info->no_wakeup_events = true;
 	} else if (upage == 0x07 && usage == 0xe3) {
 		info->name = "home";
 		info->event_code = KEY_LEFTMETA;
@@ -369,7 +372,7 @@ 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 },
+	{ "power", 0, EV_KEY, KEY_POWER, false, true, 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 },
-- 
2.14.2

Re: [PATCH resend v2 1/2] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Benjamin Tissoires]

Hi Hans,

On Oct 11 2017 or thereabouts, 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 adds support for this by adding a no_wakeup_events 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

Is there any particular reason to have a per button timer instead of a
global one?

> ---
>  drivers/input/keyboard/gpio_keys.c | 33 +++++++++++++++++++++++++++++++--
>  include/linux/gpio_keys.h          |  3 +++
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index e9f0ebf3267a..611b47261df4 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -38,6 +38,7 @@ struct gpio_button_data {
>  
>  	unsigned short *code;
>  
> +	struct timer_list unsuspend_timer;
>  	struct timer_list release_timer;
>  	unsigned int release_delay;	/* in msecs, for IRQ-only buttons */
>  
> @@ -371,6 +372,9 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
>  		return;
>  	}
>  
> +	if (state && bdata->button->no_wakeup_events && bdata->suspended)
> +		return;
> +
>  	if (type == EV_ABS) {
>  		if (state)
>  			input_event(input, type, button->code, button->value);
> @@ -400,6 +404,9 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
>  	if (bdata->button->wakeup) {
>  		const struct gpio_keys_button *button = bdata->button;
>  
> +		if (bdata->button->no_wakeup_events && bdata->suspended)
> +			return IRQ_HANDLED;
> +
>  		pm_stay_awake(bdata->input->dev.parent);
>  		if (bdata->suspended  &&
>  		    (button->type == 0 || button->type == EV_KEY)) {
> @@ -445,9 +452,13 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
>  	spin_lock_irqsave(&bdata->lock, flags);
>  
>  	if (!bdata->key_pressed) {
> -		if (bdata->button->wakeup)
> +		if (bdata->button->wakeup) {
>  			pm_wakeup_event(bdata->input->dev.parent, 0);
>  
> +			if (bdata->button->no_wakeup_events && bdata->suspended)
> +				goto out;
> +		}
> +
>  		input_event(input, EV_KEY, *bdata->code, 1);
>  		input_sync(input);
>  
> @@ -468,6 +479,13 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static void gpio_keys_unsuspend_timer(unsigned long _data)
> +{
> +	struct gpio_button_data *bdata = (struct gpio_button_data *)_data;
> +
> +	bdata->suspended = false;
> +}
> +
>  static void gpio_keys_quiesce_key(void *data)
>  {
>  	struct gpio_button_data *bdata = data;
> @@ -476,6 +494,8 @@ static void gpio_keys_quiesce_key(void *data)
>  		cancel_delayed_work_sync(&bdata->work);
>  	else
>  		del_timer_sync(&bdata->release_timer);
> +
> +	del_timer_sync(&bdata->unsuspend_timer);
>  }
>  
>  static int gpio_keys_setup_key(struct platform_device *pdev,
> @@ -496,6 +516,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  	bdata->input = input;
>  	bdata->button = button;
>  	spin_lock_init(&bdata->lock);
> +	setup_timer(&bdata->unsuspend_timer, gpio_keys_unsuspend_timer,
> +		    (unsigned long)bdata);

You'd better use the new shiny API timer_setup instead (see
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/commit/?h=for-4.15/use-timer-setup&id=0ee32774aed648854a06bc3fae636f20f5f75a68)
for an example.

>  
>  	if (child) {
>  		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
> @@ -857,6 +879,7 @@ 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);
> +			del_timer_sync(&bdata->unsuspend_timer);
>  			bdata->suspended = true;
>  		}
>  	} else {
> @@ -881,7 +904,13 @@ 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;
> +			if (bdata->button->no_wakeup_events) {
> +				mod_timer(&bdata->unsuspend_timer, jiffies +
> +					  msecs_to_jiffies(
> +						    bdata->software_debounce));
> +			} else {
> +				bdata->suspended = false;

I am not sure why this field is also duplicated in all the buttons,
while we could have only one per device.

Cheers,
Benjamin

> +			}
>  		}
>  	} else {
>  		mutex_lock(&input->mutex);
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index 0b71024c082c..d8a85e52b6bb 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -15,6 +15,8 @@ struct device;
>   * @debounce_interval:	debounce ticks interval in msecs
>   * @can_disable:	%true indicates that userspace is allowed to
>   *			disable button via sysfs
> + * @no_wakeup_events:	For wake-up source buttons only, if %true then no input
> + *			events will be generated if pressed while suspended
>   * @value:		axis value for %EV_ABS
>   * @irq:		Irq number in case of interrupt keys
>   */
> @@ -27,6 +29,7 @@ struct gpio_keys_button {
>  	int wakeup;
>  	int debounce_interval;
>  	bool can_disable;
> +	bool no_wakeup_events;
>  	int value;
>  	unsigned int irq;
>  };
> -- 
> 2.14.2
> 

Re: [PATCH resend v2 2/2] Input: soc_button_array - Suppress power button presses during suspend

[Benjamin Tissoires]

On Oct 11 2017 or thereabouts, Hans de Goede wrote:
> 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 no_wakeup_events 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 v2:
> -New patch in v2 of this patch-set
> ---
>  drivers/input/misc/soc_button_array.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 23520df7650f..0f7cce70f748 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 no_wakeup_events;
>  };
>  
>  /*
> @@ -100,6 +101,7 @@ 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].no_wakeup_events = info->no_wakeup_events;

This is an API problem in gpio_keys.c, but reading it here makes me
think "this button will never send wakeup events", so it will not wake
up the system. Can't we find a more explicit name?

>  		/* These devices often use cheap buttons, use 50 ms debounce */
>  		gpio_keys[n_buttons].debounce_interval = 50;
>  		n_buttons++;
> @@ -185,6 +187,7 @@ static int soc_button_parse_btn_desc(struct device *dev,
>  		info->name = "power";
>  		info->event_code = KEY_POWER;
>  		info->wakeup = true;
> +		info->no_wakeup_events = true;
>  	} else if (upage == 0x07 && usage == 0xe3) {
>  		info->name = "home";
>  		info->event_code = KEY_LEFTMETA;
> @@ -369,7 +372,7 @@ 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 },
> +	{ "power", 0, EV_KEY, KEY_POWER, false, true, true },
>  	{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true },

I think we might as well explicitly set the values to false for the
others, or use an explicit assignment in the whole list.

Cheers,
Benjamin

>  	{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
>  	{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false },
> -- 
> 2.14.2
> 

Re: [PATCH resend v2 1/2] Input: gpio_keys - Allow suppression of input events for wakeup button presses

[Hans de Goede]

Hi,

On 16-10-17 09:36, Benjamin Tissoires wrote:
> Hi Hans,
> 
> On Oct 11 2017 or thereabouts, 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 adds support for this by adding a no_wakeup_events 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
> 
> Is there any particular reason to have a per button timer instead of a
> global one?

Not really I was just following the example of the existing
suspended flag, for v3 I've added 2 patches to the series to make the
suspended flag be in the per-device state rather then per-button state.

This actually also has allowed me to completely remove the need for a
timer, which is much better really.

Regards,

Hans


> 
>> ---
>>   drivers/input/keyboard/gpio_keys.c | 33 +++++++++++++++++++++++++++++++--
>>   include/linux/gpio_keys.h          |  3 +++
>>   2 files changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
>> index e9f0ebf3267a..611b47261df4 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -38,6 +38,7 @@ struct gpio_button_data {
>>   
>>   	unsigned short *code;
>>   
>> +	struct timer_list unsuspend_timer;
>>   	struct timer_list release_timer;
>>   	unsigned int release_delay;	/* in msecs, for IRQ-only buttons */
>>   
>> @@ -371,6 +372,9 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
>>   		return;
>>   	}
>>   
>> +	if (state && bdata->button->no_wakeup_events && bdata->suspended)
>> +		return;
>> +
>>   	if (type == EV_ABS) {
>>   		if (state)
>>   			input_event(input, type, button->code, button->value);
>> @@ -400,6 +404,9 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
>>   	if (bdata->button->wakeup) {
>>   		const struct gpio_keys_button *button = bdata->button;
>>   
>> +		if (bdata->button->no_wakeup_events && bdata->suspended)
>> +			return IRQ_HANDLED;
>> +
>>   		pm_stay_awake(bdata->input->dev.parent);
>>   		if (bdata->suspended  &&
>>   		    (button->type == 0 || button->type == EV_KEY)) {
>> @@ -445,9 +452,13 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
>>   	spin_lock_irqsave(&bdata->lock, flags);
>>   
>>   	if (!bdata->key_pressed) {
>> -		if (bdata->button->wakeup)
>> +		if (bdata->button->wakeup) {
>>   			pm_wakeup_event(bdata->input->dev.parent, 0);
>>   
>> +			if (bdata->button->no_wakeup_events && bdata->suspended)
>> +				goto out;
>> +		}
>> +
>>   		input_event(input, EV_KEY, *bdata->code, 1);
>>   		input_sync(input);
>>   
>> @@ -468,6 +479,13 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> +static void gpio_keys_unsuspend_timer(unsigned long _data)
>> +{
>> +	struct gpio_button_data *bdata = (struct gpio_button_data *)_data;
>> +
>> +	bdata->suspended = false;
>> +}
>> +
>>   static void gpio_keys_quiesce_key(void *data)
>>   {
>>   	struct gpio_button_data *bdata = data;
>> @@ -476,6 +494,8 @@ static void gpio_keys_quiesce_key(void *data)
>>   		cancel_delayed_work_sync(&bdata->work);
>>   	else
>>   		del_timer_sync(&bdata->release_timer);
>> +
>> +	del_timer_sync(&bdata->unsuspend_timer);
>>   }
>>   
>>   static int gpio_keys_setup_key(struct platform_device *pdev,
>> @@ -496,6 +516,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>>   	bdata->input = input;
>>   	bdata->button = button;
>>   	spin_lock_init(&bdata->lock);
>> +	setup_timer(&bdata->unsuspend_timer, gpio_keys_unsuspend_timer,
>> +		    (unsigned long)bdata);
> 
> You'd better use the new shiny API timer_setup instead (see
> https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/commit/?h=for-4.15/use-timer-setup&id=0ee32774aed648854a06bc3fae636f20f5f75a68)
> for an example.
> 
>>   
>>   	if (child) {
>>   		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
>> @@ -857,6 +879,7 @@ 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);
>> +			del_timer_sync(&bdata->unsuspend_timer);
>>   			bdata->suspended = true;
>>   		}
>>   	} else {
>> @@ -881,7 +904,13 @@ 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;
>> +			if (bdata->button->no_wakeup_events) {
>> +				mod_timer(&bdata->unsuspend_timer, jiffies +
>> +					  msecs_to_jiffies(
>> +						    bdata->software_debounce));
>> +			} else {
>> +				bdata->suspended = false;
> 
> I am not sure why this field is also duplicated in all the buttons,
> while we could have only one per device.
> 
> Cheers,
> Benjamin
> 
>> +			}
>>   		}
>>   	} else {
>>   		mutex_lock(&input->mutex);
>> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
>> index 0b71024c082c..d8a85e52b6bb 100644
>> --- a/include/linux/gpio_keys.h
>> +++ b/include/linux/gpio_keys.h
>> @@ -15,6 +15,8 @@ struct device;
>>    * @debounce_interval:	debounce ticks interval in msecs
>>    * @can_disable:	%true indicates that userspace is allowed to
>>    *			disable button via sysfs
>> + * @no_wakeup_events:	For wake-up source buttons only, if %true then no input
>> + *			events will be generated if pressed while suspended
>>    * @value:		axis value for %EV_ABS
>>    * @irq:		Irq number in case of interrupt keys
>>    */
>> @@ -27,6 +29,7 @@ struct gpio_keys_button {
>>   	int wakeup;
>>   	int debounce_interval;
>>   	bool can_disable;
>> +	bool no_wakeup_events;
>>   	int value;
>>   	unsigned int irq;
>>   };
>> -- 
>> 2.14.2
>>