[PATCH 0/7] Add Samsung's s5pv310 machine with device tree enabled

[PATCH 0/7] Add Samsung's s5pv310 machine with device tree enabled

[Thomas Abraham]

This patchset adds Samsung's s5pv310 machine with device tree support. This
is based on

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

A new machine s5pv310-dt is added along with a dts file for the smdkv310 board.
Some of the features introduced in this patchset are

1. Tested on smdkv310 board and information such as bootargs, memory information
   and console port register defaults are obtained from the device tree.
2. Add basic device tree file for smdkv310 board.
3. Device tree based probe enabled in watchdog timer driver (s3c2410-wdt).
4. UART driver modified to obtain default console register values from
   device tree.

In addition, the first 5 patches have been tested with
git://git.secretlab.ca/git/linux-2.6 devicetree/arm

and (with minor modification [driver/tty/serial -> drivers/serial])
git://git.secretlab.ca/git/linux-2.6 devicetree/arm-linaro-2.6.37

Thomas Abraham (7):
  ARM: s5pv310-dt: Add s5pv310 machine with device tree support
  ARM: s5pv310-dt: Add a basic dts file for SMDKV310 machine
  ARM: s5pv310-dt: Add support for probing platform bus on s5pv310 dt-enabled machine
  watchdog: s3c2410: Add support for device tree based probe
  serial: samsung: Get console register defaults from device tree
  ARM: s5pv310-dt: Enable snooping of platform_device registrations
  serial: samsung: Get default port register settings from device tree

 arch/arm/mach-s5pv310/Kconfig           |    7 +++
 arch/arm/mach-s5pv310/Makefile          |    1 +
 arch/arm/mach-s5pv310/mach-s5pv310-dt.c |   85 +++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv310/mach-smdkv310.dts |   78 ++++++++++++++++++++++++++++
 drivers/tty/serial/samsung.c            |   27 ++++++++++
 drivers/watchdog/s3c2410_wdt.c          |   10 ++++
 6 files changed, 208 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv310/mach-s5pv310-dt.c
 create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts

[PATCH 1/7] ARM: s5pv310-dt: Add s5pv310 machine with device tree support

[Thomas Abraham]

This patch adds a new Samsung's s5pv310 machine that boots with a minimal
device tree support. The bootargs, memory information, console port
register defaults and external crystal frequency are obtained from the
device tree during boot.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-s5pv310/Kconfig           |    7 +++
 arch/arm/mach-s5pv310/Makefile          |    1 +
 arch/arm/mach-s5pv310/mach-s5pv310-dt.c |   73 +++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv310/mach-s5pv310-dt.c

diff --git a/arch/arm/mach-s5pv310/Kconfig b/arch/arm/mach-s5pv310/Kconfig
index 09c4c21..f0c1afd 100644
--- a/arch/arm/mach-s5pv310/Kconfig
+++ b/arch/arm/mach-s5pv310/Kconfig
@@ -127,6 +127,13 @@ config MACH_SMDKV310
 	help
 	  Machine support for Samsung SMDKV310
 
+config MACH_S5PV310_DT
+	bool "S5PV310 with device tree support"
+	select CPU_S5PV310
+	select USE_OF
+	help
+	  Machine support for Samsung's S5PV310 SoC with device tree based initialization.
+
 endmenu
 
 comment "Configuration for HSMMC bus width"
diff --git a/arch/arm/mach-s5pv310/Makefile b/arch/arm/mach-s5pv310/Makefile
index 036fb38..7fa7842 100644
--- a/arch/arm/mach-s5pv310/Makefile
+++ b/arch/arm/mach-s5pv310/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
 
 obj-$(CONFIG_MACH_SMDKC210)	+= mach-smdkc210.o
 obj-$(CONFIG_MACH_SMDKV310)	+= mach-smdkv310.o
+obj-$(CONFIG_MACH_S5PV310_DT)	+= mach-s5pv310-dt.o
 obj-$(CONFIG_MACH_UNIVERSAL_C210)	+= mach-universal_c210.o
 
 # device support
diff --git a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
new file mode 100644
index 0000000..a5e33ff
--- /dev/null
+++ b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
@@ -0,0 +1,73 @@
+/*
+ * Samsung's S5PV310 machine with device tree enabled.
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include <linux/serial_core.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+
+#include <plat/regs-serial.h>
+#include <plat/s5pv310.h>
+#include <plat/cpu.h>
+#include <plat/devs.h>
+
+#include <mach/map.h>
+
+/*
+ * Platfrom data for uart devices. Some elements of the platform data will
+ * be set by the s5pv310 platform code and the rest will be obtained from the
+ * device tree. TODO: remove this when driver is able to obtain all the
+ * platform data information from the device tree.
+ */
+static struct s3c2410_uartcfg uartcfgs[CONFIG_SERIAL_SAMSUNG_UARTS] __initdata;
+
+static void __init s5pv310_dt_map_io(void)
+{
+	s5p_init_io(NULL, 0, S5P_VA_CHIPID);
+}
+
+static void s5pv310_dt_init_early(void)
+{
+	int i;
+	unsigned int *xtal_freq;
+
+	xtal_freq = (u32 *)of_get_property(of_chosen, "xtal-frequency", NULL);
+	if (!xtal_freq) {
+		printk(KERN_ERR "Ext crystal clock frequency not specified\n");
+		panic("Unknown external crystal clock frequency");
+	}
+
+	s3c24xx_init_clocks(be32_to_cpu(*xtal_freq));
+
+	for (i = 0; i < CONFIG_SERIAL_SAMSUNG_UARTS; i++) {
+		uartcfgs[i].hwport = i;
+		uartcfgs[i].flags = 0;
+	}
+	s3c24xx_init_uarts(uartcfgs, ARRAY_SIZE(uartcfgs));
+}
+
+static char const *s5pv310_dt_compat[] = {
+	"samsung,s5pv310",
+	NULL
+};
+
+MACHINE_START(S5PV310_DT, "Samsung's S5PV310 with flattened device tree")
+	/* Maintainer: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
+	/* Maintainer: Changhwan Youn <chaos.youn-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
+	.init_irq	= s5pv310_init_irq,
+	.map_io		= s5pv310_dt_map_io,
+	.timer		= &s5pv310_timer,
+	.dt_compat	= s5pv310_dt_compat,
+	.init_early	= s5pv310_dt_init_early,
+MACHINE_END
-- 
1.6.6.rc2

[PATCH 2/7] ARM: s5pv310-dt: Add a basic dts file for SMDKV310 machine

[Thomas Abraham]

This patch adds a basic dts file for Samsung's SMDKV310 board which
is based on the s5pv310 machine.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-s5pv310/mach-smdkv310.dts |   78 +++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)
 create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts

diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
new file mode 100755
index 0000000..75aa76d
--- /dev/null
+++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
@@ -0,0 +1,78 @@
+/dts-v1/;
+
+/ {
+	model = "smdkv310";
+	compatible = "samsung,s5pv310","samsung,smdkv310";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		cpu@0{
+			compatible = "arm,cortex-a9";
+		};
+
+		cpu@1 {
+			compatible = "arm,cortex-a9";
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x40000000 0x08000000>;
+	};
+
+	chosen {
+		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
+		console-defaults = <0x3c5 0x3 0x111>;
+		xtal-frequency = <24000000>;
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		interrupt-parent = <&GIC>;
+		compatible = "samsung,s5pv310-soc","simple-bus";
+		ranges;
+
+		GIC:gic@0x10500000 {
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0x10500000 0x1000>;
+			compatible = "samsung,s5pv310-gic","arm,gic";
+		};
+
+		watchdog@0x10060000 {
+			reg = <0x10060000 0x400>;
+			interrupts = <552>;
+			compatible = "samsung,s5pv310-wdt","samsung,s3c2410-wdt";
+		};
+
+		serial@0x13800000 {
+			reg = <0x13800000 0x100>;
+			interrupts = <16 18 17>;
+			reg-defaults = <0x3c5 0x3 0x111>;
+			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
+		};
+
+		serial@0x13810000 {
+			reg = <0x13810000 0x100>;
+			interrupts = <20 22 21>;
+			reg-defaults = <0x3c5 0x3 0x111>;
+			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
+		};
+
+		serial@0x13820000 {
+			reg = <0x13820000 0x100>;
+			interrupts = <24 26 25>;
+			reg-defaults = <0x3c5 0x3 0x111>;
+			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
+		};
+
+		serial@0x13830000 {
+			reg = <0x13830000 0x100>;
+			interrupts = <28 30 29>;
+			reg-defaults = <0x3c5 0x3 0x111>;
+			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
+		};
+	};
+};
-- 
1.6.6.rc2

[PATCH 3/7] ARM: s5pv310-dt: Add support for probing platform bus on s5pv310 dt-enabled machine

[Thomas Abraham]

This patch adds support for probing devices from device tree
for Samsung's s5pv310 device-tree enabled machine.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-s5pv310/mach-s5pv310-dt.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
index a5e33ff..2578d73 100644
--- a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
+++ b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
@@ -57,6 +57,16 @@ static void s5pv310_dt_init_early(void)
 	s3c24xx_init_uarts(uartcfgs, ARRAY_SIZE(uartcfgs));
 }
 
+static struct of_device_id s5pv310_dt_match_table[] __initdata = {
+	{ .compatible = "simple-bus", },
+	{},
+};
+
+static void __init s5pv310_dt_machine_init(void)
+{
+	of_platform_bus_probe(NULL, s5pv310_dt_match_table, NULL);
+}
+
 static char const *s5pv310_dt_compat[] = {
 	"samsung,s5pv310",
 	NULL
@@ -67,6 +77,7 @@ MACHINE_START(S5PV310_DT, "Samsung's S5PV310 with flattened device tree")
 	/* Maintainer: Changhwan Youn <chaos.youn-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
 	.init_irq	= s5pv310_init_irq,
 	.map_io		= s5pv310_dt_map_io,
+	.init_machine	= s5pv310_dt_machine_init,
 	.timer		= &s5pv310_timer,
 	.dt_compat	= s5pv310_dt_compat,
 	.init_early	= s5pv310_dt_init_early,
-- 
1.6.6.rc2

[PATCH 4/7] watchdog: s3c2410: Add support for device tree based probe

[Thomas Abraham]

This patch adds the of_match_table to enable s3c2410-wdt driver
to be probed when watchdog device node is found in the device tree.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/watchdog/s3c2410_wdt.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index ae53662..a9edd50 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -592,6 +592,13 @@ static int s3c2410wdt_resume(struct platform_device *dev)
 #define s3c2410wdt_resume  NULL
 #endif /* CONFIG_PM */
 
+#ifdef CONFIG_OF
+static const struct of_device_id s3c2410_wdt_match[] = {
+	{ .compatible = "samsung,s3c2410-wdt" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
+#endif
 
 static struct platform_driver s3c2410wdt_driver = {
 	.probe		= s3c2410wdt_probe,
@@ -602,6 +609,9 @@ static struct platform_driver s3c2410wdt_driver = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "s3c2410-wdt",
+#ifdef CONFIG_OF
+		.of_match_table	= s3c2410_wdt_match,
+#endif
 	},
 };
 
-- 
1.6.6.rc2

[PATCH 5/7] serial: samsung: Get console register defaults from device tree

[Thomas Abraham]

The uart ports are used as console and due to console_init, the uart ports
are initialized prior to the uart driver's probe function is called.
During this intialization stage, the driver obtains the default port
register values from the platform data.

This patch adds support for obtaining the default port register values
from the device tree. The default values should be specified in the
'chosen' node of the device tree.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/tty/serial/samsung.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 2335eda..66fece9 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -43,6 +43,7 @@
 #include <linux/delay.h>
 #include <linux/clk.h>
 #include <linux/cpufreq.h>
+#include <linux/of.h>
 
 #include <asm/irq.h>
 
@@ -1384,11 +1385,24 @@ static int s3c24xx_serial_init_ports(struct s3c24xx_uart_info **info)
 	struct s3c24xx_uart_port *ptr = s3c24xx_serial_ports;
 	struct platform_device **platdev_ptr;
 	int i;
+	unsigned int *condefs , len;
+	struct s3c2410_uartcfg *cfg;
 
 	dbg("s3c24xx_serial_init_ports: initialising ports...\n");
 
 	platdev_ptr = s3c24xx_uart_devs;
+	condefs = (u32 *)of_get_property(of_chosen, "console-defaults", &len);
+	if (condefs && (len / sizeof(unsigned int)) == 3) {
+		for (i = 0; i < CONFIG_SERIAL_SAMSUNG_UARTS; i++) {
+			cfg = s3c24xx_dev_to_cfg(&(*platdev_ptr)->dev);
+			cfg->ucon = be32_to_cpu(condefs[0]);
+			cfg->ulcon = be32_to_cpu(condefs[1]);
+			cfg->ufcon = be32_to_cpu(condefs[2]);
+			platdev_ptr++;
+		}
+	}
 
+	platdev_ptr = s3c24xx_uart_devs;
 	for (i = 0; i < CONFIG_SERIAL_SAMSUNG_UARTS; i++, ptr++, platdev_ptr++) {
 		s3c24xx_serial_init_port(ptr, info[i], *platdev_ptr);
 	}
-- 
1.6.6.rc2

[PATCH 6/7] ARM: s5pv310-dt: Enable snooping of platform_device registrations

[Thomas Abraham]

Enable snooping of platform_device registrations for s5pv310-dt machine.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-s5pv310/mach-s5pv310-dt.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
index 2578d73..ed5cae4 100644
--- a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
+++ b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
@@ -64,6 +64,7 @@ static struct of_device_id s5pv310_dt_match_table[] __initdata = {
 
 static void __init s5pv310_dt_machine_init(void)
 {
+	of_platform_bus_snoop(NULL, s5pv310_dt_match_table);
 	of_platform_bus_probe(NULL, s5pv310_dt_match_table, NULL);
 }
 
-- 
1.6.6.rc2

[PATCH 7/7] serial: samsung: Get default port register settings from device tree

[Thomas Abraham]

The default uart port register settings are obtained from the platform data.
In addition to that, this patch adds support for obtaining the default uart port
register values from the uart node in device tree.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/tty/serial/samsung.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 66fece9..dda1d52 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1148,9 +1148,22 @@ int s3c24xx_serial_probe(struct platform_device *dev,
 {
 	struct s3c24xx_uart_port *ourport;
 	int ret;
+	unsigned int len, *regdef;
+	struct s3c2410_uartcfg *cfg;
 
 	dbg("s3c24xx_serial_probe(%p, %p) %d\n", dev, info, probe_index);
 
+	if (dev->dev.of_node) {
+		regdef = (u32 *)of_get_property(dev->dev.of_node,
+					"reg-defaults", &len);
+		if (regdef && (len / sizeof(unsigned int)) == 3) {
+			cfg = s3c24xx_dev_to_cfg(&dev->dev);
+			cfg->ucon = be32_to_cpu(regdef[0]);
+			cfg->ulcon = be32_to_cpu(regdef[1]);
+			cfg->ufcon = be32_to_cpu(regdef[2]);
+		}
+	}
+
 	ourport = &s3c24xx_serial_ports[probe_index];
 	probe_index++;
 
-- 
1.6.6.rc2

Re: [PATCH 4/7] watchdog: s3c2410: Add support for device tree based probe

[Grant Likely]

On Sat, Feb 12, 2011 at 06:17:02PM +0530, Thomas Abraham wrote:
> This patch adds the of_match_table to enable s3c2410-wdt driver
> to be probed when watchdog device node is found in the device tree.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/watchdog/s3c2410_wdt.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)

Need to add documentation for compatible = "samsung,s3c2410-wdt" to
Documentation/devicetree/bindings/wdt before this patch can be merged.

A couple more comments below, but I'm okay with this one.  I'd go
ahead and submit it to the linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org list and cc:
Wim.  It can be mainlined right away after fixing up the comments.
I'll ack it when you repost and then work out with Wim if it should be
merged through his tree or through mine.

> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index ae53662..a9edd50 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -592,6 +592,13 @@ static int s3c2410wdt_resume(struct platform_device *dev)
>  #define s3c2410wdt_resume  NULL
>  #endif /* CONFIG_PM */
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id s3c2410_wdt_match[] = {
> +	{ .compatible = "samsung,s3c2410-wdt" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
> +#endif

#else
#define s3c2410_wdt_match NULL
#endif

>  
>  static struct platform_driver s3c2410wdt_driver = {
>  	.probe		= s3c2410wdt_probe,
> @@ -602,6 +609,9 @@ static struct platform_driver s3c2410wdt_driver = {
>  	.driver		= {
>  		.owner	= THIS_MODULE,
>  		.name	= "s3c2410-wdt",
> +#ifdef CONFIG_OF
> +		.of_match_table	= s3c2410_wdt_match,
> +#endif

The #ifdef can be dropped here now.  .of_match_table is no longer conditional.


>  	},
>  };
>  
> -- 
> 1.6.6.rc2
> 

Re: [PATCH 5/7] serial: samsung: Get console register defaults from device tree

[Grant Likely]

On Sat, Feb 12, 2011 at 06:17:03PM +0530, Thomas Abraham wrote:
> The uart ports are used as console and due to console_init, the uart ports
> are initialized prior to the uart driver's probe function is called.
> During this intialization stage, the driver obtains the default port
> register values from the platform data.
> 
> This patch adds support for obtaining the default port register values
> from the device tree. The default values should be specified in the
> 'chosen' node of the device tree.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/tty/serial/samsung.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index 2335eda..66fece9 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -43,6 +43,7 @@
>  #include <linux/delay.h>
>  #include <linux/clk.h>
>  #include <linux/cpufreq.h>
> +#include <linux/of.h>
>  
>  #include <asm/irq.h>
>  
> @@ -1384,11 +1385,24 @@ static int s3c24xx_serial_init_ports(struct s3c24xx_uart_info **info)
>  	struct s3c24xx_uart_port *ptr = s3c24xx_serial_ports;
>  	struct platform_device **platdev_ptr;
>  	int i;
> +	unsigned int *condefs , len;
> +	struct s3c2410_uartcfg *cfg;
>  
>  	dbg("s3c24xx_serial_init_ports: initialising ports...\n");
>  
>  	platdev_ptr = s3c24xx_uart_devs;
> +	condefs = (u32 *)of_get_property(of_chosen, "console-defaults", &len);

of_get_property returns (const void *) for a reason.  Make condefs a
'const u32 *' instead of overriding the compiler warning.

Pulling the uart configuration out of a console-defaults property in
the chosen node is not the way to do this.  Each uart device should
have its own node in the device tree, and the per-port configuration
belongs there.

g.

> +	if (condefs && (len / sizeof(unsigned int)) == 3) {
> +		for (i = 0; i < CONFIG_SERIAL_SAMSUNG_UARTS; i++) {
> +			cfg = s3c24xx_dev_to_cfg(&(*platdev_ptr)->dev);
> +			cfg->ucon = be32_to_cpu(condefs[0]);
> +			cfg->ulcon = be32_to_cpu(condefs[1]);
> +			cfg->ufcon = be32_to_cpu(condefs[2]);
> +			platdev_ptr++;
> +		}
> +	}
>  
> +	platdev_ptr = s3c24xx_uart_devs;
>  	for (i = 0; i < CONFIG_SERIAL_SAMSUNG_UARTS; i++, ptr++, platdev_ptr++) {
>  		s3c24xx_serial_init_port(ptr, info[i], *platdev_ptr);
>  	}
> -- 
> 1.6.6.rc2
> 

Re: [PATCH 6/7] ARM: s5pv310-dt: Enable snooping of platform_device registrations

[Grant Likely]

On Sat, Feb 12, 2011 at 06:17:04PM +0530, Thomas Abraham wrote:
> Enable snooping of platform_device registrations for s5pv310-dt machine.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/mach-s5pv310/mach-s5pv310-dt.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
> index 2578d73..ed5cae4 100644
> --- a/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
> +++ b/arch/arm/mach-s5pv310/mach-s5pv310-dt.c
> @@ -64,6 +64,7 @@ static struct of_device_id s5pv310_dt_match_table[] __initdata = {
>  
>  static void __init s5pv310_dt_machine_init(void)
>  {
> +	of_platform_bus_snoop(NULL, s5pv310_dt_match_table);
>  	of_platform_bus_probe(NULL, s5pv310_dt_match_table, NULL);

of_platform_bus_snoop shouldn't be used with of_platform_bus_probe
because it might cause devices to get registered twice.
of_platform_bus_snoop_finish() should be used instead.  It will
register any devices found by snoop that don't have an associated
platform_device, but you need to arrange for
of_platform_bus_snoop_finish() to be called after all other platform
devices have been registered.

g.

Re: [PATCH 7/7] serial: samsung: Get default port register settings from device tree

[Grant Likely]

On Sat, Feb 12, 2011 at 06:17:05PM +0530, Thomas Abraham wrote:
> The default uart port register settings are obtained from the platform data.
> In addition to that, this patch adds support for obtaining the default uart port
> register values from the uart node in device tree.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/tty/serial/samsung.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index 66fece9..dda1d52 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -1148,9 +1148,22 @@ int s3c24xx_serial_probe(struct platform_device *dev,
>  {
>  	struct s3c24xx_uart_port *ourport;
>  	int ret;
> +	unsigned int len, *regdef;
> +	struct s3c2410_uartcfg *cfg;
>  
>  	dbg("s3c24xx_serial_probe(%p, %p) %d\n", dev, info, probe_index);
>  
> +	if (dev->dev.of_node) {
> +		regdef = (u32 *)of_get_property(dev->dev.of_node,
> +					"reg-defaults", &len);
> +		if (regdef && (len / sizeof(unsigned int)) == 3) {
> +			cfg = s3c24xx_dev_to_cfg(&dev->dev);
> +			cfg->ucon = be32_to_cpu(regdef[0]);
> +			cfg->ulcon = be32_to_cpu(regdef[1]);
> +			cfg->ufcon = be32_to_cpu(regdef[2]);
> +		}
> +	}
> +

Same comment as before, per-uart configuration belongs in each ports
device tree node.

Also, don't forget to add binding documentation do
Documentation/devicetree/bindings for the properties that you define
for the samsung uart.

g.

Re: [PATCH 2/7] ARM: s5pv310-dt: Add a basic dts file for SMDKV310 machine

[Grant Likely]

On Sat, Feb 12, 2011 at 06:17:00PM +0530, Thomas Abraham wrote:
> This patch adds a basic dts file for Samsung's SMDKV310 board which
> is based on the s5pv310 machine.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Structure looks good.  Some comments below, but it is getting close.

> ---
>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   78 +++++++++++++++++++++++++++++++
>  1 files changed, 78 insertions(+), 0 deletions(-)
>  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
> 
> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> new file mode 100755
> index 0000000..75aa76d
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> @@ -0,0 +1,78 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "smdkv310";

You can be verbose and human-friendly here.  ie. "Samsung SMDKv310 eval board"

> +	compatible = "samsung,s5pv310","samsung,smdkv310";

The specific board should come first (most compatible) followed by the
value for the soc (less specific).

> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {

#address-cells = <1>;
#size-cells = <0>;

> +		cpu@0{
> +			compatible = "arm,cortex-a9";

reg = <0>;

> +		};
> +
> +		cpu@1 {
> +			compatible = "arm,cortex-a9";

reg = <1>;

> +		};
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x40000000 0x08000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";

initrd address should be passed via the linux,initrd-start and
linux,initrd-end properties.  U-Boot already knows how to do this so
it shouldn't be encoded in the kernel parameters list.

> +		console-defaults = <0x3c5 0x3 0x111>;

Drop console-defaults.  The configuration should be encoded in the
uart's device tree node.

> +		xtal-frequency = <24000000>;

It looks like this is misplaced.  If this is the primary frequency for
the system, then it should go either in the root node,  the soc node,
or the cpu nodes.

> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		interrupt-parent = <&GIC>;
> +		compatible = "samsung,s5pv310-soc","simple-bus";
> +		ranges;
> +
> +		GIC:gic@0x10500000 {
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0x10500000 0x1000>;
> +			compatible = "samsung,s5pv310-gic","arm,gic";
> +		};
> +
> +		watchdog@0x10060000 {
> +			reg = <0x10060000 0x400>;
> +			interrupts = <552>;

This of course will need to be fixed up when we get the irq
infrastructure properly handling cascaded irq controllers.  It is fine
for the time being though.

> +			compatible = "samsung,s5pv310-wdt","samsung,s3c2410-wdt";
> +		};
> +
> +		serial@0x13800000 {
> +			reg = <0x13800000 0x100>;
> +			interrupts = <16 18 17>;
> +			reg-defaults = <0x3c5 0x3 0x111>;

reg-defaults doesn't sound like a very good binding.  It's better when
the properties reflect what is trying to be configured instead of a
set of magic values.  (That said, sometimes magic values are
appropriate, but even then it needs to be well documented so that
mere-mortals have a fighting chance of understanding and manipulating it).

> +			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
> +		};
> +
> +		serial@0x13810000 {
> +			reg = <0x13810000 0x100>;
> +			interrupts = <20 22 21>;
> +			reg-defaults = <0x3c5 0x3 0x111>;
> +			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
> +		};
> +
> +		serial@0x13820000 {
> +			reg = <0x13820000 0x100>;
> +			interrupts = <24 26 25>;
> +			reg-defaults = <0x3c5 0x3 0x111>;
> +			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
> +		};
> +
> +		serial@0x13830000 {
> +			reg = <0x13830000 0x100>;
> +			interrupts = <28 30 29>;
> +			reg-defaults = <0x3c5 0x3 0x111>;
> +			compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
> +		};
> +	};
> +};
> -- 
> 1.6.6.rc2
> 

Re: [PATCH 0/7] Add Samsung's s5pv310 machine with device tree enabled

[Grant Likely]

On Sat, Feb 12, 2011 at 5:46 AM, Thomas Abraham
<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This patchset adds Samsung's s5pv310 machine with device tree support. This
> is based on
>
> git://git.secretlab.ca/git/linux-2.6 devicetree/test
>
> A new machine s5pv310-dt is added along with a dts file for the smdkv310 board.
> Some of the features introduced in this patchset are
>
> 1. Tested on smdkv310 board and information such as bootargs, memory information
>   and console port register defaults are obtained from the device tree.
> 2. Add basic device tree file for smdkv310 board.
> 3. Device tree based probe enabled in watchdog timer driver (s3c2410-wdt).
> 4. UART driver modified to obtain default console register values from
>   device tree.

Thanks Thomas,

I've picked up patches 1-3 into my devicetree/test branch.  They still
require some work, but they are close enough that I'm happy to at
least have them in my series.  You can either send me replacement
patches as need be, or build on top of devicetree/test.

The remainder I've commented on and I'll leave alone until I get the
next version.

g.


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

Re: [PATCH 2/7] ARM: s5pv310-dt: Add a basic dts file for SMDKV310 machine

[Thomas Abraham]

Hi Grant,

On 17 February 2011 06:04, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Sat, Feb 12, 2011 at 06:17:00PM +0530, Thomas Abraham wrote:
>> This patch adds a basic dts file for Samsung's SMDKV310 board which
>> is based on the s5pv310 machine.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> Structure looks good.  Some comments below, but it is getting close.
>
>> ---
>>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   78 +++++++++++++++++++++++++++++++
>>  1 files changed, 78 insertions(+), 0 deletions(-)
>>  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
>>
>> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>> new file mode 100755
>> index 0000000..75aa76d
>> --- /dev/null
>> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>> @@ -0,0 +1,78 @@
>> +/dts-v1/;
>> +
>> +/ {
>> +     model = "smdkv310";

<snip>

>> +
>> +     chosen {
>> +             bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
>
> initrd address should be passed via the linux,initrd-start and
> linux,initrd-end properties.  U-Boot already knows how to do this so
> it shouldn't be encoded in the kernel parameters list.

Ok. "linux,initrd-start" and "linux,initrd-end" properties will be
added in chosen node and initrd parameters from bootargs removed as
below.

chosen {
        bootargs = "root=/dev/ram0 console=ttySAC1,115200 init=/linuxrc";
        linux,initrd-start=<0x41000000>;
        linux,initrd-end=<0x40800000>;
    } ;

>
>> +             console-defaults = <0x3c5 0x3 0x111>;
>
> Drop console-defaults.  The configuration should be encoded in the
> uart's device tree node.

The stage at which console_init is called, the code that initializes
the uart module for console operation does not have access to the uart
device tree node. Hence, the console-default register values are put
into the chosen node which is easier to access. If this not the right
approach, maybe the uart device tree node will have to be looked up by
using "for_each_compatible_node" or similar. Would you suggest using
for_each_compatible_node in this case.

>
>> +             xtal-frequency = <24000000>;
>
> It looks like this is misplaced.  If this is the primary frequency for
> the system, then it should go either in the root node,  the soc node,
> or the cpu nodes.

This is the frequency of the crystal clock from which all the SoC
internal clocks are derived. The crystal clock frequency depends on
the board. So it was put into the chosen node. Since this is board
specific, this is added in the chosen node.

>
>> +     };
>> +

<snip>

>> +
>> +             serial@0x13800000 {
>> +                     reg = <0x13800000 0x100>;
>> +                     interrupts = <16 18 17>;
>> +                     reg-defaults = <0x3c5 0x3 0x111>;
>
> reg-defaults doesn't sound like a very good binding.  It's better when
> the properties reflect what is trying to be configured instead of a
> set of magic values.  (That said, sometimes magic values are
> appropriate, but even then it needs to be well documented so that
> mere-mortals have a fighting chance of understanding and manipulating it).
>

Ok. This will be changed to be more informative.

>> +                     compatible = "samsung,s5pv310-uart","samsung,s3c2410-uart";
>> +             };

<snip>

Thanks,
Thomas.

Re: [PATCH 4/7] watchdog: s3c2410: Add support for device tree based probe

[Thomas Abraham]

Hi Grant,

On 17 February 2011 05:47, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Sat, Feb 12, 2011 at 06:17:02PM +0530, Thomas Abraham wrote:
>> This patch adds the of_match_table to enable s3c2410-wdt driver
>> to be probed when watchdog device node is found in the device tree.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/watchdog/s3c2410_wdt.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> Need to add documentation for compatible = "samsung,s3c2410-wdt" to
> Documentation/devicetree/bindings/wdt before this patch can be merged.
>
> A couple more comments below, but I'm okay with this one.  I'd go
> ahead and submit it to the linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org list and cc:
> Wim.  It can be mainlined right away after fixing up the comments.
> I'll ack it when you repost and then work out with Wim if it should be
> merged through his tree or through mine.
>
>>
>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
>> index ae53662..a9edd50 100644
>> --- a/drivers/watchdog/s3c2410_wdt.c
>> +++ b/drivers/watchdog/s3c2410_wdt.c
>> @@ -592,6 +592,13 @@ static int s3c2410wdt_resume(struct platform_device *dev)
>>  #define s3c2410wdt_resume  NULL
>>  #endif /* CONFIG_PM */
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id s3c2410_wdt_match[] = {
>> +     { .compatible = "samsung,s3c2410-wdt" },
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
>> +#endif
>
> #else
> #define s3c2410_wdt_match NULL
> #endif
>
>>
>>  static struct platform_driver s3c2410wdt_driver = {
>>       .probe          = s3c2410wdt_probe,
>> @@ -602,6 +609,9 @@ static struct platform_driver s3c2410wdt_driver = {
>>       .driver         = {
>>               .owner  = THIS_MODULE,
>>               .name   = "s3c2410-wdt",
>> +#ifdef CONFIG_OF
>> +             .of_match_table = s3c2410_wdt_match,
>> +#endif
>
> The #ifdef can be dropped here now.  .of_match_table is no longer conditional.
>

The of_match_table is conditional in devicetree/arm and
devicetree/test branches. Are you referring to any other branch?

Thanks,
Thomas.

>
>>       },
>>  };
>>
>> --
>> 1.6.6.rc2
>>
>

Re: [PATCH 7/7] serial: samsung: Get default port register settings from device tree

[Thomas Abraham]

Hi Grant,

On 17 February 2011 05:56, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Sat, Feb 12, 2011 at 06:17:05PM +0530, Thomas Abraham wrote:
>> The default uart port register settings are obtained from the platform data.
>> In addition to that, this patch adds support for obtaining the default uart port
>> register values from the uart node in device tree.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/tty/serial/samsung.c |   13 +++++++++++++
>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>> index 66fece9..dda1d52 100644
>> --- a/drivers/tty/serial/samsung.c
>> +++ b/drivers/tty/serial/samsung.c
>> @@ -1148,9 +1148,22 @@ int s3c24xx_serial_probe(struct platform_device *dev,
>>  {
>>       struct s3c24xx_uart_port *ourport;
>>       int ret;
>> +     unsigned int len, *regdef;
>> +     struct s3c2410_uartcfg *cfg;
>>
>>       dbg("s3c24xx_serial_probe(%p, %p) %d\n", dev, info, probe_index);
>>
>> +     if (dev->dev.of_node) {
>> +             regdef = (u32 *)of_get_property(dev->dev.of_node,
>> +                                     "reg-defaults", &len);
>> +             if (regdef && (len / sizeof(unsigned int)) == 3) {
>> +                     cfg = s3c24xx_dev_to_cfg(&dev->dev);
>> +                     cfg->ucon = be32_to_cpu(regdef[0]);
>> +                     cfg->ulcon = be32_to_cpu(regdef[1]);
>> +                     cfg->ufcon = be32_to_cpu(regdef[2]);
>> +             }
>> +     }
>> +
>
> Same comment as before, per-uart configuration belongs in each ports
> device tree node.

The above configuration is obtained from the uart device tree node
(dev->dev.of_node).

>
> Also, don't forget to add binding documentation do
> Documentation/devicetree/bindings for the properties that you define
> for the samsung uart.

Ok. The documentation will be added.

Thanks,
Thomas.

>
> g.
>
>

Re: [PATCH 0/7] Add Samsung's s5pv310 machine with device tree enabled

[Thomas Abraham]

Hi Grant,

On 17 February 2011 06:09, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Sat, Feb 12, 2011 at 5:46 AM, Thomas Abraham
> <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> This patchset adds Samsung's s5pv310 machine with device tree support. This
>> is based on
>>
>> git://git.secretlab.ca/git/linux-2.6 devicetree/test
>>
>> A new machine s5pv310-dt is added along with a dts file for the smdkv310 board.
>> Some of the features introduced in this patchset are
>>
>> 1. Tested on smdkv310 board and information such as bootargs, memory information
>>   and console port register defaults are obtained from the device tree.
>> 2. Add basic device tree file for smdkv310 board.
>> 3. Device tree based probe enabled in watchdog timer driver (s3c2410-wdt).
>> 4. UART driver modified to obtain default console register values from
>>   device tree.
>
> Thanks Thomas,
>
> I've picked up patches 1-3 into my devicetree/test branch.  They still
> require some work, but they are close enough that I'm happy to at
> least have them in my series.  You can either send me replacement
> patches as need be, or build on top of devicetree/test.
>
> The remainder I've commented on and I'll leave alone until I get the
> next version.

Thanks for reviewing the patches. I will update the patchset and
submit a newer one.

Thanks,
Thomas.

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

Re: [PATCH 2/7] ARM: s5pv310-dt: Add a basic dts file for SMDKV310 machine

[Grant Likely]

Hi Thomas, replies below...

On Thu, Feb 17, 2011 at 6:55 AM, Thomas Abraham
<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Grant,
>
> On 17 February 2011 06:04, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Sat, Feb 12, 2011 at 06:17:00PM +0530, Thomas Abraham wrote:
>>> This patch adds a basic dts file for Samsung's SMDKV310 board which
>>> is based on the s5pv310 machine.
>>>
>>> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> Structure looks good.  Some comments below, but it is getting close.
>>
>>> ---
>>>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   78 +++++++++++++++++++++++++++++++
>>>  1 files changed, 78 insertions(+), 0 deletions(-)
>>>  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
>>>
>>> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>>> new file mode 100755
>>> index 0000000..75aa76d
>>> --- /dev/null
>>> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>>> @@ -0,0 +1,78 @@
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> +     model = "smdkv310";
>
> <snip>
>
>>> +
>>> +     chosen {
>>> +             bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
>>
>> initrd address should be passed via the linux,initrd-start and
>> linux,initrd-end properties.  U-Boot already knows how to do this so
>> it shouldn't be encoded in the kernel parameters list.
>
> Ok. "linux,initrd-start" and "linux,initrd-end" properties will be
> added in chosen node and initrd parameters from bootargs removed as
> below.
>
> chosen {
>        bootargs = "root=/dev/ram0 console=ttySAC1,115200 init=/linuxrc";
>        linux,initrd-start=<0x41000000>;
>        linux,initrd-end=<0x40800000>;
>    } ;

If you're using u-boot, you shouldn't need to add these properties at
all.  U-Boot will add them for you when you use the command "bootm
<kernel-addr> <initrd-addr> <dtb-addr>"

>>
>>> +             console-defaults = <0x3c5 0x3 0x111>;
>>
>> Drop console-defaults.  The configuration should be encoded in the
>> uart's device tree node.
>
> The stage at which console_init is called, the code that initializes
> the uart module for console operation does not have access to the uart
> device tree node. Hence, the console-default register values are put
> into the chosen node which is easier to access. If this not the right
> approach, maybe the uart device tree node will have to be looked up by
> using "for_each_compatible_node" or similar. Would you suggest using
> for_each_compatible_node in this case.

Yes, you need to look up the node.  It actually isn't very hard to do.
 for_each_compatible_node() is a fine approach.

>
>>
>>> +             xtal-frequency = <24000000>;
>>
>> It looks like this is misplaced.  If this is the primary frequency for
>> the system, then it should go either in the root node,  the soc node,
>> or the cpu nodes.
>
> This is the frequency of the crystal clock from which all the SoC
> internal clocks are derived. The crystal clock frequency depends on
> the board. So it was put into the chosen node. Since this is board
> specific, this is added in the chosen node.

The chosen node is for reflecting software configuration (kernel
parameters, handle to console node, initrd, etc), not hardware
configuration.  System clock definitely falls in the category of
hardware configuration.  The SoC or the root node would be an okay
place to put it.  The chosen node is definitely the wrong place.

g.

Re: [PATCH 4/7] watchdog: s3c2410: Add support for device tree based probe

[Grant Likely]

On Thu, Feb 17, 2011 at 7:04 AM, Thomas Abraham
<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Grant,
>
> On 17 February 2011 05:47, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Sat, Feb 12, 2011 at 06:17:02PM +0530, Thomas Abraham wrote:
>>> This patch adds the of_match_table to enable s3c2410-wdt driver
>>> to be probed when watchdog device node is found in the device tree.
>>>
>>> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>  drivers/watchdog/s3c2410_wdt.c |   10 ++++++++++
>>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> Need to add documentation for compatible = "samsung,s3c2410-wdt" to
>> Documentation/devicetree/bindings/wdt before this patch can be merged.
>>
>> A couple more comments below, but I'm okay with this one.  I'd go
>> ahead and submit it to the linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org list and cc:
>> Wim.  It can be mainlined right away after fixing up the comments.
>> I'll ack it when you repost and then work out with Wim if it should be
>> merged through his tree or through mine.
>>
>>>
>>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
>>> index ae53662..a9edd50 100644
>>> --- a/drivers/watchdog/s3c2410_wdt.c
>>> +++ b/drivers/watchdog/s3c2410_wdt.c
>>> @@ -592,6 +592,13 @@ static int s3c2410wdt_resume(struct platform_device *dev)
>>>  #define s3c2410wdt_resume  NULL
>>>  #endif /* CONFIG_PM */
>>>
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id s3c2410_wdt_match[] = {
>>> +     { .compatible = "samsung,s3c2410-wdt" },
>>> +     {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
>>> +#endif
>>
>> #else
>> #define s3c2410_wdt_match NULL
>> #endif
>>
>>>
>>>  static struct platform_driver s3c2410wdt_driver = {
>>>       .probe          = s3c2410wdt_probe,
>>> @@ -602,6 +609,9 @@ static struct platform_driver s3c2410wdt_driver = {
>>>       .driver         = {
>>>               .owner  = THIS_MODULE,
>>>               .name   = "s3c2410-wdt",
>>> +#ifdef CONFIG_OF
>>> +             .of_match_table = s3c2410_wdt_match,
>>> +#endif
>>
>> The #ifdef can be dropped here now.  .of_match_table is no longer conditional.
>>
>
> The of_match_table is conditional in devicetree/arm and
> devicetree/test branches. Are you referring to any other branch?

Ah, right.  I've got that change in devicetree/next, but I've not put
it into devicetree/{test,arm}.  I'll fix that and push out an update.

g.

Re: [PATCH 7/7] serial: samsung: Get default port register settings from device tree

[Grant Likely]

On Thu, Feb 17, 2011 at 7:09 AM, Thomas Abraham
<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Grant,
>
> On 17 February 2011 05:56, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Sat, Feb 12, 2011 at 06:17:05PM +0530, Thomas Abraham wrote:
>>> The default uart port register settings are obtained from the platform data.
>>> In addition to that, this patch adds support for obtaining the default uart port
>>> register values from the uart node in device tree.
>>>
>>> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>  drivers/tty/serial/samsung.c |   13 +++++++++++++
>>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>> index 66fece9..dda1d52 100644
>>> --- a/drivers/tty/serial/samsung.c
>>> +++ b/drivers/tty/serial/samsung.c
>>> @@ -1148,9 +1148,22 @@ int s3c24xx_serial_probe(struct platform_device *dev,
>>>  {
>>>       struct s3c24xx_uart_port *ourport;
>>>       int ret;
>>> +     unsigned int len, *regdef;
>>> +     struct s3c2410_uartcfg *cfg;
>>>
>>>       dbg("s3c24xx_serial_probe(%p, %p) %d\n", dev, info, probe_index);
>>>
>>> +     if (dev->dev.of_node) {
>>> +             regdef = (u32 *)of_get_property(dev->dev.of_node,
>>> +                                     "reg-defaults", &len);
>>> +             if (regdef && (len / sizeof(unsigned int)) == 3) {
>>> +                     cfg = s3c24xx_dev_to_cfg(&dev->dev);
>>> +                     cfg->ucon = be32_to_cpu(regdef[0]);
>>> +                     cfg->ulcon = be32_to_cpu(regdef[1]);
>>> +                     cfg->ufcon = be32_to_cpu(regdef[2]);
>>> +             }
>>> +     }
>>> +
>>
>> Same comment as before, per-uart configuration belongs in each ports
>> device tree node.
>
> The above configuration is obtained from the uart device tree node
> (dev->dev.of_node).

Sorry for the noise, I wasn't paying enough attention here and I got confused.

g.