Re: [PATCH v2 02/19] OpenRISC: Device tree

[Segher Boessenkool]

> The OpenRISC architecture uses the device tree infrastructure for the
> platform description.

Hurray, world domination is one step closer ;-)

> +	memory {
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;
> +	};

Might want to write this as 0x08000000 (easier to read).

> +	cpus {
> +		cpu@0 {

Any node with a textual unit address (@0) needs to have a "reg" property
with the same address (and then the parent node, "cpus", should have
#address-cells and #size-cells defined, probably to 1 and 0 resp. in  
this
case).

You can just name the node "cpu" here.

> +		pic: pic@0 {
> +			compatible = "opencores,or1k-pic";
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +		};

Same thing here.  Although, does your interrupt controller have any  
registers?
You should describe them then.

> +
> +		serial0: serial@90000000 {
> +			/* FIXME: device_type is still needed here...
> +			   should remove requirement from of_serial driver */
> +			device_type = "serial";
> +			compatible = "opencores,uart", "ns16550a";

Opencores has and always will have only one UART implementation?  You  
should
make the "compatible" name more specific ("opencores,uart-16550a"  
perhaps).

> +		enet0: ethoc@92000000 {
> +			compatible = "opencores,ethoc", "ethoc";

Probably shouldn't have the "ethoc" there, way too generic a name.

> +			reg = <0x92000000 0x53>;

0x53?  Usually you write the size of the register block that is decoded,
not just the size of the defined registers; so it would be 0x100 or  
something
like that.


Segher

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/