Re: [PATCH v4 1/2] gpio/omap: don't create an IRQ mapping for every GPIO on DT

[Javier Martinez Canillas <martinez.javier@gmail.com>]

Hello,

Being the author of these patches I'm ashamed that they are causing a
regression. I did my best to make sure that it would work on OMAP1 and
OMAP2+ platforms booting with both legacy board files and DT.

Also, I didn't come with this solution in a vacuum. This was deeply
discussed with Jon Hunter, Grant Likely, Jean-Christophe
PLAGNIOL-VILLARD and others on a 90+ emails linux-omap thread [1] and
over IRC in #linaro-kernel channel.

The final agreement was that when a GPIO line is mapped in the IRQ
domain with irq_create_of_mapping(), the core has to take care to
request the GPIO and configure it as input.

But until we have this general solution we have to do it on a per irq
chip driver basis and the less hack-ish solution is to have a custom
.map function handler that request the GPIO used as IRQ.

Now, this assumes that irq_create_of_mapping() will always be called
for all GPIO lines that are going to be used as an IRQ. But this
doesn't seem to be true for the omap_hsmmc driver when the "cd-gpios"
mmc property is used in the omap-mmc device node.

I'm ok if you decide to revert these patches but I think we should do
it with care and be sure that the assumption that a IRQ-GPIO mapping
can happen without an explicit call to irq_create_of_mapping() is
actually true and this is not and issue in the omap_hsmmc driver or
its DT bindings.

According to Documentation/devicetree/bindings/mmc/mmc.txt:

cd-gpios: Specify GPIOs for card detection, see gpio binding

So it just says that it is a GPIO for card detection and not an IRQ so
this assumption comes from either the omap_hsmmc driver or Alexander'
DTS is missing something like:

                interrupt-parent = <&gpio6>;
                interrupts = <16 8>;

which will call irq_create_of_mapping() and the mapping for the
GPIO-IRQ will be created, thus making

irq = gpio_to_irq(gpio);
request[_threaded]_irq(irq, ...);

to succeed.

On Sun, Jul 28, 2013 at 6:29 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Jul 28, 2013 at 4:37 PM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
>
>> I think the default OMAP DT files will continue to work with
>>  these patches applied and mostly doesn't break anything
>> in default configuration.
>
> What does "mostly" mean? Hm hm. OK I feel a little
> bit better about this now...
>

I think Santosh meant that

$ grep cd-gpios arch/arm/boot/dts/omap*

doesn't find any usage of the cd-gpios property so that's why this
issue was not found on my tests.

>> Ofcourse with the DT modification as done
>> by Alexander will expose the issue.
>
> So this is all caused by non-upstream code or
> non-upstream DTS files?
>

It seems so, I'm not familiar with the omap_hsmmc so I'm not sure if
this issue is with Alexander DTS, the omap_hsmmc driver or my
assumption that irq_create_of_mapping() will always be called for GPIO
mapped as IRQ is wrong.

>> I really wanted to have the auto request GPIO supported
>> when used as IRQ line but surely not at expense of
>> breaking the client drivers.
>
> If things are working for the default DTS files in the
> kernel then I am OK with it.
>

It's working with the default DTS afaict. At least I didn't find any
regression with my tests using an OMAP3 and OMAP4 boards and neither
did Enric whose tested-by I have in the patches.

> However moving forward to support the cases that
> Alexander is highlighting, I really really thin we need
> to put the information about taken GPIOs "input-hogs"
> into the gpio DT node.
>

I do agree with you that this information should be added to the DT to
prevent this kind of issues.

> Yours,
> Linus Walleij
> --

Hope this helps...

Thanks a lot and best regards,
Javier