[lkml]   [2022]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 3/3] gpio: regmap: Support a custom ->to_irq() hook

Linus Walleij <> writes:

> On Tue, Jul 5, 2022 at 1:08 PM Aidan MacDonald
> <> wrote:
>> Linus Walleij <> 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.

Yep, that's right.

> // 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.)

If moving to hierarchical IRQ chips is the plan, could we add a note
to say .to_irq() is discouraged and shouldn't be used in new code?
Based on what you're saying (which I agree makes sense) it sounds
like there's really no reason to ever use .to_irq().

>> 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

Okay, that sounds like a good plan. I'll look more carefully at the
existing drivers and see if I can use existing gpiolib helpers.

One potential issue (from reading the code) is that hierarchical IRQ
domains seemingly can't have a non-hierarchical domain as the parent:
irq_domain_alloc_irqs_parent() calls irq_domain_alloc_irqs_hierarchy()
and the latter fails with -ENOSYS for a non-hierarchical domain.

In my case I'm using a regmap IRQ chip, which is non-hierarchical,
so perhaps that will need to be expanded?


 \ /
  Last update: 2022-07-06 16:22    [W:0.098 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site