[PATCH V4 0/5] Add MX51 basic DT support

[PATCH V4 0/5] Add MX51 basic DT support

[Jason Liu]

From: Jason Liu <Jason Liu jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

This patchset adds Freescale i.mx51 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.

Grant, I think it's almost ready for you to merge it.

V4:
- Add Linaro sign off and copyrightt

V3:
- prefix the property name with the vendor name as like:
   "fsl,has-rts-cts" and "fsl,irda-mode"
- use the ttymxc0 as the console
- add FEC support, nfs can be used now

Jason Liu (5):
  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
  net/fec: check id_entry pointer before using it
  net/fec: add device tree matching support

 arch/arm/boot/dts/babbage.dts           |  122 +++++++++++++++++++++++++++++++
 arch/arm/mach-mx5/Kconfig               |    8 ++
 arch/arm/mach-mx5/Makefile              |    1 +
 arch/arm/mach-mx5/board-dt.c            |   65 ++++++++++++++++
 arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 +++++++++++-
 arch/arm/plat-mxc/include/mach/common.h |    1 +
 drivers/net/fec.c                       |   25 +++++--
 drivers/tty/serial/imx.c                |   79 +++++++++++++++++---
 8 files changed, 327 insertions(+), 17 deletions(-)
 create mode 100644 arch/arm/boot/dts/babbage.dts
 create mode 100644 arch/arm/mach-mx5/board-dt.c

[PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Jason Liu]

Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
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            |   65 +++++++++++++++++++++++++++++++
 arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++++++++++-
 arch/arm/plat-mxc/include/mach/common.h |    1 +
 5 files changed, 117 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..19c60a4
--- /dev/null
+++ b/arch/arm/mach-mx5/board-dt.c
@@ -0,0 +1,65 @@
+/*
+ * 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
+ */
+
+#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 *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.1

Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Grant Likely]

Hi Jason,

Minor comments below.

On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

This looks wrong.  You should only have one s-o-b line.  Use one email
addr or the other.  Not both.

> ---
>  arch/arm/mach-mx5/Kconfig               |    8 ++++
>  arch/arm/mach-mx5/Makefile              |    1 +
>  arch/arm/mach-mx5/board-dt.c            |   65 +++++++++++++++++++++++++++++++
>  arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++++++++++-
>  arch/arm/plat-mxc/include/mach/common.h |    1 +
>  5 files changed, 117 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..19c60a4
> --- /dev/null
> +++ b/arch/arm/mach-mx5/board-dt.c
> @@ -0,0 +1,65 @@
> +/*
> + * 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
> + */
> +
> +#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 *mx51_dt_board_compat[] = {
> +	"fsl,mx51-babbage",
> +	NULL
> +};
> +
> +DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
> +	.boot_params  = PHYS_OFFSET + 0x100,

You should be able to drop the .boot_params line.

> +	.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 */

You can drop the #ifdef CONFIG_OF here.  linux/of*.h is safe to
include when CONFIG_OF is not selected.

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

Nitpick: Would it make sense for these 3 functions to be in a separate .c file?

> 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.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Jason Hui]

Hi, Grant,

On Tue, Mar 15, 2011 at 3:03 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> Hi Jason,
>
> Minor comments below.
>
> On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>
> This looks wrong.  You should only have one s-o-b line.  Use one email
> addr or the other.  Not both.

I just take the same approach as this link: https://lkml.org/lkml/2010/12/17/363
If you think it's not applicable, I can change it.

>
>> ---
>>  arch/arm/mach-mx5/Kconfig               |    8 ++++
>>  arch/arm/mach-mx5/Makefile              |    1 +
>>  arch/arm/mach-mx5/board-dt.c            |   65 +++++++++++++++++++++++++++++++
>>  arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++++++++++-
>>  arch/arm/plat-mxc/include/mach/common.h |    1 +
>>  5 files changed, 117 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..19c60a4
>> --- /dev/null
>> +++ b/arch/arm/mach-mx5/board-dt.c
>> @@ -0,0 +1,65 @@
>> +/*
>> + * 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
>> + */
>> +
>> +#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 *mx51_dt_board_compat[] = {
>> +     "fsl,mx51-babbage",
>> +     NULL
>> +};
>> +
>> +DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
>> +     .boot_params  = PHYS_OFFSET + 0x100,
>
> You should be able to drop the .boot_params line.

OK,

>
>> +     .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 */
>
> You can drop the #ifdef CONFIG_OF here.  linux/of*.h is safe to
> include when CONFIG_OF is not selected.

OK,

>
>> +
>>  #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
>
> Nitpick: Would it make sense for these 3 functions to be in a separate .c file?

I will do it. please check the new patch.

>
>> 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.1
>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>

Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Grant Likely]

On Tue, Mar 15, 2011 at 9:34 PM, Jason Hui <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi, Grant,
>
> On Tue, Mar 15, 2011 at 3:03 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> Hi Jason,
>>
>> Minor comments below.
>>
>> On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
>>> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>
>> This looks wrong.  You should only have one s-o-b line.  Use one email
>> addr or the other.  Not both.
>
> I just take the same approach as this link: https://lkml.org/lkml/2010/12/17/363
> If you think it's not applicable, I can change it.

Yeah, I don't think that's right.  A s-o-b is a personal assertion
that the patch is to the best of your knowledge that you have the
right to submit it for inclusion in the kernel (see section 12 of
Documentation/SubmittingPatches).  It doesn't make any statements
about who owns the copyright on the patch or other issues of corporate
ownership.  Companies may have policies about which email address
employees use when signing off, but that isn't what the s-o-b protocol
is for.

Since there isn't more than one of you, you should only have one s-o-b
line.  :-)

Paul, since your email was presented as evidence, would you care to
offer a counter-argument?  :-)

g.


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

Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Paul E. McKenney]

On Tue, Mar 15, 2011 aFt 11:03:55PM -0600, Grant Likely wrote:
> On Tue, Mar 15, 2011 at 9:34 PM, Jason Hui <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Hi, Grant,
> >
> > On Tue, Mar 15, 2011 at 3:03 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >> Hi Jason,
> >>
> >> Minor comments below.
> >>
> >> On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
> >>> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> >>
> >> This looks wrong.  You should only have one s-o-b line.  Use one email
> >> addr or the other.  Not both.
> >
> > I just take the same approach as this link: https://lkml.org/lkml/2010/12/17/363
> > If you think it's not applicable, I can change it.
> 
> Yeah, I don't think that's right.  A s-o-b is a personal assertion
> that the patch is to the best of your knowledge that you have the
> right to submit it for inclusion in the kernel (see section 12 of
> Documentation/SubmittingPatches).  It doesn't make any statements
> about who owns the copyright on the patch or other issues of corporate
> ownership.  Companies may have policies about which email address
> employees use when signing off, but that isn't what the s-o-b protocol
> is for.
> 
> Since there isn't more than one of you, you should only have one s-o-b
> line.  :-)
> 
> Paul, since your email was presented as evidence, would you care to
> offer a counter-argument?  :-)

How about https://lkml.org/lkml/2011/2/22/668?  ;-)

There is only one of me, but I am acting in two roles.

							Thanx, Paul

Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Shawn Guo]

On Tue, Mar 15, 2011 at 01:03:42AM -0600, Grant Likely wrote:
> Hi Jason,
> 
> Minor comments below.
> 
> On Thu, Mar 10, 2011 at 12:59:41PM +0800, Jason Liu wrote:
> > Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> 
> This looks wrong.  You should only have one s-o-b line.  Use one email
> addr or the other.  Not both.
> 
> > ---
> >  arch/arm/mach-mx5/Kconfig               |    8 ++++
> >  arch/arm/mach-mx5/Makefile              |    1 +
> >  arch/arm/mach-mx5/board-dt.c            |   65 +++++++++++++++++++++++++++++++
> >  arch/arm/mach-mx5/clock-mx51-mx53.c     |   43 ++++++++++++++++++++-
> >  arch/arm/plat-mxc/include/mach/common.h |    1 +
> >  5 files changed, 117 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..19c60a4
> > --- /dev/null
> > +++ b/arch/arm/mach-mx5/board-dt.c
> > @@ -0,0 +1,65 @@
> > +/*
> > + * 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
> > + */
> > +
> > +#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 *mx51_dt_board_compat[] = {
> > +	"fsl,mx51-babbage",
> > +	NULL
> > +};
> > +
> > +DT_MACHINE_START(MX51_DT, "Freescale MX51 (Flattened Device Tree)")
> > +	.boot_params  = PHYS_OFFSET + 0x100,
> 
> You should be able to drop the .boot_params line.
> 
> > +	.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 */
> 
> You can drop the #ifdef CONFIG_OF here.  linux/of*.h is safe to
> include when CONFIG_OF is not selected.
> 
> > +
> >  #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
> 
> Nitpick: Would it make sense for these 3 functions to be in a separate .c file?
> 
Sorry for that I'm late on this.  It will not make much sense to do
so if considering that dynamic dt clock codes have to be put in
clock-mx51-mx53.c anyway, since they are referring to existing
enable/disable/get_rate/set_rate/... functions.

-- 
Regards,
Shawn

Re: [PATCH V4 1/5] arm/dt: add basic mx51 device tree support

[Grant Likely]

On Thu, Mar 17, 2011 at 09:54:35AM +0800, Shawn Guo wrote:
> On Tue, Mar 15, 2011 at 01:03:42AM -0600, Grant Likely wrote:
> > > +#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
> > 
> > Nitpick: Would it make sense for these 3 functions to be in a separate .c file?
> > 
> Sorry for that I'm late on this.  It will not make much sense to do
> so if considering that dynamic dt clock codes have to be put in
> clock-mx51-mx53.c anyway, since they are referring to existing
> enable/disable/get_rate/set_rate/... functions.

Okay.

g.

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

[Jason Liu]

Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
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 |  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";
+		};
+	};
+
+	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";
+		};
+	};
+};
-- 
1.7.1

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

[Shawn Guo]

Hi Jason,

On Thu, Mar 10, 2011 at 12:59:42PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 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 |  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";
> +		};
> +	};
> +};

-- 
Regards,
Shawn

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

[Jason Hui]

Hi, Shawn,

On Fri, Mar 11, 2011 at 2:56 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> Hi Jason,
>
> On Thu, Mar 10, 2011 at 12:59:42PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> 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 |  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

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

[Shawn Guo]

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-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > Hi Jason,
> >
> > On Thu, Mar 10, 2011 at 12:59:42PM +0800, Jason Liu wrote:
> >> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> 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 |  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.

-- 
Regards,
Shawn

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

[Jason Hui]

Hi, Shawn,

On Fri, Mar 11, 2011 at 4:36 PM, Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 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 V4 3/5] serial/imx: parse from device tree support

[Jason Liu]

Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
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..c9483d1 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, "fsl,has-rts-cts", NULL))
+		sport->have_rtscts = 1;
+
+	if (of_get_property(node, "fsl,irda-mode", 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 = "fsl,imx51-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.1

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

[Grant Likely]

Minor comments, but otherwise goes first.

On Thu, Mar 10, 2011 at 12:59:43PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Jeremy's s-o-b should go at the top of the list.  He started it, and
then you took over.  s-o-b lines is the chain of people who have
handled a patch, and so it should reflect the order that they handled
it.

> ---
>  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..c9483d1 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>
> +

These should be mixed in with the rest of the <linux/*.h> includes.

>  /* 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, "fsl,has-rts-cts", NULL))
> +		sport->have_rtscts = 1;
> +
> +	if (of_get_property(node, "fsl,irda-mode", 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 = "fsl,imx51-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.1
> 

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

[Jason Hui]

Hi, Grant,

On Tue, Mar 15, 2011 at 3:07 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> Minor comments, but otherwise goes first.
>
> On Thu, Mar 10, 2011 at 12:59:43PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Signed-off-by: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>
> Jeremy's s-o-b should go at the top of the list.  He started it, and
> then you took over.  s-o-b lines is the chain of people who have
> handled a patch, and so it should reflect the order that they handled
> it.

OK, get it.

>
>> ---
>>  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..c9483d1 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>
>> +
>
> These should be mixed in with the rest of the <linux/*.h> includes.

OK,

>
>>  /* 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, "fsl,has-rts-cts", NULL))
>> +             sport->have_rtscts = 1;
>> +
>> +     if (of_get_property(node, "fsl,irda-mode", 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 = "fsl,imx51-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.1
>>
>

[PATCH V4 4/5] net/fec: check id_entry pointer before using it

[Jason Liu]

The id_entry will possibly be NULL, So, need check
id_entry and make sure it not NULL before using it.

Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/net/fec.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 2a71373..02cdd71 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -293,7 +293,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * the system that it's running on. As the result, driver has to
 	 * swap every frame going to and coming from the controller.
 	 */
-	if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+	if (id_entry && id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
 		swap_buffer(bufaddr, skb->len);
 
 	/* Save skb pointer */
@@ -529,7 +529,7 @@ fec_enet_rx(struct net_device *dev)
 	        dma_unmap_single(NULL, bdp->cbd_bufaddr, bdp->cbd_datlen,
         			DMA_FROM_DEVICE);
 
-		if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+		if (id_entry && id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
 			swap_buffer(data, pkt_len);
 
 		/* This does 16 byte alignment, exactly what we need.
@@ -808,7 +808,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	 * mdio interface in board design, and need to be configured by
 	 * fec0 mii_bus.
 	 */
-	if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) {
+	if (id_entry && (id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) {
 		/* fec1 uses fec0 mii_bus */
 		fep->mii_bus = fec0_mii_bus;
 		return 0;
@@ -851,7 +851,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 		goto err_out_free_mdio_irq;
 
 	/* save fec0 mii_bus */
-	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
+	if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC)
 		fec0_mii_bus = fep->mii_bus;
 
 	return 0;
@@ -1238,7 +1238,7 @@ fec_restart(struct net_device *dev, int duplex)
 	 * enet-mac reset will reset mac address registers too,
 	 * so need to reconfigure it.
 	 */
-	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+	if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
 		memcpy(&temp_mac, dev->dev_addr, ETH_ALEN);
 		writel(cpu_to_be32(temp_mac[0]), fep->hwp + FEC_ADDR_LOW);
 		writel(cpu_to_be32(temp_mac[1]), fep->hwp + FEC_ADDR_HIGH);
@@ -1294,7 +1294,7 @@ fec_restart(struct net_device *dev, int duplex)
 	 * The phy interface and speed need to get configured
 	 * differently on enet-mac.
 	 */
-	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+	if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
 		val = readl(fep->hwp + FEC_R_CNTRL);
 
 		/* MII or RMII */
-- 
1.7.1

Re: [PATCH V4 4/5] net/fec: check id_entry pointer before using it

[Grant Likely]

On Thu, Mar 10, 2011 at 12:59:44PM +0800, Jason Liu wrote:
> The id_entry will possibly be NULL, So, need check
> id_entry and make sure it not NULL before using it.
> 
> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Other than the double s-o-b lines, this patch looks good to me.

Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

g.

> ---
>  drivers/net/fec.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 2a71373..02cdd71 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -293,7 +293,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	 * the system that it's running on. As the result, driver has to
>  	 * swap every frame going to and coming from the controller.
>  	 */
> -	if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
> +	if (id_entry && id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
>  		swap_buffer(bufaddr, skb->len);
>  
>  	/* Save skb pointer */
> @@ -529,7 +529,7 @@ fec_enet_rx(struct net_device *dev)
>  	        dma_unmap_single(NULL, bdp->cbd_bufaddr, bdp->cbd_datlen,
>          			DMA_FROM_DEVICE);
>  
> -		if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
> +		if (id_entry && id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
>  			swap_buffer(data, pkt_len);
>  
>  		/* This does 16 byte alignment, exactly what we need.
> @@ -808,7 +808,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>  	 * mdio interface in board design, and need to be configured by
>  	 * fec0 mii_bus.
>  	 */
> -	if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) {
> +	if (id_entry && (id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) {
>  		/* fec1 uses fec0 mii_bus */
>  		fep->mii_bus = fec0_mii_bus;
>  		return 0;
> @@ -851,7 +851,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>  		goto err_out_free_mdio_irq;
>  
>  	/* save fec0 mii_bus */
> -	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
> +	if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC)
>  		fec0_mii_bus = fep->mii_bus;
>  
>  	return 0;
> @@ -1238,7 +1238,7 @@ fec_restart(struct net_device *dev, int duplex)
>  	 * enet-mac reset will reset mac address registers too,
>  	 * so need to reconfigure it.
>  	 */
> -	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> +	if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
>  		memcpy(&temp_mac, dev->dev_addr, ETH_ALEN);
>  		writel(cpu_to_be32(temp_mac[0]), fep->hwp + FEC_ADDR_LOW);
>  		writel(cpu_to_be32(temp_mac[1]), fep->hwp + FEC_ADDR_HIGH);
> @@ -1294,7 +1294,7 @@ fec_restart(struct net_device *dev, int duplex)
>  	 * The phy interface and speed need to get configured
>  	 * differently on enet-mac.
>  	 */
> -	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> +	if (id_entry && id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
>  		val = readl(fep->hwp + FEC_R_CNTRL);
>  
>  		/* MII or RMII */
> -- 
> 1.7.1
> 

[PATCH V4 5/5] net/fec: add device tree matching support

[Jason Liu]

Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/net/fec.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 02cdd71..fcb9768 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -45,6 +45,9 @@
 #include <linux/phy.h>
 #include <linux/fec.h>
 
+#include <linux/of.h>
+#include <linux/of_address.h>
+
 #include <asm/cacheflush.h>
 
 #ifndef CONFIG_ARM
@@ -1523,6 +1526,13 @@ static const struct dev_pm_ops fec_pm_ops = {
 };
 #endif
 
+#ifdef CONFIG_OF
+static struct of_device_id fec_matches[] = {
+	{ .compatible = "fsl,imx-fec" },
+	{},
+};
+#endif
+
 static struct platform_driver fec_driver = {
 	.driver	= {
 		.name	= DRIVER_NAME,
@@ -1530,6 +1540,9 @@ static struct platform_driver fec_driver = {
 #ifdef CONFIG_PM
 		.pm	= &fec_pm_ops,
 #endif
+#ifdef CONFIG_OF
+		.of_match_table = fec_matches,
+#endif
 	},
 	.id_table = fec_devtype,
 	.probe	= fec_probe,
-- 
1.7.1

Re: [PATCH V4 5/5] net/fec: add device tree matching support

[Grant Likely]

On Thu, Mar 10, 2011 at 12:59:45PM +0800, Jason Liu wrote:
> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  drivers/net/fec.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 02cdd71..fcb9768 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -45,6 +45,9 @@
>  #include <linux/phy.h>
>  #include <linux/fec.h>
>  
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +

Should be mixed in with the rest of the linux/*.h includes (don't put
a blank line between them.

>  #include <asm/cacheflush.h>
>  
>  #ifndef CONFIG_ARM
> @@ -1523,6 +1526,13 @@ static const struct dev_pm_ops fec_pm_ops = {
>  };
>  #endif
>  
> +#ifdef CONFIG_OF
> +static struct of_device_id fec_matches[] = {
> +	{ .compatible = "fsl,imx-fec" },

Must have documentation for this binding in
Documentation/devicetree/bindings before I can pick this up.  Same
goes for the uart driver patch.

Also, I recommend being more specific on the compatible property.
fsl,imx51-fec would be better.  Newer parts can claim compatibility
with this one if you're concerned about supporting multiple parts.

ie. for imx 53, this would be appropriate:

	compatible = "fsl,imx53-fec", "fsl,imx51-fec";

> +	{},
> +};
> +#endif
> +
>  static struct platform_driver fec_driver = {
>  	.driver	= {
>  		.name	= DRIVER_NAME,
> @@ -1530,6 +1540,9 @@ static struct platform_driver fec_driver = {
>  #ifdef CONFIG_PM
>  		.pm	= &fec_pm_ops,
>  #endif
> +#ifdef CONFIG_OF
> +		.of_match_table = fec_matches,
> +#endif
>  	},
>  	.id_table = fec_devtype,
>  	.probe	= fec_probe,
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH V4 5/5] net/fec: add device tree matching support

[Jason Hui]

Hi, Grant,

On Tue, Mar 15, 2011 at 3:14 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Thu, Mar 10, 2011 at 12:59:45PM +0800, Jason Liu wrote:
>> Signed-off-by: Jason Liu <jason.hui-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Jason Liu <r64343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> ---
>>  drivers/net/fec.c |   13 +++++++++++++
>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
>> index 02cdd71..fcb9768 100644
>> --- a/drivers/net/fec.c
>> +++ b/drivers/net/fec.c
>> @@ -45,6 +45,9 @@
>>  #include <linux/phy.h>
>>  #include <linux/fec.h>
>>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +
>
> Should be mixed in with the rest of the linux/*.h includes (don't put
> a blank line between them.

OK,

>
>>  #include <asm/cacheflush.h>
>>
>>  #ifndef CONFIG_ARM
>> @@ -1523,6 +1526,13 @@ static const struct dev_pm_ops fec_pm_ops = {
>>  };
>>  #endif
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id fec_matches[] = {
>> +     { .compatible = "fsl,imx-fec" },
>
> Must have documentation for this binding in
> Documentation/devicetree/bindings before I can pick this up.  Same
> goes for the uart driver patch.

OK, I will write one documentation for it and the same with uart.

>
> Also, I recommend being more specific on the compatible property.
> fsl,imx51-fec would be better.  Newer parts can claim compatibility
> with this one if you're concerned about supporting multiple parts.
>
> ie. for imx 53, this would be appropriate:
>
>        compatible = "fsl,imx53-fec", "fsl,imx51-fec";

OK,

>
>> +     {},
>> +};
>> +#endif
>> +
>>  static struct platform_driver fec_driver = {
>>       .driver = {
>>               .name   = DRIVER_NAME,
>> @@ -1530,6 +1540,9 @@ static struct platform_driver fec_driver = {
>>  #ifdef CONFIG_PM
>>               .pm     = &fec_pm_ops,
>>  #endif
>> +#ifdef CONFIG_OF
>> +             .of_match_table = fec_matches,
>> +#endif
>>       },
>>       .id_table = fec_devtype,
>>       .probe  = fec_probe,
>> --
>> 1.7.1
>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>