Re: [RFC] Input: matrix_keypad: fix interrupt regression introduced by commit 01c84b03d80a

[Siarhei Volkau]

> Siarhei, do you remember the context of that patch?

Yes. My understanding is that at least some Ingenic GPIO controllers
don't report the current input level while the pin is configured for IRQs,
but instead expose the level latched at the moment the interrupt occurred.
That's what I was trying to work around.

> That was definitely working on the SoC I tested ...

You wouldn't notice this with the gpio-keys driver because its usage
pattern matches this behavior. For example, if the IRQ is configured
for the falling edge, then when the interrupt fires the level register will
read 0, which is exactly what gpio-keys expects.

I'm not sure whether this affects all Ingenic GPIO controllers,
but it should be easy to verify. For testing, you could temporarily
modify the driver as follows:

In the IRQ handler, before reconfiguring the IRQ for the opposite edge:
- read and print the GPIO level register/bit;
- wait for an arbitrary amount of time (long enough to release the
button between the two reads);
- read and print the GPIO level register/bit again.

If both reads return 0, even though the button has been released,
then the controller behaves like the JZ4755 and latches the GPIO
level at the time of the interrupt.
If the first read returns 0 and the second returns 1, then the controller
reports the live GPIO input level, as typical GPIO controllers do.

> I wonder if you were just experiencing bouncing?

I'm sure no, I remember that without that patch, I got the same readings
for all buttons in a row, no matter which column was active and as far as I
remember I tested it with arbitrary long col-scan-delay-us in range of
10-100 ms.

BR,
Siarhei

чт, 9 июл. 2026 г. в 13:25, Paul Cercueil <paul@xxxxxxxxxxxxxxx>:
>
> Hi,
>
> Siarhei, do you remember the context of that patch?
>
> The driver will read the pin level to emulate pins configured in "both
> edges" IRQ mode, and switch to "rising edge" if low, and "falling edge"
> if high. That was definitely working on the SoC I tested it with
> (probably JZ4770). I would be surprised if it did not work on the
> JZ4755.
>
> You mention in that commit message that you would get the pin level
> before the interrupt happened, I wonder if you were just experiencing
> bouncing?
>
> Anyway, I agree that this commit can be reverted.
>
> Le mercredi 08 juillet 2026 à 18:59 +0300, Siarhei Volkau a écrit :
> > Regarding JZ4755, I agree that the [1] patch can be reverted.
> > No device requiring this workaround has landed in the mainline.
> > qi,lb60 (Ben Nanonote) seems unaffected as it was there before
> > the patch was proposed.
> >
> > However, if there are genuinely two hardware operating modes:
> > - "GPIO input mode" while scanning
> > - "GPIO interrupt input mode" while idle
> > then the keypad driver should be aware of this distinction.
> >
> > I'd like to propose pinctrl state transitions as the mechanism for
> > switching
> > these modes, wherever it makes sense (Allwinner et al).
>
> I agree.
>
> >
> > CC Paul as the maintainer of Ingenic pinctrl driver.
> >
> > BR,
> > Siarhei
>
> Cheers,
> -Paul
>
> >
> >
> > ср, 8 июл. 2026 г. в 16:30, Alexander Kochetkov
> > <al.kochet@xxxxxxxxx>:
> > >
> > > Hi everyone,
> > >
> > > I found a commit [1] that breaks the matrix_keypad behavior on the
> > > Allwinner A64.
> > >
> > > I have a PINE A64-LTS board with a connected keypad polled by
> > > matrix_keypad.
> > > This commit caused interrupts to stop arriving at the matrix_keypad
> > > driver entirely.
> > >
> > > An explicit call to gpiod_direction_input() disables interrupt
> > > reception on all chips
> > > where the GPIO input mode and GPIO interrupt input mode are
> > > configured via
> > > the pinmux register. Interrupt reception is guaranteed to break on
> > > Allwinner
> > > (A64, H3, H6, etc.), Broadcom (BCM2835/2711), and some Rockchip
> > > SoCs. It
> > > does not break on NXP i.MX (i.MX6, i.MX8), STMicroelectronics
> > > (STM32MP1),
> > > TI Sitara (AM335x), or Intel/AMD.
> > >
> > > Furthermore, the assumption that enable_row_irqs() restores the
> > > interrupt mode is
> > > also specific to the Ingenic pinctrl. In the vast majority of
> > > drivers, enable_row_irqs() is
> > > supposed to simply set the interrupt enable mask without changing
> > > the pinmux.
> > >
> > > Commit [1] was introduced to work around a hardware limitation in
> > > Ingenic's JZ4755.
> > > This specific behavior is unique to Ingenic and a small number of
> > > specialized chips.
> > > The majority of SoCs (around 90%) allow reading a GPIO input that
> > > is currently configured
> > > as an interrupt source.
> > >
> > > In my opinion, the correct approach would be to revert this commit
> > > and fix the behavior of
> > > ingenic_gpio_get_value() inside pinctrl-ingenic.c instead. However,
> > > I do not own an Ingenic
> > > board, so I won't be able to test such a patch.
> > >
> > > Alternatively, I could introduce a DTS property like read-gpio-
> > > quirk in matrix_keypad to restore
> > > the original driver behavior by default, and add this property to
> > > qi_lb60.dts so as not to break
> > > the Ingenic platform.
> > >
> > > What do you think? What is the best way to proceed here?
> > >
> > > Best regards,
> > > Alexander Kochetkov
> > >
> > > [1] commit 01c84b03d80aab9f04c4e3e1f9085f4202ff7c29 ("Input:
> > > matrix_keypad - force switch rows to input mode")