Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

Mark Brown
2009-10-07T10:07:19Z

On Wed, Oct 07, 2009 at 09:50:43AM +0100, Hennerich, Michael wrote:

> As you said there are chip internal interrupt sources for I/Os, keypad
> presses and releases, ambient light sensor comparator states, and
> overvoltage conditions.
> The current state of the driver uses only interrupts for the keypad.
> I think you agree that its common practice to only implement
> functionality for chip features that are typically used.

Yes, but equally well it's good to avoid future issues as the driver is
enhanced. To be honest the main thing that made me notice when reading
the patch was the use of the blocking notifier pattern to implement the
interrupt infrastructure, mostly since it doesn't look like an IRQ
driver and the naming hides the fat that that's what it is.

I think I forgot to mention it previously but there's some work on
getting a standard ALS interface in the kernel too. I'd really expect
the GPIOs to end up being used as GPIOs in some designs as well.

> In one of your earlier posts you mentioned: "register an irq_chip for
> the interrupt controller on it. Support for doing this on I2C devices
> was added at pretty much the same time as the IRQ_ONESHOT support."

> Can you point me to what exactly was added to support this on I2C/SPI
> devices?

These commits (which were merged during the merge window):

4dbc9ca genirq: Do not mask oneshot edge type interrupts
399b5da genirq: Support nested threaded irq handling
70aedd2 genirq: Add buslock support

There may be some other fixup patches afterwards too, but the buslock
and nested threaded handers are the key ones.