interrupts properties and API usage with GPIO controllers/Device Tree

interrupts properties and API usage with GPIO controllers/Device Tree

[Florian Fainelli]

Hi Linus, Gregory,

Recently came across an use case that looks like the following:

gpio0: gpio@deadbeef {
	compatible = "brcm,brcmstb-gpio";
	#interrrupt-cells = <2>;
	#gpio-cells = <2>;
	gpio-controller;
	interrupt-controller;
	...
};

test@cafeb00b {
	interrupt-parent = <&gpio0>;
	interrupts = <99 3>;
};

The driver consuming the test node's interrupts property tries to get
the interrupt by using platform_get_irq() or of_irq_parse_and_map() and
in the case of the gpio-brcmstb.c, this fails because the interrupt is
out of range  as flagged by kernel/irq/irqdomain.c::irq_domain_associate.

Unlike other GPIO provider drivers gpio-brcmstb.c, this driver registers
one gpiolib irqchip per each of its banks, and still uses the generic
map/unmap functions for its irq_domain_ops, so there is no way we can
provide a valid mapping outside of the gpio_to_irq() function
unfortunately since gpiochip_irq_map() does

So here are a few questions for either of you:

- is this a valid API and Device Tree use case: call
of_irq_parse_and_map on an "interrupts" property which has not been
acquired using the GPIO API and then gpio_to_irq? While gpio_to_irq()
works, are not we losing the second specifier in the interrupt cells
about what kind of interrupt type this is?

- would it be acceptable to export gpiochip_irq_map and
gpiochip_irq_export to make them accessible as helpers so we could just
wrap things a bit around or should I just open code the same things and
allow gpiochip_irqchip_add to be passed custom irq_domain_ops for instance?

Thanks!
-- 
Florian

Re: interrupts properties and API usage with GPIO controllers/Device Tree

[Grygorii Strashko]

On 05/26/2016 12:45 AM, Florian Fainelli wrote:
> Hi Linus, Gregory,
> 
> Recently came across an use case that looks like the following:
> 
> gpio0: gpio@deadbeef {
> 	compatible = "brcm,brcmstb-gpio";
> 	#interrrupt-cells = <2>;
> 	#gpio-cells = <2>;
> 	gpio-controller;
> 	interrupt-controller;
> 	...
> };
> 
> test@cafeb00b {
> 	interrupt-parent = <&gpio0>;
> 	interrupts = <99 3>;
> };
> 
> The driver consuming the test node's interrupts property tries to get
> the interrupt by using platform_get_irq() or of_irq_parse_and_map() and
> in the case of the gpio-brcmstb.c, this fails because the interrupt is
> out of range  as flagged by kernel/irq/irqdomain.c::irq_domain_associate.
> 
> Unlike other GPIO provider drivers gpio-brcmstb.c, this driver registers
> one gpiolib irqchip per each of its banks, and still uses the generic
> map/unmap functions for its irq_domain_ops, so there is no way we can
> provide a valid mapping outside of the gpio_to_irq() function
> unfortunately since gpiochip_irq_map() does

And this is not working :( Each irq_domain has to be assigned to separate DT node,
otherwise IRQ mapping will not work (most probably will work, but only for bank 0)

Such kind of GPIO controllers are incompatible with gpiochip_irqchip_add() and required
to have one, common irq_domain for all internal banks.

like 	irq_domain = irq_domain_add_xxx(dev->of_node, ngpio, irq, 0,
							&davinci_gpio_irq_ops,
							chips);

ngpio = 120 in case of bcm7445.dtsi
and custom .map() function need to be implemented 

More or less similar situation is with davinci_gpio.

> 
> So here are a few questions for either of you:
> 
> - is this a valid API and Device Tree use case: call
> of_irq_parse_and_map on an "interrupts" property which has not been
> acquired using the GPIO API and then gpio_to_irq? 

this is valid use case

> While gpio_to_irq()
> works, are not we losing the second specifier in the interrupt cells
> about what kind of interrupt type this is?
> 
> - would it be acceptable to export gpiochip_irq_map and
> gpiochip_irq_export to make them accessible as helpers so we could just
> wrap things a bit around or should I just open code the same things and
> allow gpiochip_irqchip_add to be passed custom irq_domain_ops for instance?
> 

Yah, custom implementation might be needed.

Another interesting, related question (as for me) is "Is there a limitation
 that gpio bank can have only 32 GPIO pins (from gpiolib point of view)?" 

-- 
regards,
-grygorii

Re: interrupts properties and API usage with GPIO controllers/Device Tree

[Linus Walleij]

On Fri, May 27, 2016 at 4:23 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> Another interesting, related question (as for me) is "Is there a limitation
>  that gpio bank can have only 32 GPIO pins (from gpiolib point of view)?"

Do you mean for this driver or in general?

The gpio MMIO driver has this limitation, but not the subsystem
and gpiolib as far as I know.

Yours,
Linus Walleij

Re: interrupts properties and API usage with GPIO controllers/Device Tree

[Grygorii Strashko]

On 05/31/2016 11:49 AM, Linus Walleij wrote:
> On Fri, May 27, 2016 at 4:23 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>
>> Another interesting, related question (as for me) is "Is there a limitation
>>   that gpio bank can have only 32 GPIO pins (from gpiolib point of view)?"
>
> Do you mean for this driver or in general?

In general.

>
> The gpio MMIO driver has this limitation, but not the subsystem
> and gpiolib as far as I know.

Thanks.

-- 
regards,
-grygorii

Re: interrupts properties and API usage with GPIO controllers/Device Tree

[Linus Walleij]

On Wed, May 25, 2016 at 11:45 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:

> - is this a valid API and Device Tree use case: call
> of_irq_parse_and_map on an "interrupts" property which has not been
> acquired using the GPIO API and then gpio_to_irq?

Yes. See Documentation/gpio/driver.txt:

-------

It is legal for any IRQ consumer to request an IRQ from any irqchip no matter
if that 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.

So always prepare the hardware and make it ready for action in respective
callbacks from the GPIO and irqchip APIs. Do not rely on gpiod_to_irq() having
been called first.

This orthogonality leads to ambiguities that we need to solve: if there is
competition inside the subsystem which side is using the resource (a certain
GPIO line and register for example) it needs to deny certain operations and
keep track of usage inside of the gpiolib subsystem. This is why the API
below exists.

-----

> While gpio_to_irq()
> works, are not we losing the second specifier in the interrupt cells
> about what kind of interrupt type this is?

Yeah and .set_type() needs to work. See other drivers for inspiration...

> - would it be acceptable to export gpiochip_irq_map and
> gpiochip_irq_export to make them accessible as helpers so we could just
> wrap things a bit around or should I just open code the same things and
> allow gpiochip_irqchip_add to be passed custom irq_domain_ops for instance?

Rather than poke around in gpiolib internals, make a separate
implementation for corner cases unless you can make it really
clean and nice for all consumers.

Yours,
Linus Walleij