* [PATCH v5 0/3] gpio-keys: Add support for specifying wakeup event action @ 2018-03-08 7:32 ` Jeffy Chen 0 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:32 UTC (permalink / raw) To: linux-kernel Cc: briannorris, heiko, dtor, dianders, Jeffy Chen, Guenter Roeck, Arnd Bergmann, Joseph Lo, Rob Herring, Catalin Marinas, Emil Renner Berthing, Brian Norris, Thomas Gleixner, Philippe Ombredanne, linux-rockchip, Kate Stewart, linux-input, Will Deacon, Matthias Kaehlcke, devicetree, stephen lu, Greg Kroah-Hartman, Arvind Yadav, linux-arm-kernel, Dmitry Torokhov, Mark Rutland On chromebook kevin, we are using gpio-keys for pen insert event. But we only want it to wakeup the system when ejecting the pen. So we may need to change the interrupt trigger type during suspending. Changes in v5: Remove unneeded irq_wake flag as Andy suggested. Changes in v4: Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. Include dt-binding gpio-keys.h Changes in v3: Adding more comments as Brian suggested. Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. Specify wakeup event action instead of irq trigger type as Brian suggested. Specify wakeup event action instead of irq trigger type as Brian suggested. Jeffy Chen (3): Input: gpio-keys - add support for wakeup event action Input: gpio-keys - allow setting wakeup event action in DT arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen .../devicetree/bindings/input/gpio-keys.txt | 8 +++ arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 ++ drivers/input/keyboard/gpio_keys.c | 63 ++++++++++++++++++++-- include/dt-bindings/input/gpio-keys.h | 13 +++++ include/linux/gpio_keys.h | 2 + 5 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/input/gpio-keys.h -- 2.11.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 0/3] gpio-keys: Add support for specifying wakeup event action @ 2018-03-08 7:32 ` Jeffy Chen 0 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:32 UTC (permalink / raw) To: linux-arm-kernel On chromebook kevin, we are using gpio-keys for pen insert event. But we only want it to wakeup the system when ejecting the pen. So we may need to change the interrupt trigger type during suspending. Changes in v5: Remove unneeded irq_wake flag as Andy suggested. Changes in v4: Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. Include dt-binding gpio-keys.h Changes in v3: Adding more comments as Brian suggested. Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. Specify wakeup event action instead of irq trigger type as Brian suggested. Specify wakeup event action instead of irq trigger type as Brian suggested. Jeffy Chen (3): Input: gpio-keys - add support for wakeup event action Input: gpio-keys - allow setting wakeup event action in DT arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen .../devicetree/bindings/input/gpio-keys.txt | 8 +++ arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 ++ drivers/input/keyboard/gpio_keys.c | 63 ++++++++++++++++++++-- include/dt-bindings/input/gpio-keys.h | 13 +++++ include/linux/gpio_keys.h | 2 + 5 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/input/gpio-keys.h -- 2.11.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 0/3] gpio-keys: Add support for specifying wakeup event action @ 2018-03-08 7:32 ` Jeffy Chen 0 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:32 UTC (permalink / raw) To: linux-kernel Cc: Kate Stewart, Mark Rutland, heiko, stephen lu, Catalin Marinas, Will Deacon, Joseph Lo, Emil Renner Berthing, Brian Norris, linux-rockchip, Matthias Kaehlcke, linux-input, briannorris, dtor, devicetree, Arnd Bergmann, Jeffy Chen, dianders, Rob Herring, Arvind Yadav, Thomas Gleixner, linux-arm-kernel, Greg Kroah-Hartman, Dmitry Torokhov, Guenter Roeck, Philippe On chromebook kevin, we are using gpio-keys for pen insert event. But we only want it to wakeup the system when ejecting the pen. So we may need to change the interrupt trigger type during suspending. Changes in v5: Remove unneeded irq_wake flag as Andy suggested. Changes in v4: Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. Include dt-binding gpio-keys.h Changes in v3: Adding more comments as Brian suggested. Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. Specify wakeup event action instead of irq trigger type as Brian suggested. Specify wakeup event action instead of irq trigger type as Brian suggested. Jeffy Chen (3): Input: gpio-keys - add support for wakeup event action Input: gpio-keys - allow setting wakeup event action in DT arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen .../devicetree/bindings/input/gpio-keys.txt | 8 +++ arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 ++ drivers/input/keyboard/gpio_keys.c | 63 ++++++++++++++++++++-- include/dt-bindings/input/gpio-keys.h | 13 +++++ include/linux/gpio_keys.h | 2 + 5 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/input/gpio-keys.h -- 2.11.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action 2018-03-08 7:32 ` Jeffy Chen (?) (?) @ 2018-03-08 7:32 ` Jeffy Chen 2018-03-09 23:27 ` Rob Herring 2018-03-10 18:15 ` Dmitry Torokhov -1 siblings, 2 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:32 UTC (permalink / raw) To: linux-kernel Cc: briannorris, heiko, dtor, dianders, Jeffy Chen, devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu, Rob Herring, Dmitry Torokhov, Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland, Philippe Ombredanne, Arvind Yadav 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 v5: Remove unneeded irq_wake flag as Andy suggested. Changes in v4: Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. Changes in v3: Adding more comments as Brian suggested. Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. drivers/input/keyboard/gpio_keys.c | 63 +++++++++++++++++++++++++++++++++-- include/dt-bindings/input/gpio-keys.h | 13 ++++++++ include/linux/gpio_keys.h | 2 ++ 3 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/input/gpio-keys.h diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 87e613dc33b8..f6d5cfd44833 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -30,6 +30,7 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/spinlock.h> +#include <dt-bindings/input/gpio-keys.h> struct gpio_button_data { const struct gpio_keys_button *button; @@ -45,6 +46,7 @@ struct gpio_button_data { unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */ unsigned int irq; + 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,24 @@ 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 ? + IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING; + break; + case EV_ACT_DEASSERTED: + bdata->wakeup_trigger_type = active_low ? + IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING; + break; + case EV_ACT_ANY: + /* fall through */ + default: + /* + * For other cases, we are OK letting suspend/resume + * not reconfigure the trigger type. + */ + break; + } } else { if (!button->irq) { dev_err(dev, "Found button without gpio or irq\n"); @@ -586,6 +608,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev, isr = gpio_keys_irq_isr; irqflags = 0; + + /* + * For IRQ buttons, there is no interrupt for release. + * So we don't need to reconfigure the trigger type for wakeup. + */ } bdata->code = &ddata->keymap[idx]; @@ -718,6 +745,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"); @@ -845,6 +875,31 @@ static int gpio_keys_probe(struct platform_device *pdev) return 0; } +static int gpio_keys_enable_wakeup(struct gpio_button_data *bdata) +{ + int ret; + + ret = enable_irq_wake(bdata->irq); + if (ret) + return ret; + + if (bdata->wakeup_trigger_type) + irq_set_irq_type(bdata->irq, bdata->wakeup_trigger_type); + + return 0; +} + +static void gpio_keys_disable_wakeup(struct gpio_button_data *bdata) +{ + /** + * The trigger type is always both edges for gpio-based keys and we do + * not support changing wakeup trigger for interrupt-based keys. + */ + if (bdata->wakeup_trigger_type) + irq_set_irq_type(bdata->irq, IRQ_TYPE_EDGE_BOTH); + disable_irq_wake(bdata->irq); +} + static int __maybe_unused gpio_keys_suspend(struct device *dev) { struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); @@ -854,8 +909,9 @@ 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) - enable_irq_wake(bdata->irq); + gpio_keys_enable_wakeup(bdata); bdata->suspended = true; } } else { @@ -878,9 +934,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) - disable_irq_wake(bdata->irq); + bdata->suspended = false; + if (irqd_is_wakeup_set(irq_get_irq_data(bdata->irq))) + gpio_keys_disable_wakeup(bdata); } } else { mutex_lock(&input->mutex); diff --git a/include/dt-bindings/input/gpio-keys.h b/include/dt-bindings/input/gpio-keys.h new file mode 100644 index 000000000000..8962df79e753 --- /dev/null +++ b/include/dt-bindings/input/gpio-keys.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * This header provides constants for gpio keys bindings. + */ + +#ifndef _DT_BINDINGS_GPIO_KEYS_H +#define _DT_BINDINGS_GPIO_KEYS_H + +#define EV_ACT_ANY 0x00 /* asserted or deasserted */ +#define EV_ACT_ASSERTED 0x01 /* asserted */ +#define EV_ACT_DEASSERTED 0x02 /* deasserted */ + +#endif /* _DT_BINDINGS_GPIO_KEYS_H */ 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; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action 2018-03-08 7:32 ` [PATCH v5 1/3] Input: gpio-keys - add support for " Jeffy Chen @ 2018-03-09 23:27 ` Rob Herring 2018-03-10 18:11 ` Dmitry Torokhov 2018-03-10 18:15 ` Dmitry Torokhov 1 sibling, 1 reply; 13+ messages in thread From: Rob Herring @ 2018-03-09 23:27 UTC (permalink / raw) To: Jeffy Chen Cc: linux-kernel, briannorris, heiko, dtor, dianders, devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu, Dmitry Torokhov, Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland, Philippe Ombredanne, Arvind Yadav On Thu, Mar 08, 2018 at 03:32:11PM +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 v5: > Remove unneeded irq_wake flag as Andy suggested. > > Changes in v4: > Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. > > Changes in v3: > Adding more comments as Brian suggested. > > Changes in v2: > Specify wakeup event action instead of irq trigger type as Brian > suggested. > > drivers/input/keyboard/gpio_keys.c | 63 +++++++++++++++++++++++++++++++++-- > include/dt-bindings/input/gpio-keys.h | 13 ++++++++ This file should be in the binding patch. > include/linux/gpio_keys.h | 2 ++ > 3 files changed, 75 insertions(+), 3 deletions(-) > create mode 100644 include/dt-bindings/input/gpio-keys.h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action 2018-03-09 23:27 ` Rob Herring @ 2018-03-10 18:11 ` Dmitry Torokhov 0 siblings, 0 replies; 13+ messages in thread From: Dmitry Torokhov @ 2018-03-10 18:11 UTC (permalink / raw) To: Rob Herring Cc: Jeffy Chen, linux-kernel, briannorris, heiko, dtor, dianders, devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu, Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland, Philippe Ombredanne, Arvind Yadav On Fri, Mar 09, 2018 at 05:27:21PM -0600, Rob Herring wrote: > On Thu, Mar 08, 2018 at 03:32:11PM +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 v5: > > Remove unneeded irq_wake flag as Andy suggested. > > > > Changes in v4: > > Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. > > > > Changes in v3: > > Adding more comments as Brian suggested. > > > > Changes in v2: > > Specify wakeup event action instead of irq trigger type as Brian > > suggested. > > > > drivers/input/keyboard/gpio_keys.c | 63 +++++++++++++++++++++++++++++++++-- > > include/dt-bindings/input/gpio-keys.h | 13 ++++++++ > > This file should be in the binding patch. No, this is a folly. Both the header, along with the documentation (i.e. binding doc) should be together with the patch that introduces the changes. The patches are supposed to be self contained and sufficient to stand on their own. Neither this header, not the binding doc make sense without the change to the driver itself. Next we'll start adding #defines in a separate patch before using them... Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action 2018-03-08 7:32 ` [PATCH v5 1/3] Input: gpio-keys - add support for " Jeffy Chen 2018-03-09 23:27 ` Rob Herring @ 2018-03-10 18:15 ` Dmitry Torokhov 2018-03-10 18:32 ` Andy Shevchenko 2018-03-12 10:40 ` JeffyChen 1 sibling, 2 replies; 13+ messages in thread From: Dmitry Torokhov @ 2018-03-10 18:15 UTC (permalink / raw) To: Jeffy Chen Cc: linux-kernel, briannorris, heiko, dtor, dianders, devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu, Rob Herring, Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland, Philippe Ombredanne, Arvind Yadav Hi Jeffy, On Thu, Mar 08, 2018 at 03:32:11PM +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 v5: > Remove unneeded irq_wake flag as Andy suggested. > > Changes in v4: > Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested. > > Changes in v3: > Adding more comments as Brian suggested. > > Changes in v2: > Specify wakeup event action instead of irq trigger type as Brian > suggested. > > drivers/input/keyboard/gpio_keys.c | 63 +++++++++++++++++++++++++++++++++-- > include/dt-bindings/input/gpio-keys.h | 13 ++++++++ > include/linux/gpio_keys.h | 2 ++ > 3 files changed, 75 insertions(+), 3 deletions(-) > create mode 100644 include/dt-bindings/input/gpio-keys.h > > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c > index 87e613dc33b8..f6d5cfd44833 100644 > --- a/drivers/input/keyboard/gpio_keys.c > +++ b/drivers/input/keyboard/gpio_keys.c > @@ -30,6 +30,7 @@ > #include <linux/of.h> > #include <linux/of_irq.h> > #include <linux/spinlock.h> > +#include <dt-bindings/input/gpio-keys.h> > > struct gpio_button_data { > const struct gpio_keys_button *button; > @@ -45,6 +46,7 @@ struct gpio_button_data { > unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */ > > unsigned int irq; > + 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,24 @@ 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 ? > + IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING; > + break; > + case EV_ACT_DEASSERTED: > + bdata->wakeup_trigger_type = active_low ? > + IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING; > + break; > + case EV_ACT_ANY: > + /* fall through */ > + default: > + /* > + * For other cases, we are OK letting suspend/resume > + * not reconfigure the trigger type. > + */ > + break; > + } > } else { > if (!button->irq) { > dev_err(dev, "Found button without gpio or irq\n"); > @@ -586,6 +608,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev, > > isr = gpio_keys_irq_isr; > irqflags = 0; > + > + /* > + * For IRQ buttons, there is no interrupt for release. > + * So we don't need to reconfigure the trigger type for wakeup. > + */ > } > > bdata->code = &ddata->keymap[idx]; > @@ -718,6 +745,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"); > > @@ -845,6 +875,31 @@ static int gpio_keys_probe(struct platform_device *pdev) > return 0; > } > > +static int gpio_keys_enable_wakeup(struct gpio_button_data *bdata) These new helpers need to be __maybe_unused in case we compile on system without suspend support. I also wanted a bit more of error handling, so I ended up with the version of the patch below. Can you please try it and let me know if I broke it. Thanks. -- Dmitry Input: gpio-keys - add support for wakeup event action From: Jeffy Chen <jeffy.chen@rock-chips.com> 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> Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- .../devicetree/bindings/input/gpio-keys.txt | 8 + drivers/input/keyboard/gpio_keys.c | 145 ++++++++++++++++++-- include/dt-bindings/input/gpio-keys.h | 13 ++ include/linux/gpio_keys.h | 2 4 files changed, 154 insertions(+), 14 deletions(-) create mode 100644 include/dt-bindings/input/gpio-keys.h diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt index a94940481e55a..996ce84352cbf 100644 --- a/Documentation/devicetree/bindings/input/gpio-keys.txt +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt @@ -26,6 +26,14 @@ Optional subnode-properties: If not specified defaults to 5. - wakeup-source: Boolean, button can wake-up the system. (Legacy property supported: "gpio-key,wakeup") + - wakeup-event-action: Specifies whether the key should wake the + system when asserted, when deasserted, or both. This property is + only valid for keys that wake up the system (e.g., when the + "wakeup-source" property is also provided). + Supported values are defined in linux-event-codes.h: + EV_ACT_ASSERTED - asserted + EV_ACT_DEASSERTED - deasserted + EV_ACT_ANY - both asserted and deasserted - linux,can-disable: Boolean, indicates that button is connected to dedicated (not shared) interrupt which can be disabled to suppress events from the button. diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 87e613dc33b80..966428da6bc71 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -30,6 +30,7 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/spinlock.h> +#include <dt-bindings/input/gpio-keys.h> struct gpio_button_data { const struct gpio_keys_button *button; @@ -45,6 +46,7 @@ struct gpio_button_data { unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */ unsigned int irq; + 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) { + bool 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,24 @@ 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 ? + IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING; + break; + case EV_ACT_DEASSERTED: + bdata->wakeup_trigger_type = active_low ? + IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING; + break; + case EV_ACT_ANY: + /* fall through */ + default: + /* + * For other cases, we are OK letting suspend/resume + * not reconfigure the trigger type. + */ + break; + } } else { if (!button->irq) { dev_err(dev, "Found button without gpio or irq\n"); @@ -586,6 +608,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev, isr = gpio_keys_irq_isr; irqflags = 0; + + /* + * For IRQ buttons, there is no interrupt for release. + * So we don't need to reconfigure the trigger type for wakeup. + */ } bdata->code = &ddata->keymap[idx]; @@ -718,6 +745,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"); @@ -845,19 +875,112 @@ static int gpio_keys_probe(struct platform_device *pdev) return 0; } +static int __maybe_unused +gpio_keys_button_enable_wakeup(struct gpio_button_data *bdata) +{ + int error; + + error = enable_irq_wake(bdata->irq); + if (error) { + dev_err(bdata->input->dev.parent, + "failed to configure IRQ %d as wakeup source: %d\n", + bdata->irq, error); + return error; + } + + if (bdata->wakeup_trigger_type) { + error = irq_set_irq_type(bdata->irq, + bdata->wakeup_trigger_type); + if (error) { + dev_err(bdata->input->dev.parent, + "failed to set wakeup trigger %08x for IRQ %d: %d\n", + bdata->wakeup_trigger_type, bdata->irq, error); + disable_irq_wake(bdata->irq); + return error; + } + } + + return 0; +} + +static void __maybe_unused +gpio_keys_button_disable_wakeup(struct gpio_button_data *bdata) +{ + int error; + + /* + * The trigger type is always both edges for gpio-based keys and we do + * not support changing wakeup trigger for interrupt-based keys. + */ + if (bdata->wakeup_trigger_type) { + error = irq_set_irq_type(bdata->irq, IRQ_TYPE_EDGE_BOTH); + if (error) + dev_warn(bdata->input->dev.parent, + "failed to restore interrupt trigger for IRQ %d: %d\n", + bdata->irq, error); + } + + error = disable_irq_wake(bdata->irq); + if (error) + dev_warn(bdata->input->dev.parent, + "failed to disable IRQ %d as wake source: %d\n", + bdata->irq, error); +} + +static int __maybe_unused +gpio_keys_enable_wakeup(struct gpio_keys_drvdata *ddata) +{ + struct gpio_button_data *bdata; + int error; + int i; + + for (i = 0; i < ddata->pdata->nbuttons; i++) { + bdata = &ddata->data[i]; + if (bdata->button->wakeup) { + error = gpio_keys_button_enable_wakeup(bdata); + if (error) + goto err_out; + } + bdata->suspended = true; + } + + return 0; + +err_out: + while (--i >= 0) { + bdata = &ddata->data[i]; + if (bdata->button->wakeup) + gpio_keys_button_disable_wakeup(bdata); + bdata->suspended = false; + } + + return error; +} + +static void __maybe_unused +gpio_keys_disable_wakeup(struct gpio_keys_drvdata *ddata) +{ + struct gpio_button_data *bdata; + int i; + + for (i = 0; i < ddata->pdata->nbuttons; i++) { + bdata = &ddata->data[i]; + bdata->suspended = false; + if (irqd_is_wakeup_set(irq_get_irq_data(bdata->irq))) + gpio_keys_button_disable_wakeup(bdata); + } +} + static int __maybe_unused gpio_keys_suspend(struct device *dev) { struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); struct input_dev *input = ddata->input; - int i; + int error; 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) - enable_irq_wake(bdata->irq); - bdata->suspended = true; - } + error = gpio_keys_enable_wakeup(ddata); + if (error) + return error; } else { mutex_lock(&input->mutex); if (input->users) @@ -873,15 +996,9 @@ static int __maybe_unused gpio_keys_resume(struct device *dev) struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); struct input_dev *input = ddata->input; int error = 0; - int i; if (device_may_wakeup(dev)) { - for (i = 0; i < ddata->pdata->nbuttons; i++) { - struct gpio_button_data *bdata = &ddata->data[i]; - if (bdata->button->wakeup) - disable_irq_wake(bdata->irq); - bdata->suspended = false; - } + gpio_keys_disable_wakeup(ddata); } else { mutex_lock(&input->mutex); if (input->users) diff --git a/include/dt-bindings/input/gpio-keys.h b/include/dt-bindings/input/gpio-keys.h new file mode 100644 index 0000000000000..8962df79e753b --- /dev/null +++ b/include/dt-bindings/input/gpio-keys.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * This header provides constants for gpio keys bindings. + */ + +#ifndef _DT_BINDINGS_GPIO_KEYS_H +#define _DT_BINDINGS_GPIO_KEYS_H + +#define EV_ACT_ANY 0x00 /* asserted or deasserted */ +#define EV_ACT_ASSERTED 0x01 /* asserted */ +#define EV_ACT_DEASSERTED 0x02 /* deasserted */ + +#endif /* _DT_BINDINGS_GPIO_KEYS_H */ diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h index d06bf77400f16..7160df54a6fe3 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; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action 2018-03-10 18:15 ` Dmitry Torokhov @ 2018-03-10 18:32 ` Andy Shevchenko 2018-03-12 10:40 ` JeffyChen 1 sibling, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2018-03-10 18:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: Jeffy Chen, Linux Kernel Mailing List, Brian Norris, Heiko Stübner, Dmitry Torokhov, Douglas Anderson, devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu, Rob Herring, Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland, Philippe Ombredanne, Arvind Yadav On Sat, Mar 10, 2018 at 8:15 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > + while (--i >= 0) { Just in case while (i--) { is slightly better to read. > + bdata = &ddata->data[i]; > + if (bdata->button->wakeup) > + gpio_keys_button_disable_wakeup(bdata); > + bdata->suspended = false; > + } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action 2018-03-10 18:15 ` Dmitry Torokhov 2018-03-10 18:32 ` Andy Shevchenko @ 2018-03-12 10:40 ` JeffyChen 1 sibling, 0 replies; 13+ messages in thread From: JeffyChen @ 2018-03-12 10:40 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-kernel, briannorris, heiko, dtor, dianders, devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu, Rob Herring, Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland, Philippe Ombredanne, Arvind Yadav Hi Dmitry, Thanks for your reply. On 03/11/2018 02:15 AM, Dmitry Torokhov wrote:>> +static int gpio_keys_enable_wakeup(struct gpio_button_data *bdata) > These new helpers need to be __maybe_unused in case we compile on system > without suspend support. > > I also wanted a bit more of error handling, so I ended up with the > version of the patch below. Can you please try it and let me know if I > broke it. sure, the patch you sent works :) Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com> > > Thanks. > > -- Dmitry > > Input: gpio-keys - add support for wakeup event action > > From: Jeffy Chen <jeffy.chen@rock-chips.com> > > 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> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 2/3] Input: gpio-keys - allow setting wakeup event action in DT 2018-03-08 7:32 ` Jeffy Chen ` (2 preceding siblings ...) (?) @ 2018-03-08 7:32 ` Jeffy Chen -1 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:32 UTC (permalink / raw) To: linux-kernel Cc: briannorris, heiko, dtor, dianders, Jeffy Chen, devicetree, Rob Herring, Dmitry Torokhov, linux-input, Mark Rutland Allow specifying event actions to trigger wakeup when using the gpio-keys input device as a wakeup source. Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. Documentation/devicetree/bindings/input/gpio-keys.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt index a94940481e55..996ce84352cb 100644 --- a/Documentation/devicetree/bindings/input/gpio-keys.txt +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt @@ -26,6 +26,14 @@ Optional subnode-properties: If not specified defaults to 5. - wakeup-source: Boolean, button can wake-up the system. (Legacy property supported: "gpio-key,wakeup") + - wakeup-event-action: Specifies whether the key should wake the + system when asserted, when deasserted, or both. This property is + only valid for keys that wake up the system (e.g., when the + "wakeup-source" property is also provided). + Supported values are defined in linux-event-codes.h: + EV_ACT_ASSERTED - asserted + EV_ACT_DEASSERTED - deasserted + EV_ACT_ANY - both asserted and deasserted - linux,can-disable: Boolean, indicates that button is connected to dedicated (not shared) interrupt which can be disabled to suppress events from the button. -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 3/3] arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen 2018-03-08 7:32 ` Jeffy Chen (?) @ 2018-03-08 7:42 ` Jeffy Chen -1 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:42 UTC (permalink / raw) To: linux-kernel Cc: briannorris, heiko, dtor, dianders, Jeffy Chen, Matthias Kaehlcke, devicetree, Arnd Bergmann, Emil Renner Berthing, Brian Norris, linux-rockchip, Rob Herring, Dmitry Torokhov, linux-arm-kernel, Will Deacon, Mark Rutland, Catalin Marinas Add wakeup event action for Pen Insert gpio key, to avoid wakeup when inserting the pen. Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> --- Changes in v5: None Changes in v4: Include dt-binding gpio-keys.h Changes in v3: None Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts index 191a6bcb1704..89126dbe5d91 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts @@ -44,6 +44,7 @@ /dts-v1/; #include "rk3399-gru.dtsi" +#include <dt-bindings/input/gpio-keys.h> #include <dt-bindings/input/linux-event-codes.h> /* @@ -134,6 +135,8 @@ gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; linux,code = <SW_PEN_INSERTED>; linux,input-type = <EV_SW>; + /* Wakeup only when ejecting */ + wakeup-event-action = <EV_ACT_DEASSERTED>; wakeup-source; }; }; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 3/3] arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen @ 2018-03-08 7:42 ` Jeffy Chen 0 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:42 UTC (permalink / raw) To: linux-arm-kernel Add wakeup event action for Pen Insert gpio key, to avoid wakeup when inserting the pen. Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> --- Changes in v5: None Changes in v4: Include dt-binding gpio-keys.h Changes in v3: None Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts index 191a6bcb1704..89126dbe5d91 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts @@ -44,6 +44,7 @@ /dts-v1/; #include "rk3399-gru.dtsi" +#include <dt-bindings/input/gpio-keys.h> #include <dt-bindings/input/linux-event-codes.h> /* @@ -134,6 +135,8 @@ gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; linux,code = <SW_PEN_INSERTED>; linux,input-type = <EV_SW>; + /* Wakeup only when ejecting */ + wakeup-event-action = <EV_ACT_DEASSERTED>; wakeup-source; }; }; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 3/3] arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen @ 2018-03-08 7:42 ` Jeffy Chen 0 siblings, 0 replies; 13+ messages in thread From: Jeffy Chen @ 2018-03-08 7:42 UTC (permalink / raw) To: linux-kernel Cc: Mark Rutland, devicetree, Brian Norris, heiko, Arnd Bergmann, Catalin Marinas, Jeffy Chen, Will Deacon, Rob Herring, dianders, linux-rockchip, Matthias Kaehlcke, linux-arm-kernel, briannorris, Dmitry Torokhov, dtor, Emil Renner Berthing Add wakeup event action for Pen Insert gpio key, to avoid wakeup when inserting the pen. Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> --- Changes in v5: None Changes in v4: Include dt-binding gpio-keys.h Changes in v3: None Changes in v2: Specify wakeup event action instead of irq trigger type as Brian suggested. arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts index 191a6bcb1704..89126dbe5d91 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts @@ -44,6 +44,7 @@ /dts-v1/; #include "rk3399-gru.dtsi" +#include <dt-bindings/input/gpio-keys.h> #include <dt-bindings/input/linux-event-codes.h> /* @@ -134,6 +135,8 @@ gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; linux,code = <SW_PEN_INSERTED>; linux,input-type = <EV_SW>; + /* Wakeup only when ejecting */ + wakeup-event-action = <EV_ACT_DEASSERTED>; wakeup-source; }; }; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-03-12 10:40 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-08 7:32 [PATCH v5 0/3] gpio-keys: Add support for specifying wakeup event action Jeffy Chen 2018-03-08 7:32 ` Jeffy Chen 2018-03-08 7:32 ` Jeffy Chen 2018-03-08 7:32 ` [PATCH v5 1/3] Input: gpio-keys - add support for " Jeffy Chen 2018-03-09 23:27 ` Rob Herring 2018-03-10 18:11 ` Dmitry Torokhov 2018-03-10 18:15 ` Dmitry Torokhov 2018-03-10 18:32 ` Andy Shevchenko 2018-03-12 10:40 ` JeffyChen 2018-03-08 7:32 ` [PATCH v5 2/3] Input: gpio-keys - allow setting wakeup event action in DT Jeffy Chen 2018-03-08 7:42 ` [PATCH v5 3/3] arm64: dts: rockchip: kevin: Avoid wakeup when inserting the pen Jeffy Chen 2018-03-08 7:42 ` Jeffy Chen 2018-03-08 7:42 ` Jeffy Chen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.