interrupts properties and API usage with GPIO controllers/Device Tree
Hi Linus, Gregory,
Recently came across an use case that looks like the following:
gpio0: gpio at deadbeef {
compatible = "brcm,brcmstb-gpio";
#interrrupt-cells = <2>;
#gpio-cells = <2>;
gpio-controller;
interrupt-controller;
...
};test at 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!
Signature · 1 line
Florian
Quoted 23 lines · Florian Fainelli
Hi Linus, Gregory,
Recently came across an use case that looks like the following:
gpio0: gpio at deadbeef { compatible = "brcm,brcmstb-gpio"; #interrrupt-cells = <2>; #gpio-cells = <2>; gpio-controller; interrupt-controller; ... };test at 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()orof_irq_parse_and_map()and in the case of thegpio-brcmstb.c, this fails because the interrupt is out of range as flagged by kernel/irq/irqdomain.c::irq_domain_associate.
Quoted 5 lines
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 itsirq_domain_ops, so there is no way we can provide a valid mapping outside of thegpio_to_irq()function unfortunately sincegpiochip_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.
Quoted 6 lines
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_mapon an "interrupts" property which has not been acquired using the GPIO API and thengpio_to_irq?
this is valid use case
Quoted 9 lines
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_mapandgpiochip_irq_exportto make them accessible as helpers so we could just wrap things a bit around or should I just open code the same things and allowgpiochip_irqchip_addto be passed customirq_domain_opsfor 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)?"
Signature · 2 lines
regards, -grygorii
Quoted 2 lines · Grygorii Strashko
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
Quoted 7 lines · Linus Walleij
Quoted 2 lines · Grygorii Strashko
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.
Quoted 3 lines
The gpio MMIO driver has this limitation, but not the subsystem and gpiolib as far as I know.
Thanks.
Signature · 2 lines
regards, -grygorii
Quoted 3 lines · Florian Fainelli
- is this a valid API and Device Tree use case: call
of_irq_parse_and_mapon an "interrupts" property which has not been acquired using the GPIO API and thengpio_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.
-----
Quoted 3 lines
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...
Quoted 4 lines
- would it be acceptable to export
gpiochip_irq_mapandgpiochip_irq_exportto make them accessible as helpers so we could just wrap things a bit around or should I just open code the same things and allowgpiochip_irqchip_addto be passed customirq_domain_opsfor 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