Re: [PATCH] Fix freeze in lm8333 i2c keyboard driver

[Jeff LaBundy]

Hi Tomas,

On Thu, Apr 27, 2023 at 10:19:38AM +0200, Tomáš Mudruňka wrote:
> > Yes that's correct; what I mean to say is that depending on the nature of
> > the read-to-clear mechanism in the part, there is a chance that the IRQ
> > has not been deasserted by the time the threaded handler returns. On some
> > devices for example, the IRQ is not deasserted until some time after the
> > read's stop condition.
> >
> > For these cases, I consider it best practice to measure the I2C and IRQ
> > lines on a scope and if necessary, add a small delay before the interrupt
> > handler returns. This is especially true for open-drain interrupts that
> > may need a few hundred extra us for the pin to rise.
> 
> Well before posting the patch i did some testing.
> I was watching the /proc/interrupts and checked that IRQ counter for
> lm8333 matches number of keypresses.
> Which i've only tested for like 20-30 times, but haven't seem any glitch.
> 
> But i still recognize the fact that the gpio line getting stuck for
> some reason (short circuit on PCB?) might cause troubles by
> unnecessarily loading the CPU, while with edge trigger it's more
> likely to affect only the function of keyboard itself rather than
> bringing down whole system. But i am not sure if this case is supposed
> to be expected and handled in SW.

In the case of short circuit, the hardware has failed and there is nothing
we can do.

My point is that different devices deassert their IRQ at different points
in the read-to-clear operation. For some devices, the IRQ is deasserted
immediately after the register address is latched, so we can be confident
that the IRQ has already gone high by the time the I2C read operation and
hence the interrupt handler return.

On others however, the IRQ may still remain low for 10's or even 100's of
us after the read is complete. In some cases, the threaded handler could
have already returned by then. Since you did not find any unexplained IRQ
counts, perhaps that is not the case for this device.

Kind regards,
Jeff LaBundy