Re: [PATCH] gpio: pl061: Support implementations without GPIOINTR line

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Mon, Mar 22, 2021 at 1:04 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

> The thing is that hierarchical interrupts are supposed to
> connect the lines by absolute offsets that are *not* coming
> from the device tree. This is the pattern taken by other
> in-tree hierarchical GPIO controllers. We have repeatedly
> NACKed patches adding all the IRQs to hierarchical
> GPIO interrupt controllers, in favor of using hardcoded
> offsets in the driver.
>
> Do you have some good idea of how we can achieve that?

One way would be to stack more compatible strings:

compatible = "lsi,axm5516-primary-gpio", "arm,pl061", "arm,primecell";

Going from more to less specific. We see that this is a
PL061 and that it is a primecell, but we also see that
it is a version specifically integrated into the axm5516.

I do see that today it looks like this
arch/arm/boot/dts/axm55xx.dtsi:

gpio0: gpio@2010092000 {
    #gpio-cells = <2>;
    compatible = "arm,pl061", "arm,primecell";
    gpio-controller;
    reg = <0x20 0x10092000 0x00 0x1000>;
    interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
        <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clks AXXIA_CLK_PER>;
    clock-names = "apb_pclk";
    status = "disabled";
};

(Indeed this doesn't currently work with Linux, thus this
patch.)

It is indeed specified in the schema right now as:

  interrupts:
    oneOf:
      - maxItems: 1
      - maxItems: 8

So from a devicetree PoV all is good. But it is not the
way hierarchical IRQs are supposed to be done IIUC.
The preferred solution is to use a specific compatible
string and hardcoded offsets.

It'd be nice if the interrupt or DT binding people would say
something about how they expect these hierarchical IRQs
to be specified from the device tree. I'm just representing
earlier review comments here, maybe they've changed
their mind.

Yours,
Linus Walleij