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

[PATCH] [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.

---

Posted here for review.  Patch exists in my for-2.6.24 branch. Removed
defconfig to reduce patch review size.

 arch/powerpc/boot/dts/mpc8572ds.dts       |  378 ++++++++
 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                   |    5 +-
 5 files changed, 1910 insertions(+), 2 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..9d23561
--- /dev/null
+++ b/arch/powerpc/boot/dts/mpc8572ds.dts
@@ -0,0 +1,378 @@
+/*
+ * 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 = "MPC8572DS";
+	compatible = "MPC8572DS", "MPC85xxDS";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#cpus = <1>;
+		#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>;
+			32-bit;
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <00000000 00000000>;	// Filled by U-Boot
+	};
+
+	soc8572@ffe00000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		#interrupt-cells = <2>;
+		device_type = "soc";
+		ranges = <00000000 ffe00000 00100000
+			  80000000 80000000 20000000
+			  a0000000 a0000000 20000000
+			  c0000000 c0000000 20000000
+			  ffc00000 ffc00000 00010000
+			  ffc10000 ffc10000 00010000
+			  ffc20000 ffc20000 00010000>;
+
+		reg = <ffe00000 00001000>;	// CCSRBAR 1M
+		bus-frequency = <0>;		// Filled out by uboot.
+
+		memory-controller@2000 {
+			compatible = "fsl,8572-memory-controller";
+			reg = <2000 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>;
+		};
+
+		pcie@8000 {
+			compatible = "fsl,mpc8641-pcie";
+			device_type = "pci";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <8000 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@9000 {
+			compatible = "fsl,mpc8548-pcie";
+			device_type = "pci";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <9000 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@a000 {
+			compatible = "fsl,mpc8548-pcie";
+			device_type = "pci";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <a000 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
+				>;
+		};
+
+		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>;
+			built-in;
+			compatible = "chrp,open-pic";
+			device_type = "open-pic";
+			big-endian;
+		};
+	};
+};
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 3a5c3c4..1e2eba8 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, "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..c98b867 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -374,10 +374,9 @@
 #define PCI_DEVICE_ID_ATI_IXP400_SATA   0x4379
 #define PCI_DEVICE_ID_ATI_IXP400_SATA2	0x437a
 #define PCI_DEVICE_ID_ATI_IXP600_SATA	0x4380
-#define PCI_DEVICE_ID_ATI_IXP600_SMBUS	0x4385
+#define PCI_DEVICE_ID_ATI_SBX00_SMBUS	0x4385
 #define PCI_DEVICE_ID_ATI_IXP600_IDE	0x438c
 #define PCI_DEVICE_ID_ATI_IXP700_SATA	0x4390
-#define PCI_DEVICE_ID_ATI_IXP700_SMBUS	0x4395
 #define PCI_DEVICE_ID_ATI_IXP700_IDE	0x439c

 #define PCI_VENDOR_ID_VLSI		0x1004
@@ -2100,6 +2099,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] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Scott Wood]

On Tue, Sep 11, 2007 at 01:29:18AM -0500, Kumar Gala wrote:
> diff --git a/arch/powerpc/boot/dts/mpc8572ds.dts b/arch/powerpc/boot/dts/mpc8572ds.dts
> new file mode 100644
> index 0000000..9d23561
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/mpc8572ds.dts
> @@ -0,0 +1,378 @@
> +/*
> + * 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 = "MPC8572DS";
> +	compatible = "MPC8572DS", "MPC85xxDS";

"fsl," prefix on compatible.

> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {
> +		#cpus = <1>;

Where is #cpus defined or used?  I don't see it used in any other trees, either.

> +		#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>;
> +			32-bit;

Where is 32-bit; defined or used?

> +	soc8572@ffe00000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		#interrupt-cells = <2>;
> +		device_type = "soc";
> +		ranges = <00000000 ffe00000 00100000
> +			  80000000 80000000 20000000
> +			  a0000000 a0000000 20000000
> +			  c0000000 c0000000 20000000
> +			  ffc00000 ffc00000 00010000
> +			  ffc10000 ffc10000 00010000
> +			  ffc20000 ffc20000 00010000>;
> +
> +		reg = <ffe00000 00001000>;	// CCSRBAR 1M

Comment doesn't match what's actually in reg.

> +		bus-frequency = <0>;		// Filled out by uboot.
> +
> +		memory-controller@2000 {
> +			compatible = "fsl,8572-memory-controller";

Is it compatible with any other 85xx memory controller?

> +			reg = <2000 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>;
> +		};

Should this node be referenced by an l2-cache property in the cpu node?

> +		pcie@8000 {
> +			compatible = "fsl,mpc8641-pcie";

Should probably be "fsl,mpc8572-pcie", "fsl,mpc8641-pcie".

> +			device_type = "pci";
> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			#address-cells = <3>;
> +			reg = <8000 1000>;
> +			bus-range = <0 ff>;
> +			ranges = <02000000 0 80000000 80000000 0 20000000
> +				  01000000 0 00000000 ffc00000 0 00010000>;

No prefetchable mem space?

> +			clock-frequency = <1fca055>;

Decimal would be nicer.

> +		pcie@9000 {
> +			compatible = "fsl,mpc8548-pcie";

Why is this one 8548-compatible and the previous one 8641-compatible?

> +		mpic: pic@40000 {
> +			clock-frequency = <0>;
> +			interrupt-controller;
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			reg = <40000 40000>;
> +			built-in;
> +			compatible = "chrp,open-pic";
> +			device_type = "open-pic";
> +			big-endian;
> +		};

Where is the built-in; property defined or used?

> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 06d23e1..c98b867 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -374,10 +374,9 @@
>  #define PCI_DEVICE_ID_ATI_IXP400_SATA   0x4379
>  #define PCI_DEVICE_ID_ATI_IXP400_SATA2	0x437a
>  #define PCI_DEVICE_ID_ATI_IXP600_SATA	0x4380
> -#define PCI_DEVICE_ID_ATI_IXP600_SMBUS	0x4385
> +#define PCI_DEVICE_ID_ATI_SBX00_SMBUS	0x4385
>  #define PCI_DEVICE_ID_ATI_IXP600_IDE	0x438c
>  #define PCI_DEVICE_ID_ATI_IXP700_SATA	0x4390
> -#define PCI_DEVICE_ID_ATI_IXP700_SMBUS	0x4395
>  #define PCI_DEVICE_ID_ATI_IXP700_IDE	0x439c

What's going on here?

-Scott

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

[Olof Johansson]

Hi,

On Tue, Sep 11, 2007 at 01:29:18AM -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.
> 
> ---
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> index 3a5c3c4..1e2eba8 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, "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,
> +};

How different are these boards really? Could you just detect MPC85xxDS
and have a generic platform for them, or are they different enough that
you need individual ones for it?


> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 06d23e1..c98b867 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -374,10 +374,9 @@
>  #define PCI_DEVICE_ID_ATI_IXP400_SATA   0x4379
>  #define PCI_DEVICE_ID_ATI_IXP400_SATA2	0x437a
>  #define PCI_DEVICE_ID_ATI_IXP600_SATA	0x4380
> -#define PCI_DEVICE_ID_ATI_IXP600_SMBUS	0x4385
> +#define PCI_DEVICE_ID_ATI_SBX00_SMBUS	0x4385
>  #define PCI_DEVICE_ID_ATI_IXP600_IDE	0x438c
>  #define PCI_DEVICE_ID_ATI_IXP700_SATA	0x4390
> -#define PCI_DEVICE_ID_ATI_IXP700_SMBUS	0x4395
>  #define PCI_DEVICE_ID_ATI_IXP700_IDE	0x439c

This looks like it doesn't belong in this patch.


-Olof

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

[Kumar Gala]

On Sep 11, 2007, at 9:18 AM, Olof Johansson wrote:

> Hi,
>
> On Tue, Sep 11, 2007 at 01:29:18AM -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.
>>
>> ---
>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/ 
>> powerpc/platforms/85xx/mpc85xx_ds.c
>> index 3a5c3c4..1e2eba8 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, "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,
>> +};
>
> How different are these boards really? Could you just detect MPC85xxDS
> and have a generic platform for them, or are they different enough  
> that
> you need individual ones for it?

I wanted a different probe.  I figured having a different struct was  
a simple solution.

>
>
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 06d23e1..c98b867 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -374,10 +374,9 @@
>>  #define PCI_DEVICE_ID_ATI_IXP400_SATA   0x4379
>>  #define PCI_DEVICE_ID_ATI_IXP400_SATA2	0x437a
>>  #define PCI_DEVICE_ID_ATI_IXP600_SATA	0x4380
>> -#define PCI_DEVICE_ID_ATI_IXP600_SMBUS	0x4385
>> +#define PCI_DEVICE_ID_ATI_SBX00_SMBUS	0x4385
>>  #define PCI_DEVICE_ID_ATI_IXP600_IDE	0x438c
>>  #define PCI_DEVICE_ID_ATI_IXP700_SATA	0x4390
>> -#define PCI_DEVICE_ID_ATI_IXP700_SMBUS	0x4395
>>  #define PCI_DEVICE_ID_ATI_IXP700_IDE	0x439c
>
> This looks like it doesn't belong in this patch.

oops :)  {copying from an different tree}

- k

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

[Olof Johansson]

On Tue, Sep 11, 2007 at 10:50:18AM -0500, Kumar Gala wrote:
>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c 
>>> b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> index 3a5c3c4..1e2eba8 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, "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,
>>> +};
>>
>> How different are these boards really? Could you just detect MPC85xxDS
>> and have a generic platform for them, or are they different enough that
>> you need individual ones for it?
>
> I wanted a different probe.  I figured having a different struct was a 
> simple solution.

Seems like the only reason to need that is the setting of
primary_phb_addr.  Can't that information be derived out of the device
tree instead? That'd avoid alot of code duplication (code that includes
ifdefs, FWIW :-)

It just seems like a slippery slope. I'm not objecting directly to this
patch, but I think it should be fixed for the longer term.


-Olof

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

[Kumar Gala]

On Sep 11, 2007, at 9:17 AM, Scott Wood wrote:

> On Tue, Sep 11, 2007 at 01:29:18AM -0500, Kumar Gala wrote:
>> diff --git a/arch/powerpc/boot/dts/mpc8572ds.dts b/arch/powerpc/ 
>> boot/dts/mpc8572ds.dts
>> new file mode 100644
>> index 0000000..9d23561
>> --- /dev/null
>> +++ b/arch/powerpc/boot/dts/mpc8572ds.dts
>> @@ -0,0 +1,378 @@
>> +/*
>> + * 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 = "MPC8572DS";
>> +	compatible = "MPC8572DS", "MPC85xxDS";
>
> "fsl," prefix on compatible.
>
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	cpus {
>> +		#cpus = <1>;
>
> Where is #cpus defined or used?  I don't see it used in any other  
> trees, either.

will kill.

>
>> +		#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>;
>> +			32-bit;
>
> Where is 32-bit; defined or used?

its a copy paste error.  Will kill 32-bit on other platforms.

>
>> +	soc8572@ffe00000 {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		#interrupt-cells = <2>;
>> +		device_type = "soc";
>> +		ranges = <00000000 ffe00000 00100000
>> +			  80000000 80000000 20000000
>> +			  a0000000 a0000000 20000000
>> +			  c0000000 c0000000 20000000
>> +			  ffc00000 ffc00000 00010000
>> +			  ffc10000 ffc10000 00010000
>> +			  ffc20000 ffc20000 00010000>;
>> +
>> +		reg = <ffe00000 00001000>;	// CCSRBAR 1M
>
> Comment doesn't match what's actually in reg.

will fix.

>
>> +		bus-frequency = <0>;		// Filled out by uboot.
>> +
>> +		memory-controller@2000 {
>> +			compatible = "fsl,8572-memory-controller";
>
> Is it compatible with any other 85xx memory controller?

maybe, but I don't want to get into that just yet.

>
>> +			reg = <2000 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>;
>> +		};
>
> Should this node be referenced by an l2-cache property in the cpu  
> node?

No, its a front side cache.

>
>> +		pcie@8000 {
>> +			compatible = "fsl,mpc8641-pcie";
>
> Should probably be "fsl,mpc8572-pcie", "fsl,mpc8641-pcie".

this should "fsl,mpc8548-pcie" to match other 85xx pcie.

>
>> +			device_type = "pci";
>> +			#interrupt-cells = <1>;
>> +			#size-cells = <2>;
>> +			#address-cells = <3>;
>> +			reg = <8000 1000>;
>> +			bus-range = <0 ff>;
>> +			ranges = <02000000 0 80000000 80000000 0 20000000
>> +				  01000000 0 00000000 ffc00000 0 00010000>;
>
> No prefetchable mem space?

we haven't normally provided prefetch on 85xx/86xx.. will deal with  
this later.

>
>> +			clock-frequency = <1fca055>;
>
> Decimal would be nicer.
>
>> +		pcie@9000 {
>> +			compatible = "fsl,mpc8548-pcie";
>
> Why is this one 8548-compatible and the previous one 8641-compatible?

copy paste, will fix.

>
>> +		mpic: pic@40000 {
>> +			clock-frequency = <0>;
>> +			interrupt-controller;
>> +			#address-cells = <0>;
>> +			#interrupt-cells = <2>;
>> +			reg = <40000 40000>;
>> +			built-in;
>> +			compatible = "chrp,open-pic";
>> +			device_type = "open-pic";
>> +			big-endian;
>> +		};
>
> Where is the built-in; property defined or used?

defined in chrp OF spec, not sure why we bother with it.

>
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 06d23e1..c98b867 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -374,10 +374,9 @@
>>  #define PCI_DEVICE_ID_ATI_IXP400_SATA   0x4379
>>  #define PCI_DEVICE_ID_ATI_IXP400_SATA2	0x437a
>>  #define PCI_DEVICE_ID_ATI_IXP600_SATA	0x4380
>> -#define PCI_DEVICE_ID_ATI_IXP600_SMBUS	0x4385
>> +#define PCI_DEVICE_ID_ATI_SBX00_SMBUS	0x4385
>>  #define PCI_DEVICE_ID_ATI_IXP600_IDE	0x438c
>>  #define PCI_DEVICE_ID_ATI_IXP700_SATA	0x4390
>> -#define PCI_DEVICE_ID_ATI_IXP700_SMBUS	0x4395
>>  #define PCI_DEVICE_ID_ATI_IXP700_IDE	0x439c
>
> What's going on here?

copied file over from an older tree.

- k

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

[Kumar Gala]

On Sep 11, 2007, at 10:55 AM, Olof Johansson wrote:

> On Tue, Sep 11, 2007 at 10:50:18AM -0500, Kumar Gala wrote:
>>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>>> index 3a5c3c4..1e2eba8 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, "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,
>>>> +};
>>>
>>> How different are these boards really? Could you just detect  
>>> MPC85xxDS
>>> and have a generic platform for them, or are they different  
>>> enough that
>>> you need individual ones for it?
>>
>> I wanted a different probe.  I figured having a different struct  
>> was a
>> simple solution.
>
> Seems like the only reason to need that is the setting of
> primary_phb_addr.  Can't that information be derived out of the device
> tree instead? That'd avoid alot of code duplication (code that  
> includes
> ifdefs, FWIW :-)

well the ifdefs are orthogonal.  We don't have a way of knowing  
primary from the device tree today.

> It just seems like a slippery slope. I'm not objecting directly to  
> this
> patch, but I think it should be fixed for the longer term.

Once we have a clean way of knowing primary PHB than I'm happy to fixup.

- k

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

[Olof Johansson]

On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 10:55 AM, Olof Johansson wrote:
> 
> > On Tue, Sep 11, 2007 at 10:50:18AM -0500, Kumar Gala wrote:

> >>> How different are these boards really? Could you just detect  
> >>> MPC85xxDS
> >>> and have a generic platform for them, or are they different  
> >>> enough that
> >>> you need individual ones for it?
> >>
> >> I wanted a different probe.  I figured having a different struct  
> >> was a
> >> simple solution.
> >
> > Seems like the only reason to need that is the setting of
> > primary_phb_addr.  Can't that information be derived out of the device
> > tree instead? That'd avoid alot of code duplication (code that  
> > includes
> > ifdefs, FWIW :-)
> 
> well the ifdefs are orthogonal.  We don't have a way of knowing  
> primary from the device tree today.

How about something like "fsl,primary-phb" in the bus device node? I don't
know, maybe it's already been discussed and turned down for some reason.

Or would it be sufficient to check children of that device node to see
if the ULi is on that bus?

> > It just seems like a slippery slope. I'm not objecting directly to  
> > this
> > patch, but I think it should be fixed for the longer term.
> 
> Once we have a clean way of knowing primary PHB than I'm happy to fixup.


-Olof

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

[Scott Wood]

Olof Johansson wrote:
> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
>> well the ifdefs are orthogonal.  We don't have a way of knowing  
>> primary from the device tree today.
> 
> How about something like "fsl,primary-phb" in the bus device node? I don't
> know, maybe it's already been discussed and turned down for some reason.

It's more of a Linux issue than anything to do with the hardware.

> Or would it be sufficient to check children of that device node to see
> if the ULi is on that bus?

Or more generally, see if an isa node is somewhere in that subtree.

-Scott

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

[Olof Johansson]

On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
> Olof Johansson wrote:
> > On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
> >> well the ifdefs are orthogonal.  We don't have a way of knowing  
> >> primary from the device tree today.
> > 
> > How about something like "fsl,primary-phb" in the bus device node? I don't
> > know, maybe it's already been discussed and turned down for some reason.
> 
> It's more of a Linux issue than anything to do with the hardware.

That doesn't stop firmware from telling linux which bus is the primary
one on the system to help out.

> > Or would it be sufficient to check children of that device node to see
> > if the ULi is on that bus?
> 
> Or more generally, see if an isa node is somewhere in that subtree.


-Olof

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

[Scott Wood]

Olof Johansson wrote:
> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
>> Olof Johansson wrote:
>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
>>>> well the ifdefs are orthogonal.  We don't have a way of knowing  
>>>> primary from the device tree today.
>>> How about something like "fsl,primary-phb" in the bus device node? I don't
>>> know, maybe it's already been discussed and turned down for some reason.
>> It's more of a Linux issue than anything to do with the hardware.
> 
> That doesn't stop firmware from telling linux which bus is the primary
> one on the system to help out.

The entire notion of a "primary" PCI bus is due to a Linux flaw.

If we did put it in the device tree, it should be something like 
"linux,primary-phb".  But since Linux can tell from the node's children, 
there doesn't seem to be much point.

-Scott

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

[Kumar Gala]

On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:

> Olof Johansson wrote:
>> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
>>> Olof Johansson wrote:
>>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
>>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
>>>>> primary from the device tree today.
>>>> How about something like "fsl,primary-phb" in the bus device  
>>>> node? I don't
>>>> know, maybe it's already been discussed and turned down for some  
>>>> reason.
>>> It's more of a Linux issue than anything to do with the hardware.
>>
>> That doesn't stop firmware from telling linux which bus is the  
>> primary
>> one on the system to help out.
>
> The entire notion of a "primary" PCI bus is due to a Linux flaw.
>
> If we did put it in the device tree, it should be something like
> "linux,primary-phb".  But since Linux can tell from the node's  
> children,
> there doesn't seem to be much point.

Once someone rights code to do this I'm happy to change over.  I took  
this model of explicitly knowing the primary PHB from the pmac code.

- k

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

[David Gibson]

On Tue, Sep 11, 2007 at 12:59:22PM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:
> 
> > Olof Johansson wrote:
> >> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
> >>> Olof Johansson wrote:
> >>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
> >>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
> >>>>> primary from the device tree today.
> >>>> How about something like "fsl,primary-phb" in the bus device  
> >>>> node? I don't
> >>>> know, maybe it's already been discussed and turned down for some  
> >>>> reason.
> >>> It's more of a Linux issue than anything to do with the hardware.
> >>
> >> That doesn't stop firmware from telling linux which bus is the  
> >> primary
> >> one on the system to help out.
> >
> > The entire notion of a "primary" PCI bus is due to a Linux flaw.
> >
> > If we did put it in the device tree, it should be something like
> > "linux,primary-phb".  But since Linux can tell from the node's  
> > children,
> > there doesn't seem to be much point.
> 
> Once someone rights code to do this I'm happy to change over.  I took  
> this model of explicitly knowing the primary PHB from the pmac code.

In the meantime, couldn't the code still be merged, using an explicit
test of the root node's 'compatible' or 'model' properties to decide
on the right primary bus.

-- 
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] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Kumar Gala]

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

> On Tue, Sep 11, 2007 at 12:59:22PM -0500, Kumar Gala wrote:
>>
>> On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:
>>
>>> Olof Johansson wrote:
>>>> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
>>>>> Olof Johansson wrote:
>>>>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
>>>>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
>>>>>>> primary from the device tree today.
>>>>>> How about something like "fsl,primary-phb" in the bus device
>>>>>> node? I don't
>>>>>> know, maybe it's already been discussed and turned down for some
>>>>>> reason.
>>>>> It's more of a Linux issue than anything to do with the hardware.
>>>>
>>>> That doesn't stop firmware from telling linux which bus is the
>>>> primary
>>>> one on the system to help out.
>>>
>>> The entire notion of a "primary" PCI bus is due to a Linux flaw.
>>>
>>> If we did put it in the device tree, it should be something like
>>> "linux,primary-phb".  But since Linux can tell from the node's
>>> children,
>>> there doesn't seem to be much point.
>>
>> Once someone rights code to do this I'm happy to change over.  I took
>> this model of explicitly knowing the primary PHB from the pmac code.
>
> In the meantime, couldn't the code still be merged, using an explicit
> test of the root node's 'compatible' or 'model' properties to decide
> on the right primary bus.

I will be, I'm not going to wait on having some device tree spec for  
this.  The board code can handle it until we come to some agreement  
on how to do this.  I'm in agreement with Scott in that code should  
be added to scan or allow explicit determination.  Adding a 'prop' to  
the device tree just for linux seems a bit silly.

- k

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

[David Gibson]

On Tue, Sep 11, 2007 at 10:35:01PM -0500, Kumar Gala wrote:
> 
> On Sep 11, 2007, at 10:00 PM, David Gibson wrote:
> 
> > On Tue, Sep 11, 2007 at 12:59:22PM -0500, Kumar Gala wrote:
> >>
> >> On Sep 11, 2007, at 12:48 PM, Scott Wood wrote:
> >>
> >>> Olof Johansson wrote:
> >>>> On Tue, Sep 11, 2007 at 12:21:30PM -0500, Scott Wood wrote:
> >>>>> Olof Johansson wrote:
> >>>>>> On Tue, Sep 11, 2007 at 11:00:28AM -0500, Kumar Gala wrote:
> >>>>>>> well the ifdefs are orthogonal.  We don't have a way of knowing
> >>>>>>> primary from the device tree today.
> >>>>>> How about something like "fsl,primary-phb" in the bus device
> >>>>>> node? I don't
> >>>>>> know, maybe it's already been discussed and turned down for some
> >>>>>> reason.
> >>>>> It's more of a Linux issue than anything to do with the hardware.
> >>>>
> >>>> That doesn't stop firmware from telling linux which bus is the
> >>>> primary
> >>>> one on the system to help out.
> >>>
> >>> The entire notion of a "primary" PCI bus is due to a Linux flaw.
> >>>
> >>> If we did put it in the device tree, it should be something like
> >>> "linux,primary-phb".  But since Linux can tell from the node's
> >>> children,
> >>> there doesn't seem to be much point.
> >>
> >> Once someone rights code to do this I'm happy to change over.  I took
> >> this model of explicitly knowing the primary PHB from the pmac code.
> >
> > In the meantime, couldn't the code still be merged, using an explicit
> > test of the root node's 'compatible' or 'model' properties to decide
> > on the right primary bus.
> 
> I will be, I'm not going to wait on having some device tree spec for  
> this.  The board code can handle it until we come to some agreement  
> on how to do this.  I'm in agreement with Scott in that code should  
> be added to scan or allow explicit determination.  Adding a 'prop' to  
> the device tree just for linux seems a bit silly.

Yes, saw that in your new version after I'd posted that.  Sorry.

-- 
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] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

[Olof Johansson]

On Tue, Sep 11, 2007 at 10:35:01PM -0500, Kumar Gala wrote:
> > In the meantime, couldn't the code still be merged, using an explicit
> > test of the root node's 'compatible' or 'model' properties to decide
> > on the right primary bus.
> 
> I will be, I'm not going to wait on having some device tree spec for  
> this.  The board code can handle it until we come to some agreement  
> on how to do this.  I'm in agreement with Scott in that code should  
> be added to scan or allow explicit determination.  Adding a 'prop' to  
> the device tree just for linux seems a bit silly.

Sounds good to me as well.


-Olof

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

[Segher Boessenkool]

>>> well the ifdefs are orthogonal.  We don't have a way of knowing
>>> primary from the device tree today.
>>
>> How about something like "fsl,primary-phb" in the bus device node? I 
>> don't
>> know, maybe it's already been discussed and turned down for some 
>> reason.
>
> It's more of a Linux issue than anything to do with the hardware.

Yeah, many machines actually have multiple "primary PHBs", and
Linux cannot really deal with that.  It's probably best to handle
this from platform code.

>> Or would it be sufficient to check children of that device node to see
>> if the ULi is on that bus?
>
> Or more generally, see if an isa node is somewhere in that subtree.

And if there is no ISA node, find any other legacy device (non-native
ATA controllers, ...), etc.


Segher

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

[Segher Boessenkool]

>>> +		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>;
>>> +		};
>>
>> Should this node be referenced by an l2-cache property in the cpu
>> node?
>
> No, its a front side cache.

What is a "front side cache"?  What exactly does it cache?  If it's
a cache for one CPU only, that fact should be shown in the device
tree somehow.

>>> +			device_type = "pci";
>>> +			#interrupt-cells = <1>;
>>> +			#size-cells = <2>;
>>> +			#address-cells = <3>;
>>> +			reg = <8000 1000>;
>>> +			bus-range = <0 ff>;
>>> +			ranges = <02000000 0 80000000 80000000 0 20000000
>>> +				  01000000 0 00000000 ffc00000 0 00010000>;
>>
>> No prefetchable mem space?
>
> we haven't normally provided prefetch on 85xx/86xx.. will deal with
> this later.

If you don't set up prefetchable memory regions on the PCI from
the firmware, this code is fine, sure.  It would be a good plan
to do map all BARs that say they are prefetchable in some
prefetchable PCI window, it gives a nice speed boost, even when
the kernel accesses it as simple non-cacheable space: the PCI
bridges in between can streamline loads from these areas.

In any case, the device tree should be in synch with how the
firmware set up the PCI hardware, and it seems that's what you
have now, so all is fine.


Segher

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

[Kumar Gala]

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

>>>> +		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>;
>>>> +		};
>>>
>>> Should this node be referenced by an l2-cache property in the cpu
>>> node?
>>
>> No, its a front side cache.
>
> What is a "front side cache"?  What exactly does it cache?  If it's
> a cache for one CPU only, that fact should be shown in the device
> tree somehow.

Its in front of the memory controllers.  Its not specific to a given  
CPU.

- k

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

[Segher Boessenkool]

>> What is a "front side cache"?  What exactly does it cache?  If it's
>> a cache for one CPU only, that fact should be shown in the device
>> tree somehow.
>
> Its in front of the memory controllers.  Its not specific to a given 
> CPU.

Ah, I see.  That relationship is implicit in the device tree already,
both this cache controller and that memory controller are child nodes
of the same soc node, so your device tree is fine.

Just for my own understanding, is this a coherent cache?  (I'm too
lazy to read the manual ;-) )


Segher

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

[Kumar Gala]

On Sep 13, 2007, at 11:53 AM, Segher Boessenkool wrote:

>>> What is a "front side cache"?  What exactly does it cache?  If it's
>>> a cache for one CPU only, that fact should be shown in the device
>>> tree somehow.
>>
>> Its in front of the memory controllers.  Its not specific to a  
>> given CPU.
>
> Ah, I see.  That relationship is implicit in the device tree already,
> both this cache controller and that memory controller are child nodes
> of the same soc node, so your device tree is fine.
>
> Just for my own understanding, is this a coherent cache?  (I'm too
> lazy to read the manual ;-) )

yes its coherent.  Our caches tend to be coherent.

- k