On Wed, Jun 10, 2009 at 05:30:39PM -0500, Nate Case wrote:
> Add support for X-ES single-board computers based on the Freescale
> MPC85xx processors. Changes include:
[snip]
> diff --git a/arch/powerpc/boot/dts/xcalibur1501.dts
> b/arch/powerpc/boot/dts/xcalibur1501.dts
> new file mode 100644
> index 0000000..497af7a
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/xcalibur1501.dts
> @@ -0,0 +1,759 @@
> +/*
> + * Copyright (C) 2008 Extreme Engineering Solutions, Inc.
> + * Based on MPC8572DS device tree from Freescale Semiconductor, Inc.
> + *
> + * XCalibur1501 6U CompactPCI single-board computer based on MPC8572E
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/dts-v1/;
> +/ {
> + model = "xes,xcalibur1501";
> + compatible = "xes,xcalibur1501", "xes,MPC8572";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + form-factor = "6U cPCI";
> + boot-bank = <0x0>; /* 0: Primary flash, 1: Secondary flash */
These last two aren't standard properties, so should probably be
"xes,form-factor" and "xes,boot-bank".
[snip]
> + pmcslots {
What does this structure model? Without any reg properties it's kind
of hard to see what you could do with it.
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmcs...@0 {
Since you have an unit address, you must also have a reg property to
match, so reg = <0> in this case.
> + cell-index = <0>;
Don't use cell-index here, it's redundant with the reg value that you
should have.
> + /*
> + * boolean properties (true if defined):
> + * monarch;
> + * module-present;
> + */
> + };
> +
> + pmcs...@1 {
> + cell-index = <1>;
> + /*
> + * boolean properties (true if defined):
> + * monarch;
> + * module-present;
> + */
> + };
> + };
> +
> + xmcslots {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + xmcs...@0 {
> + cell-index = <0>;
Same comments here.
> + /*
> + * boolean properties (true if defined):
> + * module-present;
> + */
> + };
> +
> + xmcs...@1 {
> + cell-index = <1>;
> + /*
> + * boolean properties (true if defined):
> + * module-present;
> + */
> + };
> + };
> +
> + cpci {
> + /*
> + * boolean properties (true if defined):
> + * system-controller;
> + */
> + system-controller;
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + PowerPC,8...@0 {
> + device_type = "cpu";
> + reg = <0x0>;
> + d-cache-line-size = <32>; // 32 bytes
> + i-cache-line-size = <32>; // 32 bytes
> + d-cache-size = <0x8000>; // L1, 32K
> + i-cache-size = <0x8000>; // L1, 32K
> + timebase-frequency = <0>;
> + bus-frequency = <0>;
> + clock-frequency = <0>;
> + next-level-cache = <&L2>;
> + };
> +
> + PowerPC,8...@1 {
> + device_type = "cpu";
> + reg = <0x1>;
> + d-cache-line-size = <32>; // 32 bytes
> + i-cache-line-size = <32>; // 32 bytes
> + d-cache-size = <0x8000>; // L1, 32K
> + i-cache-size = <0x8000>; // L1, 32K
> + timebase-frequency = <0>;
> + bus-frequency = <0>;
> + clock-frequency = <0>;
> + next-level-cache = <&L2>;
> + };
> + };
> +
> + memory {
> + device_type = "memory";
I assume this node's reg property is supposed to be filled in by the
bootloader. Best to have an explanatory comment, and/or a template
reg property here.
[snip]
> + soc8...@ef000000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "soc";
> + compatible = "simple-bus";
This compatible value needs a more specific entry for the specific SoC
type.
[snip]
> + /* eTSEC 1 front panel 0 */
> + enet0: ether...@24000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + cell-index = <0>;
> + device_type = "network";
> + model = "eTSEC";
> + compatible = "gianfar";
> + reg = <0x24000 0x1000>;
> + ranges = <0x0 0x24000 0x1000>;
> + local-mac-address = [ 00 00 00 00 00 00 ];
> + interrupts = <29 2 30 2 34 2>;
> + interrupt-parent = <&mpic>;
> + tbi-handle = <&tbi0>;
> + phy-handle = <&phy0>;
> + phy-connection-type = "sgmii";
> +
> + m...@520 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,gianfar-mdio";
> + reg = <0x520 0x20>;
> +
> + phy0: ethernet-...@1 {
> + interrupt-parent = <&mpic>;
> + interrupts = <4 1>;
> + reg = <0x1>;
> + device_type = "ethernet-phy";
Drop this device_type.
> + };
> + phy1: ethernet-...@2 {
> + interrupt-parent = <&mpic>;
> + interrupts = <4 1>;
> + reg = <0x2>;
> + device_type = "ethernet-phy";
> + };
> + phy2: ethernet-...@3 {
> + interrupt-parent = <&mpic>;
> + interrupts = <5 1>;
> + reg = <0x3>;
> + device_type = "ethernet-phy";
> + };
> + phy3: ethernet-...@4 {
> + interrupt-parent = <&mpic>;
> + interrupts = <5 1>;
> + reg = <0x4>;
> + device_type = "ethernet-phy";
> + };
> + tbi0: tbi-...@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
> + };
> + };
> + };
> +
> + /* eTSEC 2 front panel 1 */
> + enet1: ether...@25000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + cell-index = <1>;
> + device_type = "network";
> + model = "eTSEC";
> + compatible = "gianfar";
> + reg = <0x25000 0x1000>;
> + ranges = <0x0 0x25000 0x1000>;
> + local-mac-address = [ 00 00 00 00 00 00 ];
> + interrupts = <35 2 36 2 40 2>;
> + interrupt-parent = <&mpic>;
> + tbi-handle = <&tbi1>;
> + phy-handle = <&phy1>;
> + phy-connection-type = "sgmii";
> +
> + m...@520 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,gianfar-tbi";
> + reg = <0x520 0x20>;
> +
> + tbi1: tbi-...@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
> + };
> + };
> + };
> +
> + /* eTSEC 3 PICMG2.16 backplane port 0 */
> + enet2: ether...@26000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + cell-index = <2>;
> + device_type = "network";
> + model = "eTSEC";
> + compatible = "gianfar";
> + reg = <0x26000 0x1000>;
> + ranges = <0x0 0x26000 0x1000>;
> + local-mac-address = [ 00 00 00 00 00 00 ];
> + interrupts = <31 2 32 2 33 2>;
> + interrupt-parent = <&mpic>;
> + tbi-handle = <&tbi2>;
> + phy-handle = <&phy2>;
> + phy-connection-type = "sgmii";
> +
> + m...@520 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,gianfar-tbi";
> + reg = <0x520 0x20>;
> +
> + tbi2: tbi-...@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
And this one, too. Although this node should probably have a
compatible property instead.
[snip]
> + t...@2f000 {
> + device_type = "tlu";
Drop this device_type also.
> diff --git a/arch/powerpc/boot/dts/xpedite5200.dts
> b/arch/powerpc/boot/dts/xpedite5200.dts
> new file mode 100644
> index 0000000..15a5883
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/xpedite5200.dts
Analagous comments for the other device trees.
--
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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
