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

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

Commit Message

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

Comments

Grant Likely March 17, 2011, 4:42 p.m. | #1
On Wed, Mar 16, 2011 at 11:51:39PM +0800, Jason Liu wrote:
> Singed-off-by: Rob Herring <robherring2@gmail.com>
> Signed-off-by: Jason Liu <jason.hui@linaro.org>
> ---
>  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..8f9b47c
> --- /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";
> +		};

As previously discussed, 'compatible = "clock";' isn't a very good
binding; but I won't say any more on this here since Shawn reworks
this code in his series.

> +	};
> +
> +	aips@73f00000 {

Since aips isn't addressable, and there is no 'reg' property in this
node, the name can simply be 'aips' or 'aips1' (or whatever name is
used in the imx reference manual).  Same goes for the spba node below.

> +		#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,imx51-fec";
> +			reg = <0xec000 0x1000>;
> +			interrupts = <0x57>;
> +			fec_clk-clock = <&fec_clk>, "fec";

Unfortunately we're leaking Linux implementation details here by
needing to use the name "fec_clk".  This will require some more
thought on the best way to handle (but I'm not asking you to change
anything yet).

g.
Shawn Guo March 18, 2011, 1:49 a.m. | #2
On Thu, Mar 17, 2011 at 10:42:38AM -0600, Grant Likely wrote:
> On Wed, Mar 16, 2011 at 11:51:39PM +0800, Jason Liu wrote:
[...]
> > +	aips@83f00000 {
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		compatible = "simple-bus";
> > +		ranges = <0x0 0x83f00000 0x100000>;
> > +
> > +		fec@ec000 {
> > +			compatible = "fsl,imx51-fec";
> > +			reg = <0xec000 0x1000>;
> > +			interrupts = <0x57>;
> > +			fec_clk-clock = <&fec_clk>, "fec";
> 
> Unfortunately we're leaking Linux implementation details here by
> needing to use the name "fec_clk".  This will require some more
> thought on the best way to handle (but I'm not asking you to change
> anything yet).
> 
This constraint comes from function of_clk_get in drivers/of/clock.c:

struct clk *of_clk_get(struct device *dev, const char *id)
{
	[...]
        dev_dbg(dev, "Looking up %s-clock from device tree\n", id);

        snprintf(prop_name, 32, "%s-clock", id ? id : "bus");
        prop = of_get_property(dev->of_node, prop_name, &sz);
	[...]
}

The 'id' here is clk_lookup->con_id.  If we choose to use some fixed
prop name here, the name leaking Linux implementation like 'fec_clk'
will not need to be there.

What about fixing the name as 'bus-clock' used by the current
implementation, or 'module-clock', or anything you can think of
better?
Grant Likely March 18, 2011, 5:43 a.m. | #3
On Fri, Mar 18, 2011 at 09:49:17AM +0800, Shawn Guo wrote:
> On Thu, Mar 17, 2011 at 10:42:38AM -0600, Grant Likely wrote:
> > On Wed, Mar 16, 2011 at 11:51:39PM +0800, Jason Liu wrote:
> [...]
> > > +	aips@83f00000 {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <1>;
> > > +		compatible = "simple-bus";
> > > +		ranges = <0x0 0x83f00000 0x100000>;
> > > +
> > > +		fec@ec000 {
> > > +			compatible = "fsl,imx51-fec";
> > > +			reg = <0xec000 0x1000>;
> > > +			interrupts = <0x57>;
> > > +			fec_clk-clock = <&fec_clk>, "fec";
> > 
> > Unfortunately we're leaking Linux implementation details here by
> > needing to use the name "fec_clk".  This will require some more
> > thought on the best way to handle (but I'm not asking you to change
> > anything yet).
> > 
> This constraint comes from function of_clk_get in drivers/of/clock.c:
> 
> struct clk *of_clk_get(struct device *dev, const char *id)
> {
> 	[...]
>         dev_dbg(dev, "Looking up %s-clock from device tree\n", id);
> 
>         snprintf(prop_name, 32, "%s-clock", id ? id : "bus");
>         prop = of_get_property(dev->of_node, prop_name, &sz);
> 	[...]
> }
> 
> The 'id' here is clk_lookup->con_id.  If we choose to use some fixed
> prop name here, the name leaking Linux implementation like 'fec_clk'
> will not need to be there.
> 
> What about fixing the name as 'bus-clock' used by the current
> implementation, or 'module-clock', or anything you can think of
> better?

Yeah, I though about that, but I'm being very careful about hard
coding anything in the core DT code because every platform seems to
want something different here, or want a different set of clocks.  I
don't have a good solution for this yet.

g.
Shawn Guo March 18, 2011, 7:54 a.m. | #4
On Thu, Mar 17, 2011 at 11:43:34PM -0600, Grant Likely wrote:
> On Fri, Mar 18, 2011 at 09:49:17AM +0800, Shawn Guo wrote:
> > On Thu, Mar 17, 2011 at 10:42:38AM -0600, Grant Likely wrote:
> > > On Wed, Mar 16, 2011 at 11:51:39PM +0800, Jason Liu wrote:
> > [...]
> > > > +	aips@83f00000 {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <1>;
> > > > +		compatible = "simple-bus";
> > > > +		ranges = <0x0 0x83f00000 0x100000>;
> > > > +
> > > > +		fec@ec000 {
> > > > +			compatible = "fsl,imx51-fec";
> > > > +			reg = <0xec000 0x1000>;
> > > > +			interrupts = <0x57>;
> > > > +			fec_clk-clock = <&fec_clk>, "fec";
> > > 
> > > Unfortunately we're leaking Linux implementation details here by
> > > needing to use the name "fec_clk".  This will require some more
> > > thought on the best way to handle (but I'm not asking you to change
> > > anything yet).
> > > 
> > This constraint comes from function of_clk_get in drivers/of/clock.c:
> > 
> > struct clk *of_clk_get(struct device *dev, const char *id)
> > {
> > 	[...]
> >         dev_dbg(dev, "Looking up %s-clock from device tree\n", id);
> > 
> >         snprintf(prop_name, 32, "%s-clock", id ? id : "bus");
> >         prop = of_get_property(dev->of_node, prop_name, &sz);
> > 	[...]
> > }
> > 
> > The 'id' here is clk_lookup->con_id.  If we choose to use some fixed
> > prop name here, the name leaking Linux implementation like 'fec_clk'
> > will not need to be there.
> > 
> > What about fixing the name as 'bus-clock' used by the current
> > implementation, or 'module-clock', or anything you can think of
> > better?
> 
> Yeah, I though about that, but I'm being very careful about hard
> coding anything in the core DT code because every platform seems to
> want something different here, or want a different set of clocks.  I
> don't have a good solution for this yet.
> 
We are not hard coding anything but a property name here.  We are hard
coding property name everywhere in dt code, aren't we?
Grant Likely March 18, 2011, 4:02 p.m. | #5
On Fri, Mar 18, 2011 at 03:54:32PM +0800, Shawn Guo wrote:
> On Thu, Mar 17, 2011 at 11:43:34PM -0600, Grant Likely wrote:
> > On Fri, Mar 18, 2011 at 09:49:17AM +0800, Shawn Guo wrote:
> > > On Thu, Mar 17, 2011 at 10:42:38AM -0600, Grant Likely wrote:
> > > > On Wed, Mar 16, 2011 at 11:51:39PM +0800, Jason Liu wrote:
> > > [...]
> > > > > +	aips@83f00000 {
> > > > > +		#address-cells = <1>;
> > > > > +		#size-cells = <1>;
> > > > > +		compatible = "simple-bus";
> > > > > +		ranges = <0x0 0x83f00000 0x100000>;
> > > > > +
> > > > > +		fec@ec000 {
> > > > > +			compatible = "fsl,imx51-fec";
> > > > > +			reg = <0xec000 0x1000>;
> > > > > +			interrupts = <0x57>;
> > > > > +			fec_clk-clock = <&fec_clk>, "fec";
> > > > 
> > > > Unfortunately we're leaking Linux implementation details here by
> > > > needing to use the name "fec_clk".  This will require some more
> > > > thought on the best way to handle (but I'm not asking you to change
> > > > anything yet).
> > > > 
> > > This constraint comes from function of_clk_get in drivers/of/clock.c:
> > > 
> > > struct clk *of_clk_get(struct device *dev, const char *id)
> > > {
> > > 	[...]
> > >         dev_dbg(dev, "Looking up %s-clock from device tree\n", id);
> > > 
> > >         snprintf(prop_name, 32, "%s-clock", id ? id : "bus");
> > >         prop = of_get_property(dev->of_node, prop_name, &sz);
> > > 	[...]
> > > }
> > > 
> > > The 'id' here is clk_lookup->con_id.  If we choose to use some fixed
> > > prop name here, the name leaking Linux implementation like 'fec_clk'
> > > will not need to be there.
> > > 
> > > What about fixing the name as 'bus-clock' used by the current
> > > implementation, or 'module-clock', or anything you can think of
> > > better?
> > 
> > Yeah, I though about that, but I'm being very careful about hard
> > coding anything in the core DT code because every platform seems to
> > want something different here, or want a different set of clocks.  I
> > don't have a good solution for this yet.
> > 
> We are not hard coding anything but a property name here.  We are hard
> coding property name everywhere in dt code, aren't we?

In this case, each platform seems to have a different naming
conventions for the set of clocks, and a different number of clocks.
I'm not willing to hard code the translations until I've got a better
understanding of the different needs between platforms.

What I might do is allow platform code to supply the core code with a
clock name translation table which might solve the problem nicely.

g.

Patch

diff --git a/arch/arm/boot/dts/babbage.dts b/arch/arm/boot/dts/babbage.dts
new file mode 100644
index 0000000..8f9b47c
--- /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,imx51-fec";
+			reg = <0xec000 0x1000>;
+			interrupts = <0x57>;
+			fec_clk-clock = <&fec_clk>, "fec";
+		};
+	};
+};