Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
Hello Petr, On 13.06.19 21:50, Petr Štetiar wrote: > David Bauer [2019-06-11 23:52:46]: > > Hi, > >> I ran into problems booting multiple QCA9558 boards, namely the OCEDO >> Koala and the devolo WiFi pro 1200e with the current master. Both >> devices always go into failsafe mode when powering on. > > ar71xx or ath79? Can you test if it happens also with the interrupt based > `gpio-keys` variant? I did some further testing, the issue on my Koala was a hardware fault (the reset switch was physically broken m( ). With the hardware button fixed, it does not show this behavior. This was expected though as it uses polled gpio-keys instead of the interrupt based ones. The devolo WiFi pro 1200e is ath79 with the interrupt-based gpio-keys. The issue is not present when using polled gpio-keys. The device does not go into failsafe when using polled GPIO keys. > >> I haven't dug deeper into this issue, but reverting 6c5bfaac84 leads >> into both boards booting normally. > > could you please compile kernel with `CONFIG_KERNEL_DYNAMIC_DEBUG=y` and then > add to the kernel cmdline `gpio_button_hotplug.dyndbg='file gpio-button* +p'` > (or to modprobe args) and provide the output? I did some further testing and i think i've found the culprit - On probe the GPIO reads high. Shortly after probe, an interrupt is received and the GPIO switched to low which triggers a button event and therefore sends the device into failsafe. I suppose the GPIO has just switched to a stable state which triggers this interrupt. I've prepared a patch which reads the initial state after the debounce interval on probe to ensure only real button presses will trigger an event. I will send this patch shortly. Best wishes David > > Thanks. > > -- ynezz > ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
David Bauer [2019-06-11 23:52:46]: Hi, > I ran into problems booting multiple QCA9558 boards, namely the OCEDO > Koala and the devolo WiFi pro 1200e with the current master. Both > devices always go into failsafe mode when powering on. ar71xx or ath79? Can you test if it happens also with the interrupt based `gpio-keys` variant? > I haven't dug deeper into this issue, but reverting 6c5bfaac84 leads > into both boards booting normally. could you please compile kernel with `CONFIG_KERNEL_DYNAMIC_DEBUG=y` and then add to the kernel cmdline `gpio_button_hotplug.dyndbg='file gpio-button* +p'` (or to modprobe args) and provide the output? Thanks. -- ynezz ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
Hello, On 05.06.19 22:23, Christian Lamparter wrote: > I had to attach the patch as a file since gmail's webmail interface now seems > to > eat all the tabs. I hope this still gets through. I ran into problems booting multiple QCA9558 boards, namely the OCEDO Koala and the devolo WiFi pro 1200e with the current master. Both devices always go into failsafe mode when powering on. I haven't dug deeper into this issue, but reverting 6c5bfaac84 leads into both boards booting normally. Best wishes David ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
On Sunday, June 9, 2019 2:50:41 PM CEST Petr Štetiar wrote: > Christian Lamparter [2019-06-09 10:06:33]: > > Hi, > > > The APM821xx checks out with both as well. While there are spurious > > events on enabling the interrupt (one released event), > > the /etc/rc.button/ scripts are setup to handle that. So, which patch > > should we take and who gets the merge them? (I've seen that ynezz has > > more patches as well.) > > I think, that you're correct and we should stick to the previous behaviour, so > I've taken your version of the patch included in this email and will push it > with my other patches. > > -- ynezz > Ok, thanks. OT: In return, I've poked greg to include: "mtd: spinand: macronix: Fix ECC Status Read" into 4.19-stable which he did: https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=aa07b770157b1f024c807b938a6f8225f73dff04 I'll see if I can get more patches backported this way. Cheers, Christian ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
Christian Lamparter [2019-06-09 10:06:33]: Hi, > The APM821xx checks out with both as well. While there are spurious > events on enabling the interrupt (one released event), > the /etc/rc.button/ scripts are setup to handle that. So, which patch > should we take and who gets the merge them? (I've seen that ynezz has > more patches as well.) I think, that you're correct and we should stick to the previous behaviour, so I've taken your version of the patch included in this email and will push it with my other patches. -- ynezz ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
Hi, On Sun, Jun 9, 2019 at 10:06 AM Christian Lamparter wrote: > @ynezz, @Kristian > > The APM821xx checks out with both as well. While there are spurious > events on enabling the interrupt (one released event), > the /etc/rc.button/ scripts are setup to handle that. So, which patch > should we take and who gets the merge them? (I've seen that ynezz has > more patches as well.) I am unfortunately not familiar enough with the code in question to have an opinion on which patch is the most correct or the best way for going forward. I will let you two decide on which patch to merge and who gets the honor :) BR, Kristian ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
On Thursday, June 6, 2019 6:52:35 AM CEST Kristian Evensen wrote:
> Hi Christian,
>
> On Wed, Jun 5, 2019 at 10:23 PM Christian Lamparter
> wrote:
> > @Kristian Evensen, can you please check if the following patch would also
> > resolve the issues you have been experiencing?
> >
> > I had to attach the patch as a file since gmail's webmail interface now
> > seems to
> > eat all the tabs. I hope this still gets through.
>
> Patch arrived safe and sound, and I just finished my tests on the
> ZBT-WD323 (AR9344). I started out by building a fresh image from
> master (head of my tree is commit 66d1c29655a4), and with this image I
> saw the earlier reported behavior (a press of the button triggers
> factory reset). I then applied your patch on top of my tree and the
> button now works as expected. A short press triggers reboot, and
> holding the button for ~5 seconds triggers a factory reset.
@ynezz, @Kristian
The APM821xx checks out with both as well. While there are spurious
events on enabling the interrupt (one released event),
the /etc/rc.button/ scripts are setup to handle that. So, which patch
should we take and who gets the merge them? (I've seen that ynezz has
more patches as well.)
Cheers,
Christian
---
From 0a46c8adb4d0dd288c6a646dd53757c6805e584a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C5=A0tetiar?=
Date: Sat, 8 Jun 2019 01:05:32 +0200
Subject: [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first
event
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit afc056d7dc83 ("gpio-button-hotplug: support interrupt
properties") changed the gpio-keys interrupt handling logic in a way,
that it always misses first event, which causes issues with rc.button
scripts, so this patch restores the previous behaviour.
Fixes: afc056d7dc83 ("gpio-button-hotplug: support interrupt properties")
Reported-by: Kristian Evensen
Signed-off-by: Petr Štetiar
Signed-off-by: Christian Lamparter [drop state check]
---
.../gpio-button-hotplug/src/gpio-button-hotplug.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
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 11c914d4ef..6de8f56cdf 100644
--- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
+++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
@@ -348,16 +348,9 @@ 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;
- }
+ button_hotplug_event(bdata, bdata->b->type ?: EV_KEY,
+gpio_button_get_value(bdata));
}
static irqreturn_t button_handle_irq(int irq, void *_bdata)
--
2.20.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
Hi Christian, On Wed, Jun 5, 2019 at 10:23 PM Christian Lamparter wrote: > @Kristian Evensen, can you please check if the following patch would also > resolve the issues you have been experiencing? > > I had to attach the patch as a file since gmail's webmail interface now seems > to > eat all the tabs. I hope this still gets through. Patch arrived safe and sound, and I just finished my tests on the ZBT-WD323 (AR9344). I started out by building a fresh image from master (head of my tree is commit 66d1c29655a4), and with this image I saw the earlier reported behavior (a press of the button triggers factory reset). I then applied your patch on top of my tree and the button now works as expected. A short press triggers reboot, and holding the button for ~5 seconds triggers a factory reset. BR, Kristian ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
On Tue, Jun 4, 2019 at 3:05 PM Petr Štetiar wrote:
>
> Commit afc056d7dc83 ("gpio-button-hotplug: support interrupt
> properties") changed the gpio-keys interrupt handling logic in a way,
> that it always misses first event, which causes issues with rc.button
> scripts, so this patch restores the previous behaviour.
>
> Cc: Christian Lamparter
> Fixes: afc056d7dc83 ("gpio-button-hotplug: support interrupt properties")
> Reported-by: Kristian Evensen
> Signed-off-by: Petr Štetiar
> ---
>
> 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 f429f8c0271f..81697e9c4cf6 100644
> --- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
> +++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
> @@ -344,10 +344,7 @@ static void gpio_keys_irq_work_func(struct work_struct
> *work)
>
> 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);
> -
> + button_hotplug_event(bdata, type, state);
> bdata->last_state = state;
> }
> }
Thanks. initially I ran into issues with the WNDR4700 and WNDAP630 when
I was testing the interrupt-driven gpio-keys. On boot-up, they would produce
spurious ghost key presses when gpio-keys enabled the interrupt for the first
time. I'll test this again (don't have the HW at the moment... but I will on
Sunday),
Note: If we want to revert to the previous behavior (afc056d7dc83) and closer
to upstream gpio_keys.c. we have to drop even more.
@Kristian Evensen, can you please check if the following patch would also
resolve the issues you have been experiencing?
I had to attach the patch as a file since gmail's webmail interface now seems to
eat all the tabs. I hope this still gets through.
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 11c914d4ef..6de8f56cdf 100644
--- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
+++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
@@ -348,16 +348,9 @@ 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;
- }
+ button_hotplug_event(bdata, bdata->b->type ?: EV_KEY,
+ gpio_button_get_value(bdata));
}
static irqreturn_t button_handle_irq(int irq, void *_bdata)
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: fix always missing first event
Commit afc056d7dc83 ("gpio-button-hotplug: support interrupt
properties") changed the gpio-keys interrupt handling logic in a way,
that it always misses first event, which causes issues with rc.button
scripts, so this patch restores the previous behaviour.
Cc: Christian Lamparter
Fixes: afc056d7dc83 ("gpio-button-hotplug: support interrupt properties")
Reported-by: Kristian Evensen
Signed-off-by: Petr Štetiar
---
package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
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 f429f8c0271f..81697e9c4cf6 100644
--- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
+++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
@@ -344,10 +344,7 @@ static void gpio_keys_irq_work_func(struct work_struct
*work)
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);
-
+ button_hotplug_event(bdata, type, state);
bdata->last_state = state;
}
}
--
1.9.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
