[PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

The 'linux,boot-cpu' property is obsolete, so remove it from all of the DTS 
files and from booting-without-of.txt.  The boot CPU is actually defined in
the device tree header, and U-Boot sets that field.  The device tree compiler
also complains if the property exists.

Remove a stray 'chosen' section in mpc8272ads.dts.  The 'chosen' section is
initialized by U-Boot, so the definition in the DTS is not used anyway.

Signed-off-by: Timur Tabi <timur@freescale.com>
Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
---

Note: The '64-bit' property is already defined by the OF PPC
binding and is added to show an example of a property
without a value.  We're not actually adding a new property definition.

 Documentation/powerpc/booting-without-of.txt |    5 ++---
 arch/powerpc/boot/dts/kuroboxHG.dts          |    1 -
 arch/powerpc/boot/dts/mpc7448hpc2.dts        |    5 ++---
 arch/powerpc/boot/dts/mpc8272ads.dts         |    8 --------
 arch/powerpc/boot/dts/mpc8323emds.dts        |    7 +++----
 arch/powerpc/boot/dts/mpc8360emds.dts        |    1 -
 arch/powerpc/boot/dts/mpc8560ads.dts         |    1 -
 arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    3 +--
 arch/powerpc/boot/dts/mpc866ads.dts          |    1 -
 arch/powerpc/boot/dts/mpc885ads.dts          |    1 -
 10 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 3399427..310d267 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -497,7 +497,7 @@ looks like in practice.
       |   |- device_type = "cpu"
       |   |- reg = <0>
       |   |- clock-frequency = <5f5e1000>
-      |   |- linux,boot-cpu
+      |   |- 64-bit
       |   |- linux,phandle = <2>
       |
       o memory@0
@@ -519,7 +519,7 @@ physical memory layout.  It also include
 through /chosen, like in this example, the platform type (mandatory)
 and the kernel command line arguments (optional).
 
-The /cpus/PowerPC,970@0/linux,boot-cpu property is an example of a
+The /cpus/PowerPC,970@0/64-bit property is an example of a
 property without a value. All other properties have a value. The
 significance of the #address-cells and #size-cells properties will be
 explained in chapter IV which defines precisely the required nodes and
@@ -778,7 +778,6 @@ address which can extend beyond that lim
       bytes
     - d-cache-size : one cell, size of L1 data cache in bytes
     - i-cache-size : one cell, size of L1 instruction cache in bytes
-    - linux, boot-cpu : Should be defined if this cpu is the boot cpu.
 
   Recommended properties:
 
diff --git a/arch/powerpc/boot/dts/kuroboxHG.dts b/arch/powerpc/boot/dts/kuroboxHG.dts
index d06b0b0..7531027 100644
--- a/arch/powerpc/boot/dts/kuroboxHG.dts
+++ b/arch/powerpc/boot/dts/kuroboxHG.dts
@@ -35,7 +35,6 @@ build with: "dtc -f -I dts -O dtb -o kur
 
 		PowerPC,603e { /* Really 8241 */
 			linux,phandle = <2100>;
-			linux,boot-cpu;
 			device_type = "cpu";
 			reg = <0>;
 			clock-frequency = <fdad680>;	/* Fixed by bootwrapper */
diff --git a/arch/powerpc/boot/dts/mpc7448hpc2.dts b/arch/powerpc/boot/dts/mpc7448hpc2.dts
index c4d9562..4164351 100644
--- a/arch/powerpc/boot/dts/mpc7448hpc2.dts
+++ b/arch/powerpc/boot/dts/mpc7448hpc2.dts
@@ -23,7 +23,7 @@
 		#address-cells = <1>;
 		#size-cells =<0>;
 		linux,phandle = <200>;
-				
+
 		PowerPC,7448@0 {
 			device_type = "cpu";
 			reg = <0>;
@@ -36,7 +36,6 @@
 			bus-frequency = <0>;		// From U-Boot
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
@@ -152,7 +151,7 @@
 			#address-cells = <3>;
 			reg = <1000 1000>;
 			bus-range = <0 0>;
-			ranges = <02000000 0 e0000000 e0000000 0 1A000000	
+			ranges = <02000000 0 e0000000 e0000000 0 1A000000
 				  01000000 0 00000000 fa000000 0 00010000>;
 			clock-frequency = <7f28154>;
 			interrupt-parent = <7400>;
diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/boot/dts/mpc8272ads.dts
index 26b44f7..c9ffc12 100644
--- a/arch/powerpc/boot/dts/mpc8272ads.dts
+++ b/arch/powerpc/boot/dts/mpc8272ads.dts
@@ -34,7 +34,6 @@
                        clock-frequency = <0>;
                        32-bit;
                        linux,phandle = <201>;
-                       linux,boot-cpu;
                };
        };
 
@@ -53,13 +52,6 @@
                reg = <00000000 4000000 f4500000 00000020>;
        };
 
-       chosen {
-               name = "chosen";
-               linux,platform = <0>;
-               interrupt-controller = <10c00>;
-               linux,phandle = <400>;
-       };
-
        soc8272@f0000000 {
                #address-cells = <1>;
                #size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/mpc8323emds.dts b/arch/powerpc/boot/dts/mpc8323emds.dts
index fa7ef24..eaf0f86 100644
--- a/arch/powerpc/boot/dts/mpc8323emds.dts
+++ b/arch/powerpc/boot/dts/mpc8323emds.dts
@@ -34,7 +34,6 @@
 			clock-frequency = <0>;
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
@@ -174,7 +173,7 @@
 			built-in;
 			device_type = "ipic";
 		};
-		
+
 		par_io@1400 {
 			reg = <1400 100>;
 			device_type = "par_io";
@@ -237,11 +236,11 @@
 		reg = <e0100000 480>;
 		brg-frequency = <0>;
 		bus-frequency = <BCD3D80>;
-		
+
 		muram@10000 {
 			device_type = "muram";
 			ranges = <0 00010000 00004000>;
-	
+
 			data-only@0 {
 				reg = <0 4000>;
 			};
diff --git a/arch/powerpc/boot/dts/mpc8360emds.dts b/arch/powerpc/boot/dts/mpc8360emds.dts
index 9022192..303bd66 100644
--- a/arch/powerpc/boot/dts/mpc8360emds.dts
+++ b/arch/powerpc/boot/dts/mpc8360emds.dts
@@ -39,7 +39,6 @@
 			clock-frequency = <1F78A400>;
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 119bd5d..c74d6eb 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -35,7 +35,6 @@
 			clock-frequency = <312c8040>;
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index f0c7731..258fccb 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -32,7 +32,6 @@
 			bus-frequency = <0>;		// From uboot
 			clock-frequency = <0>;		// From uboot
 			32-bit;
-			linux,boot-cpu;
 		};
 		PowerPC,8641@1 {
 			device_type = "cpu";
@@ -142,7 +141,7 @@
 			interrupt-parent = <40000>;
 			phy-handle = <2452001>;
 		};
-		
+
 		ethernet@26000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/arch/powerpc/boot/dts/mpc866ads.dts b/arch/powerpc/boot/dts/mpc866ads.dts
index 5d40052..2b56b5d 100644
--- a/arch/powerpc/boot/dts/mpc866ads.dts
+++ b/arch/powerpc/boot/dts/mpc866ads.dts
@@ -37,7 +37,6 @@
 			interrupts = <f 2>;	// decrementer interrupt
 			interrupt-parent = <ff000000>;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
index cf1a19f..faecd08 100644
--- a/arch/powerpc/boot/dts/mpc885ads.dts
+++ b/arch/powerpc/boot/dts/mpc885ads.dts
@@ -37,7 +37,6 @@
 			interrupts = <f 2>;	// decrementer interrupt
 			interrupt-parent = <ff000000>;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
-- 
1.4.4

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Benjamin Herrenschmidt]

On Tue, 2007-02-13 at 17:34 -0600, Timur Tabi wrote:
> The 'linux,boot-cpu' property is obsolete, so remove it from all of the DTS 
> files and from booting-without-of.txt.  The boot CPU is actually defined in
> the device tree header, and U-Boot sets that field.  The device tree compiler
> also complains if the property exists.
> 
> Remove a stray 'chosen' section in mpc8272ads.dts.  The 'chosen' section is
> initialized by U-Boot, so the definition in the DTS is not used anyway.

I think that:

 - The /chosen node should be present in the .dts even if empty
 
 - I don't see why you are removing linux,boot-cpu from the
documentation ! It's a fairly important property. Bootloaders are
supposed to provide it on SMP environments and it's used for kexec.

Ben.


> Signed-off-by: Timur Tabi <timur@freescale.com>
> Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> ---
> 
> Note: The '64-bit' property is already defined by the OF PPC
> binding and is added to show an example of a property
> without a value.  We're not actually adding a new property definition.
> 
>  Documentation/powerpc/booting-without-of.txt |    5 ++---
>  arch/powerpc/boot/dts/kuroboxHG.dts          |    1 -
>  arch/powerpc/boot/dts/mpc7448hpc2.dts        |    5 ++---
>  arch/powerpc/boot/dts/mpc8272ads.dts         |    8 --------
>  arch/powerpc/boot/dts/mpc8323emds.dts        |    7 +++----
>  arch/powerpc/boot/dts/mpc8360emds.dts        |    1 -
>  arch/powerpc/boot/dts/mpc8560ads.dts         |    1 -
>  arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    3 +--
>  arch/powerpc/boot/dts/mpc866ads.dts          |    1 -
>  arch/powerpc/boot/dts/mpc885ads.dts          |    1 -
>  10 files changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index 3399427..310d267 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -497,7 +497,7 @@ looks like in practice.
>        |   |- device_type = "cpu"
>        |   |- reg = <0>
>        |   |- clock-frequency = <5f5e1000>
> -      |   |- linux,boot-cpu
> +      |   |- 64-bit
>        |   |- linux,phandle = <2>
>        |
>        o memory@0
> @@ -519,7 +519,7 @@ physical memory layout.  It also include
>  through /chosen, like in this example, the platform type (mandatory)
>  and the kernel command line arguments (optional).
>  
> -The /cpus/PowerPC,970@0/linux,boot-cpu property is an example of a
> +The /cpus/PowerPC,970@0/64-bit property is an example of a
>  property without a value. All other properties have a value. The
>  significance of the #address-cells and #size-cells properties will be
>  explained in chapter IV which defines precisely the required nodes and
> @@ -778,7 +778,6 @@ address which can extend beyond that lim
>        bytes
>      - d-cache-size : one cell, size of L1 data cache in bytes
>      - i-cache-size : one cell, size of L1 instruction cache in bytes
> -    - linux, boot-cpu : Should be defined if this cpu is the boot cpu.
>  
>    Recommended properties:
>  
> diff --git a/arch/powerpc/boot/dts/kuroboxHG.dts b/arch/powerpc/boot/dts/kuroboxHG.dts
> index d06b0b0..7531027 100644
> --- a/arch/powerpc/boot/dts/kuroboxHG.dts
> +++ b/arch/powerpc/boot/dts/kuroboxHG.dts
> @@ -35,7 +35,6 @@ build with: "dtc -f -I dts -O dtb -o kur
>  
>  		PowerPC,603e { /* Really 8241 */
>  			linux,phandle = <2100>;
> -			linux,boot-cpu;
>  			device_type = "cpu";
>  			reg = <0>;
>  			clock-frequency = <fdad680>;	/* Fixed by bootwrapper */
> diff --git a/arch/powerpc/boot/dts/mpc7448hpc2.dts b/arch/powerpc/boot/dts/mpc7448hpc2.dts
> index c4d9562..4164351 100644
> --- a/arch/powerpc/boot/dts/mpc7448hpc2.dts
> +++ b/arch/powerpc/boot/dts/mpc7448hpc2.dts
> @@ -23,7 +23,7 @@
>  		#address-cells = <1>;
>  		#size-cells =<0>;
>  		linux,phandle = <200>;
> -				
> +
>  		PowerPC,7448@0 {
>  			device_type = "cpu";
>  			reg = <0>;
> @@ -36,7 +36,6 @@
>  			bus-frequency = <0>;		// From U-Boot
>  			32-bit;
>  			linux,phandle = <201>;
> -			linux,boot-cpu;
>  		};
>  	};
>  
> @@ -152,7 +151,7 @@
>  			#address-cells = <3>;
>  			reg = <1000 1000>;
>  			bus-range = <0 0>;
> -			ranges = <02000000 0 e0000000 e0000000 0 1A000000	
> +			ranges = <02000000 0 e0000000 e0000000 0 1A000000
>  				  01000000 0 00000000 fa000000 0 00010000>;
>  			clock-frequency = <7f28154>;
>  			interrupt-parent = <7400>;
> diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/boot/dts/mpc8272ads.dts
> index 26b44f7..c9ffc12 100644
> --- a/arch/powerpc/boot/dts/mpc8272ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8272ads.dts
> @@ -34,7 +34,6 @@
>                         clock-frequency = <0>;
>                         32-bit;
>                         linux,phandle = <201>;
> -                       linux,boot-cpu;
>                 };
>         };
>  
> @@ -53,13 +52,6 @@
>                 reg = <00000000 4000000 f4500000 00000020>;
>         };
>  
> -       chosen {
> -               name = "chosen";
> -               linux,platform = <0>;
> -               interrupt-controller = <10c00>;
> -               linux,phandle = <400>;
> -       };
> -
>         soc8272@f0000000 {
>                 #address-cells = <1>;
>                 #size-cells = <1>;
> diff --git a/arch/powerpc/boot/dts/mpc8323emds.dts b/arch/powerpc/boot/dts/mpc8323emds.dts
> index fa7ef24..eaf0f86 100644
> --- a/arch/powerpc/boot/dts/mpc8323emds.dts
> +++ b/arch/powerpc/boot/dts/mpc8323emds.dts
> @@ -34,7 +34,6 @@
>  			clock-frequency = <0>;
>  			32-bit;
>  			linux,phandle = <201>;
> -			linux,boot-cpu;
>  		};
>  	};
>  
> @@ -174,7 +173,7 @@
>  			built-in;
>  			device_type = "ipic";
>  		};
> -		
> +
>  		par_io@1400 {
>  			reg = <1400 100>;
>  			device_type = "par_io";
> @@ -237,11 +236,11 @@
>  		reg = <e0100000 480>;
>  		brg-frequency = <0>;
>  		bus-frequency = <BCD3D80>;
> -		
> +
>  		muram@10000 {
>  			device_type = "muram";
>  			ranges = <0 00010000 00004000>;
> -	
> +
>  			data-only@0 {
>  				reg = <0 4000>;
>  			};
> diff --git a/arch/powerpc/boot/dts/mpc8360emds.dts b/arch/powerpc/boot/dts/mpc8360emds.dts
> index 9022192..303bd66 100644
> --- a/arch/powerpc/boot/dts/mpc8360emds.dts
> +++ b/arch/powerpc/boot/dts/mpc8360emds.dts
> @@ -39,7 +39,6 @@
>  			clock-frequency = <1F78A400>;
>  			32-bit;
>  			linux,phandle = <201>;
> -			linux,boot-cpu;
>  		};
>  	};
>  
> diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
> index 119bd5d..c74d6eb 100644
> --- a/arch/powerpc/boot/dts/mpc8560ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8560ads.dts
> @@ -35,7 +35,6 @@
>  			clock-frequency = <312c8040>;
>  			32-bit;
>  			linux,phandle = <201>;
> -			linux,boot-cpu;
>  		};
>  	};
>  
> diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> index f0c7731..258fccb 100644
> --- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> +++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> @@ -32,7 +32,6 @@
>  			bus-frequency = <0>;		// From uboot
>  			clock-frequency = <0>;		// From uboot
>  			32-bit;
> -			linux,boot-cpu;
>  		};
>  		PowerPC,8641@1 {
>  			device_type = "cpu";
> @@ -142,7 +141,7 @@
>  			interrupt-parent = <40000>;
>  			phy-handle = <2452001>;
>  		};
> -		
> +
>  		ethernet@26000 {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> diff --git a/arch/powerpc/boot/dts/mpc866ads.dts b/arch/powerpc/boot/dts/mpc866ads.dts
> index 5d40052..2b56b5d 100644
> --- a/arch/powerpc/boot/dts/mpc866ads.dts
> +++ b/arch/powerpc/boot/dts/mpc866ads.dts
> @@ -37,7 +37,6 @@
>  			interrupts = <f 2>;	// decrementer interrupt
>  			interrupt-parent = <ff000000>;
>  			linux,phandle = <201>;
> -			linux,boot-cpu;
>  		};
>  	};
>  
> diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
> index cf1a19f..faecd08 100644
> --- a/arch/powerpc/boot/dts/mpc885ads.dts
> +++ b/arch/powerpc/boot/dts/mpc885ads.dts
> @@ -37,7 +37,6 @@
>  			interrupts = <f 2>;	// decrementer interrupt
>  			interrupt-parent = <ff000000>;
>  			linux,phandle = <201>;
> -			linux,boot-cpu;
>  		};
>  	};
>  

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Benjamin Herrenschmidt wrote:

> I think that:
> 
>  - The /chosen node should be present in the .dts even if empty

Only one DTS file has it.  It's been removed from every other DTS.  Are 
you should that instead of removing the last one, I should *add* one to 
the other DTS files?

>  - I don't see why you are removing linux,boot-cpu from the
> documentation ! It's a fairly important property. 

No, it's a dead property.  Specifying linux,boot-cpu in the DTS is 
obsolete and deprecated.  If you specify it, the device tree compiler 
complains.  The proper way to specify the device tree is with the -b 
parameter to dtc.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[David Gibson]

On Tue, Feb 13, 2007 at 11:15:17PM -0600, Timur Tabi wrote:
> Benjamin Herrenschmidt wrote:
> 
> > I think that:
> > 
> >  - The /chosen node should be present in the .dts even if empty
> 
> Only one DTS file has it.  It's been removed from every other DTS.  Are 
> you should that instead of removing the last one, I should *add* one to 
> the other DTS files?
> 
> >  - I don't see why you are removing linux,boot-cpu from the
> > documentation ! It's a fairly important property. 
> 
> No, it's a dead property.  Specifying linux,boot-cpu in the DTS is 
> obsolete and deprecated.  If you specify it, the device tree compiler 
> complains.  The proper way to specify the device tree is with the -b 
> parameter to dtc.

Um.. you may be right in this case, but "the device tree compiler
complains" is (sadly) not in general a good reason to decide
something's bad.  The warning code in dtc is pretty bogus and
complains about a number of things it shouldn't while failing to
complain about some things it should.  I just haven't had time to make
it much better.

-- 
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: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

David Gibson wrote:

> Um.. you may be right in this case, but "the device tree compiler
> complains" is (sadly) not in general a good reason to decide
> something's bad.  The warning code in dtc is pretty bogus and
> complains about a number of things it shouldn't while failing to
> complain about some things it should.  I just haven't had time to make
> it much better.

I agree that the DTC has problems, but my understanding is that 
linux,boot-cpu is just plain wrong nowadays.  Besides, U-Boot overrides 
whatever is in the device tree, so it doesn't really matter what you say 
in the DTS.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Jon Loeliger]

So, like, the other day David Gibson mumbled:
> 
> The warning code in dtc is pretty bogus and
> complains about a number of things it shouldn't while failing to
> complain about some things it should.  I just haven't had time to make
> it much better.

David,

Would you have time here to simply state for the record
what you woul like to see fixed?  Someone might have
time to work through them.

Thanks,
jdl

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Benjamin Herrenschmidt]

On Tue, 2007-02-13 at 23:15 -0600, Timur Tabi wrote:
> Benjamin Herrenschmidt wrote:
> 
> > I think that:
> > 
> >  - The /chosen node should be present in the .dts even if empty
> 
> Only one DTS file has it.  It's been removed from every other DTS.  Are 
> you should that instead of removing the last one, I should *add* one to 
> the other DTS files?

Or just leave them alone for now.

> >  - I don't see why you are removing linux,boot-cpu from the
> > documentation ! It's a fairly important property. 
> 
> No, it's a dead property.  Specifying linux,boot-cpu in the DTS is 
> obsolete and deprecated.  If you specify it, the device tree compiler 
> complains.  The proper way to specify the device tree is with the -b 
> parameter to dtc.

Ah indeed, my bad. We moved that to a header field didn't we ?

/me grumbles about kexec... :-)

Ben.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Benjamin Herrenschmidt wrote:

>> Only one DTS file has it.  It's been removed from every other DTS.  Are 
>> you should that instead of removing the last one, I should *add* one to 
>> the other DTS files?
> 
> Or just leave them alone for now.

Well, it's inconsistent.  If you *really* don't want me to remove the 
'chosen' node from this one DTS, I'll respin the patch without it.  But 
I just don't understand why it's okay to leave it there.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Benjamin Herrenschmidt]

On Tue, 2007-02-13 at 23:31 -0600, Timur Tabi wrote:
> Benjamin Herrenschmidt wrote:
> 
> >> Only one DTS file has it.  It's been removed from every other DTS.  Are 
> >> you should that instead of removing the last one, I should *add* one to 
> >> the other DTS files?
> > 
> > Or just leave them alone for now.
> 
> Well, it's inconsistent.  If you *really* don't want me to remove the 
> 'chosen' node from this one DTS, I'll respin the patch without it.  But 
> I just don't understand why it's okay to leave it there.

To make sure there is one. I suspect kernel bits might get upset if it's
absent.

Ben.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Benjamin Herrenschmidt wrote:

>> I just don't understand why it's okay to leave it there.
> 
> To make sure there is one. I suspect kernel bits might get upset if it's
> absent.

That's not what I meant.  What I don't understand is: what's so special 
about the 8272 that its DTS should have a 'chosen' section, but no other 
DTS needs one?

Obviously, the kernel needs a 'chosen' section, and that's why U-Boot 
adds one.  And since U-Boot adds one, the DTS itself doesn't need one. 
Does the 8272 never use U-Boot?

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Guennadi Liakhovetski]

On Wed, 14 Feb 2007, Timur Tabi wrote:

> Benjamin Herrenschmidt wrote:
> 
> >> I just don't understand why it's okay to leave it there.
> > 
> > To make sure there is one. I suspect kernel bits might get upset if it's
> > absent.
> 
> That's not what I meant.  What I don't understand is: what's so special 
> about the 8272 that its DTS should have a 'chosen' section, but no other 
> DTS needs one?
> 
> Obviously, the kernel needs a 'chosen' section, and that's why U-Boot 
> adds one.  And since U-Boot adds one, the DTS itself doesn't need one. 
> Does the 8272 never use U-Boot?

Is that dts also for u-boot? maybe they have another bootloader? I know 
u-boot creates "chosen" itself, but not all possible bootloaders?

(as a matter of fact, the more I think about those .dts's, the more it 
seems to me like they belong to bootloaders, not in the kernel... Just 
one word - information duplication...)

Another thing, Timur, if you respin your patch, could you do it against 
the current powerpc.git? There's at least one more .dts there that also 
would need boot-cpu removed - kuroboxHD.dts.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Guennadi Liakhovetski wrote:

> Is that dts also for u-boot? maybe they have another bootloader? I know 
> u-boot creates "chosen" itself, but not all possible bootloaders?

Well, that's what I'm trying to figure out.  I don't see any evidence to suggest 
that the 8276 is different from any other 8xxx board, with respect to U-Boot, 
and none of the other 8xxx DTS files have a 'chosen' section.  Then there's the 
fact that the 8276's 'chosen' section is in the *middle* of the DTS, and not at 
the end where everyone else puts it, leads me to believe that its presence is an 
oversight.

So far, no one who has chimed in on this particular subject has answered this 
question: what's so special about the 8276 that its DTS should have a 'chosen' 
section when none of the others do?

> (as a matter of fact, the more I think about those .dts's, the more it 
> seems to me like they belong to bootloaders, not in the kernel... Just 
> one word - information duplication...)

Well, that's a completely separate issue altogether.  I believe that they are 
kept with the kernel as a convenience, so that we all have more control over 
their contents.

> Another thing, Timur, if you respin your patch, could you do it against 
> the current powerpc.git? There's at least one more .dts there that also 
> would need boot-cpu removed - kuroboxHD.dts.

My patch already removes linux,boot-cpu from kuroboxHD.dts:

diff --git a/arch/powerpc/boot/dts/kuroboxHG.dts 
b/arch/powerpc/boot/dts/kuroboxHG.dts
index d06b0b0..7531027 100644
--- a/arch/powerpc/boot/dts/kuroboxHG.dts
+++ b/arch/powerpc/boot/dts/kuroboxHG.dts
@@ -35,7 +35,6 @@ build with: "dtc -f -I dts -O dtb -o kur

  		PowerPC,603e { /* Really 8241 */
  			linux,phandle = <2100>;
-			linux,boot-cpu;
  			device_type = "cpu";
  			reg = <0>;
  			clock-frequency = <fdad680>;	/* Fixed by bootwrapper */

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Guennadi Liakhovetski]

On Wed, 14 Feb 2007, Timur Tabi wrote:

> Guennadi Liakhovetski wrote:
> 
> > (as a matter of fact, the more I think about those .dts's, the more it 
> > seems to me like they belong to bootloaders, not in the kernel... Just 
> > one word - information duplication...)
> 
> Well, that's a completely separate issue altogether.

that's why I put it in brackets:-)

> I believe that they are 
> kept with the kernel as a convenience, so that we all have more control over 
> their contents.

yeah... I read a __part__ of those lengthy discussions... Not willing to 
re-start them here, just couldn't help...

> > Another thing, Timur, if you respin your patch, could you do it against 
> > the current powerpc.git? There's at least one more .dts there that also 
> > would need boot-cpu removed - kuroboxHD.dts.
> 
> My patch already removes linux,boot-cpu from kuroboxHD.dts:
> 
> diff --git a/arch/powerpc/boot/dts/kuroboxHG.dts 

"HD" and "HG" are not the same - it's a new file in git now...

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Guennadi Liakhovetski wrote:

>>> Another thing, Timur, if you respin your patch, could you do it against 
>>> the current powerpc.git? There's at least one more .dts there that also 
>>> would need boot-cpu removed - kuroboxHD.dts.
>> My patch already removes linux,boot-cpu from kuroboxHD.dts:
>>
>> diff --git a/arch/powerpc/boot/dts/kuroboxHG.dts 
> 
> "HD" and "HG" are not the same - it's a new file in git now...

Oh, sorry.  I guess I need new glasses.

In that case, I will respin the patch.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Guennadi Liakhovetski]

On Wed, 14 Feb 2007, Timur Tabi wrote:

> Guennadi Liakhovetski wrote:
> 
> > "HD" and "HG" are not the same - it's a new file in git now...
> 
> Oh, sorry.  I guess I need new glasses.

I think, it's someone else who has to be more inventive:-)

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Benjamin Herrenschmidt]

On Wed, 2007-02-14 at 08:03 -0600, Timur Tabi wrote:
> Benjamin Herrenschmidt wrote:
> 
> >> I just don't understand why it's okay to leave it there.
> > 
> > To make sure there is one. I suspect kernel bits might get upset if it's
> > absent.
> 
> That's not what I meant.  What I don't understand is: what's so special 
> about the 8272 that its DTS should have a 'chosen' section, but no other 
> DTS needs one?
> 
> Obviously, the kernel needs a 'chosen' section, and that's why U-Boot 
> adds one.  And since U-Boot adds one, the DTS itself doesn't need one. 
> Does the 8272 never use U-Boot?

I'm not talking specifically about the 8272. I think we should at it
back to all dts's

Ben.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Benjamin Herrenschmidt wrote:

>> Obviously, the kernel needs a 'chosen' section, and that's why U-Boot 
>> adds one.  And since U-Boot adds one, the DTS itself doesn't need one. 
>> Does the 8272 never use U-Boot?
> 
> I'm not talking specifically about the 8272. I think we should at it
> back to all dts's

Ugh.  Considering that U-Boot overrides whatever is in the 'chosen' section, can 
you tell me why you think it should be there?  If we put an empty 'chosen' 
section in the DTS files, are we actually doing anything meaningful?

In the interest of saving time, I'm going to respin my patch without the 
'chosen' section removed.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Benjamin Herrenschmidt]

On Wed, 2007-02-14 at 15:02 -0600, Timur Tabi wrote:
> Benjamin Herrenschmidt wrote:
> 
> >> Obviously, the kernel needs a 'chosen' section, and that's why U-Boot 
> >> adds one.  And since U-Boot adds one, the DTS itself doesn't need one. 
> >> Does the 8272 never use U-Boot?
> > 
> > I'm not talking specifically about the 8272. I think we should at it
> > back to all dts's
> 
> Ugh.  Considering that U-Boot overrides whatever is in the 'chosen' section, can 
> you tell me why you think it should be there?  If we put an empty 'chosen' 
> section in the DTS files, are we actually doing anything meaningful?
> 
> In the interest of saving time, I'm going to respin my patch without the 
> 'chosen' section removed.

In case somebody wants to use something else than uboot ? I'd like the
base structure of the mandatory nodes to be there. /chosen is mandatory,
so it should be there.

Ben.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Benjamin Herrenschmidt wrote:

> In case somebody wants to use something else than uboot ? I'd like the
> base structure of the mandatory nodes to be there. /chosen is mandatory,
> so it should be there.

I just posted a new patch that just deletes linux,boot-cpu and leaves the 
'chosen' node alone.

However, there is a valid reason for *not* having a 'chosen' node in the DTS. 
Currently, U-Boot does not update the existing 'chosen' node.  Instead, it 
creates a new one.  This means that if one existed in the DTS, then there would 
be *two* in the device tree that is passed to the kernel.  There is no way to 
guarantee that the kernel will use the right one.

Of course, the real solution to this problem is to fix U-Boot.  However, there 
isn't any signed up to do that, and even if someone posted a patch right now, 
there's no telling when it will get applied.  And even if it got applied today, 
all current U-Boot installations would be broken.  Many of these systems will 
never be updated.

Therefore, I would say that having a 'chosen' node in the DTS is actually a 
*bad* thing, on systems that use U-Boot.

Another work-around would be to update the compiler to delete the chosen node 
when it builds the DTB.  That's not a bad idea, I think, but until that feature 
is implemented, the DTS files should not have a 'chosen' node.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Benjamin Herrenschmidt]

> Of course, the real solution to this problem is to fix U-Boot. 

Yup :-)

>  However, there 
> isn't any signed up to do that, and even if someone posted a patch right now, 
> there's no telling when it will get applied.  And even if it got applied today, 
> all current U-Boot installations would be broken.  Many of these systems will 
> never be updated.

But their .dts neither so it doesn't matter does it ?

> Therefore, I would say that having a 'chosen' node in the DTS is actually a 
> *bad* thing, on systems that use U-Boot.

Too bad for them..

> Another work-around would be to update the compiler to delete the chosen node 
> when it builds the DTB.  That's not a bad idea, I think, but until that feature 
> is implemented, the DTS files should not have a 'chosen' node.

Ben.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[David Gibson]

On Thu, Feb 15, 2007 at 08:17:02AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2007-02-14 at 15:02 -0600, Timur Tabi wrote:
> > Benjamin Herrenschmidt wrote:
> > 
> > >> Obviously, the kernel needs a 'chosen' section, and that's why U-Boot 
> > >> adds one.  And since U-Boot adds one, the DTS itself doesn't need one. 
> > >> Does the 8272 never use U-Boot?
> > > 
> > > I'm not talking specifically about the 8272. I think we should at it
> > > back to all dts's
> > 
> > Ugh.  Considering that U-Boot overrides whatever is in the 'chosen' section, can 
> > you tell me why you think it should be there?  If we put an empty 'chosen' 
> > section in the DTS files, are we actually doing anything meaningful?
> > 
> > In the interest of saving time, I'm going to respin my patch without the 
> > 'chosen' section removed.
> 
> In case somebody wants to use something else than uboot ? I'd like the
> base structure of the mandatory nodes to be there. /chosen is mandatory,
> so it should be there.

More specifically:
	1) Having an (even empty) /chosen makes life easier for other
	bootloaders, which may have quite limited capability to edit
	device tree blobs.

	2) It seems to me the dts files *included with the kernel*
	should be suitable for direct use in zImage wrappers, where
	possible, for use with old uboot and other non device tree
	capable firmwares.

-- 
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: delete boot-cpu and chosen nodes from all DTSfiles

[Yoder Stuart-B08248]

=20

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]=20
> Sent: Wednesday, February 14, 2007 2:55 PM

[snip]

> > Obviously, the kernel needs a 'chosen' section, and that's=20
> why U-Boot=20
> > adds one.  And since U-Boot adds one, the DTS itself=20
> doesn't need one.=20
> > Does the 8272 never use U-Boot?
>=20
> I'm not talking specifically about the 8272. I think we should at it
> back to all dts's

Why do we need the chosen node in the dts files?   Is it to generalize
the dts files for use beyond u-boot?

What meaningful properties should it have in the dts?

Stuart

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTSfiles

[David Gibson]

On Wed, Feb 14, 2007 at 02:09:19PM -0700, Yoder Stuart-B08248 wrote:
>  
> 
> > -----Original Message-----
> > From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org] 
> > Sent: Wednesday, February 14, 2007 2:55 PM
> 
> [snip]
> 
> > > Obviously, the kernel needs a 'chosen' section, and that's 
> > why U-Boot 
> > > adds one.  And since U-Boot adds one, the DTS itself 
> > doesn't need one. 
> > > Does the 8272 never use U-Boot?
> > 
> > I'm not talking specifically about the 8272. I think we should at it
> > back to all dts's
> 
> Why do we need the chosen node in the dts files?   Is it to generalize
> the dts files for use beyond u-boot?
> 
> What meaningful properties should it have in the dts?

'interrupt-controller'

-- 
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: delete boot-cpu and chosen nodes from all DTS files

[Segher Boessenkool]

>> I think that:
>>
>>  - The /chosen node should be present in the .dts even if empty
>
> Only one DTS file has it.  It's been removed from every other DTS.  Are
> you should that instead of removing the last one, I should *add* one to
> the other DTS files?

Every device tree is required to have a /chosen node.  Now
that doesn't mean DTS files need one; there are some more
options:

-- dtc could add the node whenever it is missing from the DTS;
-- it could be made a requirement of the boot loader to add it;
-- the kernel could be taught to fake one if needed.

Pick your poison :-)  I prefer the first option (dtc).


Segher

[PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

The 'linux,boot-cpu' property and the 'chosen' node do not belong in the
DTS file, because U-Boot creates these entries in the device tree when
it boots linux.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 arch/powerpc/boot/dts/kuroboxHG.dts    |    1 -
 arch/powerpc/boot/dts/mpc7448hpc2.dts  |    5 ++---
 arch/powerpc/boot/dts/mpc8272ads.dts   |    8 --------
 arch/powerpc/boot/dts/mpc8323emds.dts  |    7 +++----
 arch/powerpc/boot/dts/mpc8360emds.dts  |    1 -
 arch/powerpc/boot/dts/mpc8560ads.dts   |    1 -
 arch/powerpc/boot/dts/mpc8641_hpcn.dts |    3 +--
 arch/powerpc/boot/dts/mpc866ads.dts    |    1 -
 arch/powerpc/boot/dts/mpc885ads.dts    |    1 -
 9 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/boot/dts/kuroboxHG.dts b/arch/powerpc/boot/dts/kuroboxHG.dts
index d06b0b0..7531027 100644
--- a/arch/powerpc/boot/dts/kuroboxHG.dts
+++ b/arch/powerpc/boot/dts/kuroboxHG.dts
@@ -35,7 +35,6 @@ build with: "dtc -f -I dts -O dtb -o kur
 
 		PowerPC,603e { /* Really 8241 */
 			linux,phandle = <2100>;
-			linux,boot-cpu;
 			device_type = "cpu";
 			reg = <0>;
 			clock-frequency = <fdad680>;	/* Fixed by bootwrapper */
diff --git a/arch/powerpc/boot/dts/mpc7448hpc2.dts b/arch/powerpc/boot/dts/mpc7448hpc2.dts
index c4d9562..4164351 100644
--- a/arch/powerpc/boot/dts/mpc7448hpc2.dts
+++ b/arch/powerpc/boot/dts/mpc7448hpc2.dts
@@ -23,7 +23,7 @@
 		#address-cells = <1>;
 		#size-cells =<0>;
 		linux,phandle = <200>;
-				
+
 		PowerPC,7448@0 {
 			device_type = "cpu";
 			reg = <0>;
@@ -36,7 +36,6 @@
 			bus-frequency = <0>;		// From U-Boot
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
@@ -152,7 +151,7 @@
 			#address-cells = <3>;
 			reg = <1000 1000>;
 			bus-range = <0 0>;
-			ranges = <02000000 0 e0000000 e0000000 0 1A000000	
+			ranges = <02000000 0 e0000000 e0000000 0 1A000000
 				  01000000 0 00000000 fa000000 0 00010000>;
 			clock-frequency = <7f28154>;
 			interrupt-parent = <7400>;
diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/boot/dts/mpc8272ads.dts
index 26b44f7..c9ffc12 100644
--- a/arch/powerpc/boot/dts/mpc8272ads.dts
+++ b/arch/powerpc/boot/dts/mpc8272ads.dts
@@ -34,7 +34,6 @@
                        clock-frequency = <0>;
                        32-bit;
                        linux,phandle = <201>;
-                       linux,boot-cpu;
                };
        };
 
@@ -53,13 +52,6 @@
                reg = <00000000 4000000 f4500000 00000020>;
        };
 
-       chosen {
-               name = "chosen";
-               linux,platform = <0>;
-               interrupt-controller = <10c00>;
-               linux,phandle = <400>;
-       };
-
        soc8272@f0000000 {
                #address-cells = <1>;
                #size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/mpc8323emds.dts b/arch/powerpc/boot/dts/mpc8323emds.dts
index fa7ef24..eaf0f86 100644
--- a/arch/powerpc/boot/dts/mpc8323emds.dts
+++ b/arch/powerpc/boot/dts/mpc8323emds.dts
@@ -34,7 +34,6 @@
 			clock-frequency = <0>;
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
@@ -174,7 +173,7 @@
 			built-in;
 			device_type = "ipic";
 		};
-		
+
 		par_io@1400 {
 			reg = <1400 100>;
 			device_type = "par_io";
@@ -237,11 +236,11 @@
 		reg = <e0100000 480>;
 		brg-frequency = <0>;
 		bus-frequency = <BCD3D80>;
-		
+
 		muram@10000 {
 			device_type = "muram";
 			ranges = <0 00010000 00004000>;
-	
+
 			data-only@0 {
 				reg = <0 4000>;
 			};
diff --git a/arch/powerpc/boot/dts/mpc8360emds.dts b/arch/powerpc/boot/dts/mpc8360emds.dts
index 9022192..303bd66 100644
--- a/arch/powerpc/boot/dts/mpc8360emds.dts
+++ b/arch/powerpc/boot/dts/mpc8360emds.dts
@@ -39,7 +39,6 @@
 			clock-frequency = <1F78A400>;
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 119bd5d..c74d6eb 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -35,7 +35,6 @@
 			clock-frequency = <312c8040>;
 			32-bit;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index f0c7731..258fccb 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -32,7 +32,6 @@
 			bus-frequency = <0>;		// From uboot
 			clock-frequency = <0>;		// From uboot
 			32-bit;
-			linux,boot-cpu;
 		};
 		PowerPC,8641@1 {
 			device_type = "cpu";
@@ -142,7 +141,7 @@
 			interrupt-parent = <40000>;
 			phy-handle = <2452001>;
 		};
-		
+
 		ethernet@26000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/arch/powerpc/boot/dts/mpc866ads.dts b/arch/powerpc/boot/dts/mpc866ads.dts
index 5d40052..2b56b5d 100644
--- a/arch/powerpc/boot/dts/mpc866ads.dts
+++ b/arch/powerpc/boot/dts/mpc866ads.dts
@@ -37,7 +37,6 @@
 			interrupts = <f 2>;	// decrementer interrupt
 			interrupt-parent = <ff000000>;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
index cf1a19f..faecd08 100644
--- a/arch/powerpc/boot/dts/mpc885ads.dts
+++ b/arch/powerpc/boot/dts/mpc885ads.dts
@@ -37,7 +37,6 @@
 			interrupts = <f 2>;	// decrementer interrupt
 			interrupt-parent = <ff000000>;
 			linux,phandle = <201>;
-			linux,boot-cpu;
 		};
 	};
 
-- 
1.4.4

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Kumar Gala]

On Feb 13, 2007, at 10:47 AM, Timur Tabi wrote:

> The 'linux,boot-cpu' property and the 'chosen' node do not belong  
> in the
> DTS file, because U-Boot creates these entries in the device tree when
> it boots linux.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---

Is there some harm is having them set in the .dts?

I can see 'linux,boot-cpu' may vary on a SMP, but the majority  
of .dts's are for uniproc's

- k

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Kumar Gala wrote:
> 
> On Feb 13, 2007, at 10:47 AM, Timur Tabi wrote:
> 
>> The 'linux,boot-cpu' property and the 'chosen' node do not belong in the
>> DTS file, because U-Boot creates these entries in the device tree when
>> it boots linux.
>>
>> Signed-off-by: Timur Tabi <timur@freescale.com>
>> ---
> 
> Is there some harm is having them set in the .dts?

Well, I don't think a DTS should contain a property that isn't going to be used. 
  It might convince people that the property belongs there.

The 'chosen' section is absent from all but one DTS for the same reason.

> I can see 'linux,boot-cpu' may vary on a SMP, but the majority of .dts's 
> are for uniproc's

U-Boot sets it, so whatever the DTS contains is ignored.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

Kumar Gala wrote:
> 
> On Feb 13, 2007, at 10:47 AM, Timur Tabi wrote:
> 
>> The 'linux,boot-cpu' property and the 'chosen' node do not belong in the
>> DTS file, because U-Boot creates these entries in the device tree when
>> it boots linux.
>>
>> Signed-off-by: Timur Tabi <timur@freescale.com>
>> ---
> 
> Is there some harm is having them set in the .dts?

Actually, I just found out that the boot-cpu property is obsolete.  Instead, the 
boot CPU is specified in the device tree header, and DTC will complain if it 
sees linux,boot-cpu.

With that in mind, I'm going to respin this patch to include a change in 
booting-without-of.txt.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Segher Boessenkool]

> Is there some harm is having them set in the .dts?
>
> I can see 'linux,boot-cpu' may vary on a SMP, but the majority
> of .dts's are for uniproc's

The way this is handled in the OF binding, only multi-processor
systems are required to explicitly state their boot CPU (in
/chosen/cpu).  It's quite silly to require this for single
CPU systems.


Segher

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[David Gibson]

On Tue, Feb 13, 2007 at 10:47:19AM -0600, Timur Tabi wrote:
> The 'linux,boot-cpu' property and the 'chosen' node do not belong in the
> DTS file, because U-Boot creates these entries in the device tree when
> it boots linux.

[snip]
> @@ -53,13 +52,6 @@
>                 reg = <00000000 4000000 f4500000 00000020>;
>         };
>  
> -       chosen {
> -               name = "chosen";
> -               linux,platform = <0>;
> -               interrupt-controller = <10c00>;
> -               linux,phandle = <400>;
> -       };
> -

Um.. at least the interrupt-controller property here should be fixed,
and thus should probably remain in the dts.

-- 
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: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

David Gibson wrote:

>> -       chosen {
>> -               name = "chosen";
>> -               linux,platform = <0>;
>> -               interrupt-controller = <10c00>;
>> -               linux,phandle = <400>;
>> -       };
>> -
> 
> Um.. at least the interrupt-controller property here should be fixed,
> and thus should probably remain in the dts.

Can you be more specific?  I'm not familiar with this board, so I don't 
really know what you're talking about.

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[David Gibson]

On Tue, Feb 13, 2007 at 11:18:50PM -0600, Timur Tabi wrote:
> David Gibson wrote:
> 
> >> -       chosen {
> >> -               name = "chosen";
> >> -               linux,platform = <0>;
> >> -               interrupt-controller = <10c00>;
> >> -               linux,phandle = <400>;
> >> -       };
> >> -
> > 
> > Um.. at least the interrupt-controller property here should be fixed,
> > and thus should probably remain in the dts.
> 
> Can you be more specific?  I'm not familiar with this board, so I don't 
> really know what you're talking about.

The interrupt-controller property in /chosen just points to the
system's top-level interrupt controller to make it easier for the
kernel to find in the tree.  I think most platforms don't actually
need or use it at present, but booting-without-of does recommend it.

As such it's a property of the board, not something which will change
from boot to boot.  It would therefore be rather strange for the
bootloader to poke this property.

-- 
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: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

David Gibson wrote:

> The interrupt-controller property in /chosen just points to the
> system's top-level interrupt controller to make it easier for the
> kernel to find in the tree.  I think most platforms don't actually
> need or use it at present, but booting-without-of does recommend it.
> 
> As such it's a property of the board, not something which will change
> from boot to boot.  It would therefore be rather strange for the
> bootloader to poke this property.

That's all the more reason to delete the 'chosen' section, isn't it? 
Sicne it's wrong, deleting the entire section will just make the problem 
go away.  Or am I missing something?

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[David Gibson]

On Wed, Feb 14, 2007 at 08:05:40AM -0600, Timur Tabi wrote:
> David Gibson wrote:
> 
> > The interrupt-controller property in /chosen just points to the
> > system's top-level interrupt controller to make it easier for the
> > kernel to find in the tree.  I think most platforms don't actually
> > need or use it at present, but booting-without-of does recommend it.
> > 
> > As such it's a property of the board, not something which will change
> > from boot to boot.  It would therefore be rather strange for the
> > bootloader to poke this property.
> 
> That's all the more reason to delete the 'chosen' section, isn't it? 
> Sicne it's wrong, deleting the entire section will just make the problem 
> go away.  Or am I missing something?

Er.. I think you're missing something.

My point is that the interrupt-controller property in /chosen *should*
be there, but it doesn't make a lot of sense for it to be set by the
bootloader (because it's a fixed property of the board).  Therefore it
should be in the dts, therefore the /chosen node needs to be there to
contain it.

-- 
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: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

David Gibson wrote:

> Er.. I think you're missing something.
> 
> My point is that the interrupt-controller property in /chosen *should*
> be there, but it doesn't make a lot of sense for it to be set by the
> bootloader (because it's a fixed property of the board).  Therefore it
> should be in the dts, therefore the /chosen node needs to be there to
> contain it.

Ok, I understand.

Well, as soon as my other DTS-related patches are applied, I'll start working on 
one for the 'chosen' section.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Segher Boessenkool]

> My point is that the interrupt-controller property in /chosen *should*
> be there, but it doesn't make a lot of sense for it to be set by the
> bootloader (because it's a fixed property of the board).

And that means it shouldn't be in /chosen at all.  It's not
just a fixed property of the board, it is a physical property
of the board.  It doesn't belong in /chosen.


Segher

Re: [PATCH] powerpc: delete boot-cpu and chosen nodes from all DTS files

[Timur Tabi]

David Gibson wrote:

> My point is that the interrupt-controller property in /chosen *should*
> be there, but it doesn't make a lot of sense for it to be set by the
> bootloader (because it's a fixed property of the board).  Therefore it
> should be in the dts, therefore the /chosen node needs to be there to
> contain it.

Ok, I've added that to my to-do list.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale