[PATCH V2 0/3] Add MX51 basic DT support

[PATCH V2 0/3] Add MX51 basic DT support

[Jason Liu]

From: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

This patchset adds FSL mx51 with device tree support. This
is based on

git://git.secretlab.ca/git/linux-2.6 devicetree/test

This patch has been tested on MX51 babbage board and can
boot up succesfully to linux console with DT enabled.

Jason Liu (3):
  arm/dt: add basic mx51 device tree support
  arm/dt: add very basic dts file for babbage board
  serial/imx: parse from device tree support

 arch/arm/boot/dts/babbage.dts           |  110 +++++++++++++++++++++++++++++++
 arch/arm/mach-mx5/Kconfig               |    8 ++
 arch/arm/mach-mx5/Makefile              |    1 +
 arch/arm/mach-mx5/board-dt.c            |   64 ++++++++++++++++++
 arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++-
 arch/arm/plat-mxc/include/mach/common.h |    1 +
 drivers/tty/serial/imx.c                |   79 +++++++++++++++++++---
 7 files changed, 295 insertions(+), 11 deletions(-)
 create mode 100644 arch/arm/boot/dts/babbage.dts
 create mode 100644 arch/arm/mach-mx5/board-dt.c

[PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Jason Liu]

Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/mach-mx5/Kconfig               |    8 ++++
 arch/arm/mach-mx5/Makefile              |    1 +
 arch/arm/mach-mx5/board-dt.c            |   64 +++++++++++++++++++++++++++++++
 arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++++++++++-
 arch/arm/plat-mxc/include/mach/common.h |    1 +
 5 files changed, 116 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index de4fa99..6438f87 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -47,6 +47,14 @@ config MACH_MX51_BABBAGE
 	  u-boot. This includes specific configurations for the board and its
 	  peripherals.
 
+config MACH_MX51_DT
+	bool "Generic MX51 board (FDT support)"
+	select USE_OF
+	select SOC_IMX51
+	help
+	 Support for generic Freescale i.MX51 boards using Flattened Device
+	 Tree.
+
 config MACH_MX51_3DS
 	bool "Support MX51PDK (3DS)"
 	select SOC_IMX51
diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 0d43be9..540697e 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
 obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
 obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
 obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
+obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c
new file mode 100644
index 0000000..90593f5
--- /dev/null
+++ b/arch/arm/mach-mx5/board-dt.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * 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
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_platform.h>
+#include <linux/of_fdt.h>
+
+#include <mach/common.h>
+#include <mach/hardware.h>
+#include <mach/imx-uart.h>
+#include <mach/iomux-mx51.h>
+
+#include <asm/irq.h>
+#include <asm/setup.h>
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include "devices.h"
+
+static struct of_device_id mx51_dt_match_table[] __initdata = {
+	{ .compatible = "simple-bus", },
+	{}
+};
+
+static void __init mx51_dt_board_init(void)
+{
+	of_platform_bus_probe(NULL, mx51_dt_match_table, NULL);
+}
+
+static void __init mx51_dt_timer_init(void)
+{
+	mx51_clocks_init(32768, 24000000, 22579200, 0);
+	mx5_clk_dt_init();
+}
+
+static struct sys_timer mxc_timer = {
+	.init = mx51_dt_timer_init,
+};
+
+static const char * const mx51_dt_board_compat[] = {
+	"fsl,mx51-babbage",
+	NULL
+};
+
+DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
+	.boot_params  = PHYS_OFFSET + 0x100,
+	.map_io       = mx51_map_io,
+	.init_irq     = mx51_init_irq,
+	.init_machine = mx51_dt_board_init,
+	.dt_compat    = mx51_dt_board_compat,
+	.timer        = &mxc_timer,
+MACHINE_END
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 0a19e75..dedb7f9 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -15,13 +15,19 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/clkdev.h>
-
+#include <linux/err.h>
 #include <asm/div64.h>
 
 #include <mach/hardware.h>
 #include <mach/common.h>
 #include <mach/clock.h>
 
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_clk.h>
+#endif /* CONFIG_OF */
+
 #include "crm_regs.h"
 
 /* External clock values passed-in by the board code */
@@ -1432,3 +1438,38 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
 		MX53_INT_GPT);
 	return 0;
 }
+
+#ifdef CONFIG_OF
+static struct clk *mx5_dt_clk_get(struct device_node *np,
+					const char *output_id, void *data)
+{
+	return data;
+}
+
+static __init void mx5_dt_scan_clks(void)
+{
+	struct device_node *node;
+	struct clk *clk;
+	const char *id;
+	int rc;
+
+	for_each_compatible_node(node, NULL, "clock") {
+		id = of_get_property(node, "clock-outputs", NULL);
+		if (!id)
+			continue;
+
+		clk = clk_get_sys(id, NULL);
+		if (IS_ERR(clk))
+			continue;
+
+		rc = of_clk_add_provider(node, mx5_dt_clk_get, clk);
+		if (rc)
+			pr_err("error adding fixed clk %s\n", node->name);
+	}
+}
+
+void __init mx5_clk_dt_init(void)
+{
+	mx5_dt_scan_clks();
+}
+#endif
diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h
index aea2cd3..a28e84a 100644
--- a/arch/arm/plat-mxc/include/mach/common.h
+++ b/arch/arm/plat-mxc/include/mach/common.h
@@ -58,4 +58,5 @@ extern void mxc91231_arch_reset(int, const char *);
 extern void mxc91231_prepare_idle(void);
 extern void mx51_efikamx_reset(void);
 extern int mx53_revision(void);
+extern void mx5_clk_dt_init(void);
 #endif
-- 
1.7.0.4

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Shawn Guo]

On Mon, Mar 07, 2011 at 09:05:58PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/mach-mx5/Kconfig               |    8 ++++
>  arch/arm/mach-mx5/Makefile              |    1 +
>  arch/arm/mach-mx5/board-dt.c            |   64 +++++++++++++++++++++++++++++++
>  arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++++++++++-
>  arch/arm/plat-mxc/include/mach/common.h |    1 +
>  5 files changed, 116 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
> index de4fa99..6438f87 100644
> --- a/arch/arm/mach-mx5/Kconfig
> +++ b/arch/arm/mach-mx5/Kconfig
> @@ -47,6 +47,14 @@ config MACH_MX51_BABBAGE
>  	  u-boot. This includes specific configurations for the board and its
>  	  peripherals.
>  
> +config MACH_MX51_DT
> +	bool "Generic MX51 board (FDT support)"
> +	select USE_OF
> +	select SOC_IMX51
> +	help
> +	 Support for generic Freescale i.MX51 boards using Flattened Device
> +	 Tree.
> +
>  config MACH_MX51_3DS
>  	bool "Support MX51PDK (3DS)"
>  	select SOC_IMX51
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index 0d43be9..540697e 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
>  obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
>  obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
>  obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
> +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o

If board-dt.c is mx51 specific, would it be sane to name it something
like board-mx51-dt.c?  We have mx53 stuff in this folder as well.

-- 
Regards,
Shawn

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Arnd Bergmann]

On Monday 07 March 2011, Shawn Guo wrote:
> > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> > index 0d43be9..540697e 100644
> > --- a/arch/arm/mach-mx5/Makefile
> > +++ b/arch/arm/mach-mx5/Makefile
> > @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
> >  obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
> >  obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
> >  obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
> > +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
> 
> If board-dt.c is mx51 specific, would it be sane to name it something
> like board-mx51-dt.c?  We have mx53 stuff in this folder as well.
> 

Alternatively, it could be done the other way round: rename the identifiers
in the file from mx51_ to mx5_, and make sure that they don't contain
any mx51 specific settings but always refer to properties in the 
device tree for the differences.

	Arnd

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Shawn Guo]

Hi Arnd,

On Mon, Mar 07, 2011 at 03:37:48PM +0100, Arnd Bergmann wrote:
> On Monday 07 March 2011, Shawn Guo wrote:
> > > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> > > index 0d43be9..540697e 100644
> > > --- a/arch/arm/mach-mx5/Makefile
> > > +++ b/arch/arm/mach-mx5/Makefile
> > > @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
> > >  obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
> > >  obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
> > >  obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
> > > +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
> > 
> > If board-dt.c is mx51 specific, would it be sane to name it something
> > like board-mx51-dt.c?  We have mx53 stuff in this folder as well.
> > 
> 
> Alternatively, it could be done the other way round: rename the identifiers
> in the file from mx51_ to mx5_, and make sure that they don't contain
> any mx51 specific settings but always refer to properties in the 
> device tree for the differences.
> 
So our ultimate goal is to have only one board-dt.c in one mach-xxx?

-- 
Regards,
Shawn

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Arnd Bergmann]

On Monday 07 March 2011, Shawn Guo wrote:
> > Alternatively, it could be done the other way round: rename the identifiers
> > in the file from mx51_ to mx5_, and make sure that they don't contain
> > any mx51 specific settings but always refer to properties in the 
> > device tree for the differences.
> > 
> So our ultimate goal is to have only one board-dt.c in one mach-xxx?

I wouldn't make that a strict rule, but I'd say that one goal should be
to have as few board-dt files as possible without adding complexity.

As a start, we can say we want one file for per set of machines that
can run a common binary kernel, but there may be good reasons for
deviating from that rule on both sides.

	Arnd

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Grant Likely]

On Mon, Mar 7, 2011 at 7:52 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Monday 07 March 2011, Shawn Guo wrote:
>> > Alternatively, it could be done the other way round: rename the identifiers
>> > in the file from mx51_ to mx5_, and make sure that they don't contain
>> > any mx51 specific settings but always refer to properties in the
>> > device tree for the differences.
>> >
>> So our ultimate goal is to have only one board-dt.c in one mach-xxx?
>
> I wouldn't make that a strict rule, but I'd say that one goal should be
> to have as few board-dt files as possible without adding complexity.
>
> As a start, we can say we want one file for per set of machines that
> can run a common binary kernel, but there may be good reasons for
> deviating from that rule on both sides.

+1, I completely agree.

g.

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

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Arnd Bergmann]

On Monday 07 March 2011, Arnd Bergmann wrote:
> On Monday 07 March 2011, Shawn Guo wrote:
> > > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> > > index 0d43be9..540697e 100644
> > > --- a/arch/arm/mach-mx5/Makefile
> > > +++ b/arch/arm/mach-mx5/Makefile
> > > @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
> > >  obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
> > >  obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
> > >  obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
> > > +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
> > 
> > If board-dt.c is mx51 specific, would it be sane to name it something
> > like board-mx51-dt.c?  We have mx53 stuff in this folder as well.
> > 
> 
> Alternatively, it could be done the other way round: rename the identifiers
> in the file from mx51_ to mx5_, and make sure that they don't contain
> any mx51 specific settings but always refer to properties in the 
> device tree for the differences.

Hi Jason,

I saw that your V4 still implements neither Shawn's suggestion nor mine.

	Arnd

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Jason Hui]

Hi, Arnd,

On Thu, Mar 10, 2011 at 8:38 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Monday 07 March 2011, Arnd Bergmann wrote:
>> On Monday 07 March 2011, Shawn Guo wrote:
>> > > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
>> > > index 0d43be9..540697e 100644
>> > > --- a/arch/arm/mach-mx5/Makefile
>> > > +++ b/arch/arm/mach-mx5/Makefile
>> > > @@ -18,3 +18,4 @@ obj-$(CONFIG_MACH_EUKREA_CPUIMX51SD) += board-cpuimx51sd.o
>> > >  obj-$(CONFIG_MACH_EUKREA_MBIMXSD51_BASEBOARD) += eukrea_mbimxsd-baseboard.o
>> > >  obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
>> > >  obj-$(CONFIG_MACH_MX50_RDP) += board-mx50_rdp.o
>> > > +obj-$(CONFIG_MACH_MX51_DT) += board-dt.o
>> >
>> > If board-dt.c is mx51 specific, would it be sane to name it something
>> > like board-mx51-dt.c?  We have mx53 stuff in this folder as well.
>> >
>>
>> Alternatively, it could be done the other way round: rename the identifiers
>> in the file from mx51_ to mx5_, and make sure that they don't contain
>> any mx51 specific settings but always refer to properties in the
>> device tree for the differences.
>
> Hi Jason,
>
> I saw that your V4 still implements neither Shawn's suggestion nor mine.

As you said, I don't want to make thing complex too. I don't want to
have mx51_dt or
mx53_dt as Shawn Suggests and I still don't want to make it change to
mx5 now as you suggest
Since this patch only support mx51 currently. I think it's easy to
make change later once mx53 in-deed
added in DT support, what do you think?


>
>        Arnd
>

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Arnd Bergmann]

On Friday 11 March 2011, Jason Hui wrote:
> >> Alternatively, it could be done the other way round: rename the identifiers
> >> in the file from mx51_ to mx5_, and make sure that they don't contain
> >> any mx51 specific settings but always refer to properties in the
> >> device tree for the differences.
> >
> > I saw that your V4 still implements neither Shawn's suggestion nor mine.
> 
> As you said, I don't want to make thing complex too. I don't want to
> have mx51_dt or
> mx53_dt as Shawn Suggests and I still don't want to make it change to
> mx5 now as you suggest
> Since this patch only support mx51 currently. I think it's easy to
> make change later once mx53 in-deed
> added in DT support, what do you think?

It would also be easy to rename the file from mx51_dt to board_dt
when it becomes more generic. I don't consider it a show-stopper
though and am comfortable with leaving it to your own judgement.

Generally speaking, you don't have to do everything that reviewers
suggest, but please reply to explain your reasons if you disagree.

	Arnd

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Jason Hui]

Hi, Arnd,

On Fri, Mar 11, 2011 at 8:10 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Friday 11 March 2011, Jason Hui wrote:
>> >> Alternatively, it could be done the other way round: rename the identifiers
>> >> in the file from mx51_ to mx5_, and make sure that they don't contain
>> >> any mx51 specific settings but always refer to properties in the
>> >> device tree for the differences.
>> >
>> > I saw that your V4 still implements neither Shawn's suggestion nor mine.
>>
>> As you said, I don't want to make thing complex too. I don't want to
>> have mx51_dt or
>> mx53_dt as Shawn Suggests and I still don't want to make it change to
>> mx5 now as you suggest
>> Since this patch only support mx51 currently. I think it's easy to
>> make change later once mx53 in-deed
>> added in DT support, what do you think?
>
> It would also be easy to rename the file from mx51_dt to board_dt
> when it becomes more generic. I don't consider it a show-stopper
> though and am comfortable with leaving it to your own judgement.

Thanks for the comments. I will keep as it's now.

>
> Generally speaking, you don't have to do everything that reviewers
> suggest, but please reply to explain your reasons if you disagree.

Yes, correct. :)

>
>        Arnd
>

Re: [PATCH V2 1/3] arm/dt: add basic mx51 device tree support

[Shawn Guo]

On Mon, Mar 07, 2011 at 09:05:58PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
[...]
> diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c
> new file mode 100644
> index 0000000..90593f5
> --- /dev/null
> +++ b/arch/arm/mach-mx5/board-dt.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * 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
> + */
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_fdt.h>
> +
> +#include <mach/common.h>
> +#include <mach/hardware.h>
> +#include <mach/imx-uart.h>
> +#include <mach/iomux-mx51.h>
> +
> +#include <asm/irq.h>
> +#include <asm/setup.h>
> +#include <asm/mach-types.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/time.h>
> +
> +#include "devices.h"
> +
> +static struct of_device_id mx51_dt_match_table[] __initdata = {
> +	{ .compatible = "simple-bus", },
> +	{}
> +};
> +
> +static void __init mx51_dt_board_init(void)
> +{
> +	of_platform_bus_probe(NULL, mx51_dt_match_table, NULL);
> +}
> +
> +static void __init mx51_dt_timer_init(void)
> +{
> +	mx51_clocks_init(32768, 24000000, 22579200, 0);
> +	mx5_clk_dt_init();
> +}
> +
> +static struct sys_timer mxc_timer = {
> +	.init = mx51_dt_timer_init,
> +};
> +
> +static const char * const mx51_dt_board_compat[] = {

The extra 'const' is introducing the warning below.

arch/arm/mach-mx5/board-dt.c:69: warning: initialization discards qualifiers from pointer target type

> +	"fsl,mx51-babbage",
> +	NULL
> +};
> +
> +DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
> +	.boot_params  = PHYS_OFFSET + 0x100,
> +	.map_io       = mx51_map_io,
> +	.init_irq     = mx51_init_irq,
> +	.init_machine = mx51_dt_board_init,
> +	.dt_compat    = mx51_dt_board_compat,
> +	.timer        = &mxc_timer,
> +MACHINE_END

-- 
Regards,
Shawn

[PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Jason Liu]

Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Singed-off-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/babbage.dts |  110 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 110 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..46a3071
--- /dev/null
+++ b/arch/arm/boot/dts/babbage.dts
@@ -0,0 +1,110 @@
+/dts-v1/;
+
+/ {
+	model = "Freescale i.MX51 Babbage";
+	compatible = "fsl,mx51-babbage";
+	#address-cells = <0x1>;
+	#size-cells = <0x1>;
+	#interrupt-cells = <0x1>;
+	interrupt-parent = <&tzic>;
+
+	memory {
+		reg = <0x90000000 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttymxc1,115200n8 debug earlyprintk";
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges;
+
+		tzic: tzic@0 {
+			#address-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0xe0000000 0x1000>;
+			compatible = "fsl,imx51-tzic";
+		};
+	};
+
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		uart0_clk: uart@0 {
+			compatible = "clock";
+			clock-outputs = "imx-uart.0";
+		};
+
+		uart1_clk: uart@1 {
+			compatible = "clock";
+			clock-outputs = "imx-uart.1";
+		};
+
+		uart2_clk: uart@2 {
+			compatible = "clock";
+			clock-outputs = "imx-uart.2";
+		};
+
+		fec_clk: fec@0 {
+			compatible = "clock";
+			clock-outputs = "fec.0";
+		};
+	};
+
+	spba@70000000 {
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+		compatible = "simple-bus";
+		ranges = <0x0 0x70000000 0x100000>;
+
+		imx-uart@c000 {
+			compatible = "imx-uart";
+			reg = <0xc000 0x1000>;
+			interrupts = <0x21>;
+			rts-cts;
+			uart-clock = <&uart2_clk>, "uart";
+		};
+	};
+
+	aips@73f00000 {
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+		compatible = "simple-bus";
+		ranges = <0x0 0x73f00000 0x100000>;
+
+		imx-uart@bc000 {
+			compatible = "imx-uart";
+			reg = <0xbc000 0x1000>;
+			interrupts = <0x1f>;
+			rts-cts;
+			uart-clock = <&uart0_clk>, "uart";
+		};
+
+		imx-uart@c0000 {
+			compatible = "imx-uart";
+			reg = <0xc0000 0x1000>;
+			interrupts = <0x20>;
+			rts-cts;
+			uart-clock = <&uart1_clk>, "uart";
+		};
+	};
+
+	aips@83f00000 {
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+		compatible = "simple-bus";
+		ranges = <0x0 0x83f00000 0x100000>;
+
+		fec@ec000 {
+			compatible = "fec";
+			reg = <0xec000 0x1000>;
+			interrupts = <0x57>;
+			fec_clk-clock = <&fec_clk>, "fec";
+		};
+	};
+};
-- 
1.7.0.4

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Shawn Guo]

Hi Jason,

On Mon, Mar 07, 2011 at 09:05:59PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Singed-off-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/babbage.dts |  110 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 110 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..46a3071
> --- /dev/null
> +++ b/arch/arm/boot/dts/babbage.dts
> @@ -0,0 +1,110 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "Freescale i.MX51 Babbage";
> +	compatible = "fsl,mx51-babbage";
> +	#address-cells = <0x1>;
> +	#size-cells = <0x1>;
> +	#interrupt-cells = <0x1>;
> +	interrupt-parent = <&tzic>;
> +
> +	memory {
> +		reg = <0x90000000 0x20000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttymxc1,115200n8 debug earlyprintk";

So you do not fix it.  You still put ttymxc1 here while ttymxc0 port
is being used.

> +	};
> +

-- 
Regards,
Shawn

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Shawn Guo]

Aha, Jason.  Grant and I put many comments on v1 of this patch, but it
seems that v2 is identical to v1, except adding Rob Herring sign-off.

On Mon, Mar 07, 2011 at 09:05:59PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Singed-off-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/babbage.dts |  110 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 110 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..46a3071
> --- /dev/null
> +++ b/arch/arm/boot/dts/babbage.dts
> @@ -0,0 +1,110 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "Freescale i.MX51 Babbage";
> +	compatible = "fsl,mx51-babbage";
> +	#address-cells = <0x1>;
> +	#size-cells = <0x1>;
> +	#interrupt-cells = <0x1>;
> +	interrupt-parent = <&tzic>;
> +
> +	memory {
> +		reg = <0x90000000 0x20000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttymxc1,115200n8 debug earlyprintk";
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		tzic: tzic@0 {
> +			#address-cells = <0>;
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0xe0000000 0x1000>;
> +			compatible = "fsl,imx51-tzic";
> +		};
> +	};
> +
> +	clocks {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		uart0_clk: uart@0 {
> +			compatible = "clock";
> +			clock-outputs = "imx-uart.0";
> +		};
> +
> +		uart1_clk: uart@1 {
> +			compatible = "clock";
> +			clock-outputs = "imx-uart.1";
> +		};
> +
> +		uart2_clk: uart@2 {
> +			compatible = "clock";
> +			clock-outputs = "imx-uart.2";
> +		};
> +
> +		fec_clk: fec@0 {
> +			compatible = "clock";
> +			clock-outputs = "fec.0";
> +		};
> +	};
> +
> +	spba@70000000 {
> +		#address-cells = <0x1>;
> +		#size-cells = <0x1>;
> +		compatible = "simple-bus";
> +		ranges = <0x0 0x70000000 0x100000>;
> +
> +		imx-uart@c000 {
> +			compatible = "imx-uart";
> +			reg = <0xc000 0x1000>;
> +			interrupts = <0x21>;
> +			rts-cts;
> +			uart-clock = <&uart2_clk>, "uart";
> +		};
> +	};
> +
> +	aips@73f00000 {
> +		#address-cells = <0x1>;
> +		#size-cells = <0x1>;
> +		compatible = "simple-bus";
> +		ranges = <0x0 0x73f00000 0x100000>;
> +
> +		imx-uart@bc000 {
> +			compatible = "imx-uart";
> +			reg = <0xbc000 0x1000>;
> +			interrupts = <0x1f>;
> +			rts-cts;
> +			uart-clock = <&uart0_clk>, "uart";
> +		};
> +
> +		imx-uart@c0000 {
> +			compatible = "imx-uart";
> +			reg = <0xc0000 0x1000>;
> +			interrupts = <0x20>;
> +			rts-cts;
> +			uart-clock = <&uart1_clk>, "uart";
> +		};
> +	};
> +
> +	aips@83f00000 {
> +		#address-cells = <0x1>;
> +		#size-cells = <0x1>;
> +		compatible = "simple-bus";
> +		ranges = <0x0 0x83f00000 0x100000>;
> +
> +		fec@ec000 {
> +			compatible = "fec";
> +			reg = <0xec000 0x1000>;
> +			interrupts = <0x57>;
> +			fec_clk-clock = <&fec_clk>, "fec";
> +		};
> +	};
> +};
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
> 

-- 
Regards,
Shawn

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Jason Hui]

Hi, Shawn,

On Mon, Mar 7, 2011 at 10:51 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> Aha, Jason.  Grant and I put many comments on v1 of this patch, but it
> seems that v2 is identical to v1, except adding Rob Herring sign-off.

Please compare with v1 and say it once again, It should include the
grant and your comments except the mxctty1
due to I think that it's not one issue, So, I did not take your comments.

But I observed that some comments as Grant suggest to use decimal not
hex value for #address-cells and #size-cells,
not applied to this file globally, I will fix it in the V3 patch if need.

>
> On Mon, Mar 07, 2011 at 09:05:59PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Singed-off-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  arch/arm/boot/dts/babbage.dts |  110 +++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 110 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..46a3071
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/babbage.dts
>> @@ -0,0 +1,110 @@
>> +/dts-v1/;
>> +
>> +/ {
>> +     model = "Freescale i.MX51 Babbage";
>> +     compatible = "fsl,mx51-babbage";
>> +     #address-cells = <0x1>;
>> +     #size-cells = <0x1>;
>> +     #interrupt-cells = <0x1>;
>> +     interrupt-parent = <&tzic>;
>> +
>> +     memory {
>> +             reg = <0x90000000 0x20000000>;
>> +     };
>> +
>> +     chosen {
>> +             bootargs = "console=ttymxc1,115200n8 debug earlyprintk";
>> +     };
>> +
>> +     soc {
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             compatible = "simple-bus";
>> +             ranges;
>> +
>> +             tzic: tzic@0 {
>> +                     #address-cells = <0>;
>> +                     #interrupt-cells = <1>;
>> +                     interrupt-controller;
>> +                     reg = <0xe0000000 0x1000>;
>> +                     compatible = "fsl,imx51-tzic";
>> +             };
>> +     };
>> +
>> +     clocks {
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +
>> +             uart0_clk: uart@0 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "imx-uart.0";
>> +             };
>> +
>> +             uart1_clk: uart@1 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "imx-uart.1";
>> +             };
>> +
>> +             uart2_clk: uart@2 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "imx-uart.2";
>> +             };
>> +
>> +             fec_clk: fec@0 {
>> +                     compatible = "clock";
>> +                     clock-outputs = "fec.0";
>> +             };
>> +     };
>> +
>> +     spba@70000000 {
>> +             #address-cells = <0x1>;
>> +             #size-cells = <0x1>;
>> +             compatible = "simple-bus";
>> +             ranges = <0x0 0x70000000 0x100000>;
>> +
>> +             imx-uart@c000 {
>> +                     compatible = "imx-uart";
>> +                     reg = <0xc000 0x1000>;
>> +                     interrupts = <0x21>;
>> +                     rts-cts;
>> +                     uart-clock = <&uart2_clk>, "uart";
>> +             };
>> +     };
>> +
>> +     aips@73f00000 {
>> +             #address-cells = <0x1>;
>> +             #size-cells = <0x1>;
>> +             compatible = "simple-bus";
>> +             ranges = <0x0 0x73f00000 0x100000>;
>> +
>> +             imx-uart@bc000 {
>> +                     compatible = "imx-uart";
>> +                     reg = <0xbc000 0x1000>;
>> +                     interrupts = <0x1f>;
>> +                     rts-cts;
>> +                     uart-clock = <&uart0_clk>, "uart";
>> +             };
>> +
>> +             imx-uart@c0000 {
>> +                     compatible = "imx-uart";
>> +                     reg = <0xc0000 0x1000>;
>> +                     interrupts = <0x20>;
>> +                     rts-cts;
>> +                     uart-clock = <&uart1_clk>, "uart";
>> +             };
>> +     };
>> +
>> +     aips@83f00000 {
>> +             #address-cells = <0x1>;
>> +             #size-cells = <0x1>;
>> +             compatible = "simple-bus";
>> +             ranges = <0x0 0x83f00000 0x100000>;
>> +
>> +             fec@ec000 {
>> +                     compatible = "fec";
>> +                     reg = <0xec000 0x1000>;
>> +                     interrupts = <0x57>;
>> +                     fec_clk-clock = <&fec_clk>, "fec";
>> +             };
>> +     };
>> +};
>> --
>> 1.7.0.4
>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>>
>
> --
> Regards,
> Shawn
>
>

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Shawn Guo]

On Tue, Mar 08, 2011 at 10:20:33AM +0800, Jason Hui wrote:
> Hi, Shawn,
> 
> On Mon, Mar 7, 2011 at 10:51 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > Aha, Jason.  Grant and I put many comments on v1 of this patch, but it
> > seems that v2 is identical to v1, except adding Rob Herring sign-off.
> 
> Please compare with v1 and say it once again, It should include the

Oops!  Sorry, I messed up with my local branches last night, probably
because it was late :)

> grant and your comments except the mxctty1
> due to I think that it's not one issue, So, I did not take your comments.
> 
I still think it's an issue that the dt kernel needs a different
console cmdline from the non-dt one.

> But I observed that some comments as Grant suggest to use decimal not
> hex value for #address-cells and #size-cells,
> not applied to this file globally, I will fix it in the V3 patch if need.
> 
> >
> > On Mon, Mar 07, 2011 at 09:05:59PM +0800, Jason Liu wrote:
> >> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> >> Singed-off-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  arch/arm/boot/dts/babbage.dts |  110 +++++++++++++++++++++++++++++++++++++++++
> >>  1 files changed, 110 insertions(+), 0 deletions(-)

-- 
Regards,
Shawn

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Jason Hui]

Hi, Shawn,

On Tue, Mar 8, 2011 at 11:11 AM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> On Tue, Mar 08, 2011 at 10:20:33AM +0800, Jason Hui wrote:
>> Hi, Shawn,
>>
>> On Mon, Mar 7, 2011 at 10:51 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> > Aha, Jason.  Grant and I put many comments on v1 of this patch, but it
>> > seems that v2 is identical to v1, except adding Rob Herring sign-off.
>>
>> Please compare with v1 and say it once again, It should include the
>
> Oops!  Sorry, I messed up with my local branches last night, probably
> because it was late :)
>
>> grant and your comments except the mxctty1
>> due to I think that it's not one issue, So, I did not take your comments.
>>
> I still think it's an issue that the dt kernel needs a different
> console cmdline from the non-dt one.

In fact, this is user defined, I think But why we need mxctty1 in dt
here, the reason is that,

uart3 will be probed first and then uart0, uart1, which lead to
mxctty0 ---> uart3,

mxctty1---> uart0 and mxctty2 --->uart1.

The debug uart of mx51 babbage board is uart0, so we need define
mxctty1 in the kernel command line.

We can put spba@70000000 at the back of aips@73f00000, but it will
make the memory address not sorted
from lower to higher.


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>;
                        rts-cts;
                        uart-clock = <&uart2_clk>, "uart";
                };
        };

        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 = "imx-uart";
                        reg = <0xc0000 0x1000>;
                        interrupts = <0x20>;
                        fsl,has-rts-cts;
                        uart-clock = <&uart1_clk>, "uart";
                };


>
>> But I observed that some comments as Grant suggest to use decimal not
>> hex value for #address-cells and #size-cells,
>> not applied to this file globally, I will fix it in the V3 patch if need.
>>
>> >
>> > On Mon, Mar 07, 2011 at 09:05:59PM +0800, Jason Liu wrote:
>> >> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> >> Singed-off-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> ---
>> >>  arch/arm/boot/dts/babbage.dts |  110 +++++++++++++++++++++++++++++++++++++++++
>> >>  1 files changed, 110 insertions(+), 0 deletions(-)
>
> --
> Regards,
> Shawn
>
>

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Shawn Guo]

On Tue, Mar 08, 2011 at 11:31:07AM +0800, Jason Hui wrote:
> Hi, Shawn,
> 
> On Tue, Mar 8, 2011 at 11:11 AM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > On Tue, Mar 08, 2011 at 10:20:33AM +0800, Jason Hui wrote:
> >> Hi, Shawn,
> >>
> >> On Mon, Mar 7, 2011 at 10:51 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> >> > Aha, Jason.  Grant and I put many comments on v1 of this patch, but it
> >> > seems that v2 is identical to v1, except adding Rob Herring sign-off.
> >>
> >> Please compare with v1 and say it once again, It should include the
> >
> > Oops!  Sorry, I messed up with my local branches last night, probably
> > because it was late :)
> >
> >> grant and your comments except the mxctty1
> >> due to I think that it's not one issue, So, I did not take your comments.
> >>
> > I still think it's an issue that the dt kernel needs a different
> > console cmdline from the non-dt one.
> 
> In fact, this is user defined, I think But why we need mxctty1 in dt
> here, the reason is that,
> 
> uart3 will be probed first and then uart0, uart1, which lead to
> mxctty0 ---> uart3,
> 
> mxctty1---> uart0 and mxctty2 --->uart1.
> 
> The debug uart of mx51 babbage board is uart0, so we need define
> mxctty1 in the kernel command line.
> 
> We can put spba@70000000 at the back of aips@73f00000, but it will
> make the memory address not sorted
> from lower to higher.
> 
> 
> 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>;
>                         rts-cts;
>                         uart-clock = <&uart2_clk>, "uart";
>                 };
>         };
> 
>         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 = "imx-uart";
>                         reg = <0xc0000 0x1000>;
>                         interrupts = <0x20>;
>                         fsl,has-rts-cts;
>                         uart-clock = <&uart1_clk>, "uart";
>                 };
> 
> 
So we probably need one more number in 'reg' to tell the port id?

-- 
Regards,
Shawn

Re: [PATCH V2 2/3] arm/dt: add very basic dts file for babbage board

[Grant Likely]

On Tue, Mar 08, 2011 at 11:46:47AM +0800, Shawn Guo wrote:
> On Tue, Mar 08, 2011 at 11:31:07AM +0800, Jason Hui wrote:
> > Hi, Shawn,
> > 
> > On Tue, Mar 8, 2011 at 11:11 AM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > > On Tue, Mar 08, 2011 at 10:20:33AM +0800, Jason Hui wrote:
> > >> Hi, Shawn,
> > >>
> > >> On Mon, Mar 7, 2011 at 10:51 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > >> > Aha, Jason.  Grant and I put many comments on v1 of this patch, but it
> > >> > seems that v2 is identical to v1, except adding Rob Herring sign-off.
> > >>
> > >> Please compare with v1 and say it once again, It should include the
> > >
> > > Oops!  Sorry, I messed up with my local branches last night, probably
> > > because it was late :)
> > >
> > >> grant and your comments except the mxctty1
> > >> due to I think that it's not one issue, So, I did not take your comments.
> > >>
> > > I still think it's an issue that the dt kernel needs a different
> > > console cmdline from the non-dt one.
> > 
> > In fact, this is user defined, I think But why we need mxctty1 in dt
> > here, the reason is that,
> > 
> > uart3 will be probed first and then uart0, uart1, which lead to
> > mxctty0 ---> uart3,
> > 
> > mxctty1---> uart0 and mxctty2 --->uart1.
> > 
> > The debug uart of mx51 babbage board is uart0, so we need define
> > mxctty1 in the kernel command line.
> > 
> > We can put spba@70000000 at the back of aips@73f00000, but it will
> > make the memory address not sorted
> > from lower to higher.
> > 
> So we probably need one more number in 'reg' to tell the port id?

As discussed in my other reply; it is a bad idea to try and encoded
the port id into a device node.  Use /aliases instead.

g.

[PATCH V2 3/3] serial/imx: parse from device tree support

[Jason Liu]

Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/tty/serial/imx.c |   79 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfcf4b1..1ebca28 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -53,6 +53,9 @@
 #include <mach/hardware.h>
 #include <mach/imx-uart.h>
 
+#include <linux/of.h>
+#include <linux/of_address.h>
+
 /* Register definitions */
 #define URXD0 0x0  /* Receiver Register */
 #define URTX0 0x40 /* Transmitter Register */
@@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static int serial_imx_probe_dt(struct imx_port *sport,
+		struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	static int line;
+
+	if (!node)
+		return -ENODEV;
+
+	if (of_get_property(node, "rts-cts", NULL))
+		sport->have_rtscts = 1;
+
+	if (of_get_property(node, "irda", NULL))
+		sport->use_irda = 1;
+
+	sport->port.line = line++;
+
+	return 0;
+}
+#else
+static int serial_imx_probe_dt(struct imx_port *sport,
+		struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif
+
+static int serial_imx_probe_pdata(struct imx_port *sport,
+		struct platform_device *pdev)
+{
+	struct imxuart_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!pdata)
+		return 0;
+
+	if (pdata->flags & IMXUART_HAVE_RTSCTS)
+		sport->have_rtscts = 1;
+
+#ifdef CONFIG_IRDA
+	if (pdata->flags & IMXUART_IRDA)
+		sport->use_irda = 1;
+#endif
+	sport->port.line = pdev->id;
+
+	return 0;
+}
 static int serial_imx_probe(struct platform_device *pdev)
 {
 	struct imx_port *sport;
@@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device *pdev)
 	if (!sport)
 		return -ENOMEM;
 
+	ret = serial_imx_probe_dt(sport, pdev);
+	if (ret == -ENODEV)
+		ret = serial_imx_probe_pdata(sport, pdev);
+	if (ret)
+		goto free;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		ret = -ENODEV;
@@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device *pdev)
 	sport->port.fifosize = 32;
 	sport->port.ops = &imx_pops;
 	sport->port.flags = UPF_BOOT_AUTOCONF;
-	sport->port.line = pdev->id;
 	init_timer(&sport->timer);
 	sport->timer.function = imx_timeout;
 	sport->timer.data     = (unsigned long)sport;
@@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device *pdev)
 
 	sport->port.uartclk = clk_get_rate(sport->clk);
 
-	imx_ports[pdev->id] = sport;
+	if (imx_ports[sport->port.line]) {
+		ret = -EBUSY;
+		goto clkput;
+	}
+	imx_ports[sport->port.line] = sport;
 
 	pdata = pdev->dev.platform_data;
-	if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
-		sport->have_rtscts = 1;
-
-#ifdef CONFIG_IRDA
-	if (pdata && (pdata->flags & IMXUART_IRDA))
-		sport->use_irda = 1;
-#endif
-
 	if (pdata && pdata->init) {
 		ret = pdata->init(pdev);
 		if (ret)
@@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id imx_uart_matches[] = {
+	{ .compatible = "imx-uart" },
+	{},
+};
+
 static struct platform_driver serial_imx_driver = {
 	.probe		= serial_imx_probe,
 	.remove		= serial_imx_remove,
@@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
 	.driver		= {
 		.name	= "imx-uart",
 		.owner	= THIS_MODULE,
+#if defined(CONFIG_OF)
+		.of_match_table = imx_uart_matches,
+#endif
 	},
 };
 
-- 
1.7.0.4

Re: [PATCH V2 3/3] serial/imx: parse from device tree support

[Grant Likely]

On Mon, Mar 7, 2011 at 6:06 AM, Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Hi Jason,

comments below.

> ---
>  drivers/tty/serial/imx.c |   79 ++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 69 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index dfcf4b1..1ebca28 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -53,6 +53,9 @@
>  #include <mach/hardware.h>
>  #include <mach/imx-uart.h>
>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
>  /* Register definitions */
>  #define URXD0 0x0  /* Receiver Register */
>  #define URTX0 0x40 /* Transmitter Register */
> @@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device *dev)
>        return 0;
>  }
>
> +#ifdef CONFIG_OF
> +static int serial_imx_probe_dt(struct imx_port *sport,
> +               struct platform_device *pdev)
> +{
> +       struct device_node *node = pdev->dev.of_node;
> +       static int line;
> +
> +       if (!node)
> +               return -ENODEV;
> +
> +       if (of_get_property(node, "rts-cts", NULL))
> +               sport->have_rtscts = 1;
> +
> +       if (of_get_property(node, "irda", NULL))
> +               sport->use_irda = 1;

These properties need to be documented in
Documentation/devicetree/bindings/serial.  Also, for device specific
properties, it is considered good practice to prefix the property name
with the vendor name; so "fsl,rts-cts" and "fsl,irda".  Also, slightly
more descriptive property names would probably be helpful:
"fsl,has-rts-cts" and "fsl,irda-mode"

> +
> +       sport->port.line = line++;
> +
> +       return 0;
> +}
> +#else
> +static int serial_imx_probe_dt(struct imx_port *sport,
> +               struct platform_device *pdev)
> +{
> +       return -ENODEV;
> +}
> +#endif
> +
> +static int serial_imx_probe_pdata(struct imx_port *sport,
> +               struct platform_device *pdev)
> +{
> +       struct imxuart_platform_data *pdata = pdev->dev.platform_data;
> +
> +       if (!pdata)
> +               return 0;
> +
> +       if (pdata->flags & IMXUART_HAVE_RTSCTS)
> +               sport->have_rtscts = 1;
> +
> +#ifdef CONFIG_IRDA
> +       if (pdata->flags & IMXUART_IRDA)
> +               sport->use_irda = 1;
> +#endif
> +       sport->port.line = pdev->id;
> +
> +       return 0;
> +}
>  static int serial_imx_probe(struct platform_device *pdev)
>  {
>        struct imx_port *sport;
> @@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device *pdev)
>        if (!sport)
>                return -ENOMEM;
>
> +       ret = serial_imx_probe_dt(sport, pdev);
> +       if (ret == -ENODEV)
> +               ret = serial_imx_probe_pdata(sport, pdev);
> +       if (ret)
> +               goto free;
> +
>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>        if (!res) {
>                ret = -ENODEV;
> @@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device *pdev)
>        sport->port.fifosize = 32;
>        sport->port.ops = &imx_pops;
>        sport->port.flags = UPF_BOOT_AUTOCONF;
> -       sport->port.line = pdev->id;
>        init_timer(&sport->timer);
>        sport->timer.function = imx_timeout;
>        sport->timer.data     = (unsigned long)sport;
> @@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device *pdev)
>
>        sport->port.uartclk = clk_get_rate(sport->clk);
>
> -       imx_ports[pdev->id] = sport;
> +       if (imx_ports[sport->port.line]) {
> +               ret = -EBUSY;
> +               goto clkput;
> +       }
> +       imx_ports[sport->port.line] = sport;
>
>        pdata = pdev->dev.platform_data;
> -       if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
> -               sport->have_rtscts = 1;
> -
> -#ifdef CONFIG_IRDA
> -       if (pdata && (pdata->flags & IMXUART_IRDA))
> -               sport->use_irda = 1;
> -#endif
> -
>        if (pdata && pdata->init) {
>                ret = pdata->init(pdev);
>                if (ret)
> @@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device *pdev)
>        return 0;
>  }
>
> +static struct of_device_id imx_uart_matches[] = {
> +       { .compatible = "imx-uart" },

Should be "fsl,imx51-uart".  It is always a good idea to specify the exact part.

> +       {},
> +};
> +
>  static struct platform_driver serial_imx_driver = {
>        .probe          = serial_imx_probe,
>        .remove         = serial_imx_remove,
> @@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
>        .driver         = {
>                .name   = "imx-uart",
>                .owner  = THIS_MODULE,
> +#if defined(CONFIG_OF)
> +               .of_match_table = imx_uart_matches,
> +#endif
>        },
>  };
>
> --
> 1.7.0.4
>
>



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

Re: [PATCH V2 3/3] serial/imx: parse from device tree support

[Jason Hui]

Hi, Grant,

On Tue, Mar 8, 2011 at 12:22 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Mon, Mar 7, 2011 at 6:06 AM, Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>
> Hi Jason,
>
> comments below.
>
>> ---
>>  drivers/tty/serial/imx.c |   79 ++++++++++++++++++++++++++++++++++++++++------
>>  1 files changed, 69 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index dfcf4b1..1ebca28 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -53,6 +53,9 @@
>>  #include <mach/hardware.h>
>>  #include <mach/imx-uart.h>
>>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +
>>  /* Register definitions */
>>  #define URXD0 0x0  /* Receiver Register */
>>  #define URTX0 0x40 /* Transmitter Register */
>> @@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device *dev)
>>        return 0;
>>  }
>>
>> +#ifdef CONFIG_OF
>> +static int serial_imx_probe_dt(struct imx_port *sport,
>> +               struct platform_device *pdev)
>> +{
>> +       struct device_node *node = pdev->dev.of_node;
>> +       static int line;
>> +
>> +       if (!node)
>> +               return -ENODEV;
>> +
>> +       if (of_get_property(node, "rts-cts", NULL))
>> +               sport->have_rtscts = 1;
>> +
>> +       if (of_get_property(node, "irda", NULL))
>> +               sport->use_irda = 1;
>
> These properties need to be documented in
> Documentation/devicetree/bindings/serial.  Also, for device specific
> properties, it is considered good practice to prefix the property name
> with the vendor name; so "fsl,rts-cts" and "fsl,irda".  Also, slightly
> more descriptive property names would probably be helpful:
> "fsl,has-rts-cts" and "fsl,irda-mode"

OK,

>
>> +
>> +       sport->port.line = line++;
>> +
>> +       return 0;
>> +}
>> +#else
>> +static int serial_imx_probe_dt(struct imx_port *sport,
>> +               struct platform_device *pdev)
>> +{
>> +       return -ENODEV;
>> +}
>> +#endif
>> +
>> +static int serial_imx_probe_pdata(struct imx_port *sport,
>> +               struct platform_device *pdev)
>> +{
>> +       struct imxuart_platform_data *pdata = pdev->dev.platform_data;
>> +
>> +       if (!pdata)
>> +               return 0;
>> +
>> +       if (pdata->flags & IMXUART_HAVE_RTSCTS)
>> +               sport->have_rtscts = 1;
>> +
>> +#ifdef CONFIG_IRDA
>> +       if (pdata->flags & IMXUART_IRDA)
>> +               sport->use_irda = 1;
>> +#endif
>> +       sport->port.line = pdev->id;
>> +
>> +       return 0;
>> +}
>>  static int serial_imx_probe(struct platform_device *pdev)
>>  {
>>        struct imx_port *sport;
>> @@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device *pdev)
>>        if (!sport)
>>                return -ENOMEM;
>>
>> +       ret = serial_imx_probe_dt(sport, pdev);
>> +       if (ret == -ENODEV)
>> +               ret = serial_imx_probe_pdata(sport, pdev);
>> +       if (ret)
>> +               goto free;
>> +
>>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>        if (!res) {
>>                ret = -ENODEV;
>> @@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device *pdev)
>>        sport->port.fifosize = 32;
>>        sport->port.ops = &imx_pops;
>>        sport->port.flags = UPF_BOOT_AUTOCONF;
>> -       sport->port.line = pdev->id;
>>        init_timer(&sport->timer);
>>        sport->timer.function = imx_timeout;
>>        sport->timer.data     = (unsigned long)sport;
>> @@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device *pdev)
>>
>>        sport->port.uartclk = clk_get_rate(sport->clk);
>>
>> -       imx_ports[pdev->id] = sport;
>> +       if (imx_ports[sport->port.line]) {
>> +               ret = -EBUSY;
>> +               goto clkput;
>> +       }
>> +       imx_ports[sport->port.line] = sport;
>>
>>        pdata = pdev->dev.platform_data;
>> -       if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
>> -               sport->have_rtscts = 1;
>> -
>> -#ifdef CONFIG_IRDA
>> -       if (pdata && (pdata->flags & IMXUART_IRDA))
>> -               sport->use_irda = 1;
>> -#endif
>> -
>>        if (pdata && pdata->init) {
>>                ret = pdata->init(pdev);
>>                if (ret)
>> @@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device *pdev)
>>        return 0;
>>  }
>>
>> +static struct of_device_id imx_uart_matches[] = {
>> +       { .compatible = "imx-uart" },
>
> Should be "fsl,imx51-uart".  It is always a good idea to specify the exact part.

OK,
>
>> +       {},
>> +};
>> +
>>  static struct platform_driver serial_imx_driver = {
>>        .probe          = serial_imx_probe,
>>        .remove         = serial_imx_remove,
>> @@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
>>        .driver         = {
>>                .name   = "imx-uart",
>>                .owner  = THIS_MODULE,
>> +#if defined(CONFIG_OF)
>> +               .of_match_table = imx_uart_matches,
>> +#endif
>>        },
>>  };
>>
>> --
>> 1.7.0.4
>>
>>
>
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>