Re: [RFC PATCH 02/10] MIPS: Octeon: Add device tree source files.

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

On Wed, Feb 23, 2011 at 11:06:30AM -0800, David Daney wrote:
> On 02/22/2011 04:07 PM, David Gibson wrote:
> >On Tue, Feb 22, 2011 at 12:57:46PM -0800, David Daney wrote:
> >>Signed-off-by: David Daney<ddaney-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> >>---
> >>  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.

What I meant was, where are you generating .dtb.S files that you need
to ignore them?

> >>+  compatible = "octeon,octeon";
> >
> >There's no model number at all for this board?
> 
> 
> I think it should be:
> 
> 	compatible = "octeon,octeon-3860";

That looks better.

Also, the part before the comma is generally the vendor, so I would
have expected cavium,XXX throughout rather than octeon,XXX.

[snip]
> >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";

That looks better.

[snip]
> >>+      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.

Ok.  'cell-index' is the normal property name for this sort of
purpose.

-- 
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