[PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

Added basic board port for MPC8572 DS reference platform that is
similiar to the MPC8544/33 DS reference platform in uniprocessor mode.

---

Rev 3 -- updates to the device tree based on discussion on how we want
to handle memory busses going forward.

 arch/powerpc/boot/dts/mpc8572ds.dts       |  374 +++++++
 arch/powerpc/configs/mpc8572_ds_defconfig | 1496 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/85xx/mpc85xx_ds.c  |   31 +
 arch/powerpc/sysdev/fsl_pci.c             |    2 +
 include/linux/pci_ids.h                   |    2 +
 5 files changed, 1905 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/mpc8572ds.dts
 create mode 100644 arch/powerpc/configs/mpc8572_ds_defconfig

diff --git a/arch/powerpc/boot/dts/mpc8572ds.dts b/arch/powerpc/boot/dts/mpc8572ds.dts
new file mode 100644
index 0000000..bc8feaf
--- /dev/null
+++ b/arch/powerpc/boot/dts/mpc8572ds.dts
@@ -0,0 +1,374 @@
+/*
+ * MPC8572 DS Device Tree Source
+ *
+ * Copyright 2007 Freescale Semiconductor Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+/ {
+	model = "fsl,MPC8572DS";
+	compatible = "fsl,MPC8572DS", "fsl,MPC85xxDS";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		PowerPC,8572@0 {
+			device_type = "cpu";
+			reg = <0>;
+			d-cache-line-size = <20>;	// 32 bytes
+			i-cache-line-size = <20>;	// 32 bytes
+			d-cache-size = <8000>;		// L1, 32K
+			i-cache-size = <8000>;		// L1, 32K
+			timebase-frequency = <0>;
+			bus-frequency = <0>;
+			clock-frequency = <0>;
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <00000000 00000000>;	// Filled by U-Boot
+	};
+
+	soc8572@ffe00000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		device_type = "soc";
+		ranges = <00000000 ffe00000 00100000>;
+		reg = <ffe00000 00001000>;	// CCSRBAR & soc regs, remove once parse code for immrbase fixed
+		bus-frequency = <0>;		// Filled out by uboot.
+
+		memory-controller@2000 {
+			compatible = "fsl,8572-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <12 2>;
+		};
+
+		memory-controller@6000 {
+			compatible = "fsl,8572-memory-controller";
+			reg = <6000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <12 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,8572-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <80000>;	// L2, 512K
+			interrupt-parent = <&mpic>;
+			interrupts = <10 2>;
+		};
+
+		i2c@3000 {
+			device_type = "i2c";
+			compatible = "fsl-i2c";
+			reg = <3000 100>;
+			interrupts = <2b 2>;
+			interrupt-parent = <&mpic>;
+			dfsrr;
+		};
+
+		mdio@24520 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			device_type = "mdio";
+			compatible = "gianfar";
+			reg = <24520 20>;
+			phy0: ethernet-phy@0 {
+				interrupt-parent = <&mpic>;
+				interrupts = <a 1>;
+				reg = <0>;
+				device_type = "ethernet-phy";
+			};
+			phy1: ethernet-phy@1 {
+				interrupt-parent = <&mpic>;
+				interrupts = <a 1>;
+				reg = <1>;
+				device_type = "ethernet-phy";
+			};
+			phy2: ethernet-phy@2 {
+				interrupt-parent = <&mpic>;
+				interrupts = <a 1>;
+				reg = <2>;
+				device_type = "ethernet-phy";
+			};
+			phy3: ethernet-phy@3 {
+				interrupt-parent = <&mpic>;
+				interrupts = <a 1>;
+				reg = <3>;
+				device_type = "ethernet-phy";
+			};
+		};
+
+		ethernet@24000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			device_type = "network";
+			model = "eTSEC";
+			compatible = "gianfar";
+			reg = <24000 1000>;
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			interrupts = <1d 2 1e 2 22 2>;
+			interrupt-parent = <&mpic>;
+			phy-handle = <&phy0>;
+			phy-connection-type = "rgmii-id";
+		};
+
+		ethernet@25000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			device_type = "network";
+			model = "eTSEC";
+			compatible = "gianfar";
+			reg = <25000 1000>;
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			interrupts = <23 2 24 2 28 2>;
+			interrupt-parent = <&mpic>;
+			phy-handle = <&phy1>;
+			phy-connection-type = "rgmii-id";
+		};
+
+		ethernet@26000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			device_type = "network";
+			model = "eTSEC";
+			compatible = "gianfar";
+			reg = <26000 1000>;
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			interrupts = <1f 2 20 2 21 2>;
+			interrupt-parent = <&mpic>;
+			phy-handle = <&phy2>;
+			phy-connection-type = "rgmii-id";
+		};
+
+		ethernet@27000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			device_type = "network";
+			model = "eTSEC";
+			compatible = "gianfar";
+			reg = <27000 1000>;
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			interrupts = <25 2 26 2 27 2>;
+			interrupt-parent = <&mpic>;
+			phy-handle = <&phy3>;
+			phy-connection-type = "rgmii-id";
+		};
+
+		serial@4500 {
+			device_type = "serial";
+			compatible = "ns16550";
+			reg = <4500 100>;
+			clock-frequency = <0>;
+			interrupts = <2a 2>;
+			interrupt-parent = <&mpic>;
+		};
+
+		serial@4600 {
+			device_type = "serial";
+			compatible = "ns16550";
+			reg = <4600 100>;
+			clock-frequency = <0>;
+			interrupts = <2a 2>;
+			interrupt-parent = <&mpic>;
+		};
+
+		global-utilities@e0000 {	//global utilities block
+			compatible = "fsl,mpc8548-guts";
+			reg = <e0000 1000>;
+			fsl,has-rstcr;
+		};
+
+		mpic: pic@40000 {
+			clock-frequency = <0>;
+			interrupt-controller;
+			#address-cells = <0>;
+			#interrupt-cells = <2>;
+			reg = <40000 40000>;
+			compatible = "chrp,open-pic";
+			device_type = "open-pic";
+			big-endian;
+		};
+	};
+
+	pcie@ffe08000 {
+		compatible = "fsl,mpc8548-pcie";
+		device_type = "pci";
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <ffe08000 1000>;
+		bus-range = <0 ff>;
+		ranges = <02000000 0 80000000 80000000 0 20000000
+			  01000000 0 00000000 ffc00000 0 00010000>;
+		clock-frequency = <1fca055>;
+		interrupt-parent = <&mpic>;
+		interrupts = <18 2>;
+		interrupt-map-mask = <fb00 0 0 0>;
+		interrupt-map = <
+			/* IDSEL 0x11 - PCI slot 1 */
+			8800 0 0 1 &mpic 2 1
+			8800 0 0 2 &mpic 3 1
+			8800 0 0 3 &mpic 4 1
+			8800 0 0 4 &mpic 1 1
+
+			/* IDSEL 0x12 - PCI slot 2 */
+			9000 0 0 1 &mpic 3 1
+			9000 0 0 2 &mpic 4 1
+			9000 0 0 3 &mpic 1 1
+			9000 0 0 4 &mpic 2 1
+
+			// IDSEL 0x1c  USB
+			e000 0 0 0 &i8259 c 2
+			e100 0 0 0 &i8259 9 2
+			e200 0 0 0 &i8259 a 2
+			e300 0 0 0 &i8259 b 2
+
+			// IDSEL 0x1d  Audio
+			e800 0 0 0 &i8259 6 2
+
+			// IDSEL 0x1e Legacy
+			f000 0 0 0 &i8259 7 2
+			f100 0 0 0 &i8259 7 2
+
+			// IDSEL 0x1f IDE/SATA
+			f800 0 0 0 &i8259 e 2
+			f900 0 0 0 &i8259 5 2
+
+			>;
+		uli1575@0 {
+			reg = <0 0 0 0 0>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			ranges = <02000000 0 80000000
+				  02000000 0 80000000
+				  0 20000000
+				  01000000 0 00000000
+				  01000000 0 00000000
+				  0 00100000>;
+
+			pci_bridge@0 {
+				reg = <0 0 0 0 0>;
+				#size-cells = <2>;
+				#address-cells = <3>;
+				ranges = <02000000 0 80000000
+					  02000000 0 80000000
+					  0 20000000
+					  01000000 0 00000000
+					  01000000 0 00000000
+					  0 00100000>;
+
+				isa@1e {
+					device_type = "isa";
+					#interrupt-cells = <2>;
+					#size-cells = <1>;
+					#address-cells = <2>;
+					reg = <f000 0 0 0 0>;
+					ranges = <1 0 01000000 0 0
+						  00001000>;
+					interrupt-parent = <&i8259>;
+
+					i8259: interrupt-controller@20 {
+						reg = <1 20 2
+						       1 a0 2
+						       1 4d0 2>;
+						clock-frequency = <0>;
+						interrupt-controller;
+						device_type = "interrupt-controller";
+						#address-cells = <0>;
+						#interrupt-cells = <2>;
+						built-in;
+						compatible = "chrp,iic";
+						interrupts = <9 2>;
+						interrupt-parent =
+							<&mpic>;
+					};
+
+					i8042@60 {
+						#size-cells = <0>;
+						#address-cells = <1>;
+						reg = <1 60 1 1 64 1>;
+						interrupts = <1 3 c 3>;
+						interrupt-parent =
+							<&i8259>;
+
+						keyboard@0 {
+							reg = <0>;
+							compatible = "pnpPNP,303";
+						};
+
+						mouse@1 {
+							reg = <1>;
+							compatible = "pnpPNP,f03";
+						};
+					};
+
+					rtc@70 {
+						compatible = "pnpPNP,b00";
+						reg = <1 70 2>;
+					};
+
+					gpio@400 {
+						reg = <1 400 80>;
+					};
+				};
+			};
+		};
+
+	};
+
+	pcie@ffe09000 {
+		compatible = "fsl,mpc8548-pcie";
+		device_type = "pci";
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <ffe09000 1000>;
+		bus-range = <0 ff>;
+		ranges = <02000000 0 a0000000 a0000000 0 20000000
+			  01000000 0 00000000 ffc10000 0 00010000>;
+		clock-frequency = <1fca055>;
+		interrupt-parent = <&mpic>;
+		interrupts = <1a 2>;
+		interrupt-map-mask = <f800 0 0 7>;
+		interrupt-map = <
+			/* IDSEL 0x0 */
+			0000 0 0 1 &mpic 4 1
+			0000 0 0 2 &mpic 5 1
+			0000 0 0 3 &mpic 6 1
+			0000 0 0 4 &mpic 7 1
+			>;
+	};
+
+	pcie@ffe0a000 {
+		compatible = "fsl,mpc8548-pcie";
+		device_type = "pci";
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <ffe0a000 1000>;
+		bus-range = <0 ff>;
+		ranges = <02000000 0 c0000000 c0000000 0 20000000
+			  01000000 0 00000000 ffc20000 0 00010000>;
+		clock-frequency = <1fca055>;
+		interrupt-parent = <&mpic>;
+		interrupts = <1b 2>;
+		interrupt-map = <
+			/* IDSEL 0x0 */
+			0000 0 0 1 &mpic 0 1
+			0000 0 0 2 &mpic 1 1
+			0000 0 0 3 &mpic 2 1
+			0000 0 0 4 &mpic 3 1
+			>;
+	};
+};
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 3a5c3c4..4d44902 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
@@ -181,6 +181,23 @@ static int __init mpc8544_ds_probe(void)
 	}
 }

+/*
+ * Called very early, device-tree isn't unflattened
+ */
+static int __init mpc8572_ds_probe(void)
+{
+	unsigned long root = of_get_flat_dt_root();
+
+	if (of_flat_dt_is_compatible(root, "fsl,MPC8572DS")) {
+#ifdef CONFIG_PCI
+		primary_phb_addr = 0x8000;
+#endif
+		return 1;
+	} else {
+		return 0;
+	}
+}
+
 define_machine(mpc8544_ds) {
 	.name			= "MPC8544 DS",
 	.probe			= mpc8544_ds_probe,
@@ -194,3 +211,17 @@ define_machine(mpc8544_ds) {
 	.calibrate_decr		= generic_calibrate_decr,
 	.progress		= udbg_progress,
 };
+
+define_machine(mpc8572_ds) {
+	.name			= "MPC8572 DS",
+	.probe			= mpc8572_ds_probe,
+	.setup_arch		= mpc85xx_ds_setup_arch,
+	.init_IRQ		= mpc85xx_ds_pic_init,
+#ifdef CONFIG_PCI
+	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+#endif
+	.get_irq		= mpic_get_irq,
+	.restart		= mpc85xx_restart,
+	.calibrate_decr		= generic_calibrate_decr,
+	.progress		= udbg_progress,
+};
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 114c90f..34cad96 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -255,5 +255,7 @@ DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8533E, quirk_fsl_pcie_transpare
 DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8533, quirk_fsl_pcie_transparent);
 DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8544E, quirk_fsl_pcie_transparent);
 DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8544, quirk_fsl_pcie_transparent);
+DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8572E, quirk_fsl_pcie_transparent)
+DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8572, quirk_fsl_pcie_transparent);
 DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8641, quirk_fsl_pcie_transparent);
 DECLARE_PCI_FIXUP_EARLY(0x1957, PCI_DEVICE_ID_MPC8641D, quirk_fsl_pcie_transparent);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 06d23e1..daedb60 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2100,6 +2100,8 @@
 #define PCI_DEVICE_ID_MPC8533		0x0031
 #define PCI_DEVICE_ID_MPC8544E		0x0032
 #define PCI_DEVICE_ID_MPC8544		0x0033
+#define PCI_DEVICE_ID_MPC8572E		0x0040
+#define PCI_DEVICE_ID_MPC8572		0x0041
 #define PCI_DEVICE_ID_MPC8641		0x7010
 #define PCI_DEVICE_ID_MPC8641D		0x7011

-- 
1.5.2.4

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[David Gibson]

On Tue, Sep 11, 2007 at 02:37:56PM -0500, Kumar Gala wrote:
> Added basic board port for MPC8572 DS reference platform that is
> similiar to the MPC8544/33 DS reference platform in uniprocessor mode.
> 
> ---
> 
> Rev 3 -- updates to the device tree based on discussion on how we want
> to handle memory busses going forward.

[snip]
> +		mdio@24520 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "mdio";

I don't think we actually have an mdio device_type binding defined.


> +			compatible = "gianfar";

This needs to be more specific.  And it certainly shouldn't be the
same compatible string as the ethernet MACs have.

> +			reg = <24520 20>;
> +			phy0: ethernet-phy@0 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <0>;
> +				device_type = "ethernet-phy";

Do we actually have an ethernet-phy device_type binding?  If not, we
should kill this.  'compatible' properties for phys would probably be
a good idea, though (giving the actual phy model).

> +			};
> +			phy1: ethernet-phy@1 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <1>;
> +				device_type = "ethernet-phy";
> +			};
> +			phy2: ethernet-phy@2 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <2>;
> +				device_type = "ethernet-phy";
> +			};
> +			phy3: ethernet-phy@3 {
> +				interrupt-parent = <&mpic>;
> +				interrupts = <a 1>;
> +				reg = <3>;
> +				device_type = "ethernet-phy";
> +			};
> +		};
> +
> +		ethernet@24000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <24000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <1d 2 1e 2 22 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy0>;
> +			phy-connection-type = "rgmii-id";
> +		};
> +
> +		ethernet@25000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <25000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <23 2 24 2 28 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy1>;
> +			phy-connection-type = "rgmii-id";
> +		};
> +
> +		ethernet@26000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <26000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <1f 2 20 2 21 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy2>;
> +			phy-connection-type = "rgmii-id";
> +		};
> +
> +		ethernet@27000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "eTSEC";
> +			compatible = "gianfar";
> +			reg = <27000 1000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <25 2 26 2 27 2>;
> +			interrupt-parent = <&mpic>;
> +			phy-handle = <&phy3>;
> +			phy-connection-type = "rgmii-id";
> +		};


> +
> +		serial@4500 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <4500 100>;
> +			clock-frequency = <0>;
> +			interrupts = <2a 2>;
> +			interrupt-parent = <&mpic>;
> +		};
> +
> +		serial@4600 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <4600 100>;
> +			clock-frequency = <0>;
> +			interrupts = <2a 2>;
> +			interrupt-parent = <&mpic>;
> +		};
> +
> +		global-utilities@e0000 {	//global utilities block
> +			compatible = "fsl,mpc8548-guts";

Hrm... this should have "fsp,mpc8572-guts" in addition to the older
model with which it is compatible.

> +			reg = <e0000 1000>;
> +			fsl,has-rstcr;
> +		};
> +
> +		mpic: pic@40000 {
> +			clock-frequency = <0>;
> +			interrupt-controller;
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			reg = <40000 40000>;
> +			compatible = "chrp,open-pic";
> +			device_type = "open-pic";
> +			big-endian;
> +		};
> +	};
> +
> +	pcie@ffe08000 {
> +		compatible = "fsl,mpc8548-pcie";

And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".

> +		device_type = "pci";
> +		#interrupt-cells = <1>;
> +		#size-cells = <2>;
> +		#address-cells = <3>;
> +		reg = <ffe08000 1000>;
> +		bus-range = <0 ff>;
> +		ranges = <02000000 0 80000000 80000000 0 20000000
> +			  01000000 0 00000000 ffc00000 0 00010000>;
> +		clock-frequency = <1fca055>;
> +		interrupt-parent = <&mpic>;
> +		interrupts = <18 2>;
> +		interrupt-map-mask = <fb00 0 0 0>;
> +		interrupt-map = <
> +			/* IDSEL 0x11 - PCI slot 1 */
> +			8800 0 0 1 &mpic 2 1
> +			8800 0 0 2 &mpic 3 1
> +			8800 0 0 3 &mpic 4 1
> +			8800 0 0 4 &mpic 1 1
> +
> +			/* IDSEL 0x12 - PCI slot 2 */
> +			9000 0 0 1 &mpic 3 1
> +			9000 0 0 2 &mpic 4 1
> +			9000 0 0 3 &mpic 1 1
> +			9000 0 0 4 &mpic 2 1
> +
> +			// IDSEL 0x1c  USB
> +			e000 0 0 0 &i8259 c 2
> +			e100 0 0 0 &i8259 9 2
> +			e200 0 0 0 &i8259 a 2
> +			e300 0 0 0 &i8259 b 2
> +
> +			// IDSEL 0x1d  Audio
> +			e800 0 0 0 &i8259 6 2
> +
> +			// IDSEL 0x1e Legacy
> +			f000 0 0 0 &i8259 7 2
> +			f100 0 0 0 &i8259 7 2
> +
> +			// IDSEL 0x1f IDE/SATA
> +			f800 0 0 0 &i8259 e 2
> +			f900 0 0 0 &i8259 5 2
> +
> +			>;
> +		uli1575@0 {
> +			reg = <0 0 0 0 0>;

This looks kind of bogus...

> +			#size-cells = <2>;
> +			#address-cells = <3>;
> +			ranges = <02000000 0 80000000
> +				  02000000 0 80000000
> +				  0 20000000
> +				  01000000 0 00000000
> +				  01000000 0 00000000
> +				  0 00100000>;
> +
> +			pci_bridge@0 {

Ok.. why is pci_bridge nested within uli1575 - with the matching reg
and ranges, it looks like they ought to be one device.  Also if this
is a PCI<->PCI bridge, I believe it shold have device_type = "pci".

> +				reg = <0 0 0 0 0>;
> +				#size-cells = <2>;
> +				#address-cells = <3>;
> +				ranges = <02000000 0 80000000
> +					  02000000 0 80000000
> +					  0 20000000
> +					  01000000 0 00000000
> +					  01000000 0 00000000
> +					  0 00100000>;
> +
> +				isa@1e {
> +					device_type = "isa";
> +					#interrupt-cells = <2>;
> +					#size-cells = <1>;
> +					#address-cells = <2>;
> +					reg = <f000 0 0 0 0>;
> +					ranges = <1 0 01000000 0 0
> +						  00001000>;
> +					interrupt-parent = <&i8259>;
> +
> +					i8259: interrupt-controller@20 {
> +						reg = <1 20 2
> +						       1 a0 2
> +						       1 4d0 2>;
> +						clock-frequency = <0>;

Hrm.. what is clock-frequency for on an i8259?  I see that other 8259
descriptions have this as well, so it's not a problem with this patch
specifically.

-- 
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

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

On Sep 11, 2007, at 10:11 PM, David Gibson wrote:

> On Tue, Sep 11, 2007 at 02:37:56PM -0500, Kumar Gala wrote:
>> Added basic board port for MPC8572 DS reference platform that is
>> similiar to the MPC8544/33 DS reference platform in uniprocessor  
>> mode.
>>
>> ---
>>
>> Rev 3 -- updates to the device tree based on discussion on how we  
>> want
>> to handle memory busses going forward.
>
> [snip]
>> +		mdio@24520 {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			device_type = "mdio";
>
> I don't think we actually have an mdio device_type binding defined.

We've had this on 83xx/85xx/86xx device trees for a far amount of  
time now.  Look at section 2a in booting-without-of.txt

>
>
>> +			compatible = "gianfar";
>
> This needs to be more specific.  And it certainly shouldn't be the
> same compatible string as the ethernet MACs have.

Why not its the same controller?  Again, we've been doing this for  
some period of time already.

>
>> +			reg = <24520 20>;
>> +			phy0: ethernet-phy@0 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <0>;
>> +				device_type = "ethernet-phy";
>
> Do we actually have an ethernet-phy device_type binding?  If not, we
> should kill this.  'compatible' properties for phys would probably be
> a good idea, though (giving the actual phy model).

Look section 2c in booting-without-of.txt

>> +			};
>> +			phy1: ethernet-phy@1 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <1>;
>> +				device_type = "ethernet-phy";
>> +			};
>> +			phy2: ethernet-phy@2 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <2>;
>> +				device_type = "ethernet-phy";
>> +			};
>> +			phy3: ethernet-phy@3 {
>> +				interrupt-parent = <&mpic>;
>> +				interrupts = <a 1>;
>> +				reg = <3>;
>> +				device_type = "ethernet-phy";
>> +			};
>> +		};

[snip]

>>
>> +		global-utilities@e0000 {	//global utilities block
>> +			compatible = "fsl,mpc8548-guts";
>
> Hrm... this should have "fsp,mpc8572-guts" in addition to the older
> model with which it is compatible.

I've changed this to just fsl,mpc8572-guts

>
>> +			reg = <e0000 1000>;
>> +			fsl,has-rstcr;
>> +		};
>> +
>> +		mpic: pic@40000 {
>> +			clock-frequency = <0>;
>> +			interrupt-controller;
>> +			#address-cells = <0>;
>> +			#interrupt-cells = <2>;
>> +			reg = <40000 40000>;
>> +			compatible = "chrp,open-pic";
>> +			device_type = "open-pic";
>> +			big-endian;
>> +		};
>> +	};
>> +
>> +	pcie@ffe08000 {
>> +		compatible = "fsl,mpc8548-pcie";
>
> And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".

But why?  there is no difference between the PCIe controller in  
mpc8548 and mpc8572?

>
>> +		device_type = "pci";
>> +		#interrupt-cells = <1>;
>> +		#size-cells = <2>;
>> +		#address-cells = <3>;
>> +		reg = <ffe08000 1000>;
>> +		bus-range = <0 ff>;
>> +		ranges = <02000000 0 80000000 80000000 0 20000000
>> +			  01000000 0 00000000 ffc00000 0 00010000>;
>> +		clock-frequency = <1fca055>;
>> +		interrupt-parent = <&mpic>;
>> +		interrupts = <18 2>;
>> +		interrupt-map-mask = <fb00 0 0 0>;
>> +		interrupt-map = <
>> +			/* IDSEL 0x11 - PCI slot 1 */
>> +			8800 0 0 1 &mpic 2 1
>> +			8800 0 0 2 &mpic 3 1
>> +			8800 0 0 3 &mpic 4 1
>> +			8800 0 0 4 &mpic 1 1
>> +
>> +			/* IDSEL 0x12 - PCI slot 2 */
>> +			9000 0 0 1 &mpic 3 1
>> +			9000 0 0 2 &mpic 4 1
>> +			9000 0 0 3 &mpic 1 1
>> +			9000 0 0 4 &mpic 2 1
>> +
>> +			// IDSEL 0x1c  USB
>> +			e000 0 0 0 &i8259 c 2
>> +			e100 0 0 0 &i8259 9 2
>> +			e200 0 0 0 &i8259 a 2
>> +			e300 0 0 0 &i8259 b 2
>> +
>> +			// IDSEL 0x1d  Audio
>> +			e800 0 0 0 &i8259 6 2
>> +
>> +			// IDSEL 0x1e Legacy
>> +			f000 0 0 0 &i8259 7 2
>> +			f100 0 0 0 &i8259 7 2
>> +
>> +			// IDSEL 0x1f IDE/SATA
>> +			f800 0 0 0 &i8259 e 2
>> +			f900 0 0 0 &i8259 5 2
>> +
>> +			>;
>> +		uli1575@0 {
>> +			reg = <0 0 0 0 0>;
>
> This looks kind of bogus...

Its a PCIe to PCI bridge that is transparent.

>> +			#size-cells = <2>;
>> +			#address-cells = <3>;
>> +			ranges = <02000000 0 80000000
>> +				  02000000 0 80000000
>> +				  0 20000000
>> +				  01000000 0 00000000
>> +				  01000000 0 00000000
>> +				  0 00100000>;
>> +
>> +			pci_bridge@0 {
>
> Ok.. why is pci_bridge nested within uli1575 - with the matching reg
> and ranges, it looks like they ought to be one device.  Also if this
> is a PCI<->PCI bridge, I believe it shold have device_type = "pci".

We've been using this as it stands for a while.  If there are some  
changes here that make sense I'm willing to make them.

>> +				reg = <0 0 0 0 0>;
>> +				#size-cells = <2>;
>> +				#address-cells = <3>;
>> +				ranges = <02000000 0 80000000
>> +					  02000000 0 80000000
>> +					  0 20000000
>> +					  01000000 0 00000000
>> +					  01000000 0 00000000
>> +					  0 00100000>;
>> +
>> +				isa@1e {
>> +					device_type = "isa";
>> +					#interrupt-cells = <2>;
>> +					#size-cells = <1>;
>> +					#address-cells = <2>;
>> +					reg = <f000 0 0 0 0>;
>> +					ranges = <1 0 01000000 0 0
>> +						  00001000>;
>> +					interrupt-parent = <&i8259>;
>> +
>> +					i8259: interrupt-controller@20 {
>> +						reg = <1 20 2
>> +						       1 a0 2
>> +						       1 4d0 2>;
>> +						clock-frequency = <0>;
>
> Hrm.. what is clock-frequency for on an i8259?  I see that other 8259
> descriptions have this as well, so it's not a problem with this patch
> specifically.

Its a copy-paste thing so I don't know.

- k

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[David Gibson]

On Tue, Sep 11, 2007 at 10:33:20PM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 10:11 PM, David Gibson wrote:
> 
> > On Tue, Sep 11, 2007 at 02:37:56PM -0500, Kumar Gala wrote:
> >> Added basic board port for MPC8572 DS reference platform that is
> >> similiar to the MPC8544/33 DS reference platform in uniprocessor  
> >> mode.
> >>
> >> ---
> >>
> >> Rev 3 -- updates to the device tree based on discussion on how we  
> >> want
> >> to handle memory busses going forward.
> >
> > [snip]
> >> +		mdio@24520 {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +			device_type = "mdio";
> >
> > I don't think we actually have an mdio device_type binding defined.
> 
> We've had this on 83xx/85xx/86xx device trees for a far amount of  
> time now.  Look at section 2a in booting-without-of.txt

Ah, so we have; sorry.  Although the binding as it is currently
written is pretty much pointless - it should actually define some
mappings between dt properties / addresses and the standards defining
the MDIO bus.x

> >
> >> +			compatible = "gianfar";
> >
> > This needs to be more specific.  And it certainly shouldn't be the
> > same compatible string as the ethernet MACs have.
> 
> Why not its the same controller?  Again, we've been doing this for  
> some period of time already.

Yes you have, but it's still crap.  'compatible' should be sufficient
to distinguish the driver needed for device nodes, but the MACs and
MDIO should clearly have different drivers (or at least, different
parts of a driver).

> >> +			reg = <24520 20>;
> >> +			phy0: ethernet-phy@0 {
> >> +				interrupt-parent = <&mpic>;
> >> +				interrupts = <a 1>;
> >> +				reg = <0>;
> >> +				device_type = "ethernet-phy";
> >
> > Do we actually have an ethernet-phy device_type binding?  If not, we
> > should kill this.  'compatible' properties for phys would probably be
> > a good idea, though (giving the actual phy model).
> 
> Look section 2c in booting-without-of.txt

Ah, yes.  That one's a rather less redeemable pointless device_type
binding.  We should kill it from booting-without-of.txt.

> >> +			reg = <e0000 1000>;
> >> +			fsl,has-rstcr;
> >> +		};
> >> +
> >> +		mpic: pic@40000 {
> >> +			clock-frequency = <0>;
> >> +			interrupt-controller;
> >> +			#address-cells = <0>;
> >> +			#interrupt-cells = <2>;
> >> +			reg = <40000 40000>;
> >> +			compatible = "chrp,open-pic";
> >> +			device_type = "open-pic";
> >> +			big-endian;
> >> +		};
> >> +	};
> >> +
> >> +	pcie@ffe08000 {
> >> +		compatible = "fsl,mpc8548-pcie";
> >
> > And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".
> 
> But why?  there is no difference between the PCIe controller in  
> mpc8548 and mpc8572?

As far as you've yet discovered...

> >> +		uli1575@0 {
> >> +			reg = <0 0 0 0 0>;
> >
> > This looks kind of bogus...
> 
> Its a PCIe to PCI bridge that is transparent.

Right.... if it has no control registers, I think it should just lack
'reg', not define a zero-length register block.

> >> +			#size-cells = <2>;
> >> +			#address-cells = <3>;
> >> +			ranges = <02000000 0 80000000
> >> +				  02000000 0 80000000
> >> +				  0 20000000
> >> +				  01000000 0 00000000
> >> +				  01000000 0 00000000
> >> +				  0 00100000>;

And if truly transparent, it should perhaps have just ranges;
indicating that child addresses are identity mapped to parent
addresses.

> >> +
> >> +			pci_bridge@0 {
> >
> > Ok.. why is pci_bridge nested within uli1575 - with the matching reg
> > and ranges, it looks like they ought to be one device.  Also if this
> > is a PCI<->PCI bridge, I believe it shold have device_type = "pci".
> 
> We've been using this as it stands for a while.  If there are some  
> changes here that make sense I'm willing to make them.

Right, at present I don't see why you couldn't just ditch the
pci_bridge node, and drop its contents straight into the uli1575 node.

-- 
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

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Segher Boessenkool]

>>>> +		uli1575@0 {
>>>> +			reg = <0 0 0 0 0>;
>>>
>>> This looks kind of bogus...
>>
>> Its a PCIe to PCI bridge that is transparent.
>
> Right.... if it has no control registers, I think it should just lack
> 'reg', not define a zero-length register block.

"reg" for PCI config registers has length 0 always, it's defined that
way in the PCI binding.

But if this thing is transparent, it doesn't have PCI config regs.

>>>> +			#size-cells = <2>;
>>>> +			#address-cells = <3>;
>>>> +			ranges = <02000000 0 80000000
>>>> +				  02000000 0 80000000
>>>> +				  0 20000000
>>>> +				  01000000 0 00000000
>>>> +				  01000000 0 00000000
>>>> +				  0 00100000>;
>
> And if truly transparent, it should perhaps have just ranges;
> indicating that child addresses are identity mapped to parent
> addresses.

If truly transparent, the node should just not be there at all!

>>>> +			pci_bridge@0 {
>>>
>>> Ok.. why is pci_bridge nested within uli1575 - with the matching reg
>>> and ranges, it looks like they ought to be one device.  Also if this
>>> is a PCI<->PCI bridge, I believe it shold have device_type = "pci".
>>
>> We've been using this as it stands for a while.  If there are some
>> changes here that make sense I'm willing to make them.
>
> Right, at present I don't see why you couldn't just ditch the
> pci_bridge node, and drop its contents straight into the uli1575 node.

Yeah.  The preferred name for PCI-to-PCI bridge nodes is simply "pci", 
btw.


Segher

MDIO & phy device tree bindings (was Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port)

[Kumar Gala]

David,

I'm splitting this up since the mdio & phy comments are more general  
that the 8572.dts

>>> [snip]
>>>> +		mdio@24520 {
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +			device_type = "mdio";
>>>
>>> I don't think we actually have an mdio device_type binding defined.
>>
>> We've had this on 83xx/85xx/86xx device trees for a far amount of
>> time now.  Look at section 2a in booting-without-of.txt
>
> Ah, so we have; sorry.  Although the binding as it is currently
> written is pretty much pointless - it should actually define some
> mappings between dt properties / addresses and the standards defining
> the MDIO bus.x

that's a doc issue at this point

>>>> +			compatible = "gianfar";
>>>
>>> This needs to be more specific.  And it certainly shouldn't be the
>>> same compatible string as the ethernet MACs have.
>>
>> Why not its the same controller?  Again, we've been doing this for
>> some period of time already.
>
> Yes you have, but it's still crap.  'compatible' should be sufficient
> to distinguish the driver needed for device nodes, but the MACs and
> MDIO should clearly have different drivers (or at least, different
> parts of a driver).

don't disagree will see about coming up with a better name, and  
deprecating 'gianfar'.

>>>> +			reg = <24520 20>;
>>>> +			phy0: ethernet-phy@0 {
>>>> +				interrupt-parent = <&mpic>;
>>>> +				interrupts = <a 1>;
>>>> +				reg = <0>;
>>>> +				device_type = "ethernet-phy";
>>>
>>> Do we actually have an ethernet-phy device_type binding?  If not, we
>>> should kill this.  'compatible' properties for phys would  
>>> probably be
>>> a good idea, though (giving the actual phy model).
>>
>> Look section 2c in booting-without-of.txt
>
> Ah, yes.  That one's a rather less redeemable pointless device_type
> binding.  We should kill it from booting-without-of.txt.

agreed, will poke andy on this.

- k

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

>>>> +			reg = <e0000 1000>;
>>>> +			fsl,has-rstcr;
>>>> +		};
>>>> +
>>>> +		mpic: pic@40000 {
>>>> +			clock-frequency = <0>;
>>>> +			interrupt-controller;
>>>> +			#address-cells = <0>;
>>>> +			#interrupt-cells = <2>;
>>>> +			reg = <40000 40000>;
>>>> +			compatible = "chrp,open-pic";
>>>> +			device_type = "open-pic";
>>>> +			big-endian;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	pcie@ffe08000 {
>>>> +		compatible = "fsl,mpc8548-pcie";
>>>
>>> And again, "fsl,mpc8572-pcie", "fsl,mpc8548-pcie".
>>
>> But why?  there is no difference between the PCIe controller in
>> mpc8548 and mpc8572?
>
> As far as you've yet discovered...

Its the same actual block from design.  I'll think some on this.  If  
I had some macro support in the dtc I wouldn't feel so bad about  
doing this.  Its the edit/modify/fix cycle that's a pain.

>>>> +		uli1575@0 {
>>>> +			reg = <0 0 0 0 0>;
>>>
>>> This looks kind of bogus...
>>
>> Its a PCIe to PCI bridge that is transparent.
>
> Right.... if it has no control registers, I think it should just lack
> 'reg', not define a zero-length register block.
>
>>>> +			#size-cells = <2>;
>>>> +			#address-cells = <3>;
>>>> +			ranges = <02000000 0 80000000
>>>> +				  02000000 0 80000000
>>>> +				  0 20000000
>>>> +				  01000000 0 00000000
>>>> +				  01000000 0 00000000
>>>> +				  0 00100000>;
>
> And if truly transparent, it should perhaps have just ranges;
> indicating that child addresses are identity mapped to parent
> addresses.
>
>>>> +
>>>> +			pci_bridge@0 {
>>>
>>> Ok.. why is pci_bridge nested within uli1575 - with the matching reg
>>> and ranges, it looks like they ought to be one device.  Also if this
>>> is a PCI<->PCI bridge, I believe it shold have device_type = "pci".
>>
>> We've been using this as it stands for a while.  If there are some
>> changes here that make sense I'm willing to make them.
>
> Right, at present I don't see why you couldn't just ditch the
> pci_bridge node, and drop its contents straight into the uli1575 node.

upon further review and discussion you are right about dropping the  
pci_bridge@0 node from the ULI.  However we do need to add a pcie@0  
node to cover the virtual P2P bridge in the PHB. So we have something  
like:

pcie@ff808000 {
   pcie@0 {
     uli@0 {
     }
   }
}

- k

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Segher Boessenkool]

>>> +					i8259: interrupt-controller@20 {
>>> +						reg = <1 20 2
>>> +						       1 a0 2
>>> +						       1 4d0 2>;
>>> +						clock-frequency = <0>;
>>
>> Hrm.. what is clock-frequency for on an i8259?  I see that other 8259
>> descriptions have this as well, so it's not a problem with this patch
>> specifically.
>
> Its a copy-paste thing so I don't know.

If your bootwrapper doesn't fill in this value, you should get rid
of this property -- better to have no value than to have the wrong
value, esp. since it's probably unused anyway.


Segher

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

On Sep 12, 2007, at 9:10 AM, Segher Boessenkool wrote:

>>>> +					i8259: interrupt-controller@20 {
>>>> +						reg = <1 20 2
>>>> +						       1 a0 2
>>>> +						       1 4d0 2>;
>>>> +						clock-frequency = <0>;
>>>
>>> Hrm.. what is clock-frequency for on an i8259?  I see that other  
>>> 8259
>>> descriptions have this as well, so it's not a problem with this  
>>> patch
>>> specifically.
>>
>> Its a copy-paste thing so I don't know.
>
> If your bootwrapper doesn't fill in this value, you should get rid
> of this property -- better to have no value than to have the wrong
> value, esp. since it's probably unused anyway.

I'm going to kill this since I can't find it spec'd anywhere.

- k

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Segher Boessenkool]

Looks a lot better, thanks!

Some minor nits and suggestions...

> +/ {
> +	model = "fsl,MPC8572DS";
> +	compatible = "fsl,MPC8572DS", "fsl,MPC85xxDS";

We don't want "xx" compatible entries; especially here it makes
no sense at all.  If the board is compatible to some other (older)
board, just name that board explicitly.

> +		PowerPC,8572@0 {

Maybe it would be good to use "PowerPC,e500" instead -- it would
make it easier to probe for the actual CPU type, that way.  Not
that Linux uses the name/compatible here at all ;-)

> +	soc8572@ffe00000 {

You should put an interrupt-parent in here, so you can get rid of
it in all the children.


And then there's the pci_bridge thing we're discussing on IRC, of
course -- basically, get rid of the pci_bridge pseudo-node, and
move the interrupt-map for the south-bridge devices into the
south-bridge node.


Segher

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

On Sep 12, 2007, at 8:36 AM, Segher Boessenkool wrote:

> Looks a lot better, thanks!
>
> Some minor nits and suggestions...
>
>> +/ {
>> +	model = "fsl,MPC8572DS";
>> +	compatible = "fsl,MPC8572DS", "fsl,MPC85xxDS";
>
> We don't want "xx" compatible entries; especially here it makes
> no sense at all.  If the board is compatible to some other (older)
> board, just name that board explicitly.

removed.

>
>> +		PowerPC,8572@0 {
>
> Maybe it would be good to use "PowerPC,e500" instead -- it would
> make it easier to probe for the actual CPU type, that way.  Not
> that Linux uses the name/compatible here at all ;-)

I thought about this, not sure what the best solution is.

>> +	soc8572@ffe00000 {
>
> You should put an interrupt-parent in here, so you can get rid of
> it in all the children.

Are interrupt-parent's inherited by child nodes?

> And then there's the pci_bridge thing we're discussing on IRC, of
> course -- basically, get rid of the pci_bridge pseudo-node, and
> move the interrupt-map for the south-bridge devices into the
> south-bridge node.

Leaving the interrupt-map in the PHB because that works and moving it  
down has issues.

- k

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[David Gibson]

On Wed, Sep 12, 2007 at 10:28:24PM -0500, Kumar Gala wrote:
[snip]
> >> +	soc8572@ffe00000 {
> >
> > You should put an interrupt-parent in here, so you can get rid of
> > it in all the children.
> 
> Are interrupt-parent's inherited by child nodes?

Strictly speaking, a node's default interrupt-parent is its physical
parent.  And a node without interrupt-map identity maps all interrupts
to *its* parent.  So the interrupt is notionally wired from the child
to its parent and so forth up the bus tree until it reaches a node
with a specified interrupt-parent or interrupt-map.

-- 
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

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Segher Boessenkool]

>>> +		PowerPC,8572@0 {
>>
>> Maybe it would be good to use "PowerPC,e500" instead -- it would
>> make it easier to probe for the actual CPU type, that way.  Not
>> that Linux uses the name/compatible here at all ;-)
>
> I thought about this, not sure what the best solution is.

Since the CPU cores on all these SoCs are identical (well, there
might be a few revisions, or different cache sizes or such -- minor
differences that can be probed for separately), it probably is a
good idea to name them in the tree instead of having each client
have its own table.

Or is there anything about the CPU that can be derived from "8572"
but not from "e500"?

>>> +	soc8572@ffe00000 {
>>
>> You should put an interrupt-parent in here, so you can get rid of
>> it in all the children.
>
> Are interrupt-parent's inherited by child nodes?

A node without "interrupt-parent" uses the regular tree parent for
walking the interrupt "tree".

>> And then there's the pci_bridge thing we're discussing on IRC, of
>> course -- basically, get rid of the pci_bridge pseudo-node, and
>> move the interrupt-map for the south-bridge devices into the
>> south-bridge node.
>
> Leaving the interrupt-map in the PHB because that works and moving it 
> down has issues.

Okay, fair enough.  Are you looking at resolving those kernel issues?


Segher

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

On Sep 13, 2007, at 12:06 PM, Segher Boessenkool wrote:

>>>> +		PowerPC,8572@0 {
>>>
>>> Maybe it would be good to use "PowerPC,e500" instead -- it would
>>> make it easier to probe for the actual CPU type, that way.  Not
>>> that Linux uses the name/compatible here at all ;-)
>>
>> I thought about this, not sure what the best solution is.
>
> Since the CPU cores on all these SoCs are identical (well, there
> might be a few revisions, or different cache sizes or such -- minor
> differences that can be probed for separately), it probably is a
> good idea to name them in the tree instead of having each client
> have its own table.
>
> Or is there anything about the CPU that can be derived from "8572"
> but not from "e500"?

Only in so much that we need something that states what the actual  
processor is.

>>>> +	soc8572@ffe00000 {
>>>
>>> You should put an interrupt-parent in here, so you can get rid of
>>> it in all the children.
>>
>> Are interrupt-parent's inherited by child nodes?
>
> A node without "interrupt-parent" uses the regular tree parent for
> walking the interrupt "tree".
>
>>> And then there's the pci_bridge thing we're discussing on IRC, of
>>> course -- basically, get rid of the pci_bridge pseudo-node, and
>>> move the interrupt-map for the south-bridge devices into the
>>> south-bridge node.
>>
>> Leaving the interrupt-map in the PHB because that works and moving  
>> it down has issues.
>
> Okay, fair enough.  Are you looking at resolving those kernel issues?

No.  I've had enough of this device tree foo for a while :)

[I'm happy to test any patches related to this, if someone else comes  
up with them]

- k

Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Segher Boessenkool]

>>>>> +		PowerPC,8572@0 {
>>>>
>>>> Maybe it would be good to use "PowerPC,e500" instead -- it would
>>>> make it easier to probe for the actual CPU type, that way.  Not
>>>> that Linux uses the name/compatible here at all ;-)
>>>
>>> I thought about this, not sure what the best solution is.
>>
>> Since the CPU cores on all these SoCs are identical (well, there
>> might be a few revisions, or different cache sizes or such -- minor
>> differences that can be probed for separately), it probably is a
>> good idea to name them in the tree instead of having each client
>> have its own table.
>>
>> Or is there anything about the CPU that can be derived from "8572"
>> but not from "e500"?
>
> Only in so much that we need something that states what the actual 
> processor is.

You mean, something needs to say "8572"?  I think the "soc" node
would be best for that.

It's all not terribly important, just something to think about.

>>>> And then there's the pci_bridge thing we're discussing on IRC, of
>>>> course -- basically, get rid of the pci_bridge pseudo-node, and
>>>> move the interrupt-map for the south-bridge devices into the
>>>> south-bridge node.
>>>
>>> Leaving the interrupt-map in the PHB because that works and moving 
>>> it down has issues.
>>
>> Okay, fair enough.  Are you looking at resolving those kernel issues?
>
> No.  I've had enough of this device tree foo for a while :)

Heh okay :-)

> [I'm happy to test any patches related to this, if someone else comes 
> up with them]

Well I don't know what the problem is ("it doesn't work" doesn't
say much), and don't have your hardware to test.  Maybe we can do
it on IRC again ;-)


Segher