[OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: read initial state

[David Bauer]

Hello Linus,

On 17.06.19 01:42, Linus Walleij wrote:
> It shows what the problem is for sure. delays are forbidden in
> irqchip callback functions since they are all called from
> IRQ context with IRQs disabled though.

Good to know, thanks!

>> @@ -96,6 +97,12 @@ static void ath79_gpio_irq_enable(struct irq_data *data)
> 
>>         u32 mask = BIT(irqd_to_hwirq(data));
>>         unsigned long flags;
>>
>> +       /*
>> +        * The input can be unstable after configuring GPIO direction.
>> +        * Wait a bit to assert the input is stable.
>> +        */
>> +       msleep(25);
>> +
>>         raw_spin_lock_irqsave(&ctrl->lock, flags);
>>         ath79_gpio_update_bits(ctrl, AR71XX_GPIO_REG_INT_ENABLE, mask, mask);
>>         ath79_gpio_update_bits(ctrl, AR71XX_GPIO_REG_INT_MASK, mask, mask);
> 
> After this and before the raw_spin_unlock() try to read the status
> register until it
> eads zero for the requested IRQ:
> 
> while (ath79_gpio_read(ctrl, AR71XX_GPIO_REG_INT_PENDING) & mask) {}
> 
> This way we wait for the status to go low before we allow any IRQs to
> fire after enableing.

Thanks for your suggestion. However, with your suggestion i still see the ghost press on bootup :(
Just to clarify - I do not see multiple ghost presses, just one. The button is active low. 

After configuring the GPIO direction to input, the value of the GPIO reads 0 (pressed).
After ~10ms, this changes to 1 (not pressed). I suppose your proposed solution does not work
as interrupts are only registered after configuring the GPIO line as input and the GPIO line
changes after registering the interrupt. So we are reading the interrupt state too early.

(I might have repeated myself with this explanation but I was not sure if I've given
all the relevant facts "in one piece" yet)

I'm not sure where we can go from here. I have two ideas,
but I'm not sure if they are feasible:

First:
If delays are allowed there, we could add a 20ms delay when configuring the GPIO line
direction as input. This way we could also assure the line is stable for "normal" value
reads. We would need to override the direction_input method provided by gpio-mmio for this.

Second:
We could store the kernel uptime together with the GPIO line and for how long interrupts
should be ignored within the driver. This could be checked when an interrupt is fired.

I do not really like the second idea as dropping interrupts without feedback is probably not
what we want.

Do you have another idea how we could solve this?

Best wishes
David


> 
> Maybe not so good since we may want to turn on IRQs that are asserted
> at some point, but worth a try.
> 
> Yours,
> Linus Walleij
> 

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel