Small fixes for the Ebony device tree

9 messages, 4 authors, 2007-05-16 · open the first message on its own page

Small fixes for the Ebony device tree

From: David Gibson <hidden>
Date: 2007-05-14 04:54:04

This patch corrects a number of minor errors in the Ebony device tree:
	- Missing (given as 0) cache sizes are added to the CPU node
	- device_type properties are removed from nodes which don't
have a reasonably well defined device_type binding.  This does require
a very small code change to locate the busses to be probed for
of_platform devices by 'compatible' instead of 'device_type'.
	- A node is added for the SRAM controller
	- The unit address of the small-flash node is adjusted to
correctly reflect the reg property.
	- device_type values for the MAL and ZMII are updated to
reflected more up-to-date versions of the binding.
	- An incorrect offset in the partition map for the large-flash
node is corrected.
	- Some redundant values, already commented out are removed
entirely.

Signed-off-by: David Gibson <redacted>
---

The flash partition offset correction, at least, should go into
2.6.22.  I think the rest while only borderline "bugfixes" is also
reasonable for inclusion in 2.6.22.

Index: working-2.6/arch/powerpc/boot/dts/ebony.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/ebony.dts	2007-05-08 15:07:45.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/ebony.dts	2007-05-14 14:38:39.000000000 +1000
@@ -33,8 +33,8 @@
 			timebase-frequency = <0>; // Filled in by zImage
 			i-cache-line-size = <32>;
 			d-cache-line-size = <32>;
-			i-cache-size = <0>;
-			d-cache-size = <0>;
+			i-cache-size = <2000000>; /* 32 kB */
+			d-cache-size = <2000000>; /* 32 kB */
 			dcr-controller;
 			dcr-access-method = "native";
 		};
@@ -46,7 +46,6 @@
 	};
 
 	UIC0: interrupt-controller0 {
-		device_type = "ibm,uic";
 		compatible = "ibm,uic-440gp", "ibm,uic";
 		interrupt-controller;
 		cell-index = <0>;
@@ -58,7 +57,6 @@
 	};
 
 	UIC1: interrupt-controller1 {
-		device_type = "ibm,uic";
 		compatible = "ibm,uic-440gp", "ibm,uic";
 		interrupt-controller;
 		cell-index = <1>;
@@ -71,14 +69,12 @@
 	};
 
 	CPC0: cpc {
-		device_type = "ibm,cpc";
 		compatible = "ibm,cpc-440gp";
 		dcr-reg = <0b0 003 0e0 010>;
 		// FIXME: anything else?
 	};
 
 	plb {
-		device_type = "ibm,plb";
 		compatible = "ibm,plb-440gp", "ibm,plb4";
 		#address-cells = <2>;
 		#size-cells = <1>;
@@ -86,21 +82,24 @@
 		clock-frequency = <0>; // Filled in by zImage
 
 		SDRAM0: sdram {
-			device_type = "memory-controller";
 			compatible = "ibm,sdram-440gp", "ibm,sdram";
 			dcr-reg = <010 2>;
 			// FIXME: anything else?
 		};
 
+		SRAM0: sram {
+			compatible = "ibm,sram440gp";
+			dcr-reg = <020 8 00a 1>;
+		};
+
 		DMA0: dma {
 			// FIXME: ???
-			device_type = "ibm,dma-4xx";
 			compatible = "ibm,dma-440gp", "ibm,dma-4xx";
 			dcr-reg = <100 027>;
 		};
 
 		MAL0: mcmal {
-			device_type = "mcmal-dma";
+			device_type = "dma-controller";
 			compatible = "ibm,mcmal-440gp", "ibm,mcmal";
 			dcr-reg = <180 62>;
 			num-tx-chans = <4>;
@@ -119,7 +118,6 @@
 		};
 
 		POB0: opb {
-			device_type = "ibm,opb";
 			compatible = "ibm,opb-440gp", "ibm,opb";
 			#address-cells = <1>;
 			#size-cells = <1>;
@@ -133,7 +131,6 @@
 			clock-frequency = <0>; // Filled in by zImage
 
 			EBC0: ebc {
-				device_type = "ibm,ebc";
 				compatible = "ibm,ebc-440gp";
 				dcr-reg = <012 2>;
 				#address-cells = <2>;
@@ -147,7 +144,7 @@
 				interrupts = <5 4>;
 				interrupt-parent = <&UIC1>;
 
-				small-flash@0,0 {
+				small-flash@0,80000 {
 					device_type = "rom";
 					compatible = "direct-mapped";
 					probe-type = "JEDEC";
@@ -159,7 +156,6 @@
 
 				ds1743@1,0 {
 					/* NVRAM & RTC */
-					device_type = "nvram";
 					compatible = "ds1743";
 					reg = <1 0 2000>;
 				};
@@ -170,7 +166,7 @@
 					probe-type = "JEDEC";
 					bank-width = <1>;
 					partitions = <0 380000
-						      280000 80000>;
+						      380000 80000>;
 					partition-names = "fs", "firmware";
 					reg = <2 0 400000>;
 				};
@@ -226,13 +222,12 @@
 
 			GPIO0: gpio@40000700 {
 				/* FIXME */
-				device_type = "gpio";
 				compatible = "ibm,gpio-440gp";
 				reg = <40000700 20>;
 			};
 
 			ZMII0: emac-zmii@40000780 {
-				device_type = "emac-zmii";
+				device_type = "zmii-interface";
 				compatible = "ibm,zmii-440gp", "ibm,zmii";
 				reg = <40000780 c>;
 			};
@@ -299,9 +294,5 @@
 
 	chosen {
 		linux,stdout-path = "/plb/opb/serial@40000200";
-//		linux,initrd-start = <0>; /* FIXME */
-//		linux,initrd-end = <0>;
-//		bootargs = "";
 	};
 };
-
Index: working-2.6/arch/powerpc/platforms/44x/ebony.c
===================================================================
--- working-2.6.orig/arch/powerpc/platforms/44x/ebony.c	2007-05-08 15:07:45.000000000 +1000
+++ working-2.6/arch/powerpc/platforms/44x/ebony.c	2007-05-14 14:37:51.000000000 +1000
@@ -27,9 +27,9 @@
 #include "44x.h"
 
 static struct of_device_id ebony_of_bus[] = {
-	{ .type = "ibm,plb", },
-	{ .type = "ibm,opb", },
-	{ .type = "ibm,ebc", },
+	{ .compatible = "ibm,plb", },
+	{ .compatible = "ibm,opb", },
+	{ .compatible = "ibm,ebc", },
 	{},
 };
 
-- 
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: Small fixes for the Ebony device tree

From: Josh Boyer <hidden>
Date: 2007-05-14 13:26:26

On Mon, 2007-05-14 at 14:54 +1000, David Gibson wrote:
This patch corrects a number of minor errors in the Ebony device tree:
	- Missing (given as 0) cache sizes are added to the CPU node
	- device_type properties are removed from nodes which don't
have a reasonably well defined device_type binding.  This does require
a very small code change to locate the busses to be probed for
of_platform devices by 'compatible' instead of 'device_type'.
	- A node is added for the SRAM controller
	- The unit address of the small-flash node is adjusted to
correctly reflect the reg property.
	- device_type values for the MAL and ZMII are updated to
reflected more up-to-date versions of the binding.
	- An incorrect offset in the partition map for the large-flash
node is corrected.
	- Some redundant values, already commented out are removed
entirely.

Signed-off-by: David Gibson <redacted>
Acked-by: Josh Boyer <redacted>

Re: Small fixes for the Ebony device tree

From: Segher Boessenkool <hidden>
Date: 2007-05-14 15:03:52

-			i-cache-size = <0>;
-			d-cache-size = <0>;
+			i-cache-size = <2000000>; /* 32 kB */
+			d-cache-size = <2000000>; /* 32 kB */
That's 32MB, not 32kB.  Better fix this :-)
 	UIC0: interrupt-controller0 {
 	UIC1: interrupt-controller1 {
It's a shame you can't use unit addresses for these since
you use "dcr-reg" instead of "reg".  Oh well.
 		SDRAM0: sdram {
-			device_type = "memory-controller";
 			compatible = "ibm,sdram-440gp", "ibm,sdram";
Maybe rename the node to "memory-controller"?
+		SRAM0: sram {
+			compatible = "ibm,sram440gp";
+			dcr-reg = <020 8 00a 1>;
+		};
Is this thing _only_ addressable over DCRs?  Weird.
 		MAL0: mcmal {
-			device_type = "mcmal-dma";
+			device_type = "dma-controller";
 			compatible = "ibm,mcmal-440gp", "ibm,mcmal";
Remove "device_type", change name to "dma-controller"?
 			EBC0: ebc {
-				device_type = "ibm,ebc";
 				compatible = "ibm,ebc-440gp";
You forgot "ibm,ebc" here.


Segher

Re: Small fixes for the Ebony device tree

From: Josh Boyer <hidden>
Date: 2007-05-14 15:09:35

On Mon, 2007-05-14 at 14:59 +0200, Segher Boessenkool wrote:
quoted
-			i-cache-size = <0>;
-			d-cache-size = <0>;
+			i-cache-size = <2000000>; /* 32 kB */
+			d-cache-size = <2000000>; /* 32 kB */
That's 32MB, not 32kB.  Better fix this :-)
Argh.  What's even sadder is I just did the exact same thing for Bamboo.
quoted
+		SRAM0: sram {
+			compatible = "ibm,sram440gp";
+			dcr-reg = <020 8 00a 1>;
+		};
Is this thing _only_ addressable over DCRs?  Weird.
Yes.
quoted
 		MAL0: mcmal {
-			device_type = "mcmal-dma";
+			device_type = "dma-controller";
 			compatible = "ibm,mcmal-440gp", "ibm,mcmal";
Remove "device_type", change name to "dma-controller"?
quoted
 			EBC0: ebc {
-				device_type = "ibm,ebc";
 				compatible = "ibm,ebc-440gp";
You forgot "ibm,ebc" here.
/me notes that for Bamboo

josh

Re: Small fixes for the Ebony device tree

From: David Gibson <hidden>
Date: 2007-05-15 01:17:04

On Mon, May 14, 2007 at 02:59:31PM +0200, Segher Boessenkool wrote:
quoted
-			i-cache-size = <0>;
-			d-cache-size = <0>;
+			i-cache-size = <2000000>; /* 32 kB */
+			d-cache-size = <2000000>; /* 32 kB */
That's 32MB, not 32kB.  Better fix this :-)
Duh.  Fixed.
quoted
 	UIC0: interrupt-controller0 {
quoted
 	UIC1: interrupt-controller1 {
It's a shame you can't use unit addresses for these since
you use "dcr-reg" instead of "reg".  Oh well.
quoted
 		SDRAM0: sdram {
-			device_type = "memory-controller";
 			compatible = "ibm,sdram-440gp", "ibm,sdram";
Maybe rename the node to "memory-controller"?
Hmm, yeah, I guess so.
quoted
+		SRAM0: sram {
+			compatible = "ibm,sram440gp";
+			dcr-reg = <020 8 00a 1>;
+		};
Is this thing _only_ addressable over DCRs?  Weird.
Well... the control registers are certainly DCR only.  I guess there's
the actual SRAM itself, though whether this belongs in this node, or
elsewhere isn't immediately clear.  I haven't yet investigated how the
SRAM is mapped (it depends on DIP switch settings) so I'm certainly
not considering this node complete yet.
quoted
 		MAL0: mcmal {
-			device_type = "mcmal-dma";
+			device_type = "dma-controller";
 			compatible = "ibm,mcmal-440gp", "ibm,mcmal";
Remove "device_type", change name to "dma-controller"?
Don't really want to remove the device_type, because the MAL driver
looks for it at present.  Don't really want to change the name, since
that might encourage confusion with the other (more conventional) DMA
controller.
quoted
 			EBC0: ebc {
-				device_type = "ibm,ebc";
 				compatible = "ibm,ebc-440gp";
You forgot "ibm,ebc" here.
Hmm.. yeah, I guess.

Revised patch coming shortly.

-- 
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: Small fixes for the Ebony device tree

From: Segher Boessenkool <hidden>
Date: 2007-05-15 04:59:55

quoted
quoted
+		SRAM0: sram {
+			compatible = "ibm,sram440gp";
+			dcr-reg = <020 8 00a 1>;
+		};
Is this thing _only_ addressable over DCRs?  Weird.
Well... the control registers are certainly DCR only.  I guess there's
the actual SRAM itself, though whether this belongs in this node, or
elsewhere isn't immediately clear.  I haven't yet investigated how the
SRAM is mapped (it depends on DIP switch settings) so I'm certainly
not considering this node complete yet.
If it is supposed to have a "reg" property, and it doesn't
yet, it might be a good idea to comment it out in the DTS
for now, so later kernels can work with the older device
tree correctly.
quoted
quoted
 		MAL0: mcmal {
-			device_type = "mcmal-dma";
+			device_type = "dma-controller";
 			compatible = "ibm,mcmal-440gp", "ibm,mcmal";
Remove "device_type", change name to "dma-controller"?
Don't really want to remove the device_type, because the MAL driver
looks for it at present.
Fair enough.  But you change the "device_type" in
this patch, so presumably you change it in the kernel
driver as well -- can't you just *fix* the kernel driver,
instead?
Don't really want to change the name, since
that might encourage confusion with the other (more conventional) DMA
controller.
Nah, just look at the other properties in the node and
you know what is what.  It is quite common to have nodes
with the same name representing different devices (for
example, "ethernet" devices -- "dma-controller" would be
a bit more unusual, sure).

I have no strong feelings about the name, "mcmal" is
generic enough a name as far as I'm concerned.
quoted
quoted
 			EBC0: ebc {
-				device_type = "ibm,ebc";
 				compatible = "ibm,ebc-440gp";
You forgot "ibm,ebc" here.
Hmm.. yeah, I guess.
Well that's what the kernel code matches on ;-)
Revised patch coming shortly.
Looking forward to it!


Segher

Re: Small fixes for the Ebony device tree

From: David Gibson <hidden>
Date: 2007-05-15 05:46:26

On Tue, May 15, 2007 at 06:59:49AM +0200, Segher Boessenkool wrote:
quoted
quoted
quoted
+		SRAM0: sram {
+			compatible = "ibm,sram440gp";
+			dcr-reg = <020 8 00a 1>;
+		};
Is this thing _only_ addressable over DCRs?  Weird.
Well... the control registers are certainly DCR only.  I guess there's
the actual SRAM itself, though whether this belongs in this node, or
elsewhere isn't immediately clear.  I haven't yet investigated how the
SRAM is mapped (it depends on DIP switch settings) so I'm certainly
not considering this node complete yet.
If it is supposed to have a "reg" property, and it doesn't
yet, it might be a good idea to comment it out in the DTS
for now, so later kernels can work with the older device
tree correctly.
Given that I'm not aware of any Ebony firmwares that actually supply a
device tree, so in practice the kernel's tree will always come from an
attached zImage, I don't think this is really a big consideration.
quoted
quoted
quoted
 		MAL0: mcmal {
-			device_type = "mcmal-dma";
+			device_type = "dma-controller";
 			compatible = "ibm,mcmal-440gp", "ibm,mcmal";
Remove "device_type", change name to "dma-controller"?
Don't really want to remove the device_type, because the MAL driver
looks for it at present.
Fair enough.  But you change the "device_type" in
this patch, so presumably you change it in the kernel
The kernel driver recognizes both variants, but the one I had
previously is marked deprecated.
driver as well -- can't you just *fix* the kernel driver,
instead?
Well.. I guess, but I'd prefer to leave that to BenH, who wrote the
driver.
quoted
Don't really want to change the name, since
that might encourage confusion with the other (more conventional) DMA
controller.
Nah, just look at the other properties in the node and
you know what is what.  It is quite common to have nodes
with the same name representing different devices (for
example, "ethernet" devices -- "dma-controller" would be
a bit more unusual, sure).

I have no strong feelings about the name, "mcmal" is
generic enough a name as far as I'm concerned.
quoted
quoted
quoted
 			EBC0: ebc {
-				device_type = "ibm,ebc";
 				compatible = "ibm,ebc-440gp";
You forgot "ibm,ebc" here.
Hmm.. yeah, I guess.
Well that's what the kernel code matches on ;-)
Um.. yes.  I wonder how it was working before...

-- 
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: Small fixes for the Ebony device tree

From: Mark A. Greer <hidden>
Date: 2007-05-15 18:15:54

On Mon, May 14, 2007 at 02:54:04PM +1000, David Gibson wrote:
quoted hunk
Index: working-2.6/arch/powerpc/boot/dts/ebony.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/ebony.dts	2007-05-08 15:07:45.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/ebony.dts	2007-05-14 14:38:39.000000000 +1000
@@ -33,8 +33,8 @@
 			timebase-frequency = <0>; // Filled in by zImage
 			i-cache-line-size = <32>;
 			d-cache-line-size = <32>;
-			i-cache-size = <0>;
-			d-cache-size = <0>;
+			i-cache-size = <2000000>; /* 32 kB */
+			d-cache-size = <2000000>; /* 32 kB */
 			dcr-controller;
 			dcr-access-method = "native";
 		};
Ahh HA!  You did it too! :)

Re: Small fixes for the Ebony device tree

From: David Gibson <hidden>
Date: 2007-05-16 03:47:01

On Tue, May 15, 2007 at 03:46:26PM +1000, David Gibson wrote:
On Tue, May 15, 2007 at 06:59:49AM +0200, Segher Boessenkool wrote:
[snip]
quoted
driver as well -- can't you just *fix* the kernel driver,
instead?
Well.. I guess, but I'd prefer to leave that to BenH, who wrote the
driver.
On second thoughts I will alter the driver, the new version I sent out
today has the fix to only look at 'compatible'.
quoted
quoted
Don't really want to change the name, since
that might encourage confusion with the other (more conventional) DMA
controller.
Nah, just look at the other properties in the node and
you know what is what.  It is quite common to have nodes
with the same name representing different devices (for
example, "ethernet" devices -- "dma-controller" would be
a bit more unusual, sure).

I have no strong feelings about the name, "mcmal" is
generic enough a name as far as I'm concerned.
I'll leave it then.

-- 
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help