Re: [PATCH 2/2] gpio: lpc18xx: add GPIO pin interrupt controller support

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Wed, Nov 28, 2018 at 11:48 PM Vladimir Zapolskiy <vz@xxxxxxxxx> wrote:

> The change adds support of LPC18xx/LPC43xx GPIO pin interrupt controller
> block within SoC GPIO controller. The new interrupt controller driver
> allows to configure and capture edge or level interrupts on 8 arbitrary
> selectedinput GPIO pins, and lift the signals to be reported as NVIC rising
> edge interrupts. Configuration of a particular GPIO pin to serve as
> interrupt and its mapping to an interrupt on NVIC is done by SCU pin
> controller, for more details see description of 'nxp,gpio-pin-interrupt'
> device tree property of a GPIO pin [1].
>
> From LPC18xx and LPC43xx User Manuals the GPIO controller consists of
> the following blocks:
> * GPIO pin interrupt block at 0x40087000, this change adds its support,
> * GPIO GROUP0 interrupt block at 0x40088000,
> * GPIO GROUP1 interrupt block at 0x40089000,
> * GPIO port block at 0x400F4000, it is supported by the original driver.
>
> While all 4 sub-controller blocks have their own I/O addresses, moreover
> all 3 interrupt blocks are APB0 peripherals and high-speed GPIO block is
> an AHB slave, according to the hardware manual the GPIO controller is
> seen as a single block, and 4 sub-controllers have the shared reset signal
> RGU #28 and clock to register interface CLK_CPU_GPIO on CCU1.
>
> Likely support of two GPIO group interrupt blocks won't be added in short
> term, because the mechanism to mask several interrupt sources is not well
> defined.
>
> [1] Documentation/devicetree/bindings/pinctrl/nxp,lpc1850-scu.txt
>
> Signed-off-by: Vladimir Zapolskiy <vz@xxxxxxxxx>

It's quite complex, but your implementation is so crisp and clean
that there is nothing to complain about really.

We are discussing putting some hierarchy handling into the
gpiolib core, so you might want to simplify the code later to
make use of this, but for now it's perfectly fine.

Patch applied.

Yours,
Linus Walleij