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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <61570CA6-9450-4B9C-A472-FE62A5B23B49@kernel.crashing.org>
Date:	Sun, 3 Jul 2011 20:07:45 +0200
From:	Segher Boessenkool <segher@...nel.crashing.org>
To:	Jonas Bonn <jonas@...thpole.se>
Cc:	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	devicetree-discuss@...ts.ozlabs.org
Subject: Re: [PATCH v2 02/19] OpenRISC: Device tree

> 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@...00000 {
> +			/* 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@...00000 {
> +			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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/