Re: [PATCH] powerpc: document new interrupt-array property

[Segher Boessenkool <segher@kernel.crashing.org>]

> Option #2 -- new interrupt-array property.  MAL would look
> like this:
>
>   MAL0: mcmal {
>       /* FIXME */
>       device_type = "mcmal-dma";
>       compatible = "ibm,mcmal-440gp", "ibm,mcmal";
>       dcr-reg = <180 62>;
>       num-tx-chans = <4>;
>       num-rx-chans = <4>;
>       interrupt-array = <&UIC0 a 4
>                          &UIC0 b 4
>                          &UIC1 0 4
>                          &UIC1 1 4
>                          &UIC1 2 4>;
>   };

I'd rather write it like

>       interrupts        = < a 4   b 4   0 4   1 4   2 4 >
>       interrupt-parents = <&UIC0 &UIC0 &UIC1 &UIC1 &UIC1>

to stay closer to the existing binding, but yes.

> Option #3 -- define new, logical interrupt nexus to do
> the mapping.   Not sure if I got this right but here is
> my take on what this might look like:
>
>   MALINT: malint_nexus {
>       #interrupt-cells = <1>;
>       #address-cells = <0>;
>       #size-cells = <0>;
>       interrupt-map = </*TXEOB*/ 0 &UIC0 a 4
>                /*RXEOB*/ 1 &UIC0 b 4
>                /*SERR*/  2 &UIC1 0 4
>                /*TXDE*/  3 &UIC1 1 4
>                /*RXDE*/  4 &UIC1 2 4>;
>       interrupt-map-mask = <ffffffff>;
>   }
>
>   MAL0: mcmal {
>       /* FIXME */
>       device_type = "mcmal-dma";
>       compatible = "ibm,mcmal-440gp", "ibm,mcmal";
>       dcr-reg = <180 62>;
>       num-tx-chans = <4>;
>       num-rx-chans = <4>;
>       interrupt-parent = <&MALINT>;
>       interrupts = <0 1 2 3 4>;
>   };
>
> The malint_nexus node is attache to / I guess??  Segher
> is this what you had in mind?

I'd put it as a child node of the mcmal node, like this:

   MAL0: mcmal {
       /* FIXME */
       device_type = "mcmal-dma";
       compatible = "ibm,mcmal-440gp", "ibm,mcmal";
       dcr-reg = <180 62>;
       num-tx-chans = <4>;
       num-rx-chans = <4>;
       interrupt-parent = <&MALINT>;
       interrupts = <0 1 2 3 4>;

       MALINT: mal-nexus {
           device_type = "interrupt-nexus"
           #interrupt-cells = <1>;
           interrupt-map = </*TXEOB*/  0   &UIC0   a 4
                            /*RXEOB*/  1   &UIC0   b 4
                            /*SERR*/   2   &UIC1   0 4
                            /*TXDE*/   3   &UIC1   1 4
                            /*RXDE*/   4   &UIC1   2 4>;
           interrupt-map-mask = <ff>;
       }
   }

> The question is-- is option #3 clear enough?

I think so.  A separate nexus pseudo-node needs hardly
any properties (no "#address-cells", no "reg", no nothing)
and nicely expresses the whole thing, within the existing
framework.

> Is a new property warranted?

The proposed "interrupt-array" property, apart from being
a horrible name, only handles one very specific manifestation
of a much more general problem: random weirdness in the
interrupt routing that doesn't conform to the "normal" device
tree.  The existing interrupt mapping binding can handle the
generic case just fine, with a very simple but extremely
flexible framework.  The fact that you sometimes need an
extra "pseudo" device node is actually very natural: it
represent an interrupt "bus".  There is no really good place
for such a node though; depending on the situation, it seems
best to hang it either from the node that represents the
origin of the interrupts (in case this is routing from one
device only), or the node that represents their destination
(typically, an interrupt controller).


Segher