linux-kernel - Re: [PATCH 3/3] gpio: regmap: Support a custom ->to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <GrX3yDSwNOGIBcWmKqusaJ3dDqNGLr3Y@localhost>
Date:   Tue, 05 Jul 2022 12:09:28 +0100
From:   Aidan MacDonald <aidanmacdonald.0x0@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     michael@...le.cc, brgl@...ev.pl, linux-gpio@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] gpio: regmap: Support a custom ->to_irq() hook

Linus Walleij <linus.walleij@...aro.org> writes:

> On Sun, Jul 3, 2022 at 1:10 PM Aidan MacDonald
> <aidanmacdonald.0x0@...il.com> wrote:
>
>> Some GPIO chips require a custom to_irq() callback for mapping
>> their IRQs, eg. because their interrupts come from a parent IRQ
>> chip where the GPIO offset doesn't map 1-to-1 with hwirq number.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@...il.com>
>
> What is the usecase for this?

This is a generic GPIO chip; so I guess any use case for ->to_irq()?

> Since GPIO chips and IRQ chips are orthogonal there is absolutely
> no guarantee that ->to_irq() is called before a driver start to use
> an IRQ from the irqchip side:
>
> (quoting Documentation/driver-api/gpio/driver.rst)
>
>  It is legal for any IRQ consumer to request an IRQ from any irqchip even if it
>  is a combined GPIO+IRQ driver. The basic premise is that gpio_chip and
>  irq_chip are orthogonal, and offering their services independent of each
>  other.
>
>  gpiod_to_irq() is just a convenience function to figure out the IRQ for a
>  certain GPIO line and should not be relied upon to have been called before
>  the IRQ is used.
>
>  Always prepare the hardware and make it ready for action in respective
>  callbacks from the GPIO and irq_chip APIs. Do not rely on gpiod_to_irq() having
>  been called first.
>
> (end quote)

That's fine, and I don't require ->to_irq() to be called. The IRQ chip
in my case is provided by an MFD driver (axp20x to be specific) and it
*does* work orthogonally to the GPIO driver -- the GPIO driver neither
knows nor cares about the IRQ chip.

> Using ->to_irq() makes sense in a few cases such as when
> a GPIO key that can also poll for state want to get hold of an
> IRQ to react to edges.

This is my use case; specifically, GPIO keys and ASoC jack detection.

> Now: if a consumer requests IRQ nr 3 from your driver say from ACPI or
> from a device tree, and as you say GPIOs and IRQs are not 1-to-1 mapped,
> so IRQ nr 3 may be coming from GPIO 314, isn't this going to be really
> messy for users? One local numberspace for GPIO and another local
> numberspace for IRQs?

Well, this is how MFD drivers with GPIO functionality often work, they
aren't creating a special IRQ sub-domain for GPIOs and it doesn't seem
to be a problem there. Most likely because those MFD devices are being
used for GPIO keys or something similar.

Referring to the interrupt directly would make sense if the GPIO was
wired to another chip's IRQ line, but that is unlikely to be the case
for MFD devices because they're behind a slow bus.

> To me it seems like the reasoning is something like
>
> - I only use GPIO line numbers like <&gpio 3>;
> - Then I call gpiod_to_irq() on that number so I do not need to
>   deal with looking up the IRQ some other way
> - request_irq();
> - Profit.
>

Yeah, that's accurate for my use case.

> There is no guarantee that the API will be used like that at all, actually
> it is uncommon.
>
> Yours,
> Linus Walleij

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.

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()?

Regards,
Aidan