Re: [PATCH v4 03/10] of/irq: document properties for wakeup interrupt parent

[Lina Iyer]

On Tue, Apr 16 2019 at 10:54 -0600, Stephen Boyd wrote:
> Quoting Lina Iyer (2019-04-04 08:58:38)
> > On Mon, Mar 18 2019 at 11:54 -0600, Marc Zyngier wrote:
> > >On Wed, 13 Mar 2019 15:18:37 -0600
> > >Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:
> > >
> > >Please do Cc Rob when posting DT related patches.
> > >
> > >> Some interrupt controllers in a SoC, are always powered on and have a
> > >> select interrupts routed to them, so that they can wakeup the SoC from
> > >> suspend. Add wakeup-parent DT property to refer to these interrupt
> > >> controllers.
> > >>
> > >> If the interrupts routed to the wakeup parent are not sequential, than a
> > >> map needs to exist to associate the same interrupt line on multiple
> > >> interrupt controllers. Providing this map in every driver is cumbersome.
> > >> Let's add this in the device tree and document the properties to map the
> > >> interrupt specifiers
> > >>
> > >> Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
> > >> ---
> > >> Changes in v4:
> > >>      - Added this documentation
> > >> ---
> > >>  .../interrupt-controller/interrupts.txt       | 39 +++++++++++++++++++
> > >>  1 file changed, 39 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> > >> index 8a3c40829899..917b598317f5 100644
> > >> --- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> > >> +++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> > >> @@ -108,3 +108,42 @@ commonly used:
> > >>                      sensitivity = <7>;
> > >>              };
> > >>      };
> > >> +
> > >> +3) Interrupt wakeup parent
> > >> +--------------------------
> > >> +
> > >> +Some interrupt controllers in a SoC, are always powered on and have a select
> > >> +interrupts routed to them, so that they can wakeup the SoC from suspend. These
> > >> +interrupt controllers do not fall into the category of a parent interrupt
> > >> +controller and can be specified by the "wakeup-parent" property and contain a
> > >> +single phandle referring to the wakeup capable interrupt controller.
> > >> +
> > >> +   Example:
> > >> +    wakeup-parent = <&pdc_intc>;
> > >> +
> > >> +
> > >> +4) Interrupt mapping
> > >> +--------------------
> > >> +
> > >> +Sometimes interrupts may be detected by more than one interrupt controller
> > >> +(depending on which controller is active). The interrupt controllers may not
> > >> +be in hierarchy and therefore the interrupt controller driver is required to
> > >> +establish the relationship between the same interrupt at different interrupt
> > >> +controllers. If these interrupts are not sequential then a map needs to be
> > >> +specified to help identify these interrupts.
> > >> +
> > >> +Mapping the interrupt specifiers in the device tree can be done using the
> > >> +"irqdomain-map" property. The property contains interrupt specifier at the
> > >> +current interrupt controller followed by the interrupt specifier at the mapped
> > >> +interrupt controller.
> > >> +
> > >> +   irqdomain-map = <incoming-interrupt-specifier mapped-interrupt-specifier>
> > >> +
> > >> +The optional properties "irqdomain-map-mask" and "irqdomain-map-pass-thru" may
> > >> +be provided to help interpret the valid bits of the incoming and mapped
> > >> +interrupt specifiers respectively.
> > >> +
> > >> +   Example:
> > >> +    irqdomain-map = <22 0 &intc 36 0>, <24 0 &intc 37 0>;
> > >> +    irqdomain-map-mask = <0xff 0>;
> > >> +    irqdomain-map-pass-thru = <0 0xff>;
> > >
> > >
> > >This doesn't quite explain how the mask and pass-thru properties are
> > >used. I guess that the mask is used to define the 'useful bits' on the
> > >incoming side, but pass-thru puzzles me. In your example, does it mean
> > >that incoming lines map to outgoing interrupt <0 0>?
> > >
> > Sorry about the late reply.
> >
> > How about this to go with the rest of the documentation -
> >
> > In the above example, the input interrupt specifier map-mask <0xff 0> applied
> > on the incoming interrupt specifier of the map <22 0>, <24 0>, returns the
> > input interrupt 22, 24 etc. The second argument being irq type is immaterial
> > from the map and is used from the incoming request instead. The pass-thru
> > specifier parses the output interrupt specifier from the rest of the unparsed
> > argments from the map <&intc 36 0>, <&intc 37 0> etc to return the output
> > interrupt 36, 37 etc.
>
> I see two things going on here. Do both need to happen?
>
> 	#1: Specifying wakeup parent phandle
> 	#2: Mapping GPIO interrupts to a parent irqdomain
>
> Do we need the method of specifying the wakeup parent if with a dt
> property if we have a way to map irqdomains from one to another? I think
> I may have already said on the list that we must have #1 but now I'm not
> so sure. It looks like we could get away with just looking into the
> irqdomain-map and then pick out the wakeup parent that way.
I thought about it. But the wakeup-parent seems to be needed outside the
irqdomain-map to setup the gpiochip's hierarchy. This could be done by
reading the map, but I am not sure if that approach is clean enough.

> The way the bindings are written shows one way to map interrupts between
> domains but I don't know if it lets us differentiate which irqs go from
> which domain to what other domain. It seems that we assume we're looking
> at only the GPIO to wakeup parent irqdomain mapping from the
> irqdomain-map property in this series. If we had a way to do this with
> the irqdomain map then we could avoid needing a special 'wakeup-parent'
> property.