Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[JeffyChen]

Hi Brain,

Thanks for your reply.

On 03/02/2018 10:32 AM, Brian Norris wrote:

> >What about the 'else' case? Shouldn't we try to handle that?

>i think the else case is for irq key, which would generate down and up
>events in one irq, so it would use the same trigger type for all these 3
>cases.

Not necessarily. It uses whatever trigger was provided in
platform/DT/etc. data. You could retrieve that with
irq_get_trigger_type() and try to interpret that. Or you could just
outlaw using that combination (e.g., in the binding documentation).
i think for the IRQ button case the assert/deassert/any are using the 
same irq trigger type, so it should be ok to leave the wakeup trigger 
type to be 0(not reconfigure the trigger type)...


i've made a v3 to add a comment about that, but forgot to send it :(



Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[JeffyChen]

Hi Brain,

Thanks for your reply.

On 03/02/2018 10:32 AM, Brian Norris wrote:

> >What about the 'else' case? Shouldn't we try to handle that?

>i think the else case is for irq key, which would generate down and up
>events in one irq, so it would use the same trigger type for all these 3
>cases.

Not necessarily. It uses whatever trigger was provided in
platform/DT/etc. data. You could retrieve that with
irq_get_trigger_type() and try to interpret that. Or you could just
outlaw using that combination (e.g., in the binding documentation).
i think for the IRQ button case the assert/deassert/any are using the 
same irq trigger type, so it should be ok to leave the wakeup trigger 
type to be 0(not reconfigure the trigger type)...


i've made a v3 to add a comment about that, but forgot to send it :(



Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Brian Norris]

Hi,

On Fri, Feb 23, 2018 at 06:04:22PM +0800, Jeffy Chen wrote:
> On 02/13/2018 06:13 AM, Brian Norris wrote:
> > > >
> > > > if (bdata->gpiod) {
> > > >+int active_low = gpiod_is_active_low(bdata->gpiod);
> > > >+
> > > > if (button->debounce_interval) {
> > > > error = gpiod_set_debounce(bdata->gpiod,
> > > > button->debounce_interval * 
> > > > 1000);
> > > >@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct 
> > > >platform_device *pdev,
> > > > isr = gpio_keys_gpio_isr;
> > > > irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
> > > >
> > > >+switch (button->wakeup_event_action) {
> > > >+case EV_ACT_ASSERTED:
> > > >+bdata->wakeup_trigger_type = active_low ?
> > > >+IRQF_TRIGGER_FALLING : 
> > > >IRQF_TRIGGER_RISING;
> > > >+break;
> > > >+case EV_ACT_DEASSERTED:
> > > >+bdata->wakeup_trigger_type = active_low ?
> > > >+IRQF_TRIGGER_RISING : 
> > > >IRQF_TRIGGER_FALLING;
> > > >+break;
> > What about EV_ACT_ANY? And default case? I think for ANY, we're OK
> > letting suspend/resume not reconfigure the trigger type, but maybe a
> > comment here to note that?
> right, will add comment in the next version.
> > 
> > > >+}
> > > > } else {
> > What about the 'else' case? Shouldn't we try to handle that?
> i think the else case is for irq key, which would generate down and up
> events in one irq, so it would use the same trigger type for all these 3
> cases.

Not necessarily. It uses whatever trigger was provided in
platform/DT/etc. data. You could retrieve that with
irq_get_trigger_type() and try to interpret that. Or you could just
outlaw using that combination (e.g., in the binding documentation).

Brian

> i'll add comment in the next version too.
> > 
> > Brian
> > 
> 
> 

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Brian Norris]

Hi,

On Fri, Feb 23, 2018 at 06:04:22PM +0800, Jeffy Chen wrote:
> On 02/13/2018 06:13 AM, Brian Norris wrote:
> > > >
> > > > if (bdata->gpiod) {
> > > >+int active_low = gpiod_is_active_low(bdata->gpiod);
> > > >+
> > > > if (button->debounce_interval) {
> > > > error = gpiod_set_debounce(bdata->gpiod,
> > > > button->debounce_interval * 
> > > > 1000);
> > > >@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct 
> > > >platform_device *pdev,
> > > > isr = gpio_keys_gpio_isr;
> > > > irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
> > > >
> > > >+switch (button->wakeup_event_action) {
> > > >+case EV_ACT_ASSERTED:
> > > >+bdata->wakeup_trigger_type = active_low ?
> > > >+IRQF_TRIGGER_FALLING : 
> > > >IRQF_TRIGGER_RISING;
> > > >+break;
> > > >+case EV_ACT_DEASSERTED:
> > > >+bdata->wakeup_trigger_type = active_low ?
> > > >+IRQF_TRIGGER_RISING : 
> > > >IRQF_TRIGGER_FALLING;
> > > >+break;
> > What about EV_ACT_ANY? And default case? I think for ANY, we're OK
> > letting suspend/resume not reconfigure the trigger type, but maybe a
> > comment here to note that?
> right, will add comment in the next version.
> > 
> > > >+}
> > > > } else {
> > What about the 'else' case? Shouldn't we try to handle that?
> i think the else case is for irq key, which would generate down and up
> events in one irq, so it would use the same trigger type for all these 3
> cases.

Not necessarily. It uses whatever trigger was provided in
platform/DT/etc. data. You could retrieve that with
irq_get_trigger_type() and try to interpret that. Or you could just
outlaw using that combination (e.g., in the binding documentation).

Brian

> i'll add comment in the next version too.
> > 
> > Brian
> > 
> 
> 

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[JeffyChen]

Hi Enric,

Thanks for your reply.

On 02/14/2018 06:25 AM, Enric Balletbo Serra wrote:

Hi,

2018-02-13 19:25 GMT+01:00 Brian Norris :

Hi Enric,

On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:

On 12/02/18 23:13, Brian Norris wrote:

On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:

Add support for specifying event actions to trigger wakeup when using
the gpio-keys input device as a wakeup source.

This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.

Suggested-by: Brian Norris 
Signed-off-by: Jeffy Chen 
---

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

[...]

Not sure if you were aware but there is also some discussion related to this,
maybe we can join the efforts?

v1: https://patchwork.kernel.org/patch/10208261/
v2: https://patchwork.kernel.org/patch/10211147/


Thanks for the pointers. IIUC, that's talking about a different problem:
how to utilize a GPIO key in level-triggered mode. That touches similar
code, but it doesn't really have anything to do with configuring a
different wakeup trigger type.



Right, sorry. I see now what you are doing.


The two patches would need to be reconciled, if they both are going to
be merged. But otherwise, I think they're perfectly fine to be separate.



Yes, that's why I got confused, I had those patches applied on my tree
and when I tried to apply these failed and I wrongly assumed that were
doing the same. Waiting to test the third version ;)
right, they are not related, and i should add the level irq case after 
that series merged :)




Thanks,
  Enric


Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[JeffyChen]

Hi Enric,

Thanks for your reply.

On 02/14/2018 06:25 AM, Enric Balletbo Serra wrote:

Hi,

2018-02-13 19:25 GMT+01:00 Brian Norris :

Hi Enric,

On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:

On 12/02/18 23:13, Brian Norris wrote:

On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:

Add support for specifying event actions to trigger wakeup when using
the gpio-keys input device as a wakeup source.

This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.

Suggested-by: Brian Norris 
Signed-off-by: Jeffy Chen 
---

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

[...]

Not sure if you were aware but there is also some discussion related to this,
maybe we can join the efforts?

v1: https://patchwork.kernel.org/patch/10208261/
v2: https://patchwork.kernel.org/patch/10211147/


Thanks for the pointers. IIUC, that's talking about a different problem:
how to utilize a GPIO key in level-triggered mode. That touches similar
code, but it doesn't really have anything to do with configuring a
different wakeup trigger type.



Right, sorry. I see now what you are doing.


The two patches would need to be reconciled, if they both are going to
be merged. But otherwise, I think they're perfectly fine to be separate.



Yes, that's why I got confused, I had those patches applied on my tree
and when I tried to apply these failed and I wrongly assumed that were
doing the same. Waiting to test the third version ;)
right, they are not related, and i should add the level irq case after 
that series merged :)




Thanks,
  Enric


Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[JeffyChen]

Hi Brian,

Thanks for your reply.

On 02/13/2018 06:13 AM, Brian Norris wrote:

>
>if (bdata->gpiod) {
>+   int active_low = gpiod_is_active_low(bdata->gpiod);
>+
>if (button->debounce_interval) {
>error = gpiod_set_debounce(bdata->gpiod,
>button->debounce_interval * 1000);
>@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
>isr = gpio_keys_gpio_isr;
>irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>
>+   switch (button->wakeup_event_action) {
>+   case EV_ACT_ASSERTED:
>+   bdata->wakeup_trigger_type = active_low ?
>+   IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
>+   break;
>+   case EV_ACT_DEASSERTED:
>+   bdata->wakeup_trigger_type = active_low ?
>+   IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>+   break;

What about EV_ACT_ANY? And default case? I think for ANY, we're OK
letting suspend/resume not reconfigure the trigger type, but maybe a
comment here to note that?

right, will add comment in the next version.



>+   }
>} else {

What about the 'else' case? Shouldn't we try to handle that?
i think the else case is for irq key, which would generate down and up 
events in one irq, so it would use the same trigger type for all these 3 
cases.


i'll add comment in the next version too.


Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[JeffyChen]

Hi Brian,

Thanks for your reply.

On 02/13/2018 06:13 AM, Brian Norris wrote:

>
>if (bdata->gpiod) {
>+   int active_low = gpiod_is_active_low(bdata->gpiod);
>+
>if (button->debounce_interval) {
>error = gpiod_set_debounce(bdata->gpiod,
>button->debounce_interval * 1000);
>@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
>isr = gpio_keys_gpio_isr;
>irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>
>+   switch (button->wakeup_event_action) {
>+   case EV_ACT_ASSERTED:
>+   bdata->wakeup_trigger_type = active_low ?
>+   IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
>+   break;
>+   case EV_ACT_DEASSERTED:
>+   bdata->wakeup_trigger_type = active_low ?
>+   IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>+   break;

What about EV_ACT_ANY? And default case? I think for ANY, we're OK
letting suspend/resume not reconfigure the trigger type, but maybe a
comment here to note that?

right, will add comment in the next version.



>+   }
>} else {

What about the 'else' case? Shouldn't we try to handle that?
i think the else case is for irq key, which would generate down and up 
events in one irq, so it would use the same trigger type for all these 3 
cases.


i'll add comment in the next version too.


Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Enric Balletbo Serra]

Hi,

2018-02-13 19:25 GMT+01:00 Brian Norris :
> Hi Enric,
>
> On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:
>> On 12/02/18 23:13, Brian Norris wrote:
>> > On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
>> >> Add support for specifying event actions to trigger wakeup when using
>> >> the gpio-keys input device as a wakeup source.
>> >>
>> >> This would allow the device to configure when to wakeup the system. For
>> >> example a gpio-keys input device for pen insert, may only want to wakeup
>> >> the system when ejecting the pen.
>> >>
>> >> Suggested-by: Brian Norris 
>> >> Signed-off-by: Jeffy Chen 
>> >> ---
>> >>
>> >> Changes in v2:
>> >> Specify wakeup event action instead of irq trigger type as Brian
>> >> suggested.
> [...]
>> Not sure if you were aware but there is also some discussion related to this,
>> maybe we can join the efforts?
>>
>> v1: https://patchwork.kernel.org/patch/10208261/
>> v2: https://patchwork.kernel.org/patch/10211147/
>
> Thanks for the pointers. IIUC, that's talking about a different problem:
> how to utilize a GPIO key in level-triggered mode. That touches similar
> code, but it doesn't really have anything to do with configuring a
> different wakeup trigger type.
>

Right, sorry. I see now what you are doing.

> The two patches would need to be reconciled, if they both are going to
> be merged. But otherwise, I think they're perfectly fine to be separate.
>

Yes, that's why I got confused, I had those patches applied on my tree
and when I tried to apply these failed and I wrongly assumed that were
doing the same. Waiting to test the third version ;)

Thanks,
 Enric

> Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Enric Balletbo Serra]

Hi,

2018-02-13 19:25 GMT+01:00 Brian Norris :
> Hi Enric,
>
> On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:
>> On 12/02/18 23:13, Brian Norris wrote:
>> > On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
>> >> Add support for specifying event actions to trigger wakeup when using
>> >> the gpio-keys input device as a wakeup source.
>> >>
>> >> This would allow the device to configure when to wakeup the system. For
>> >> example a gpio-keys input device for pen insert, may only want to wakeup
>> >> the system when ejecting the pen.
>> >>
>> >> Suggested-by: Brian Norris 
>> >> Signed-off-by: Jeffy Chen 
>> >> ---
>> >>
>> >> Changes in v2:
>> >> Specify wakeup event action instead of irq trigger type as Brian
>> >> suggested.
> [...]
>> Not sure if you were aware but there is also some discussion related to this,
>> maybe we can join the efforts?
>>
>> v1: https://patchwork.kernel.org/patch/10208261/
>> v2: https://patchwork.kernel.org/patch/10211147/
>
> Thanks for the pointers. IIUC, that's talking about a different problem:
> how to utilize a GPIO key in level-triggered mode. That touches similar
> code, but it doesn't really have anything to do with configuring a
> different wakeup trigger type.
>

Right, sorry. I see now what you are doing.

> The two patches would need to be reconciled, if they both are going to
> be merged. But otherwise, I think they're perfectly fine to be separate.
>

Yes, that's why I got confused, I had those patches applied on my tree
and when I tried to apply these failed and I wrongly assumed that were
doing the same. Waiting to test the third version ;)

Thanks,
 Enric

> Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Brian Norris]

Hi Enric,

On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:
> On 12/02/18 23:13, Brian Norris wrote:
> > On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
> >> Add support for specifying event actions to trigger wakeup when using
> >> the gpio-keys input device as a wakeup source.
> >>
> >> This would allow the device to configure when to wakeup the system. For
> >> example a gpio-keys input device for pen insert, may only want to wakeup
> >> the system when ejecting the pen.
> >>
> >> Suggested-by: Brian Norris 
> >> Signed-off-by: Jeffy Chen 
> >> ---
> >>
> >> Changes in v2:
> >> Specify wakeup event action instead of irq trigger type as Brian
> >> suggested.
[...]
> Not sure if you were aware but there is also some discussion related to this,
> maybe we can join the efforts?
> 
> v1: https://patchwork.kernel.org/patch/10208261/
> v2: https://patchwork.kernel.org/patch/10211147/

Thanks for the pointers. IIUC, that's talking about a different problem:
how to utilize a GPIO key in level-triggered mode. That touches similar
code, but it doesn't really have anything to do with configuring a
different wakeup trigger type.

The two patches would need to be reconciled, if they both are going to
be merged. But otherwise, I think they're perfectly fine to be separate.

Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Brian Norris]

Hi Enric,

On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:
> On 12/02/18 23:13, Brian Norris wrote:
> > On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
> >> Add support for specifying event actions to trigger wakeup when using
> >> the gpio-keys input device as a wakeup source.
> >>
> >> This would allow the device to configure when to wakeup the system. For
> >> example a gpio-keys input device for pen insert, may only want to wakeup
> >> the system when ejecting the pen.
> >>
> >> Suggested-by: Brian Norris 
> >> Signed-off-by: Jeffy Chen 
> >> ---
> >>
> >> Changes in v2:
> >> Specify wakeup event action instead of irq trigger type as Brian
> >> suggested.
[...]
> Not sure if you were aware but there is also some discussion related to this,
> maybe we can join the efforts?
> 
> v1: https://patchwork.kernel.org/patch/10208261/
> v2: https://patchwork.kernel.org/patch/10211147/

Thanks for the pointers. IIUC, that's talking about a different problem:
how to utilize a GPIO key in level-triggered mode. That touches similar
code, but it doesn't really have anything to do with configuring a
different wakeup trigger type.

The two patches would need to be reconciled, if they both are going to
be merged. But otherwise, I think they're perfectly fine to be separate.

Brian

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Enric Balletbo i Serra]

Hi Jeffy,

On 12/02/18 23:13, Brian Norris wrote:
> Hi Jeffy,
> 
> On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
>> Add support for specifying event actions to trigger wakeup when using
>> the gpio-keys input device as a wakeup source.
>>
>> This would allow the device to configure when to wakeup the system. For
>> example a gpio-keys input device for pen insert, may only want to wakeup
>> the system when ejecting the pen.
>>
>> Suggested-by: Brian Norris 
>> Signed-off-by: Jeffy Chen 
>> ---
>>
>> Changes in v2:
>> Specify wakeup event action instead of irq trigger type as Brian
>> suggested.
>>
>>  drivers/input/keyboard/gpio_keys.c | 27 +++
>>  include/linux/gpio_keys.h  |  2 ++
>>  include/uapi/linux/input-event-codes.h |  9 +
>>  3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c 
>> b/drivers/input/keyboard/gpio_keys.c
>> index 87e613dc33b8..5c57339d3999 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -45,6 +45,8 @@ struct gpio_button_data {
>>  unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>>  
>>  unsigned int irq;
>> +unsigned int irq_trigger_type;
>> +unsigned int wakeup_trigger_type;
>>  spinlock_t lock;
>>  bool disabled;
>>  bool key_pressed;
>> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device 
>> *pdev,
>>  }
>>  
>>  if (bdata->gpiod) {
>> +int active_low = gpiod_is_active_low(bdata->gpiod);
>> +
>>  if (button->debounce_interval) {
>>  error = gpiod_set_debounce(bdata->gpiod,
>>  button->debounce_interval * 1000);
>> @@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
>> *pdev,
>>  isr = gpio_keys_gpio_isr;
>>  irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>>  
>> +switch (button->wakeup_event_action) {
>> +case EV_ACT_ASSERTED:
>> +bdata->wakeup_trigger_type = active_low ?
>> +IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
>> +break;
>> +case EV_ACT_DEASSERTED:
>> +bdata->wakeup_trigger_type = active_low ?
>> +IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>> +break;
> 
> What about EV_ACT_ANY? And default case? I think for ANY, we're OK
> letting suspend/resume not reconfigure the trigger type, but maybe a
> comment here to note that?
> 
>> +}
>>  } else {
> 
> What about the 'else' case? Shouldn't we try to handle that?
> 
> Brian
> 
>>  if (!button->irq) {
>>  dev_err(dev, "Found button without gpio or irq\n");
>> @@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device 
>> *pdev,
>>  return error;
>>  }
>>  
>> +bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
>> +
>>  return 0;
>>  }
>>  
>> @@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>>  /* legacy name */
>>  fwnode_property_read_bool(child, "gpio-key,wakeup");
>>  
>> +fwnode_property_read_u32(child, "wakeup-event-action",
>> + >wakeup_event_action);
>> +
>>  button->can_disable =
>>  fwnode_property_read_bool(child, "linux,can-disable");
>>  
>> @@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct 
>> device *dev)
>>  if (device_may_wakeup(dev)) {
>>  for (i = 0; i < ddata->pdata->nbuttons; i++) {
>>  struct gpio_button_data *bdata = >data[i];
>> +
>> +if (bdata->button->wakeup && bdata->wakeup_trigger_type)
>> +irq_set_irq_type(bdata->irq,
>> + bdata->wakeup_trigger_type);
>>  if (bdata->button->wakeup)
>>  enable_irq_wake(bdata->irq);
>>  bdata->suspended = true;
>> @@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct 
>> device *dev)
>>  if (device_may_wakeup(dev)) {
>>  for (i = 0; i < ddata->pdata->nbuttons; i++) {
>>  struct gpio_button_data *bdata = >data[i];
>> +
>> +if (bdata->button->wakeup && bdata->wakeup_trigger_type)
>> +irq_set_irq_type(bdata->irq,
>> + bdata->irq_trigger_type);
>>  if (bdata->button->wakeup)
>>  disable_irq_wake(bdata->irq);
>>  bdata->suspended = false;
>> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
>> index 

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Enric Balletbo i Serra]

Hi Jeffy,

On 12/02/18 23:13, Brian Norris wrote:
> Hi Jeffy,
> 
> On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
>> Add support for specifying event actions to trigger wakeup when using
>> the gpio-keys input device as a wakeup source.
>>
>> This would allow the device to configure when to wakeup the system. For
>> example a gpio-keys input device for pen insert, may only want to wakeup
>> the system when ejecting the pen.
>>
>> Suggested-by: Brian Norris 
>> Signed-off-by: Jeffy Chen 
>> ---
>>
>> Changes in v2:
>> Specify wakeup event action instead of irq trigger type as Brian
>> suggested.
>>
>>  drivers/input/keyboard/gpio_keys.c | 27 +++
>>  include/linux/gpio_keys.h  |  2 ++
>>  include/uapi/linux/input-event-codes.h |  9 +
>>  3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c 
>> b/drivers/input/keyboard/gpio_keys.c
>> index 87e613dc33b8..5c57339d3999 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -45,6 +45,8 @@ struct gpio_button_data {
>>  unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>>  
>>  unsigned int irq;
>> +unsigned int irq_trigger_type;
>> +unsigned int wakeup_trigger_type;
>>  spinlock_t lock;
>>  bool disabled;
>>  bool key_pressed;
>> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device 
>> *pdev,
>>  }
>>  
>>  if (bdata->gpiod) {
>> +int active_low = gpiod_is_active_low(bdata->gpiod);
>> +
>>  if (button->debounce_interval) {
>>  error = gpiod_set_debounce(bdata->gpiod,
>>  button->debounce_interval * 1000);
>> @@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
>> *pdev,
>>  isr = gpio_keys_gpio_isr;
>>  irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>>  
>> +switch (button->wakeup_event_action) {
>> +case EV_ACT_ASSERTED:
>> +bdata->wakeup_trigger_type = active_low ?
>> +IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
>> +break;
>> +case EV_ACT_DEASSERTED:
>> +bdata->wakeup_trigger_type = active_low ?
>> +IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>> +break;
> 
> What about EV_ACT_ANY? And default case? I think for ANY, we're OK
> letting suspend/resume not reconfigure the trigger type, but maybe a
> comment here to note that?
> 
>> +}
>>  } else {
> 
> What about the 'else' case? Shouldn't we try to handle that?
> 
> Brian
> 
>>  if (!button->irq) {
>>  dev_err(dev, "Found button without gpio or irq\n");
>> @@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device 
>> *pdev,
>>  return error;
>>  }
>>  
>> +bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
>> +
>>  return 0;
>>  }
>>  
>> @@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>>  /* legacy name */
>>  fwnode_property_read_bool(child, "gpio-key,wakeup");
>>  
>> +fwnode_property_read_u32(child, "wakeup-event-action",
>> + >wakeup_event_action);
>> +
>>  button->can_disable =
>>  fwnode_property_read_bool(child, "linux,can-disable");
>>  
>> @@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct 
>> device *dev)
>>  if (device_may_wakeup(dev)) {
>>  for (i = 0; i < ddata->pdata->nbuttons; i++) {
>>  struct gpio_button_data *bdata = >data[i];
>> +
>> +if (bdata->button->wakeup && bdata->wakeup_trigger_type)
>> +irq_set_irq_type(bdata->irq,
>> + bdata->wakeup_trigger_type);
>>  if (bdata->button->wakeup)
>>  enable_irq_wake(bdata->irq);
>>  bdata->suspended = true;
>> @@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct 
>> device *dev)
>>  if (device_may_wakeup(dev)) {
>>  for (i = 0; i < ddata->pdata->nbuttons; i++) {
>>  struct gpio_button_data *bdata = >data[i];
>> +
>> +if (bdata->button->wakeup && bdata->wakeup_trigger_type)
>> +irq_set_irq_type(bdata->irq,
>> + bdata->irq_trigger_type);
>>  if (bdata->button->wakeup)
>>  disable_irq_wake(bdata->irq);
>>  bdata->suspended = false;
>> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
>> index d06bf77400f1..7160df54a6fe 100644
>> --- a/include/linux/gpio_keys.h
>> +++ 

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Brian Norris]

Hi Jeffy,

On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
> Add support for specifying event actions to trigger wakeup when using
> the gpio-keys input device as a wakeup source.
> 
> This would allow the device to configure when to wakeup the system. For
> example a gpio-keys input device for pen insert, may only want to wakeup
> the system when ejecting the pen.
> 
> Suggested-by: Brian Norris 
> Signed-off-by: Jeffy Chen 
> ---
> 
> Changes in v2:
> Specify wakeup event action instead of irq trigger type as Brian
> suggested.
> 
>  drivers/input/keyboard/gpio_keys.c | 27 +++
>  include/linux/gpio_keys.h  |  2 ++
>  include/uapi/linux/input-event-codes.h |  9 +
>  3 files changed, 38 insertions(+)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c 
> b/drivers/input/keyboard/gpio_keys.c
> index 87e613dc33b8..5c57339d3999 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -45,6 +45,8 @@ struct gpio_button_data {
>   unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>  
>   unsigned int irq;
> + unsigned int irq_trigger_type;
> + unsigned int wakeup_trigger_type;
>   spinlock_t lock;
>   bool disabled;
>   bool key_pressed;
> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device 
> *pdev,
>   }
>  
>   if (bdata->gpiod) {
> + int active_low = gpiod_is_active_low(bdata->gpiod);
> +
>   if (button->debounce_interval) {
>   error = gpiod_set_debounce(bdata->gpiod,
>   button->debounce_interval * 1000);
> @@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
> *pdev,
>   isr = gpio_keys_gpio_isr;
>   irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>  
> + switch (button->wakeup_event_action) {
> + case EV_ACT_ASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
> + break;
> + case EV_ACT_DEASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
> + break;

What about EV_ACT_ANY? And default case? I think for ANY, we're OK
letting suspend/resume not reconfigure the trigger type, but maybe a
comment here to note that?

> + }
>   } else {

What about the 'else' case? Shouldn't we try to handle that?

Brian

>   if (!button->irq) {
>   dev_err(dev, "Found button without gpio or irq\n");
> @@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device 
> *pdev,
>   return error;
>   }
>  
> + bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
> +
>   return 0;
>  }
>  
> @@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>   /* legacy name */
>   fwnode_property_read_bool(child, "gpio-key,wakeup");
>  
> + fwnode_property_read_u32(child, "wakeup-event-action",
> +  >wakeup_event_action);
> +
>   button->can_disable =
>   fwnode_property_read_bool(child, "linux,can-disable");
>  
> @@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct 
> device *dev)
>   if (device_may_wakeup(dev)) {
>   for (i = 0; i < ddata->pdata->nbuttons; i++) {
>   struct gpio_button_data *bdata = >data[i];
> +
> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
> + irq_set_irq_type(bdata->irq,
> +  bdata->wakeup_trigger_type);
>   if (bdata->button->wakeup)
>   enable_irq_wake(bdata->irq);
>   bdata->suspended = true;
> @@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device 
> *dev)
>   if (device_may_wakeup(dev)) {
>   for (i = 0; i < ddata->pdata->nbuttons; i++) {
>   struct gpio_button_data *bdata = >data[i];
> +
> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
> + irq_set_irq_type(bdata->irq,
> +  bdata->irq_trigger_type);
>   if (bdata->button->wakeup)
>   disable_irq_wake(bdata->irq);
>   bdata->suspended = false;
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index d06bf77400f1..7160df54a6fe 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -13,6 +13,7 @@ struct device;
>   * 

Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Brian Norris]

Hi Jeffy,

On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
> Add support for specifying event actions to trigger wakeup when using
> the gpio-keys input device as a wakeup source.
> 
> This would allow the device to configure when to wakeup the system. For
> example a gpio-keys input device for pen insert, may only want to wakeup
> the system when ejecting the pen.
> 
> Suggested-by: Brian Norris 
> Signed-off-by: Jeffy Chen 
> ---
> 
> Changes in v2:
> Specify wakeup event action instead of irq trigger type as Brian
> suggested.
> 
>  drivers/input/keyboard/gpio_keys.c | 27 +++
>  include/linux/gpio_keys.h  |  2 ++
>  include/uapi/linux/input-event-codes.h |  9 +
>  3 files changed, 38 insertions(+)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c 
> b/drivers/input/keyboard/gpio_keys.c
> index 87e613dc33b8..5c57339d3999 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -45,6 +45,8 @@ struct gpio_button_data {
>   unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>  
>   unsigned int irq;
> + unsigned int irq_trigger_type;
> + unsigned int wakeup_trigger_type;
>   spinlock_t lock;
>   bool disabled;
>   bool key_pressed;
> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device 
> *pdev,
>   }
>  
>   if (bdata->gpiod) {
> + int active_low = gpiod_is_active_low(bdata->gpiod);
> +
>   if (button->debounce_interval) {
>   error = gpiod_set_debounce(bdata->gpiod,
>   button->debounce_interval * 1000);
> @@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
> *pdev,
>   isr = gpio_keys_gpio_isr;
>   irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>  
> + switch (button->wakeup_event_action) {
> + case EV_ACT_ASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
> + break;
> + case EV_ACT_DEASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
> + break;

What about EV_ACT_ANY? And default case? I think for ANY, we're OK
letting suspend/resume not reconfigure the trigger type, but maybe a
comment here to note that?

> + }
>   } else {

What about the 'else' case? Shouldn't we try to handle that?

Brian

>   if (!button->irq) {
>   dev_err(dev, "Found button without gpio or irq\n");
> @@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device 
> *pdev,
>   return error;
>   }
>  
> + bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
> +
>   return 0;
>  }
>  
> @@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>   /* legacy name */
>   fwnode_property_read_bool(child, "gpio-key,wakeup");
>  
> + fwnode_property_read_u32(child, "wakeup-event-action",
> +  >wakeup_event_action);
> +
>   button->can_disable =
>   fwnode_property_read_bool(child, "linux,can-disable");
>  
> @@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct 
> device *dev)
>   if (device_may_wakeup(dev)) {
>   for (i = 0; i < ddata->pdata->nbuttons; i++) {
>   struct gpio_button_data *bdata = >data[i];
> +
> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
> + irq_set_irq_type(bdata->irq,
> +  bdata->wakeup_trigger_type);
>   if (bdata->button->wakeup)
>   enable_irq_wake(bdata->irq);
>   bdata->suspended = true;
> @@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device 
> *dev)
>   if (device_may_wakeup(dev)) {
>   for (i = 0; i < ddata->pdata->nbuttons; i++) {
>   struct gpio_button_data *bdata = >data[i];
> +
> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
> + irq_set_irq_type(bdata->irq,
> +  bdata->irq_trigger_type);
>   if (bdata->button->wakeup)
>   disable_irq_wake(bdata->irq);
>   bdata->suspended = false;
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index d06bf77400f1..7160df54a6fe 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -13,6 +13,7 @@ struct device;
>   * @desc:label that will be attached to 

[PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Jeffy Chen]

Add support for specifying event actions to trigger wakeup when using
the gpio-keys input device as a wakeup source.

This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.

Suggested-by: Brian Norris 
Signed-off-by: Jeffy Chen 
---

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

 drivers/input/keyboard/gpio_keys.c | 27 +++
 include/linux/gpio_keys.h  |  2 ++
 include/uapi/linux/input-event-codes.h |  9 +
 3 files changed, 38 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c 
b/drivers/input/keyboard/gpio_keys.c
index 87e613dc33b8..5c57339d3999 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,6 +45,8 @@ struct gpio_button_data {
unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
 
unsigned int irq;
+   unsigned int irq_trigger_type;
+   unsigned int wakeup_trigger_type;
spinlock_t lock;
bool disabled;
bool key_pressed;
@@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
}
 
if (bdata->gpiod) {
+   int active_low = gpiod_is_active_low(bdata->gpiod);
+
if (button->debounce_interval) {
error = gpiod_set_debounce(bdata->gpiod,
button->debounce_interval * 1000);
@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
isr = gpio_keys_gpio_isr;
irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
+   switch (button->wakeup_event_action) {
+   case EV_ACT_ASSERTED:
+   bdata->wakeup_trigger_type = active_low ?
+   IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
+   break;
+   case EV_ACT_DEASSERTED:
+   bdata->wakeup_trigger_type = active_low ?
+   IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
+   break;
+   }
} else {
if (!button->irq) {
dev_err(dev, "Found button without gpio or irq\n");
@@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
return error;
}
 
+   bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
+
return 0;
 }
 
@@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
/* legacy name */
fwnode_property_read_bool(child, "gpio-key,wakeup");
 
+   fwnode_property_read_u32(child, "wakeup-event-action",
+>wakeup_event_action);
+
button->can_disable =
fwnode_property_read_bool(child, "linux,can-disable");
 
@@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct device 
*dev)
if (device_may_wakeup(dev)) {
for (i = 0; i < ddata->pdata->nbuttons; i++) {
struct gpio_button_data *bdata = >data[i];
+
+   if (bdata->button->wakeup && bdata->wakeup_trigger_type)
+   irq_set_irq_type(bdata->irq,
+bdata->wakeup_trigger_type);
if (bdata->button->wakeup)
enable_irq_wake(bdata->irq);
bdata->suspended = true;
@@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device 
*dev)
if (device_may_wakeup(dev)) {
for (i = 0; i < ddata->pdata->nbuttons; i++) {
struct gpio_button_data *bdata = >data[i];
+
+   if (bdata->button->wakeup && bdata->wakeup_trigger_type)
+   irq_set_irq_type(bdata->irq,
+bdata->irq_trigger_type);
if (bdata->button->wakeup)
disable_irq_wake(bdata->irq);
bdata->suspended = false;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77400f1..7160df54a6fe 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -13,6 +13,7 @@ struct device;
  * @desc:  label that will be attached to button's gpio
  * @type:  input event type (%EV_KEY, %EV_SW, %EV_ABS)
  * @wakeup:configure the button as a wake-up source
+ * @wakeup_event_action:   event action to trigger wakeup
  * @debounce_interval: debounce ticks interval in msecs
  * @can_disable:   %true indicates that userspace is allowed to
  * disable 

[PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action

[Jeffy Chen]

Add support for specifying event actions to trigger wakeup when using
the gpio-keys input device as a wakeup source.

This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.

Suggested-by: Brian Norris 
Signed-off-by: Jeffy Chen 
---

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

 drivers/input/keyboard/gpio_keys.c | 27 +++
 include/linux/gpio_keys.h  |  2 ++
 include/uapi/linux/input-event-codes.h |  9 +
 3 files changed, 38 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c 
b/drivers/input/keyboard/gpio_keys.c
index 87e613dc33b8..5c57339d3999 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,6 +45,8 @@ struct gpio_button_data {
unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
 
unsigned int irq;
+   unsigned int irq_trigger_type;
+   unsigned int wakeup_trigger_type;
spinlock_t lock;
bool disabled;
bool key_pressed;
@@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
}
 
if (bdata->gpiod) {
+   int active_low = gpiod_is_active_low(bdata->gpiod);
+
if (button->debounce_interval) {
error = gpiod_set_debounce(bdata->gpiod,
button->debounce_interval * 1000);
@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
isr = gpio_keys_gpio_isr;
irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
+   switch (button->wakeup_event_action) {
+   case EV_ACT_ASSERTED:
+   bdata->wakeup_trigger_type = active_low ?
+   IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
+   break;
+   case EV_ACT_DEASSERTED:
+   bdata->wakeup_trigger_type = active_low ?
+   IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
+   break;
+   }
} else {
if (!button->irq) {
dev_err(dev, "Found button without gpio or irq\n");
@@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
return error;
}
 
+   bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
+
return 0;
 }
 
@@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
/* legacy name */
fwnode_property_read_bool(child, "gpio-key,wakeup");
 
+   fwnode_property_read_u32(child, "wakeup-event-action",
+>wakeup_event_action);
+
button->can_disable =
fwnode_property_read_bool(child, "linux,can-disable");
 
@@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct device 
*dev)
if (device_may_wakeup(dev)) {
for (i = 0; i < ddata->pdata->nbuttons; i++) {
struct gpio_button_data *bdata = >data[i];
+
+   if (bdata->button->wakeup && bdata->wakeup_trigger_type)
+   irq_set_irq_type(bdata->irq,
+bdata->wakeup_trigger_type);
if (bdata->button->wakeup)
enable_irq_wake(bdata->irq);
bdata->suspended = true;
@@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device 
*dev)
if (device_may_wakeup(dev)) {
for (i = 0; i < ddata->pdata->nbuttons; i++) {
struct gpio_button_data *bdata = >data[i];
+
+   if (bdata->button->wakeup && bdata->wakeup_trigger_type)
+   irq_set_irq_type(bdata->irq,
+bdata->irq_trigger_type);
if (bdata->button->wakeup)
disable_irq_wake(bdata->irq);
bdata->suspended = false;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77400f1..7160df54a6fe 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -13,6 +13,7 @@ struct device;
  * @desc:  label that will be attached to button's gpio
  * @type:  input event type (%EV_KEY, %EV_SW, %EV_ABS)
  * @wakeup:configure the button as a wake-up source
+ * @wakeup_event_action:   event action to trigger wakeup
  * @debounce_interval: debounce ticks interval in msecs
  * @can_disable:   %true indicates that userspace is allowed to
  * disable button via sysfs
@@ -26,6 +27,7 @@ struct