diff mbox

[03/11] powerpc/85xx: Rework PCI nodes on P1020RDB

Message ID 1319318452-27036-3-git-send-email-galak@kernel.crashing.org (mailing list archive)
State Accepted, archived
Commit fc2478e728b252c33d9655b234584a24c524d7e3
Headers show

Commit Message

Kumar Gala Oct. 22, 2011, 9:20 p.m. UTC
* Move SoC specific details like irq mapping to SoC dtsi
* Update interrupt property to cover both error interrupt and PCIe
  runtime interrupts

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/boot/dts/p1020rdb.dts |   26 +---------------------
 arch/powerpc/boot/dts/p1020si.dtsi |   40 ++++++++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 28 deletions(-)

Comments

Tabi Timur-B04825 Oct. 23, 2011, 2:37 p.m. UTC | #1
On Sat, Oct 22, 2011 at 4:20 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> * Move SoC specific details like irq mapping to SoC dtsi
> * Update interrupt property to cover both error interrupt and PCIe
>  runtime interrupts

Are we going to be doing this for all our device trees?  If so, then I
think we need to document what properties the board dts should be
defining when it includes a node from a dtsi.  Something like this:

dtsi:
               pcie@0 {
                       /* dts should define 'reg' and 'ranges' */
                       reg = <0 0 0 0 0>;
                       #interrupt-cells = <1>;

I suppose it's obvious that 'reg' and 'ranges' should be defined, so
this isn't the best example.  But we should document if any other
properties should be defined.

For example, the SSI nodes contain a bunch of SOC- and board-specific
properties.
Kumar Gala Oct. 24, 2011, 2:02 p.m. UTC | #2
On Oct 23, 2011, at 9:37 AM, Tabi Timur-B04825 wrote:

> On Sat, Oct 22, 2011 at 4:20 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>> * Move SoC specific details like irq mapping to SoC dtsi
>> * Update interrupt property to cover both error interrupt and PCIe
>>  runtime interrupts
> 
> Are we going to be doing this for all our device trees?  If so, then I
> think we need to document what properties the board dts should be
> defining when it includes a node from a dtsi.  Something like this:

Yes, I intend we do this as much as possible.

> dtsi:
>               pcie@0 {
>                       /* dts should define 'reg' and 'ranges' */
>                       reg = <0 0 0 0 0>;
>                       #interrupt-cells = <1>;
> 
> I suppose it's obvious that 'reg' and 'ranges' should be defined, so
> this isn't the best example.  But we should document if any other
> properties should be defined.
> 
> For example, the SSI nodes contain a bunch of SOC- and board-specific
> properties.

I would have hoped the bindings had made it clear already what was board info vs what was SoC.

If not, they should be clarify that in the binding specs.

- k
Timur Tabi Oct. 24, 2011, 2:05 p.m. UTC | #3
Kumar Gala wrote:
> I would have hoped the bindings had made it clear already what was board info vs what was SoC.

When it comes to device trees, I never assume anything is "clear".

> If not, they should be clarify that in the binding specs.

I'm okay with that.
diff mbox

Patch

diff --git a/arch/powerpc/boot/dts/p1020rdb.dts b/arch/powerpc/boot/dts/p1020rdb.dts
index d6a8ae4..8b1a7ee 100644
--- a/arch/powerpc/boot/dts/p1020rdb.dts
+++ b/arch/powerpc/boot/dts/p1020rdb.dts
@@ -257,19 +257,8 @@ 
 	pci0: pcie@ffe09000 {
 		ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc10000 0x0 0x10000>;
-		interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
-		interrupt-map = <
-			/* IDSEL 0x0 */
-			0000 0x0 0x0 0x1 &mpic 0x4 0x1
-			0000 0x0 0x0 0x2 &mpic 0x5 0x1
-			0000 0x0 0x0 0x3 &mpic 0x6 0x1
-			0000 0x0 0x0 0x4 &mpic 0x7 0x1
-			>;
+		reg = <0 0xffe09000 0 0x1000>;
 		pcie@0 {
-			reg = <0x0 0x0 0x0 0x0 0x0>;
-			#size-cells = <2>;
-			#address-cells = <3>;
-			device_type = "pci";
 			ranges = <0x2000000 0x0 0xa0000000
 				  0x2000000 0x0 0xa0000000
 				  0x0 0x20000000
@@ -281,21 +270,10 @@ 
 	};
 
 	pci1: pcie@ffe0a000 {
+		reg = <0 0xffe0a000 0 0x1000>;
 		ranges = <0x2000000 0x0 0x80000000 0 0x80000000 0x0 0x20000000
 			  0x1000000 0x0 0x00000000 0 0xffc00000 0x0 0x10000>;
-		interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
-		interrupt-map = <
-			/* IDSEL 0x0 */
-			0000 0x0 0x0 0x1 &mpic 0x0 0x1
-			0000 0x0 0x0 0x2 &mpic 0x1 0x1
-			0000 0x0 0x0 0x3 &mpic 0x2 0x1
-			0000 0x0 0x0 0x4 &mpic 0x3 0x1
-			>;
 		pcie@0 {
-			reg = <0x0 0x0 0x0 0x0 0x0>;
-			#size-cells = <2>;
-			#address-cells = <3>;
-			device_type = "pci";
 			ranges = <0x2000000 0x0 0x80000000
 				  0x2000000 0x0 0x80000000
 				  0x0 0x20000000
diff --git a/arch/powerpc/boot/dts/p1020si.dtsi b/arch/powerpc/boot/dts/p1020si.dtsi
index 5c5acb6..58f6b30 100644
--- a/arch/powerpc/boot/dts/p1020si.dtsi
+++ b/arch/powerpc/boot/dts/p1020si.dtsi
@@ -352,26 +352,58 @@ 
 	pci0: pcie@ffe09000 {
 		compatible = "fsl,mpc8548-pcie";
 		device_type = "pci";
-		#interrupt-cells = <1>;
 		#size-cells = <2>;
 		#address-cells = <3>;
-		reg = <0 0xffe09000 0 0x1000>;
 		bus-range = <0 255>;
 		clock-frequency = <33333333>;
 		interrupt-parent = <&mpic>;
 		interrupts = <16 2>;
+
+		pcie@0 {
+			reg = <0 0 0 0 0>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			device_type = "pci";
+			interrupts = <16 2>;
+			interrupt-map-mask = <0xf800 0 0 7>;
+			interrupt-map = <
+				/* IDSEL 0x0 */
+				0000 0x0 0x0 0x1 &mpic 0x4 0x1
+				0000 0x0 0x0 0x2 &mpic 0x5 0x1
+				0000 0x0 0x0 0x3 &mpic 0x6 0x1
+				0000 0x0 0x0 0x4 &mpic 0x7 0x1
+				>;
+		};
+
 	};
 
 	pci1: pcie@ffe0a000 {
 		compatible = "fsl,mpc8548-pcie";
 		device_type = "pci";
-		#interrupt-cells = <1>;
 		#size-cells = <2>;
 		#address-cells = <3>;
-		reg = <0 0xffe0a000 0 0x1000>;
 		bus-range = <0 255>;
 		clock-frequency = <33333333>;
 		interrupt-parent = <&mpic>;
 		interrupts = <16 2>;
+
+		pcie@0 {
+			reg = <0 0 0 0 0>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			device_type = "pci";
+			interrupts = <16 2>;
+			interrupt-map-mask = <0xf800 0 0 7>;
+
+			interrupt-map = <
+				/* IDSEL 0x0 */
+				0000 0x0 0x0 0x1 &mpic 0x0 0x1
+				0000 0x0 0x0 0x2 &mpic 0x1 0x1
+				0000 0x0 0x0 0x3 &mpic 0x2 0x1
+				0000 0x0 0x0 0x4 &mpic 0x3 0x1
+				>;
+		};
 	};
 };