[RFC] powerpc: Document new property called interrupt-parents

Document new property called interrupt-parents which can
be used to represent interrupts for devices that route
interrupts to more than one interrupt controller.

Signed-off-by: Stuart Yoder <redacted>
---

 This is based on the discussions on the list last week and
 this week.  I realize there are still differences of opinion
 on interrupts-array vs interrupt-parents but I think the
 differences between the two options are not that significant.

 Note: this patch depends on the patch updating the interrupt info
 in booting-without-of.txt

 Documentation/powerpc/booting-without-of.txt |   65 +++++++++++++++++++++++++-
 1 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 0238a64..89e1c4a 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt

@@ -1759,7 +1759,68 @@ phandle to the parent node.
 If the interrupt-parent property is not defined for a node, it's
 interrupt parent is assumed to be it's device tree_ parent.
 
-3) OpenPIC Interrupt Controllers
+3) interrupt-parents property
+-----------------------------
+
+For devices that generate interrupts to multiple interrupt
+controllers, the  interrupt-parent representation
+is not sufficient because it only describes a single
+interrupt parent. The 'interrupt-parents' property should be used
+to represent this (note the plural in the property name).
+
+The interrupt-parents property consists of one or more 
+interrupt parent phandle values each representing the interrupt
+parent for the corresponding interrupt specifier in the interrupts
+property:
+
+  interrupt-parents = <parent-phandle0 parent-phandle1 ... parent-phandleN>
+  interrupts = <int-specifier0 int-specifier1 ... int-specifierN>
+
+It is required that the number of interrupts encoded in the interrupt-parents
+property exacly match the number of interrupt specifiers in the interrupts
+property.
+
+See the example below:
+
+   pic0: pic@700 {
+       interrupt-controller;
+       #address-cells = <0>;
+       #interrupt-cells = <2>;
+       reg = <700 100>;
+       device_type = "open-pic";
+   };
+
+   pic1: pic@800 {
+       interrupt-controller;
+       #address-cells = <0>;
+       #interrupt-cells = <2>;
+       reg = <800 100>;
+       device_type = "open-pic";
+   };
+
+   ethernet@25000 {
+       #address-cells = <1>;
+       #size-cells = <0>;
+       device_type = "network";
+       model = "fsl,TSEC";
+       compatible = "gianfar";
+       reg = <25000 1000>;
+       mac-address = [ 00 E0 0C 00 73 01 ];
+       interrupt-parents = <&pic0 &pic0 &pic1>;
+       interrupts = <13 3 14 3 18 3>;
+       phy-handle = <2452001>;
+   };
+
+In the example, the interrupts property and the interrupt-parents
+property defines three interrupts-- interrupt 0x13 and 0x14 go to
+the pic0 interrupt controller and interrupt 0x18 goes to pic1.
+
+Note: each interrupt parent in the interrupt-parents array can
+potentially specify a different value for #interrupt-cells.  The number
+of #interrupt-cells for each parent must be determined to correctly
+parser the interrupts property.
+
+4) OpenPIC Interrupt Controllers
 --------------------------------
 
 OpenPIC interrupt controllers required 2 cells to encode

@@ -1774,7 +1835,7 @@ Sense and level information should be en
 	2 = active high level sensitive type enabled
 	3 = high to low edge sensitive type enabled
 
-4) ISA Interrupt Controllers
+5) ISA Interrupt Controllers
 ----------------------------
 
 ISA PIC interrupt controllers required 2 cells to encode

-- 
1.4.4

+For devices that generate interrupts to multiple interrupt
+controllers, the  interrupt-parent representation
+is not sufficient because it only describes a single
+interrupt parent. The 'interrupt-parents' property should be used
+to represent this (note the plural in the property name).

s/should be used/can be used/

+   pic0: pic@700 {
+       interrupt-controller;
+       #address-cells = <0>;
+       #interrupt-cells = <2>;
+       reg = <700 100>;
+       device_type = "open-pic";
+   };

I still say the #address-cells should be removed.

+   ethernet@25000 {
+       #address-cells = <1>;
+       #size-cells = <0>;
+       device_type = "network";
+       model = "fsl,TSEC";
+       compatible = "gianfar";
+       reg = <25000 1000>;
+       mac-address = [ 00 E0 0C 00 73 01 ];
+       interrupt-parents = <&pic0 &pic0 &pic1>;
+       interrupts = <13 3 14 3 18 3>;
+       phy-handle = <2452001>;
+   };

Maybe write this like

ethernet@25000 {
         device_type = "network";
         reg = <25000 1000>;
         ...
         interrupt-parents = <&pic0 &pic0 &pic1>;
         interrupts = <13 3 14 3 18 3>;
};

to make the example more readable?  And some spacing in
the properties, so maybe

ethernet@25000 {
         device_type = "network";
         reg = <25000 1000>;
         ...
         interrupt-parents = <&pic0 &pic0 &pic1>;
         interrupts        = <13 3  14 3  18 3 >;
};

+Note: each interrupt parent in the interrupt-parents array can
+potentially specify a different value for #interrupt-cells.  The 
number
+of #interrupt-cells for each parent must be determined to correctly
+parser the interrupts property.

s/parser/parse/


Segher

On Tue, Feb 27, 2007 at 04:54:45PM -0600, Stuart Yoder wrote:

Document new property called interrupt-parents which can
be used to represent interrupts for devices that route
interrupts to more than one interrupt controller.

Well, whatever Segher says, I'm still very concerned about the
potential for extra difficulty in debugging with this approach.

Update to say it's assumed be the device tree parent if neither
interrupt-parent nor interrupt-parents exists.

-3) OpenPIC Interrupt Controllers
+3) interrupt-parents property
+-----------------------------
+
+For devices that generate interrupts to multiple interrupt
+controllers, the  interrupt-parent representation
+is not sufficient because it only describes a single
+interrupt parent. The 'interrupt-parents' property should be used
+to represent this (note the plural in the property name).
+
+The interrupt-parents property consists of one or more 
+interrupt parent phandle values each representing the interrupt
+parent for the corresponding interrupt specifier in the interrupts
+property:
+
+  interrupt-parents = <parent-phandle0 parent-phandle1 ... parent-phandleN>
+  interrupts = <int-specifier0 int-specifier1 ... int-specifierN>
+
+It is required that the number of interrupts encoded in the interrupt-parents
+property exacly match the number of interrupt specifiers in the interrupts
+property.
+

[snip]

+In the example, the interrupts property and the interrupt-parents
+property defines three interrupts-- interrupt 0x13 and 0x14 go to
+the pic0 interrupt controller and interrupt 0x18 goes to pic1.

This is potentially misleading: the interrupt numbers are properties
of the pic, not of the device.  Better to say that for this device,
the first two interrupt lines are routed to irqs 0x13 and 0x14 on
pic0, and the third is routed to irq 0x18 on pic1.  On a board with 3
PICs, it would also be valid to have a device with:
	interrupts = <0x10 0x4 0x10 0x4 0x10 0x4>;
	interrupt-parents = <&pic0 &pic1 &pic2>;

+Note: each interrupt parent in the interrupt-parents array can
+potentially specify a different value for #interrupt-cells.  The number
+of #interrupt-cells for each parent must be determined to correctly
+parser the interrupts property.

I suggest your example actually show this case, since it's the more
general.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

quoted

+Note: each interrupt parent in the interrupt-parents array can
+potentially specify a different value for #interrupt-cells.  The 
number
+of #interrupt-cells for each parent must be determined to correctly
+parser the interrupts property.

I suggest your example actually show this case, since it's the more
general.

But it won't very likely happen in reality, unlike
the nested-identical-PICs case.


Segher

-----Original Message-----
From: David Gibson [mailto:david@gibson.dropbear.id.au]=20
Sent: Tuesday, February 27, 2007 6:56 PM
To: Yoder Stuart-B08248
Cc: paulus@samba.org; linuxppc-dev@ozlabs.org
Subject: Re: [RFC] powerpc: Document new property called=20
interrupt-parents
=20
On Tue, Feb 27, 2007 at 04:54:45PM -0600, Stuart Yoder wrote:

quoted

=20
Document new property called interrupt-parents which can
be used to represent interrupts for devices that route
interrupts to more than one interrupt controller.

=20
Well, whatever Segher says, I'm still very concerned about the
potential for extra difficulty in debugging with this approach.

The issue, as I understand it, is the potential for an old
kernel parser to silently misparse a new dev tree with this
new property.

Do the version and last_comp_version in the blob header
help here?

If new dev trees with this new property were encoded with a
version=3D0x17 and last_comp_version=3D0x17 would the old
kernel parser error out if it only supported up to 0x16?

I'm not sure or not if that is the intended use of those
fields, but I think parser/DTS compatibility issues could
be solved by some kind of versioning.

It's more of a general problem.

Stuart

quoted

Well, whatever Segher says, I'm still very concerned about the
potential for extra difficulty in debugging with this approach.

The issue, as I understand it, is the potential for an old
kernel parser to silently misparse a new dev tree with this
new property.

Exactly.  Note that such such an old parser will misparse
the "interrupt-array" property as well, namely, not at all.
Debugging *could* be a bit easier though, who knows.

Do the version and last_comp_version in the blob header
help here?

That's not what those version fields are for.

The real solution is to just not use too old kernels with
too new a firmware/bootloader/device tree.  This issue
has existed for since forever and is in no way specific
to adding new OF bindings, or specific to Linux, or
anything like that.

It's more of a general problem.

Quite so.

It might be worthwhile to add a device tree property
"minimum-linux-version" (in the tree root) that the kernel
will parse and complain about loudly if it doesn't match;
at a minimum it will make it super-duper clear to users
of a DTS with that property which combos work.  Documentation
rules ;-)


Segher

On Wed, Feb 28, 2007 at 08:56:55AM -0700, Yoder Stuart-B08248 wrote:

quoted

-----Original Message-----
From: David Gibson [mailto:david@gibson.dropbear.id.au] 
Sent: Tuesday, February 27, 2007 6:56 PM
To: Yoder Stuart-B08248
Cc: paulus@samba.org; linuxppc-dev@ozlabs.org
Subject: Re: [RFC] powerpc: Document new property called 
interrupt-parents

On Tue, Feb 27, 2007 at 04:54:45PM -0600, Stuart Yoder wrote:

quoted

Document new property called interrupt-parents which can
be used to represent interrupts for devices that route
interrupts to more than one interrupt controller.

Well, whatever Segher says, I'm still very concerned about the
potential for extra difficulty in debugging with this approach.

The issue, as I understand it, is the potential for an old
kernel parser to silently misparse a new dev tree with this
new property.

Do the version and last_comp_version in the blob header
help here?

Unfortunately not.  Those fields version the the serialization format
of the device tree, not the content of the device tree.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

quoted

Do the version and last_comp_version in the blob header
help here?

Unfortunately not.  Those fields version the the serialization format
of the device tree, not the content of the device tree.

I'm glad you agree :-)


Segher