On 02/22/2011 04:07 PM, David Gibson wrote:

On Tue, Feb 22, 2011 at 12:57:46PM -0800, David Daney wrote:

quoted

Signed-off-by: David Daney<redacted>
---
  arch/mips/cavium-octeon/.gitignore      |    2 +
  arch/mips/cavium-octeon/Makefile        |   13 ++
  arch/mips/cavium-octeon/octeon_3xxx.dts |  314 +++++++++++++++++++++++++++++++
  arch/mips/cavium-octeon/octeon_68xx.dts |   99 ++++++++++
  4 files changed, 428 insertions(+), 0 deletions(-)
  create mode 100644 arch/mips/cavium-octeon/.gitignore
  create mode 100644 arch/mips/cavium-octeon/octeon_3xxx.dts
  create mode 100644 arch/mips/cavium-octeon/octeon_68xx.dts

diff --git a/arch/mips/cavium-octeon/.gitignore b/arch/mips/cavium-octeon/.gitignore
new file mode 100644
index 0000000..39c9686
--- /dev/null
+++ b/arch/mips/cavium-octeon/.gitignore

@@ -0,0 +1,2 @@
+*.dtb.S

.dtb.S?

I think I have the correct .gitignore syntax.

[snip]

quoted

+/dts-v1/;
+/* OCTEON 3XXX, 5XXX, 63XX device tree skeleton. */
+/ {
+  model = "OCTEON";

1 tab indents are the usual convention for device trees.

OK.

quoted

+  compatible = "octeon,octeon";

There's no model number at all for this board?

I think it should be:

	compatible = "octeon,octeon-3860";

quoted

+  #address-cells =<2>;
+  #size-cells =<2>;
+
+  soc@0 {
+    device_type = "soc";

Drop this device_type.

OK.

quoted

+    compatible = "simple-bus";
+    #address-cells =<2>;
+    #size-cells =<2>;
+    ranges; /* Direct mapping */
+
+    ciu: ciu-3xxx@1070000000000 {
+      compatible = "octeon,ciu-3xxx";

So, names or compatible values with "wildcards" like 3xxx should be
avoided.  Instead, use the specific model number of this device, then
future devices can claim compatibility with the earlier one.

But, in addition the generic names convention means that the node name
should be "interrupt-controller" rather than something model specific.

Let's try:

ciu: interrupt-controller@1070000000000 {
       compatible = "octeon,octeon-3860-ciu";

quoted

+      interrupt-controller;
+      #address-cells =<0>;
+      #interrupt-cells =<2>;
+      reg =<0x10700 0x00000000 0x0 0x7000>;
+    };
+
+    /* SMI0 */
+    mdio0: mdio@1180000001800 {

If SMI0 is the name generally used in the documentation, using that in
the label instead of mdio0 might be more useful.

quoted

+      compatible = "octeon,mdio";

No model or revision number?

Let's try:

	smi0: mdio@1180000001800 {
		compatible = "octeon,octeon-3860-mdio";

quoted

+      #address-cells =<1>;
+      #size-cells =<0>;
+      reg =<0x11800 0x00001800 0x0 0x40>;
+      device_type = "mdio";

Drop this device_type.

OK.

[...]

quoted

+    mgmt0: ethernet@1070000100000 {
+      compatible = "octeon,mgmt";

This becomes:

	mgmt0: ethernet@1070000100000 {
		compatible = "octeon,octeon-5230-mii";

quoted

+      device_type = "network";
+      model = "mgmt";
+      reg =<0x10700 0x00100000 0x0 0x100>, /* MIX */
+<0x11800 0xE0000000 0x0 0x300>, /* AGL */
+<0x11800 0xE0000400 0x0 0x400>, /* AGL_SHARED  */
+<0x11800 0xE0002000 0x0 0x8>;   /* AGL_PRT_CTL */
+      unit-number =<0>;

What is this 'unit-number' property for?

The AGL_SHARED register bank is shared among all the octeon-5230-mii 
devices.  the 'unit-number' indicates the bit-field index that this 
device should use within those registers.

quoted

+      interrupt-parent =<&ciu>;
+      interrupts =<0 62>,<1 46>;
+      local-mac-address = [ 00 00 00 00 00 00 ];

That's not a valid MAC address of course.  If this has to be patched
in by the bootloader / later processing, you should add a comment to
that effect.

Right.

quoted

+      phy-handle =<&phy0>;
+    };
+
+    mgmt1: ethernet@1070000100800 {
+      compatible = "octeon,mgmt";
+      device_type = "network";
+      model = "mgmt";
+      reg =<0x10700 0x00100800 0x0 0x100>, /* MIX */
+<0x11800 0xE0000800 0x0 0x300>, /* AGL */
+<0x11800 0xE0000400 0x0 0x400>, /* AGL_SHARED  */
+<0x11800 0xE0002008 0x0 0x8>;   /* AGL_PRT_CTL */
+      unit-number =<1>;
+      interrupt-parent =<&ciu>;
+      interrupts =<1 18>,<  1 46>;
+      local-mac-address = [ 00 00 00 00 00 00 ];
+      phy-handle =<&phy1>;
+    };
+
+    pip: pip@11800a0000000 {
+      compatible = "octeon,pip";
+      #address-cells =<1>;
+      #size-cells =<0>;
+      reg =<0x11800 0xa0000000 0x0 0x2000>;
+
+      interface@0 {

These subnodes and subsubnodes should have compatible values too, even
if it's just "octeon,pip-interface" and "octeon,pip-ethernet".

OK.

quoted

+        #address-cells =<1>;
+        #size-cells =<0>;
+        reg =<0>; /* interface */
+
+        ethernet@0 {
+          device_type = "network";
+          model = "pip";

This model property doesn't look very useful.

I will remove it.

[snip]

Uh.. where are the CPUs?

Answered in other e-mail.

Thanks,
David Daney