Re: [PATCH 3/3] gpio: regmap: Support a custom ->to_irq() hook

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Tue, Jul 5, 2022 at 1:08 PM Aidan MacDonald
<aidanmacdonald.0x0@xxxxxxxxx> wrote:
> Linus Walleij <linus.walleij@xxxxxxxxxx> writes:

> I'm not trying to argue that hierarchical IRQ domains are always a bad
> thing -- I'm just pointing out they're not always useful or necessary.
> All your points make sense when the GPIO controller is a large distinct
> block with potentially many GPIOs. When we're dealing with an MFD device
> with just a few GPIOs, maybe even just one, having a separate IRQ domain
> makes less sense; the added structure is generally not useful.

Do you mean your driver does this:

MFD main device
MFD irqchip
 |
 +->  MFD gpiochip
         No irqchip here, so .to_irq() just refers ^ to that one up there

IIUC you mean that if I want to use the irqchip directly then
I have to refer to the MFD irqchip, I just cannot refer to the
gpiochip subnode because that one does not have an irqchip.

// Getting GPIO from gpiochip and irq from MFD device
// for the same GPIO line
gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
irqs = <&mfd 114 IRQ_EDGE_RISING>;

Then for a Linux driver this can be papered over by using the
.to_irq() callback and just defining gpios.

This isn't very good, if you created a separate gpiochip then you
should have a separate (hierarchical) irqchip associated with that
gpiochip as well.

// Getting GPIO and irq from the same gpiochip node
gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
irqs = <&gpio 3 IRQ_EDGE_RISING>;

I made this mistake with the ab8500 driver and
I would not do it like this today. I would use hierarchical gpio
irqchip. And I should go and fix it. (Is on my TODO.)

> Looking at other GPIO drivers using a hierarchical IRQ domain, they
> include their own IRQ chips with specialized ops. In my case I don't
> need any of that (and it'd be the same with other MFD devices) so it
> looks like using an IRQ domain would mean I'd have to create a fake
> IRQ chip and domain just to translate between two number spaces.
>
> Is that really better than simply using ->to_irq()?

To be honest most irqchips are "fake", what they mostly do is figure
out which of a few internal sources that fired the irq, so it models the
different things connected to a single IRQ line.

So yeah, I think the hierarchical irqchip is worth it, especially if that
means the offset of the irqs and gpios become the same.

Maybe we can add more helpers in the core to make it dirt simple
though? It would help others with the same problem.

Yours,
Linus Walleij