| Message ID | 20180210110907.5504-2-jeffy.chen@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <briannorris@chromium.org> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > --- > > 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", > + &button->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 = &ddata->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 = &ddata->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 gpio_keys_button { > const char *desc; > unsigned int type; > int wakeup; > + int wakeup_event_action; > int debounce_interval; > bool can_disable; > int value; > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h > index 53fbae27b280..d7917b0bd438 100644 > --- a/include/uapi/linux/input-event-codes.h > +++ b/include/uapi/linux/input-event-codes.h > @@ -32,6 +32,15 @@ > #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1) > > /* > + * Event action types > + */ > +#define EV_ACT_ANY 0x00 /* asserted or deasserted */ > +#define EV_ACT_ASSERTED 0x01 /* asserted */ > +#define EV_ACT_DEASSERTED 0x02 /* deasserted */ > +#define EV_ACT_MAX 0x02 > +#define EV_ACT_CNT (EV_ACT_MAX+1) > + > +/* > * Event types > */ > > -- > 2.11.0 > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <briannorris@chromium.org> >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >> --- >> >> 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", >> + &button->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 = &ddata->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 = &ddata->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 gpio_keys_button { >> const char *desc; >> unsigned int type; >> int wakeup; >> + int wakeup_event_action; >> int debounce_interval; >> bool can_disable; >> int value; >> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h >> index 53fbae27b280..d7917b0bd438 100644 >> --- a/include/uapi/linux/input-event-codes.h >> +++ b/include/uapi/linux/input-event-codes.h >> @@ -32,6 +32,15 @@ >> #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1) >> >> /* >> + * Event action types >> + */ >> +#define EV_ACT_ANY 0x00 /* asserted or deasserted */ >> +#define EV_ACT_ASSERTED 0x01 /* asserted */ >> +#define EV_ACT_DEASSERTED 0x02 /* deasserted */ >> +#define EV_ACT_MAX 0x02 >> +#define EV_ACT_CNT (EV_ACT_MAX+1) >> + >> +/* >> * Event types >> */ >> >> -- >> 2.11.0 >> >> 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/ Best regards, Enric -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <briannorris@chromium.org> > >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > >> --- > >> > >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, 2018-02-13 19:25 GMT+01:00 Brian Norris <briannorris@chromium.org>: > 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 <briannorris@chromium.org> >> >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >> >> --- >> >> >> >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <briannorris@chromium.org>: >> 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 <briannorris@chromium.org> >>>>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >>>>> --- >>>>> >>>>> 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 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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", + &button->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 = &ddata->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 = &ddata->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 gpio_keys_button { const char *desc; unsigned int type; int wakeup; + int wakeup_event_action; int debounce_interval; bool can_disable; int value; diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h index 53fbae27b280..d7917b0bd438 100644 --- a/include/uapi/linux/input-event-codes.h +++ b/include/uapi/linux/input-event-codes.h @@ -32,6 +32,15 @@ #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1) /* + * Event action types + */ +#define EV_ACT_ANY 0x00 /* asserted or deasserted */ +#define EV_ACT_ASSERTED 0x01 /* asserted */ +#define EV_ACT_DEASSERTED 0x02 /* deasserted */ +#define EV_ACT_MAX 0x02 +#define EV_ACT_CNT (EV_ACT_MAX+1) + +/* * Event types */
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 <briannorris@chromium.org> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> --- 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(+)