Re: 0001-add-amlogic-gpio-to-irq

Jerome Brunet
2020-12-07T11:07:54Z

On Mon 07 Dec 2020 at 11:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Dec 4, 2020 at 4:25 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>> On Fri 04 Dec 2020 at 10:13, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> This HW only has 8 irqs that can each be mapped to a pin. No direct
>> translation can be made, we have to allocate an irq to monitor the line.
>> So when gpio_to_irq() was called, we had to do that allocation dynamically
>> to return a valid irq number. Since there was no counter part to
>> gpio_to_irq(), those allocation cannot be freed during the lifetime of
>> the device.
>
> I'm not sure why we are talking about legacy API which should not be
> used.

I would have been happy to forget about it, but it seems to be the topic
of the thread :)

> Besides that I didn't get what you meant under counterpart API (IRQ
> descriptor has a mapping to the IRQ chip which keeps the mapping to
> whatever hardware wants).

* This HW has to create the mapping between GPIO and irq number
dynamically. The number of irqs available is very limited.
* We only get to know a mapping is required when gpio_to_irq() is called
* There is no way to know when it is safe to dispose of the created
mapping
* Some drivers require a trigger type we don't support. These will create
mappings and not use it because of the failure when .set_type() is
called

To answer your question, there an API which lets us know a mapping is
needed, but none to inform that it is not required anymore. The GPIO API
was not meant to used like this. Not saying it is good or bad, this is
just how it is.

If there was a way to know it is safe to dispose of the mapping, then
letting users of gpio_to_irq() try and fail would be OK, but we don't
have that AFAIK.

This is why gpio_to_irq() or gpiolib irqchip had not been added so far
on this HW. I don't think it is worth fixing, especially if the API is
considered to be legacy.

On this HW, getting an interupt from a pin is done by going directly
after the interrupt controller, like here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts#n173

AFAICT, making pps-gpio parse an "interrupt" property should be doable
too.