On Fri, 23 Mar 2012 16:18:09 +0530, Thomas Abraham <thomas.abraham@xxxxxxxxxx> wrote:
> Hi Grant,
>
> On 21 March 2012 20:43, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> > 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?
>
> #1 is applicable but not #2 and #3 (if I have interpreted the above
> correctly). The wakeup interrupt controller looks for an edge or level
> on pins (which are connected to external devices) and generates a
> interrupt (to gic or combiner) when the set condition on that pin is
> found (edge or level).
>
> >
> > 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).
>
> Ok.
>
> >
> > --- 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.
>
> Other devices use wakeup node as interrupt controller. The wakeup
> interrupt controller supports masking, unmasking and prioritization of
> the wakeup interrupts. The interrupt-controller property can be
> dropped but then of_irq_init() function cannot be used.
>
> >
> > --- 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>;
> > Â Â Â Â Â Â Â Â };
> > Â Â Â Â };
>
> I have tested with this and it works but of_irq_init() function cannot
> be used because 'wakeup_map' is set as interrupt parent and
> 'wakeup_map' is not a interrupt-controller. So of_irq_init() does not
> invoke the initialization function for the wakeup node. If the machine
> init code explicitly looks for this node and calls the corresponding
> initialization function, it works fine.
That's a bug in of_irq_init() then. It should be fixed.
>
> >
> > --- 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> [...]].
>
> This would be the ideal case. In addition to this, the
> interrupt-parent property handling should be modified to support
> multiple parent phandles like <&parent1 [&parent2] [&parent3] ....>.
> This will be useful to extend the capability of of_irq_init() to
> handle interrupt-controller node with multiple interrupt parents.
>
> >
> > 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.
>
> Ok. For now, I will go ahead and use solution #2 which you and David
> have suggested. And correspondingly add explicit lookup of wakeup node
> and call to its initialization function in the machine init code.
I'd really prefer a fix to of_irq_init() instead of hacking around it.
g.
--
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
