Re: Device node for a controller with two interrupt parents
From: Grant Likely <hidden>
Date: 2012-03-21 15:13:21
Also in:
linux-samsung-soc
On Wed, 21 Mar 2012 19:05:26 +0530, Thomas Abraham [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Hi David, On 21 March 2012 09:11, David Gibson [off-list ref] wrote:quoted
On Wed, Mar 21, 2012 at 07:55:43AM +0530, Thomas Abraham wrote:quoted
Hi, Exynos5 includes a gpio wakeup interrupt controller that generates 32 interrupts. The first 16 interrupts are routed to the interrupt combiner controller. The last 16 are muxed into one interrupt and this interrupt line is connected to the GIC interrupt controller. So, the wakeup interrupt controller node in device tree requires two interrupt parents. I do not know how to handle this. Any suggestions will be very helpful.This has occurred before, for example on the MAL device on 440EP (see the bamboo board dts for example). Â The semi-standard approach is to make the node an interrupt-nexus for itself. Â That is in the node's interrupts property, just list 0..N giving as many interrupts as you need. Â Set the node's interrupt-parent to point to the node itself, then add interrupt-map and interrupt-map-mask properties which remap those interrupts 0..N to the correct interrupts on the actual interrupt controllers. Â Each entry in the interrupt map specifies an interrupt parent phandle, so you can distribute the irqs to multiple interrupt controllers that way.Thanks for your suggestion and pointing out an example. I tried this approach for Exynos4 and Exynos5. It mostly works but there are two issues here. 1. In the Exynos5 case, the wakeup interrupt controller (which has two separate interrupt parents - gic and combiner) is itself a interrupt controller and has the 'interrupt-controller' property. So of_irq_map_raw() function does not process the interrupt-map in the wakeup interrupt controller device node. I did the following change to get past this but I am not sure if this the correct thing to do.diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 9cf0060..892ac95 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c@@ -152,7 +152,8 @@ int of_irq_map_raw(struct device_node *parent,const __be32 *intspec, /* Now check if cursor is an interrupt-controller and if it is * then we are done */ - if (of_get_property(ipar, "interrupt-controller", NULL) != + if (!of_get_property(ipar, "interrupt-map", &imaplen) && + of_get_property(ipar, "interrupt-controller", NULL) != NULL) { pr_debug(" -> got it !\n"); for (i = 0; i < intsize; i++)
Okay, so you're saying there are three important aspects to this device: 1) it terminates interrupts from other devices (therefore needs an interrupt controller driver) 2) it passes some interrupts through untouched (interrupt controller driver doesn't need to touch them; it directly raises an irq on the gic or combiner) 3) It is able generate interrupt signals on it's own (independent of any attached devices) Do I understand correctly? Your patch above solves the problem for #2 above, but it breaks #1 because interrupts from external devices can no longer be terminated on the wakeup controller node (they'll always get passed through).
--- Possible solution 1 ---If other devices *don't* use the wakeup node as their interrupt parent, then you should be able to simply drop the interrupt-controller property and make the other devices directly reference the gic and combiner.
--- Possible solution 2 ---Split the interrupt map into a separate node:
wakeup_eint: interrupt-controller@11000000 {
compatible = "samsung,exynos4210-wakeup-eint";
reg = <0x11000000 0x1000>;
interrupt-controller;
#interrupt-cells = <1>;
interrupt-parent = <&wakeup_map>;
interrupts = <0 1 2 3 f 5 6 7 8 9 10 11 12 13 14 15 16>;
wakeup_map: interrupt-map {
#interrupt-cells = <1>;
#address-cells = <0>;
interrupt-map = <0 &gic 0 16 0>,
<1 &gic 0 17 0>,
<2 &gic 0 18 0>,
<3 &gic 0 19 0>,
<4 &gic 0 20 0>,
<5 &gic 0 21 0>,
<6 &gic 0 22 0>,
<7 &gic 0 23 0>,
<8 &gic 0 24 0>,
<9 &gic 0 25 0>,
<10 &gic 0 26 0>,
<11 &gic 0 27 0>,
<12 &gic 0 28 0>,
<13 &gic 0 29 0>,
<14 &gic 0 30 0>,
<15 &gic 0 31 0>,
<16 &combiner 2 4>;
};
};
--- possible solution 3 ---'interrupts' just isn't sufficient for some devices; add a binding for a 'interrupts-multiparent' that can be used instead of 'interrupts' and uses the format <phandle> <specifier> [<phandle> <specifier> [...]]. I'm not opposed to this solution since it is a more natural binding for multiparented interrupt controllers, but I won't commit to it without feedback and agreement from Mitch, Ben, David Gibson, etc.
2. The of_irq_init() function (mainly used on the arm platforms) looks
for nodes with 'interrupt-controller' and initializes them with the
parents initialized first. In the Exynos4/5 case, the wakeup interrupt
has two interrupt parents and of_irq_init() function does not seem to
be consider this case. And hence, the wakeup interrupt controller is
being initialized, without the combiner being initialized.
The following are the interrupt-controller nodes, that I have been
testing with (slightly modified for testing)
gic:interrupt-controller@10490000 {
compatible = "arm,cortex-a9-gic";
#interrupt-cells = <3>;
#address-cells = <0>;
#size-cells = <0>;
interrupt-controller;
cpu-offset = <0x8000>;
reg = <0x10490000 0x1000>, <0x10480000 0x100>;
};
combiner:interrupt-controller@10440000 {
compatible = "samsung,exynos4210-combiner";
#interrupt-cells = <2>;
interrupt-controller;
samsung,combiner-nr = <16>;
reg = <0x10440000 0x1000>;
interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
<0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
<0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
<0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>;
};
wakeup_eint: interrupt-controller@11000000 {
compatible = "samsung,exynos4210-wakeup-eint";
reg = <0x11000000 0x1000>;
interrupt-controller;
#interrupt-cells = <1>;
interrupt-parent = <&wakeup_eint>;
interrupts = <0x0>, <0x1>, <0x2>, <0x3>,
<0x4>, <0x5>, <0x6>, <0x7>,
<0x8>, <0x9>, <0xa>, <0xb>,
<0xc>, <0xd>, <0xe>, <0xf>,
<0x10>;
#address-cells = <0>;
#size-cells = <0>;
interrupt-map = <0x0 &gic 0 16 0>,
<0x1 &gic 0 17 0>,
<0x2 &gic 0 18 0>,
<0x3 &gic 0 19 0>,
<0x4 &gic 0 20 0>,
<0x5 &gic 0 21 0>,
<0x6 &gic 0 22 0>,
<0x7 &gic 0 23 0>,
<0x8 &gic 0 24 0>,
<0x9 &gic 0 25 0>,
<0xa &gic 0 26 0>,
<0xb &gic 0 27 0>,
<0xc &gic 0 28 0>,
<0xd &gic 0 29 0>,
<0xe &gic 0 30 0>,
<0xf &gic 0 31 0>,
<0x10 &combiner 2 4>;
};
Thanks,
Thomas.
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss-- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd.