[V4,2/5] arm/dt: add very basic dts file for babbage board

Message ID 1299733185-2172-3-git-send-email-jason.hui@linaro.org
State New
Headers show

Commit Message

Jason Hui March 10, 2011, 4:59 a.m.
Signed-off-by: Jason Liu <jason.hui@linaro.org>
Signed-off-by: Jason Liu <r64343@freescale.com>
Singed-off-by: Rob Herring <robherring2@gmail.com>
---
 arch/arm/boot/dts/babbage.dts |  122 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 122 insertions(+), 0 deletions(-)

Comments

Shawn Guo March 11, 2011, 6:56 a.m. | #1
Hi Jason,

On Thu, Mar 10, 2011 at 12:59:42PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <jason.hui@linaro.org>
> Signed-off-by: Jason Liu <r64343@freescale.com>
> Singed-off-by: Rob Herring <robherring2@gmail.com>
> ---
>  arch/arm/boot/dts/babbage.dts |  122 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 122 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/babbage.dts b/arch/arm/boot/dts/babbage.dts
> new file mode 100644
> index 0000000..ab87a1b
> --- /dev/null
> +++ b/arch/arm/boot/dts/babbage.dts
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright 2011 Linaro Ltd.
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +	model = "Freescale i.MX51 Babbage";
> +	compatible = "fsl,mx51-babbage";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	#interrupt-cells = <1>;
> +	interrupt-parent = <&tzic>;
> +
> +	memory {
> +		reg = <0x90000000 0x20000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttymxc0,115200n8 debug earlyprintk ip=dhcp";
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		tzic: tz-interrupt-controller {
> +			#address-cells = <0>;
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0xe0000000 0x1000>;
> +			compatible = "fsl,imx51-tzic";
> +		};
> +	};
> +
> +	clocks {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		uart0_clk: uart0 {
> +			compatible = "clock";
> +			clock-outputs = "imx-uart.0";
> +		};
> +
> +		uart1_clk: uart1 {
> +			compatible = "clock";
> +			clock-outputs = "imx-uart.1";
> +		};
> +
> +		uart2_clk: uart2 {
> +			compatible = "clock";
> +			clock-outputs = "imx-uart.2";
> +		};
> +
> +		fec_clk: fec {
> +			compatible = "clock";
> +			clock-outputs = "fec.0";
> +		};
> +	};
> +
> +	aips@73f00000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges = <0x0 0x73f00000 0x100000>;
> +
> +		imx-uart@bc000 {
> +			compatible = "fsl,imx51-uart";
> +			reg = <0xbc000 0x1000>;
> +			interrupts = <0x1f>;
> +			fsl,has-rts-cts;
> +			uart-clock = <&uart0_clk>, "uart";
> +		};
> +
> +		imx-uart@c0000 {
> +			compatible = "fsl,imx51-uart";
> +			reg = <0xc0000 0x1000>;
> +			interrupts = <0x20>;
> +			fsl,has-rts-cts;
> +			uart-clock = <&uart1_clk>, "uart";
> +		};
> +	};
> +
> +	spba@70000000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges = <0x0 0x70000000 0x100000>;
> +
> +		imx-uart@c000 {
> +			compatible = "fsl,imx51-uart";
> +			reg = <0xc000 0x1000>;
> +			interrupts = <0x21>;
> +			fsl,has-rts-cts;
> +			uart-clock = <&uart2_clk>, "uart";
> +		};
> +	};
> +
Moving spba@70000000 section after aips@73f00000 seems a quick fix to
get console=ttymxc0, but not a right fix to me.

I do not find a real example on mx51, but let's make one, saying
there are two instance of one IP block, xyz1 and xyz2, and xyz1 is on
bus spba@70000000) while xyz2 is on bus apis@73f00000.  Your fix is
broken here, as you need to put spba@70000000 after aips@73f00000 for
uart driver, while xyz driver requires spba@70000000 stays before
aips@73f00000. 

Also this quick fix is working for uart, but will not for some others,
for example, eCSPI and SSI, which requires spba@70000000 even behind
aips@83f00000 with your solution.

> +	aips@83f00000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges = <0x0 0x83f00000 0x100000>;
> +
> +		fec@ec000 {
> +			compatible = "fsl,imx-fec";
> +			reg = <0xec000 0x1000>;
> +			interrupts = <0x57>;
> +			fec_clk-clock = <&fec_clk>, "fec";
> +		};
> +	};
> +};
Jason Hui March 11, 2011, 7:33 a.m. | #2
Hi, Shawn,

On Fri, Mar 11, 2011 at 2:56 PM, Shawn Guo <shawn.guo@freescale.com> wrote:
> Hi Jason,
>
> On Thu, Mar 10, 2011 at 12:59:42PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu <jason.hui@linaro.org>
>> Signed-off-by: Jason Liu <r64343@freescale.com>
>> Singed-off-by: Rob Herring <robherring2@gmail.com>
>> ---
>>  arch/arm/boot/dts/babbage.dts |  122 +++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 122 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/babbage.dts b/arch/arm/boot/dts/babbage.dts
>> new file mode 100644
>> index 0000000..ab87a1b
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/babbage.dts
>> @@ -0,0 +1,122 @@
>> +/*
>> + * Copyright 2011 Linaro Ltd.
>> + * Copyright 2011 Freescale Semiconductor, Inc.
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +/dts-v1/;
>> +
>> +/ {
>> +     model = "Freescale i.MX51 Babbage";
>> +     compatible = "fsl,mx51-babbage";
>> +     #address-cells = <1>;
>> +     #size-cells = <1>;
>> +     #interrupt-cells = <1>;
>> +     interrupt-parent = <&tzic>;
>> +
>> +     memory {
>> +             reg = <0x90000000 0x20000000>;
>> +     };
>> +
>> +     chosen {
>> +             bootargs = "console=ttymxc0,115200n8 debug earlyprintk ip=dhcp";
>> +     };
>> +
>> +     soc {
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             compatible = "simple-bus";
>> +             ranges;
>> +
>> +             tzic: tz-interrupt-controller {
>> +                     #address-cells = <0>;
>> +                     #interrupt-cells = <1>;
>> +                     interrupt-controller;
>> +                     reg = <0xe0000000 0x1000>;
>> +                     compatible = "fsl,imx51-tzic";
>> +             };
>> +     };
>> +
>> +     clocks {
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +
>> +             uart0_clk: uart0 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "imx-uart.0";
>> +             };
>> +
>> +             uart1_clk: uart1 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "imx-uart.1";
>> +             };
>> +
>> +             uart2_clk: uart2 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "imx-uart.2";
>> +             };
>> +
>> +             fec_clk: fec {
>> +                     compatible = "clock";
>> +                     clock-outputs = "fec.0";
>> +             };
>> +     };
>> +
>> +     aips@73f00000 {
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             compatible = "simple-bus";
>> +             ranges = <0x0 0x73f00000 0x100000>;
>> +
>> +             imx-uart@bc000 {
>> +                     compatible = "fsl,imx51-uart";
>> +                     reg = <0xbc000 0x1000>;
>> +                     interrupts = <0x1f>;
>> +                     fsl,has-rts-cts;
>> +                     uart-clock = <&uart0_clk>, "uart";
>> +             };
>> +
>> +             imx-uart@c0000 {
>> +                     compatible = "fsl,imx51-uart";
>> +                     reg = <0xc0000 0x1000>;
>> +                     interrupts = <0x20>;
>> +                     fsl,has-rts-cts;
>> +                     uart-clock = <&uart1_clk>, "uart";
>> +             };
>> +     };
>> +
>> +     spba@70000000 {
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             compatible = "simple-bus";
>> +             ranges = <0x0 0x70000000 0x100000>;
>> +
>> +             imx-uart@c000 {
>> +                     compatible = "fsl,imx51-uart";
>> +                     reg = <0xc000 0x1000>;
>> +                     interrupts = <0x21>;
>> +                     fsl,has-rts-cts;
>> +                     uart-clock = <&uart2_clk>, "uart";
>> +             };
>> +     };
>> +
> Moving spba@70000000 section after aips@73f00000 seems a quick fix to
> get console=ttymxc0, but not a right fix to me.
>
> I do not find a real example on mx51, but let's make one, saying
> there are two instance of one IP block, xyz1 and xyz2, and xyz1 is on
> bus spba@70000000) while xyz2 is on bus apis@73f00000.  Your fix is
> broken here, as you need to put spba@70000000 after aips@73f00000 for
> uart driver, while xyz driver requires spba@70000000 stays before
> aips@73f00000.

No, I don't think so. Where the section is put is not one strict rule,
take a look at
powerpc dts file, you will see that.

>
> Also this quick fix is working for uart, but will not for some others,
> for example, eCSPI and SSI, which requires spba@70000000 even behind
> aips@83f00000 with your solution.

I don't see what's wrong with eCSPI and SSI when put spba behind aips
just like uart.
I will not take your comments.

BR,
Jason
Shawn Guo March 11, 2011, 8:36 a.m. | #3
On Fri, Mar 11, 2011 at 03:33:24PM +0800, Jason Hui wrote:
> Hi, Shawn,
> 
> On Fri, Mar 11, 2011 at 2:56 PM, Shawn Guo <shawn.guo@freescale.com> wrote:
> > Hi Jason,
> >
> > On Thu, Mar 10, 2011 at 12:59:42PM +0800, Jason Liu wrote:
> >> Signed-off-by: Jason Liu <jason.hui@linaro.org>
> >> Signed-off-by: Jason Liu <r64343@freescale.com>
> >> Singed-off-by: Rob Herring <robherring2@gmail.com>
> >> ---
> >>  arch/arm/boot/dts/babbage.dts |  122 +++++++++++++++++++++++++++++++++++++++++
> >>  1 files changed, 122 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/babbage.dts b/arch/arm/boot/dts/babbage.dts
> >> new file mode 100644
> >> index 0000000..ab87a1b
> >> --- /dev/null
> >> +++ b/arch/arm/boot/dts/babbage.dts
> >> @@ -0,0 +1,122 @@
> >> +/*
> >> + * Copyright 2011 Linaro Ltd.
> >> + * Copyright 2011 Freescale Semiconductor, Inc.
> >> + *
> >> + * The code contained herein is licensed under the GNU General Public
> >> + * License. You may obtain a copy of the GNU General Public License
> >> + * Version 2 or later at the following locations:
> >> + *
> >> + * http://www.opensource.org/licenses/gpl-license.html
> >> + * http://www.gnu.org/copyleft/gpl.html
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +/ {
> >> +     model = "Freescale i.MX51 Babbage";
> >> +     compatible = "fsl,mx51-babbage";
> >> +     #address-cells = <1>;
> >> +     #size-cells = <1>;
> >> +     #interrupt-cells = <1>;
> >> +     interrupt-parent = <&tzic>;
> >> +
> >> +     memory {
> >> +             reg = <0x90000000 0x20000000>;
> >> +     };
> >> +
> >> +     chosen {
> >> +             bootargs = "console=ttymxc0,115200n8 debug earlyprintk ip=dhcp";
> >> +     };
> >> +
> >> +     soc {
> >> +             #address-cells = <1>;
> >> +             #size-cells = <1>;
> >> +             compatible = "simple-bus";
> >> +             ranges;
> >> +
> >> +             tzic: tz-interrupt-controller {
> >> +                     #address-cells = <0>;
> >> +                     #interrupt-cells = <1>;
> >> +                     interrupt-controller;
> >> +                     reg = <0xe0000000 0x1000>;
> >> +                     compatible = "fsl,imx51-tzic";
> >> +             };
> >> +     };
> >> +
> >> +     clocks {
> >> +             #address-cells = <1>;
> >> +             #size-cells = <0>;
> >> +
> >> +             uart0_clk: uart0 {
> >> +                     compatible = "clock";
> >> +                     clock-outputs = "imx-uart.0";
> >> +             };
> >> +
> >> +             uart1_clk: uart1 {
> >> +                     compatible = "clock";
> >> +                     clock-outputs = "imx-uart.1";
> >> +             };
> >> +
> >> +             uart2_clk: uart2 {
> >> +                     compatible = "clock";
> >> +                     clock-outputs = "imx-uart.2";
> >> +             };
> >> +
> >> +             fec_clk: fec {
> >> +                     compatible = "clock";
> >> +                     clock-outputs = "fec.0";
> >> +             };
> >> +     };
> >> +
> >> +     aips@73f00000 {
> >> +             #address-cells = <1>;
> >> +             #size-cells = <1>;
> >> +             compatible = "simple-bus";
> >> +             ranges = <0x0 0x73f00000 0x100000>;
> >> +
> >> +             imx-uart@bc000 {
> >> +                     compatible = "fsl,imx51-uart";
> >> +                     reg = <0xbc000 0x1000>;
> >> +                     interrupts = <0x1f>;
> >> +                     fsl,has-rts-cts;
> >> +                     uart-clock = <&uart0_clk>, "uart";
> >> +             };
> >> +
> >> +             imx-uart@c0000 {
> >> +                     compatible = "fsl,imx51-uart";
> >> +                     reg = <0xc0000 0x1000>;
> >> +                     interrupts = <0x20>;
> >> +                     fsl,has-rts-cts;
> >> +                     uart-clock = <&uart1_clk>, "uart";
> >> +             };
> >> +     };
> >> +
> >> +     spba@70000000 {
> >> +             #address-cells = <1>;
> >> +             #size-cells = <1>;
> >> +             compatible = "simple-bus";
> >> +             ranges = <0x0 0x70000000 0x100000>;
> >> +
> >> +             imx-uart@c000 {
> >> +                     compatible = "fsl,imx51-uart";
> >> +                     reg = <0xc000 0x1000>;
> >> +                     interrupts = <0x21>;
> >> +                     fsl,has-rts-cts;
> >> +                     uart-clock = <&uart2_clk>, "uart";
> >> +             };
> >> +     };
> >> +
> > Moving spba@70000000 section after aips@73f00000 seems a quick fix to
> > get console=ttymxc0, but not a right fix to me.
> >
> > I do not find a real example on mx51, but let's make one, saying
> > there are two instance of one IP block, xyz1 and xyz2, and xyz1 is on
> > bus spba@70000000) while xyz2 is on bus apis@73f00000.  Your fix is
> > broken here, as you need to put spba@70000000 after aips@73f00000 for
> > uart driver, while xyz driver requires spba@70000000 stays before
> > aips@73f00000.
> 
> No, I don't think so. Where the section is put is not one strict rule,
> take a look at
> powerpc dts file, you will see that.
> 
Maybe I did not make my point clear.  I would try it one more time.
Let's see this virtual example.

aips@73f00000 {
	[...]

	/* uart1 */
	imx-uart@bc000 {
		[...]
	};

	/* uart 2 */
	imx-uart@c0000 {
		[...]
	};

	/* xyz 2*/
	imx-xyz@offset {
		[...]
	};
};

spba@70000000 {
	[...]

	/* uart 3 */
	imx-uart@c000 {
		[...]
	};

	/* xyz 1 */
	imx-xyz@offset {
		[...]
	};
}

In this case, xyz2 will get probed before xyz1 while driver imx-xyz
assumes that xyz1 is always probed before xyz2, which is the exact
problem that we are trying to resolve with uart.  Though the following
way works, I'm not the fan of it?

spba@70000000 {
	[...]

	/* XYZ 1 */
	imx-xyz@offset {
		[...]
	};
}

aips@73f00000 {
	[...]

	/* UART 1 */
	imx-uart@bc000 {
		[...]
	};

	/* UART 2 */
	imx-uart@c0000 {
		[...]
	};

	/* XYZ 2*/
	imx-xyz@offset {
		[...]
	};
};

spba@70000000 {
	[...]

	/* UART 3 */
	imx-uart@c000 {
		[...]
	};
}

> >
> > Also this quick fix is working for uart, but will not for some others,
> > for example, eCSPI and SSI, which requires spba@70000000 even behind
> > aips@83f00000 with your solution.
> 
> I don't see what's wrong with eCSPI and SSI when put spba behind aips
> just like uart.

Sorry.  Allow me correct the point here.  eCSPI is not the example I
intended to put.  Let's see SSI below.  You do not get the correct
order anyway, either you put spba@70000000 before or after
aips@83f00000, unless you split the aips@83f00000 section into
mulitples, which again I'm not sure if we should go for.

spba@70000000 {
	[...]

	/* SSI 2 */
	imx-ssi@cc000 {
		[...]
	};
}

aips@83f00000 {
	[...]

	/* SSI 1 */
	imx-ssi@cc000 {
		[...]
	};

	/* SSI 3 */
	imx-ssi@e8000 {
		[...]
	};
};

> I will not take your comments.
> 
It does not matter at all whether you take my comments or not.  The
thing matters is if we have the solution working for all the possible
cases.
Jason Hui March 11, 2011, 9:10 a.m. | #4
Hi, Shawn,

On Fri, Mar 11, 2011 at 4:36 PM, Shawn Guo <shawn.guo@freescale.com> wrote:
> On Fri, Mar 11, 2011 at 03:33:24PM +0800, Jason Hui wrote:
>> Hi, Shawn,
[...]
>>
>> No, I don't think so. Where the section is put is not one strict rule,
>> take a look at
>> powerpc dts file, you will see that.
>>
> Maybe I did not make my point clear.  I would try it one more time.
> Let's see this virtual example.
>
[...]
>> I don't see what's wrong with eCSPI and SSI when put spba behind aips
>> just like uart.
>
> Sorry.  Allow me correct the point here.  eCSPI is not the example I
> intended to put.  Let's see SSI below.  You do not get the correct
> order anyway, either you put spba@70000000 before or after
> aips@83f00000, unless you split the aips@83f00000 section into
> mulitples, which again I'm not sure if we should go for.
>
[...]
> It does not matter at all whether you take my comments or not.  The
> thing matters is if we have the solution working for all the possible
> cases.

I know your meanings from the beginning. The case is that,  There always
will be solution for this case. I will adjust it when I met such issue in-deed.
for example when adding SSI support at that time.
I don't want to make things complex here for the basic DT support.

BR,
Jason.

>
> --
> Regards,
> Shawn
>
>

Patch

diff --git a/arch/arm/boot/dts/babbage.dts b/arch/arm/boot/dts/babbage.dts
new file mode 100644
index 0000000..ab87a1b
--- /dev/null
+++ b/arch/arm/boot/dts/babbage.dts
@@ -0,0 +1,122 @@ 
+/*
+ * Copyright 2011 Linaro Ltd.
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+
+/ {
+	model = "Freescale i.MX51 Babbage";
+	compatible = "fsl,mx51-babbage";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	#interrupt-cells = <1>;
+	interrupt-parent = <&tzic>;
+
+	memory {
+		reg = <0x90000000 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttymxc0,115200n8 debug earlyprintk ip=dhcp";
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges;
+
+		tzic: tz-interrupt-controller {
+			#address-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0xe0000000 0x1000>;
+			compatible = "fsl,imx51-tzic";
+		};
+	};
+
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		uart0_clk: uart0 {
+			compatible = "clock";
+			clock-outputs = "imx-uart.0";
+		};
+
+		uart1_clk: uart1 {
+			compatible = "clock";
+			clock-outputs = "imx-uart.1";
+		};
+
+		uart2_clk: uart2 {
+			compatible = "clock";
+			clock-outputs = "imx-uart.2";
+		};
+
+		fec_clk: fec {
+			compatible = "clock";
+			clock-outputs = "fec.0";
+		};
+	};
+
+	aips@73f00000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges = <0x0 0x73f00000 0x100000>;
+
+		imx-uart@bc000 {
+			compatible = "fsl,imx51-uart";
+			reg = <0xbc000 0x1000>;
+			interrupts = <0x1f>;
+			fsl,has-rts-cts;
+			uart-clock = <&uart0_clk>, "uart";
+		};
+
+		imx-uart@c0000 {
+			compatible = "fsl,imx51-uart";
+			reg = <0xc0000 0x1000>;
+			interrupts = <0x20>;
+			fsl,has-rts-cts;
+			uart-clock = <&uart1_clk>, "uart";
+		};
+	};
+
+	spba@70000000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges = <0x0 0x70000000 0x100000>;
+
+		imx-uart@c000 {
+			compatible = "fsl,imx51-uart";
+			reg = <0xc000 0x1000>;
+			interrupts = <0x21>;
+			fsl,has-rts-cts;
+			uart-clock = <&uart2_clk>, "uart";
+		};
+	};
+
+	aips@83f00000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges = <0x0 0x83f00000 0x100000>;
+
+		fec@ec000 {
+			compatible = "fsl,imx-fec";
+			reg = <0xec000 0x1000>;
+			interrupts = <0x57>;
+			fec_clk-clock = <&fec_clk>, "fec";
+		};
+	};
+};