On Wed, 21 Mar 2012 19:05:26 +0530, Thomas Abraham <thomas.abraham@xxxxxxxxxx> wrote:
> Hi David,
>
> On 21 March 2012 09:11, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Wed, Mar 21, 2012 at 07:55:43AM +0530, Thomas Abraham wrote:
> >> 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@xxxxxxxxxxxxxxxx
> https://lists.ozlabs.org/listinfo/devicetree-discuss
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
