[PATCH] [POWERPC] 83xx: Add support for the Thecus N1200 NAS device
David Gibson
david at gibson.dropbear.id.au
Fri Jun 5 13:08:50 EST 2009
On Thu, Jun 04, 2009 at 09:59:04PM +0100, Byron Bradley wrote:
> The Thecus N1200 is a NAS device with a single internal SATA disk and
> an eSATA port based on an MPC8347 SoC.
Comments on a number of fairly minor device tree nits below:
[snip]
> + soc8349 at e0000000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "soc";
> + compatible = "simple-bus";
The compatible value should have something more specific
(e.g. "fsl,mpc8340-soc") before listing "simple-bus".
> + ranges = <0x0 0xe0000000 0x00100000
> + 0xfe000000 0xfe000000 0x0800000>;
> + reg = <0xe0000000 0x00000200>;
> + bus-frequency = <0>; // from bootloader
> +
> + physmap-flash at fe000000 {
Calling this just "flash" would be more normal.
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "cfi-flash";
Ideally this should list the actual model of flash chip, before the
generic "cfi-flash".
> + reg = <0xfe000000 0x0800000>;
> + bank-width = <2>;
> + device-width = <1>;
> +
> + u-boot at 0 {
> + reg = <0x0 0x040000>;
> + read-only;
> + };
> +
> + u-boot-config at 40000 {
> + reg = <0x040000 0x040000>;
> + label = "u-boot config";
> + };
> +
> + user at 080000 {
> + reg = <0x080000 0x100000>;
> + };
> +
> + kernel at 180000 {
> + reg = <0x180000 0x1a0000>;
> + };
> +
> + ramdisk at 320000 {
> + reg = <0x320000 0x4e0000>;
> + };
> + };
> +
> + wdt at 200 {
> + device_type = "watchdog";
No device_type here.
> + compatible = "mpc83xx_wdt";
> + reg = <0x200 0x100>;
> + };
> +
> + i2c at 3000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + cell-index = <0>;
> + compatible = "fsl-i2c";
> + reg = <0x3000 0x100>;
> + interrupts = <14 0x8>;
> + interrupt-parent = <&ipic>;
> + dfsrr;
> +
> + fanctrl at 32 {
A less abbreviated name, like "fan-control" would be more usual here.
> + device_type = "hwmon";
No device_type here either.
> + compatible = "fintek,f75375";
> + reg = <0x2e>;
> + };
> +
> + rtc at 32 {
> + device_type = "rtc";
Or here.
> + compatible = "ricoh,rs5c372a";
> + reg = <0x32>;
> + };
> +
> + boardctrl at 32 {
Again "board-control" would be preferable.
> + device_type = "misc";
Especially no device_type here.
[snip]
> + ipic: pic at 700 {
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <2>;
> + reg = <0x700 0x100>;
> + device_type = "ipic";
This should have a compatible property. It shouldn't really have
device_type, but I suspect that's a bug in the ipic binding, rather
than your tree per-se.
> + };
> +
> + gpio1: gpio-controller at c00 {
> + #gpio-cells = <2>;
> + compatible = "fsl,mpc8347-gpio", "fsl,mpc8349-gpio";
This actually is an 8349 board, yes? Generally compatible should be
listed from most specific to least specific, so the 8349 entry should
go first.
--
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
More information about the Linuxppc-dev
mailing list