[PATCH v2 6/7] Holly DTS

[Segher Boessenkool]

> +/ {
> +	model = "ppc750-tsi109";

"model" should be something really specific; typically
a (unique!) model number.  This means you can't use the
same device tree for Holly and Hickory (but there are
more reasons for that; see below).

> +	compatible = "ppc750-tsi";

The needs to be more specific as well; "ibm,holly"
or something.

> +		PowerPC,750 at 0 {

Needs to be PowerPC,750CL etc.

> +			name = "PowerPC,750";

You don't need to specify "name" in modern DTS.

> +  	tsi109 at c0000000 {
> +		device_type = "tsi-bridge";

Don't put a "device_type" here, it is useless
(and undefined).  There are more like this, but
perhaps Linux (wrongly) probes on "device_type"
for those, so the kernel would need updating
first.

> +		bus-frequency = <0>;

0?

> +		ethernet at 6200 {
> +			model = "TSI-ETH";

"model" should be an exact model name.  If you don't
have one, just leave out the "model" property.

> +			address = [ 00 0d 60 f4 3f 04 ];

"local-mac-address" instead.  And you don't want to
hardcode one specific address like this; just keep
it open so if the bootloader fails to fill it in you
notice.  If the bootloader cannot do that, just leave
out this property completely (not compliant, but neither
is filling in the wrong value).

> +		serial at 7808 {
> +			reg-shift = <0>;

This is the default value so you can leave it out.

> +	  	MPIC: pic at 7400 {
> +			built-in;

Why say this for MPIC only, not for most other nodes?
What binding defines "built-in", anyway?

> +				/*--------------------------------------------+
> +				| AD19.
> +				+--------------------------------------------*/
> +				1800 0 0 1 &RT0 26 0
> +				1800 0 0 2 &RT0 27 0
> +				1800 0 0 3 &RT0 24 0
> +				1800 0 0 4 &RT0 25 0
> +
> +				2000 0 0 1 &RT0 27 0
> +				2000 0 0 2 &RT0 24 0
> +				2000 0 0 3 &RT0 25 0
> +				2000 0 0 4 &RT0 26 0
> +				>;

You forgot to say "AD20" ;-)

> +	chosen {
> +		linux,platform = <0>;
> +		linux,initrd-start = <0>;
> +		linux,initrd-end = <0>;

Do you need to set those zero properties?


Segher