[PATCH 1/3 v3] Add StorCenter DTS first draft.

[PATCH 1/3 v3] Add StorCenter DTS first draft.

[Jon Loeliger]

Based on the Kurobox DTS files.

Signed-off-by: Andy Wilcox <andy@protium.com>
Signed-off-by: Jon Loeliger <jdl@jdl.com>
---
 arch/powerpc/boot/dts/storcenter.dts |  139 ++++++++++++++++++++++++++++++++++
 1 files changed, 139 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/storcenter.dts

diff --git a/arch/powerpc/boot/dts/storcenter.dts b/arch/powerpc/boot/dts/storcenter.dts
new file mode 100644
index 0000000..fd2f3aa
--- /dev/null
+++ b/arch/powerpc/boot/dts/storcenter.dts
@@ -0,0 +1,139 @@
+/*
+ * Device Tree Source for IOMEGA StorCenter
+ *
+ * Copyright 2007 Oyvind Repvik
+ * Copyright 2007 Jon Loeliger
+ *
+ * Based on the Kurobox DTS by G. Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/ {
+	model = "StorCenter";
+	compatible = "storcenter";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	aliases {
+		serial0 = &serial0;
+		serial1 = &serial1;
+		pci0 = &pci0;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		PowerPC,8241@0 {
+			device_type = "cpu";
+			reg = <0>;
+			clock-frequency = <d# 200000000>;	/* Hz */
+			timebase-frequency = <d# 25000000>;	/* Hz */
+			bus-frequency = <0>;	/* from bootwrapper */
+			i-cache-line-size = <d# 32>;	/* bytes */
+			d-cache-line-size = <d# 32>;	/* bytes */
+			i-cache-size = <4000>;
+			d-cache-size = <4000>;
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <00000000 04000000>;	/* 64MB @ 0x0 */
+	};
+
+	soc@fc000000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		device_type = "soc";
+		compatible = "fsl,mpc8241", "mpc10x";
+		store-gathering = <0>; /* 0 == off, !0 == on */
+		ranges = <0 fc000000 100000>;
+		reg = <fc000000 100000>;	/* EUMB */
+		bus-frequency = <0>;		/* fixed by loader */
+
+		i2c@3000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "fsl-i2c";
+			reg = <3000 1000>;
+			interrupts = <5 2>;
+			interrupt-parent = <&mpic>;
+
+			rtc@68 {
+				compatible = "dallas,ds1337";
+				reg = <68>;
+			};
+		};
+
+		serial0: serial@4500 {
+			cell-index = <0>;
+			device_type = "serial";
+			compatible = "ns16550";
+			reg = <4500 20>;
+			clock-frequency = <d# 97553800>; /* Hz */
+			current-speed = <d# 115200>;
+			interrupts = <9 2>;
+			interrupt-parent = <&mpic>;
+		};
+
+		serial1: serial@4600 {
+			cell-index = <1>;
+			device_type = "serial";
+			compatible = "ns16550";
+			reg = <4600 20>;
+			clock-frequency = <d# 97553800>; /* Hz */
+			current-speed = <d# 9600>;
+			interrupts = <a 2>;
+			interrupt-parent = <&mpic>;
+		};
+
+		mpic: interrupt-controller@40000 {
+			#interrupt-cells = <2>;
+			#address-cells = <0>;
+			device_type = "open-pic";
+			compatible = "chrp,open-pic";
+			interrupt-controller;
+			reg = <40000 40000>;
+		};
+
+	};
+
+	pci0: pci@fe800000 {
+		#address-cells = <3>;
+		#size-cells = <2>;
+		#interrupt-cells = <1>;
+		device_type = "pci";
+		compatible = "mpc10x-pci";
+		reg = <fe800000 1000>;
+		ranges = <01000000 0        0 fe000000 0 00c00000
+			  02000000 0 80000000 80000000 0 70000000>;
+		bus-range = <0 ff>;
+		clock-frequency = <d# 97553800>; /* Hz */
+		interrupt-parent = <&mpic>;
+		interrupt-map-mask = <f800 0 0 7>;
+		interrupt-map = <
+			/* IDSEL 13 - IDE */
+			6800 0 0 1 &mpic 0 1
+			6800 0 0 2 &mpic 0 1
+			6800 0 0 3 &mpic 0 1
+			/* IDSEL 14 - USB */
+			7000 0 0 1 &mpic 0 1
+			7000 0 0 2 &mpic 0 1
+			7000 0 0 3 &mpic 0 1
+			7000 0 0 4 &mpic 0 1
+			/* IDSEL 15 - ETH */
+			7800 0 0 1 &mpic 0 1
+			7800 0 0 2 &mpic 0 1
+			7800 0 0 3 &mpic 0 1
+			7800 0 0 4 &mpic 0 1
+		>;
+	};
+
+	chosen {
+		linux,stdout-path = "/soc/serial@4500";
+	};
+};
-- 
1.5.4.rc0

Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Grant Likely]

On 1/22/08, Jon Loeliger <jdl@jdl.com> wrote:
> Based on the Kurobox DTS files.
>
> Signed-off-by: Andy Wilcox <andy@protium.com>
> Signed-off-by: Jon Loeliger <jdl@jdl.com>

Comments below

> +
> +/ {
> +       model = "StorCenter";
> +       compatible = "storcenter";

Be specific!  "iomega,storcenter".  Even better if you put in the model number.

> +
> +       soc@fc000000 {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               device_type = "soc";

device_type should be dropped (but I know that requires changes to the
existing mpc82xx support code).

> +               compatible = "fsl,mpc8241", "mpc10x";

fsl,mpc8241-immr would be better; this node describes the internally
memory mapped registers; not the entire soc.

> +
> +               mpic: interrupt-controller@40000 {
> +                       #interrupt-cells = <2>;
> +                       #address-cells = <0>;

Is #address-cells needed?  There are no child nodes.

> +       chosen {
> +               linux,stdout-path = "/soc/serial@4500";

/soc@fc000000/ perhaps?

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Jon Loeliger]

Grant Likely wrote:
> On 1/22/08, Jon Loeliger <jdl@jdl.com> wrote:
>> Based on the Kurobox DTS files.
>>
>> Signed-off-by: Andy Wilcox <andy@protium.com>
>> Signed-off-by: Jon Loeliger <jdl@jdl.com>
> 
> Comments below
> 
>> +
>> +/ {
>> +       model = "StorCenter";
>> +       compatible = "storcenter";
> 
> Be specific!  "iomega,storcenter".  Even better if you put in the model number.

As I mentioned vefore, there is no further model number.
That _is_ the model name.

>> +
>> +       soc@fc000000 {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               device_type = "soc";
> 
> device_type should be dropped (but I know that requires changes to the
> existing mpc82xx support code).

And when the code is fixed, we can fix the DTS too... :-)

>> +               compatible = "fsl,mpc8241", "mpc10x";
> 
> fsl,mpc8241-immr would be better; this node describes the internally
> memory mapped registers; not the entire soc.

Uh, whatever? :-)  'Cuz how many other DTS files say that?

>> +
>> +               mpic: interrupt-controller@40000 {
>> +                       #interrupt-cells = <2>;
>> +                       #address-cells = <0>;
> 
> Is #address-cells needed?  There are no child nodes.

I thought so.  Could be wrong.

>> +       chosen {
>> +               linux,stdout-path = "/soc/serial@4500";
> 
> /soc@fc000000/ perhaps?

Not really necessary to specify the unit number.

jdl

Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Grant Likely]

On 1/22/08, Jon Loeliger <jdl@freescale.com> wrote:
> Grant Likely wrote:
> > On 1/22/08, Jon Loeliger <jdl@jdl.com> wrote:
> >> Based on the Kurobox DTS files.
> >>
> >> Signed-off-by: Andy Wilcox <andy@protium.com>
> >> Signed-off-by: Jon Loeliger <jdl@jdl.com>
> >
> > Comments below
> >
> >> +
> >> +/ {
> >> +       model = "StorCenter";
> >> +       compatible = "storcenter";
> >
> > Be specific!  "iomega,storcenter".  Even better if you put in the model number.
>
> As I mentioned vefore, there is no further model number.
> That _is_ the model name.

/me has a short memory.

> >> +               compatible = "fsl,mpc8241", "mpc10x";
> >
> > fsl,mpc8241-immr would be better; this node describes the internally
> > memory mapped registers; not the entire soc.
>
> Uh, whatever? :-)  'Cuz how many other DTS files say that?

True, we haven't been doing this until recently; but we've also been
stumbling about over the last 2 years figuring out how best to use
this fancy device tree thing in embedded systems effectively.  Best
practices are starting to emerge (wadda you know, the open firmware
recommended practices actually have good thought behind them) and
being specific with the compatible property is one aspect.  I'm moving
the 5200 over to specifying -immr and some of the other soc ports are
doing so also.  Chat with Scott Wood.

> >> +               linux,stdout-path = "/soc/serial@4500";
> >
> > /soc@fc000000/ perhaps?
>
> Not really necessary to specify the unit number.

Okay, I wasn't sure if we could get away with that with the fdt.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Benjamin Herrenschmidt]

> > +
> > +               mpic: interrupt-controller@40000 {
> > +                       #interrupt-cells = <2>;
> > +                       #address-cells = <0>;
> 
> Is #address-cells needed?  There are no child nodes.

It's preferrable for interrupt controllers as #address-cells of the
parent interrupt controller defines the size of the interrupt unit
specifier in the child in the interrupt tree.

I think there's an ongoing argument as to whether the absence of
#address-cells should be the same as #address-cells = 0 in that specific
case but I'm not sure the code does the right thing so let's have it
explicit.

Ben.

Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Scott Wood]

Benjamin Herrenschmidt wrote:
> I think there's an ongoing argument as to whether the absence of
> #address-cells should be the same as #address-cells = 0 in that specific
> case but I'm not sure the code does the right thing so let's have it
> explicit.

I've had no problems leaving it out.

-Scott

Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Josh Boyer]

On Wed, 23 Jan 2008 11:33:16 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> > > +
> > > +               mpic: interrupt-controller@40000 {
> > > +                       #interrupt-cells = <2>;
> > > +                       #address-cells = <0>;
> > 
> > Is #address-cells needed?  There are no child nodes.
> 
> It's preferrable for interrupt controllers as #address-cells of the
> parent interrupt controller defines the size of the interrupt unit
> specifier in the child in the interrupt tree.
> 
> I think there's an ongoing argument as to whether the absence of
> #address-cells should be the same as #address-cells = 0 in that specific
> case but I'm not sure the code does the right thing so let's have it
> explicit.

The code doesn't handle the lack of #address-cells in nodes where
there is an interrupt-map. It pukes if there is no reg property and
addrsize != 0. In the absence of the #address-cells property, it will
try to look at the parent's #address-cells, and if that lacks it, it
will assign 2 which it then merrily decides is incorrect.

josh