| Message ID | a3db69d2fc4b460dcb2e58e2608f7fdd8640c5c8.1559159872.git.chunkeey@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | Christian Lamparter |
| Headers | show |
| Series | [OpenWrt-Devel,1/2] gpio-button-hotplug: support interrupt properties | expand |
Christian Lamparter <chunkeey@gmail.com> [2019-05-29 21:58:29]: Hi, > Upstream Linux's input gpio-keys driver supports > specifying a external interrupt for a gpio via the > 'interrupts' properties as well as having support > for software debounce. [...] I've just checked this on ath79 (archer-c7-v5) and on ramips/mt7620 (bdcom,wap2100-sk) with WPS buttons. Acked-by: Petr Štetiar <ynezz@true.cz>
Hello, On Thursday, May 30, 2019 12:00:27 PM CEST Petr Štetiar wrote: > Christian Lamparter <chunkeey@gmail.com> [2019-05-29 21:58:29]: > > Upstream Linux's input gpio-keys driver supports > > specifying a external interrupt for a gpio via the > > 'interrupts' properties as well as having support > > for software debounce. > > [...] > > I've just checked this on ath79 (archer-c7-v5) and on ramips/mt7620 > (bdcom,wap2100-sk) with WPS buttons. > > Acked-by: Petr Štetiar <ynezz@true.cz> Can you tell me what you tested? Was it the software debounce? Because this should be the only bit that will affect the ath79 platform I think (since it already has support for interrupts through the gpio controller). From what I can tell, ramips should use gpio-keys-polled exclusivly for now. This is because the rt2880-pinmux driver doesn't implement and irq support (though some of the chips should support it). all gpio-keys-polled should work as before. Cheers, Christian
Christian Lamparter <chunkeey@gmail.com> [2019-05-30 17:09:08]: Hi, > On Thursday, May 30, 2019 12:00:27 PM CEST Petr Štetiar wrote: > > > > I've just checked this on ath79 (archer-c7-v5) and on ramips/mt7620 > > (bdcom,wap2100-sk) with WPS buttons. > > > > Acked-by: Petr Štetiar <ynezz@true.cz> > > Can you tell me what you tested? Was it the software debounce? > Because this should be the only bit that will affect the ath79 > platform I think (since it already has support for interrupts > through the gpio controller). it was just FYI, that I've checked it (run tested) and didn't noticed any side effects, possible regressions. I've simply added this patch on top of fix for FS#1965 and run tested it together. -- ynezz
On Thu, May 30, 2019 at 8:09 AM Christian Lamparter <chunkeey@gmail.com> wrote: > > Hello, > > On Thursday, May 30, 2019 12:00:27 PM CEST Petr Štetiar wrote: > > Christian Lamparter <chunkeey@gmail.com> [2019-05-29 21:58:29]: > > > Upstream Linux's input gpio-keys driver supports > > > specifying a external interrupt for a gpio via the > > > 'interrupts' properties as well as having support > > > for software debounce. > > > > [...] > > > > I've just checked this on ath79 (archer-c7-v5) and on ramips/mt7620 > > (bdcom,wap2100-sk) with WPS buttons. > > > > Acked-by: Petr Štetiar <ynezz@true.cz> > > Can you tell me what you tested? Was it the software debounce? > Because this should be the only bit that will affect the ath79 > platform I think (since it already has support for interrupts > through the gpio controller). > > From what I can tell, ramips should use gpio-keys-polled exclusivly > for now. This is because the rt2880-pinmux driver doesn't implement > and irq support (though some of the chips should support it). > all gpio-keys-polled should work as before. The upstream driver fixes this. See https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpio/gpio-mt7621.c?h=v4.19.47 > > Cheers, > Christian > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Petr Štetiar <ynezz@true.cz> [2019-05-30 17:30:18]: Hi, > > On Thursday, May 30, 2019 12:00:27 PM CEST Petr Štetiar wrote: > > > > > > I've just checked this on ath79 (archer-c7-v5) and on ramips/mt7620 > > > (bdcom,wap2100-sk) with WPS buttons. > > > > > > Acked-by: Petr Štetiar <ynezz@true.cz> > > > > Can you tell me what you tested? Was it the software debounce? > > Because this should be the only bit that will affect the ath79 > > platform I think (since it already has support for interrupts > > through the gpio controller). > > it was just FYI, that I've checked it (run tested) and didn't noticed any side > effects, possible regressions. I've simply added this patch on top of fix for > FS#1965 and run tested it together. apparently I didn't tested it well, someone has reported following on the IRC: 04:39:23 < kyli> After commit afc056d7dc, the button stopped working properly on my ramips/mt7620 target. The first press of a button after reboot will get a LARGE SEEN, and it will be interpreted as a long press. So even a short press on reset will trigger FACTORY RESET.... Is anyone experiencing the same problem? I've told him to report it on bugs.openwrt.org, I can do some testing and attempt to fix it on my mt7620 in the upcoming days. Cheers, Petr
On Github there is a PR about adding EFI image generation to the x86 target, but it has not been picked by anyone for a while. This is an important feature, can anyone look into merging it https://github.com/openwrt/openwrt/pull/1968 -Alberto
Petr Štetiar <ynezz@true.cz> [2019-06-02 14:06:30]: Hi, > > > On Thursday, May 30, 2019 12:00:27 PM CEST Petr Štetiar wrote: > > > > > > > > I've just checked this on ath79 (archer-c7-v5) and on ramips/mt7620 > > > > (bdcom,wap2100-sk) with WPS buttons. > > > > > > > > Acked-by: Petr Štetiar <ynezz@true.cz> > > > > > > Can you tell me what you tested? Was it the software debounce? > > > Because this should be the only bit that will affect the ath79 > > > platform I think (since it already has support for interrupts > > > through the gpio controller). > > > > it was just FYI, that I've checked it (run tested) and didn't noticed any side > > effects, possible regressions. I've simply added this patch on top of fix for > > FS#1965 and run tested it together. > > apparently I didn't tested it well, someone has reported following on the IRC: > > 04:39:23 < kyli> After commit afc056d7dc, the button stopped working properly > on my ramips/mt7620 target. The first press of a button after reboot will get > a LARGE SEEN, and it will be interpreted as a long press. So even a short > press on reset will trigger FACTORY RESET.... Is anyone experiencing the same > problem? so I can confirm this behaviour, but it's not caused by your patch as it doesn't work even with your patch reverted. There's simply unhandled corner case, where the seen is wrong if the button is pressed for the first time and I'll shortly send a patch which should fix this. -- ynezz
Also tested on an EFI only Asrock J5005-ITX. Builds, writes and boots fine. However since it is not from 18.06 dev and is built from LEDE you really cant do anything else with as luci wont install via opkg. If the commits can be pulled into openwrt-dev, I can test it on my Gigabit connection. On Sun, Jun 2, 2019 at 7:59 AM Alberto Bursi <bobafetthotmail@gmail.com> wrote: > On Github there is a PR about adding EFI image generation > > to the x86 target, but it has not been picked by anyone for a while. > > This is an important feature, can anyone look into merging it > > https://github.com/openwrt/openwrt/pull/1968 > > > -Alberto > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
On 05/06/19 07:22, John Braley wrote: > Also tested on an EFI only Asrock J5005-ITX. Builds, writes and boots > fine. However since it is not from 18.06 dev and is built from LEDE > you really cant do anything else with as luci wont install via opkg. > > If the commits can be pulled into openwrt-dev, I can test it on my > Gigabit connection. > > On Sun, Jun 2, 2019 at 7:59 AM Alberto Bursi > <bobafetthotmail@gmail.com <mailto:bobafetthotmail@gmail.com>> wrote: > > On Github there is a PR about adding EFI image generation > > I'm not sure about what you mean with "is built from LEDE". I built test images with luci, available here https://mega.nz/#F!HipgRIyS!_VxhEB5nqhU0rpmU4Rr8Tw since I have built them directly from the PR, you may or may not be able to install kernel-related packages from the repository. If you need specific packages in the test image I can include them. -Alberto
I've been running with some version or another of the EFI patches for about six months now; sysupgrade works fine, and there aren't any reboot or stability issues that I've seen. I'd be happy to contribute a review, especially if it helps EFI support land in master. Is there any desire to move forward with this? On Wed, Jun 5, 2019 at 3:33 AM Alberto Bursi <bobafetthotmail@gmail.com> wrote: > > > On 05/06/19 07:22, John Braley wrote: > > Also tested on an EFI only Asrock J5005-ITX. Builds, writes and boots fine. However since it is not from 18.06 dev and is built from LEDE you really cant do anything else with as luci wont install via opkg. > > If the commits can be pulled into openwrt-dev, I can test it on my Gigabit connection. > > On Sun, Jun 2, 2019 at 7:59 AM Alberto Bursi <bobafetthotmail@gmail.com> wrote: >> >> On Github there is a PR about adding EFI image generation > > > I'm not sure about what you mean with "is built from LEDE". > > I built test images with luci, available here > > https://mega.nz/#F!HipgRIyS!_VxhEB5nqhU0rpmU4Rr8Tw > > since I have built them directly from the PR, you may or may not be > > able to install kernel-related packages from the repository. > > If you need specific packages in the test image I can include them. > > -Alberto > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff --git a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c index 1aef23e876..6e730cdabe 100644 --- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c +++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c @@ -26,6 +26,7 @@ #include <linux/interrupt.h> #include <linux/platform_device.h> #include <linux/of_gpio.h> +#include <linux/of_irq.h> #include <linux/gpio_keys.h> #define DRV_NAME "gpio-keys" @@ -70,7 +71,10 @@ struct gpio_keys_button_data { int count; int threshold; int can_sleep; - struct gpio_keys_button *b; + int irq; + unsigned int software_debounce; + struct gpio_desc *gpiod; + const struct gpio_keys_button *b; }; extern u64 uevent_next_seqnum(void); @@ -297,7 +301,7 @@ static void gpio_keys_polled_check_state(struct gpio_keys_button_data *bdata) return; } - if ((bdata->last_state != -1) || (type == EV_SW)) + if (bdata->last_state != -1 || type == EV_SW) button_hotplug_event(bdata, type, state); bdata->last_state = state; @@ -339,11 +343,29 @@ static void gpio_keys_polled_close(struct gpio_keys_button_dev *bdev) pdata->disable(bdev->dev); } +static void gpio_keys_irq_work_func(struct work_struct *work) +{ + struct gpio_keys_button_data *bdata = container_of(work, + struct gpio_keys_button_data, work.work); + int state = gpio_button_get_value(bdata); + + if (state != bdata->last_state) { + unsigned int type = bdata->b->type ?: EV_KEY; + + if (bdata->last_state != -1 || type == EV_SW) + button_hotplug_event(bdata, type, state); + + bdata->last_state = state; + } +} + static irqreturn_t button_handle_irq(int irq, void *_bdata) { - struct gpio_keys_button_data *bdata = (struct gpio_keys_button_data *) _bdata; + struct gpio_keys_button_data *bdata = + (struct gpio_keys_button_data *) _bdata; - button_hotplug_event(bdata, bdata->b->type ?: EV_KEY, gpio_button_get_value(bdata)); + schedule_delayed_work(&bdata->work, + msecs_to_jiffies(bdata->software_debounce)); return IRQ_HANDLED; } @@ -389,7 +411,9 @@ gpio_keys_get_devtree_pdata(struct device *dev) continue; } - button = &pdata->buttons[i++]; + button = (struct gpio_keys_button *)(&pdata->buttons[i++]); + + button->irq = irq_of_parse_and_map(pp, 0); button->gpio = of_get_gpio_flags(pp, 0, &flags); if (button->gpio < 0) { @@ -516,6 +540,9 @@ static int gpio_keys_button_probe(struct platform_device *pdev, gpio, error); return error; } + bdata->gpiod = gpio_to_desc(gpio); + if (!bdata->gpiod) + return -EINVAL; error = gpio_direction_input(gpio); if (error) { @@ -528,12 +555,26 @@ static int gpio_keys_button_probe(struct platform_device *pdev, bdata->can_sleep = gpio_cansleep(gpio); bdata->last_state = -1; - if (bdev->polled) + if (bdev->polled) { bdata->threshold = DIV_ROUND_UP(button->debounce_interval, - pdata->poll_interval); - else + pdata->poll_interval); + } else { bdata->threshold = 1; + if (button->debounce_interval) { + error = gpiod_set_debounce(bdata->gpiod, + button->debounce_interval * 1000); + /* + * use timer if gpiolib doesn't provide + * debounce. + */ + if (error < 0) { + bdata->software_debounce = + button->debounce_interval; + } + } + } + bdata->b = &pdata->buttons[i]; } @@ -560,23 +601,39 @@ static int gpio_keys_probe(struct platform_device *pdev) pdata = bdev->pdata; for (i = 0; i < pdata->nbuttons; i++) { - struct gpio_keys_button *button = &pdata->buttons[i]; + const struct gpio_keys_button *button = &pdata->buttons[i]; struct gpio_keys_button_data *bdata = &bdev->data[i]; + unsigned long irqflags = IRQF_ONESHOT; - if (!button->irq) - button->irq = gpio_to_irq(button->gpio); - if (button->irq < 0) { - dev_err(&pdev->dev, "failed to get irq for gpio:%d\n", button->gpio); - continue; + if (!button->irq) { + bdata->irq = gpio_to_irq(button->gpio); + + if (bdata->irq < 0) { + dev_err(&pdev->dev, "failed to get irq for gpio:%d\n", + button->gpio); + continue; + } + + irqflags |= IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING; + } else { + bdata->irq = button->irq; } - ret = devm_request_threaded_irq(&pdev->dev, button->irq, NULL, button_handle_irq, - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT, - dev_name(&pdev->dev), bdata); - if (ret < 0) - dev_err(&pdev->dev, "failed to request irq:%d for gpio:%d\n", button->irq, button->gpio); - else - dev_dbg(&pdev->dev, "gpio:%d has irq:%d\n", button->gpio, button->irq); + INIT_DELAYED_WORK(&bdata->work, gpio_keys_irq_work_func); + + ret = devm_request_threaded_irq(&pdev->dev, + bdata->irq, NULL, button_handle_irq, + irqflags, dev_name(&pdev->dev), bdata); + + if (ret < 0) { + bdata->irq = 0; + dev_err(&pdev->dev, "failed to request irq:%d for gpio:%d\n", + bdata->irq, button->gpio); + continue; + } else { + dev_dbg(&pdev->dev, "gpio:%d has irq:%d\n", + button->gpio, bdata->irq); + } if (bdata->b->type == EV_SW) button_hotplug_event(bdata, EV_SW, gpio_button_get_value(bdata)); @@ -612,6 +669,19 @@ static int gpio_keys_polled_probe(struct platform_device *pdev) return ret; } +static void gpio_keys_irq_close(struct gpio_keys_button_dev *bdev) +{ + struct gpio_keys_platform_data *pdata = bdev->pdata; + size_t i; + + for (i = 0; i < pdata->nbuttons; i++) { + struct gpio_keys_button_data *bdata = &bdev->data[i]; + + disable_irq(bdata->irq); + cancel_delayed_work_sync(&bdata->work); + } +} + static int gpio_keys_remove(struct platform_device *pdev) { struct gpio_keys_button_dev *bdev = platform_get_drvdata(pdev); @@ -620,6 +690,8 @@ static int gpio_keys_remove(struct platform_device *pdev) if (bdev->polled) gpio_keys_polled_close(bdev); + else + gpio_keys_irq_close(bdev); return 0; }
Upstream Linux's input gpio-keys driver supports specifying a external interrupt for a gpio via the 'interrupts' properties as well as having support for software debounce. This patch ports these features to OpenWrt's event version. Only the "pure" interrupt-driven support is left behind, since this goes a bit against the "gpio" in the "gpio-keys" and I don't have a real device to test this with. This patch also silences the generated warnings showing up since 4.14 due to the 'constification' of the struct gpio_keys_button *buttons variable in the upstream struct gpio_keys_platform_data declaration. gpio-button-hotplug.c: In function 'gpio_keys_get_devtree_pdata': gpio-button-hotplug.c:392:10: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] button = &pdata->buttons[i++]; ^ gpio-button-hotplug.c: In function 'gpio_keys_button_probe': gpio-button-hotplug.c:537:12: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] bdata->b = &pdata->buttons[i]; ^ gpio-button-hotplug.c: In function 'gpio_keys_probe': gpio-button-hotplug.c:563:37: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] struct gpio_keys_button *button = &pdata->buttons[i]; ^ Signed-off-by: Christian Lamparter <chunkeey@gmail.com> --- .../src/gpio-button-hotplug.c | 114 ++++++++++++++---- 1 file changed, 93 insertions(+), 21 deletions(-)