[0/3] gpio: lpc32xx: enable interrupts for port 3

Hi,

This series enables interrupt support for GPIOs on port 3. Those are the
GPIOs that are connected directl to an interrupt controller (sic1 or
sic2). This was tested on a custom LPC3250 design.

The binding includes support for interrupts on port 0 and port 1 but
this requires the registration of a proper irqchip and I don't have any
way to test it.

Patch 1 and 2 should probably go through the gpio tree while patch 3
should go through arm-soc. They are quite independant and this shoud
cause no issue.

Alexandre Belloni (3):
  dt-bindings: gpio: lpc32xx: document interrupt bindings
  gpio: lpc32xx: enable interrupt lookup for port 3
  ARM: dts: lpc32xx: add GPIO interrupts

 .../devicetree/bindings/gpio/gpio_lpc32xx.txt |  4 +++
 arch/arm/boot/dts/lpc32xx.dtsi                | 25 +++++++++++++++++++
 drivers/gpio/gpio-lpc32xx.c                   | 14 ++++-------
 3 files changed, 34 insertions(+), 9 deletions(-)

On Wed, Apr 10, 2019 at 12:39 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:

> Interrupt support was disabled "temporarily" in commit 320a6480ef24 ("gpio:
> lpc32xx: disable broken to_irq support").
>
> Reenable to_irq for port 3 as they are directly connected to an interrupt
> controller and a simple lookup is working.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Unfortunately this seems to be one of these hacks for
hierarchical interrupts that does not quite cut it.

> +static int lpc32xx_gpio_to_irq_p3(struct gpio_chip *chip, unsigned int offset)
> +       return of_irq_get_byname(chip->of_node, chip->names[offset]);

This works as long as consuming drivers pick a GPIO first
using gpiod_get() (and variants) and then convert it to an
IRQ using gpiod_to_irq().

But it doesn't work when some consumer is just picking an
IRQ off of the node without picking the GPIO first.
And in DT that is OK, this DT node is definately an interrupt
controller. (Leaving out the attribute for interrupt controller
as is currently the case doesn't really fix the issue, it is
just inconsistent.)

Look at Brian Masney's series for the qualcomm pin controller
chips for inspiration on how to do things right, see:
commit 9d2b563bc23acfa93e7716b3396fd2f79fa8f0cd
and down.

Also:
https://marc.info/?l=linux-gpio&m=154959228527643&w=2

Especially note how he removes the kind of hack you are
adding in e.g.
commit a796fab2c605d6340512c51c6c3fa1c581357993

Yours,
Linus Walleij

On Thu, Apr 11, 2019 at 9:55 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Apr 10, 2019 at 12:39 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
>
> > Interrupt support was disabled "temporarily" in commit 320a6480ef24 ("gpio:
> > lpc32xx: disable broken to_irq support").
> >
> > Reenable to_irq for port 3 as they are directly connected to an interrupt
> > controller and a simple lookup is working.
> >
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>
> Unfortunately this seems to be one of these hacks for
> hierarchical interrupts that does not quite cut it.
>
> > +static int lpc32xx_gpio_to_irq_p3(struct gpio_chip *chip, unsigned int offset)
> > +       return of_irq_get_byname(chip->of_node, chip->names[offset]);
>
> This works as long as consuming drivers pick a GPIO first
> using gpiod_get() (and variants) and then convert it to an
> IRQ using gpiod_to_irq().
>
> But it doesn't work when some consumer is just picking an
> IRQ off of the node without picking the GPIO first.
> And in DT that is OK, this DT node is definately an interrupt
> controller. (Leaving out the attribute for interrupt controller
> as is currently the case doesn't really fix the issue, it is
> just inconsistent.)
>
> Look at Brian Masney's series for the qualcomm pin controller
> chips for inspiration on how to do things right, see:
> commit 9d2b563bc23acfa93e7716b3396fd2f79fa8f0cd
> and down.
>
> Also:
> https://marc.info/?l=linux-gpio&m=154959228527643&w=2
>
> Especially note how he removes the kind of hack you are
> adding in e.g.
> commit a796fab2c605d6340512c51c6c3fa1c581357993
>
> Yours,
> Linus Walleij
>

Hi Alexandre,

you can also look at the original attempt, in 2015, to replace
this driver by a new version:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/386708.html

Regards,
Sylvain

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel