Message-ID: <CACRpkdbZxPwGNF427T85hmNHw6ncnYExKmXzLJM1rq8FCZOpxw@mail.gmail.com>
Date:   Mon, 7 Dec 2020 14:43:36 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Jerome Brunet <jbrunet@...libre.com>
Cc:     Marc Zyngier <maz@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        林圣欢 <linshenghuan@...gtu-china.com>,
        khilman <khilman@...libre.com>,
        narmstrong <narmstrong@...libre.com>,
        "martin.blumenstingl" <martin.blumenstingl@...glemail.com>,
        linux-gpio <linux-gpio@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-amlogic <linux-amlogic@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: 0001-add-amlogic-gpio-to-irq

On Mon, Dec 7, 2020 at 2:25 PM Jerome Brunet <jbrunet@...libre.com> wrote:
> [Me]

> > So maybe the problem is that you need to go back and think about
> > updating the DT bindings for this thing to include interrupt-controller
> > as well?
>
> We do
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-meson-gpio.c
>
> That's actually the only thing we provide, on purpose.

Aha I see now.

> >>  * We only get to know a mapping is required when gpio_to_irq() is called
> >
> > No that callback should not be used for that.
>
> Agreed ... I was trying explain why we did *not* push a patch similar to what
> was proposed here, or use gpiolib irqchip.

The gpiolib irqchip kind of suppose there is a 1-to-1 mapping between a
GPIO line and an IRQ, so I see the reasoning. That said, the callbacks
are code so a deviant remapping irq line "pool" could possibly be used.

> > I don't quite understand this. Do you mean you are bombarded by pointless
> > requests for interrupts that will not work anyways?
>
> When we tried the approach suggested in this patch (again I agree it is
> bad, which is why I'm against it), some drivers out there (I don't
> remember which one TBH - that was 3 years ago) parsed the "gpio"
> property and tried gpio_to_irq() and if it did not work then go
> something else (like polling).
>
> However the allocation stayed behind. It does not take much
> "bombardment" when you only have 8.

I don't see any problem with gpio_to_irq() always returning -EINVAL
in situations like this.

> We control the ressources of the devices through DT, not the necessarily
> drivers (which may be generic)
>
> Some device needs the gpio, even if we don't want the irq.
> We can't always prevent the driver to try gpio_to_irq().

True. But you can say "no" to anything trying to do that, that way you
will only hand out the irqs on a first-come-first serve basis to the clients
that use the irqs directly and thus you get it under control.

> This why I don't want gpio_to_irq() to be enabled on this HW, because it
> would not be under our control anymore.

I think you can enable it and use gpiolibs hierarchical irqchip but let
gpio_to_irq() say no to everything.

> Again agreed. I'm really sorry if I have been that unclear about my
> motive here. We already had that discussion 3 years ago, I totally
> understand your point and agree. I was trying (and failing) to tell the
> author of the patch that this approach had already been discussed in
> past and that, unless gpiolib dramatically changed since then,
> gpio_to_irq() should be used in this way and he should use irqchip we
> already provide.

OK I get it.

It's just that from my point of view using the hierarchical gpiolib irqchip
has a value in itself even if gpio_to_irq() isn't used at all because it brings
the criss-cross under control.

If the author wants to get some driver to work, such as MMC card detect
or ethernet phy or similar, what s/he should do is to go and fix the driver
to ask for an irq directly from the platform device or similar if it
can't get an
IRQ from the GPIO line by using gpiod_to_irq().

Yours,
Linus Walleij

Powered by blists - more mailing lists