Thread (3 messages) 3 messages, 2 authors, 2021-03-22

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

From: Alexander Sverdlin <hidden>
Date: 2021-03-22 12:38:12
Also in: linux-gpio

Hello Linus,

On 22/03/2021 13:17, Linus Walleij wrote:
quoted
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.
The problem is, it's not the only SoC with this "issue".
AXM56xx and AXC67xx will follow, and these "hardcoded offsets"
will be different. We are not going to add a compatible for
PL061 per SoC, are we?

Well, you can always merge v1:
https://lore.kernel.org/linux-gpio/20170222123049.17588-1-alexander.sverdlin@nokia.com/ (local)

I have a ported version of it as well.
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.
-- 
Best regards,
Alexander Sverdlin.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help