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

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

On Tue, Sep 11, 2007 at 01:29:18AM -0500, Kumar Gala wrote:

quoted

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.

quoted

+	#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.

quoted

+		#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.

quoted

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

quoted

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

quoted

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

quoted

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

quoted

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

quoted

+			clock-frequency = <1fca055>;

Decimal would be nicer.

quoted

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

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

copy paste, will fix.

quoted

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

quoted

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