Re: [PATCH V2 6/6] powerpc/476: Create a dts files for two 476 AMP instances under ISS

[Dave Kleikamp <shaggy@linux.vnet.ibm.com>]

On Wed, 2011-02-02 at 13:43 +1100, David Gibson wrote:
> On Tue, Feb 01, 2011 at 12:48:46PM -0600, Dave Kleikamp wrote:
> > These are completely independent OS instances, each running on 2
> > cores.
> 
> [snip]
> > +/memreserve/ 0x01f00000 0x00100000;
> 
> A comment describing what this reserved section is for would be good.

I with I knew what it was for.  I've blindly carried it along for a
while.  Removing it doesn't appear to do any harm.  Ben, any idea why
this was ever in here?

> > +/ {
> > +	#address-cells = <2>;
> > +	#size-cells = <1>;
> > +	model = "ibm,iss-4xx";
> > +	compatible = "ibm,iss-4xx", "ibm,47x-AMP";
> > +	dcr-parent = <&{/cpus/cpu@0}>;
> > +
> > +	aliases {
> > +		serial0 = &UART0;
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > +			device_type = "cpu";
> > +			model = "PowerPC,4xx"; // real CPU changed in sim
> 
> If the comment is true, then it's probably simpler to just omit the
> model property.  I'm pretty sure nothing will look at it.

It doesn't appear to be true.  Another bit I've been carrying along
without checking it.  Removing the comment.

> 
> > +			reg = <0>;
> > +			clock-frequency = <100000000>; // 100Mhz :-)
> > +			timebase-frequency = <100000000>;> +			i-cache-line-size = <32>;
> > +			d-cache-line-size = <32>;
> > +			i-cache-size = <32768>;
> > +			d-cache-size = <32768>;
> > +			dcr-controller;
> > +			dcr-access-method = "native";
> > +			status = "ok";
> 
> Should be "okay" rather than "ok".

okay :-)

> 
> [snip]
> > +			UART0: serial@40000200 {
> > +				device_type = "serial";
> > +				compatible = "ns16550a";
> > +				reg = <0x40000200 0x00000008>;
> > +				virtual-reg = <0xe0000200>;
> > +				clock-frequency = <11059200>;
> > +				current-speed = <115200>;
> > +				interrupt-parent = <&MPIC>;
> > +				interrupts = <0x0 0x2>;
> > +			};
> > +		};
> > +	};
> > +
> > +	nvrtc {
> > +		compatible = "ds1743-nvram", "ds1743", "rtc-ds1743";
> > +		reg = <0 0xEF703000 0x2000>;
> > +	};
> > +
> > +	chosen {
> > +		linux,stdout-path = "/plb/opb/serial@40000200";
> 
> You can use a string reference here:
> 	linux,stdout-path = &UART0;

no problem

Shaggy

-- 
Dave Kleikamp
IBM Linux Technology Center