[PATCH 0/4] Add basic device tree support for Samsung's SMDKV310 machine.

[PATCH 0/4] Add basic device tree support for Samsung's SMDKV310 machine.

[Thomas Abraham]

This patchset adds Samsung's SMDKV310 machine with device tree support. The
bootargs and memory information is obtained from the device tree. The
watchdog driver is also modified such that it is probed based on the information
from the device tree.

Thomas Abraham (4):
 ARM: DT: Add SMDKV310 machine with device tree support.
 ARM: DT: Add a basic dts file for SMDKV310 machine
 ARM: DT: Add support for probing platform bus on SMDKV310 machine
 watchdog: s3c2410: Add support for device tree based probe

 arch/arm/mach-s5pv310/Kconfig            |    7 ++
 arch/arm/mach-s5pv310/Makefile           |    1 +
 arch/arm/mach-s5pv310/mach-smdkv310-dt.c |  107 ++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv310/mach-smdkv310.dts  |   38 +++++++++++
 drivers/watchdog/s3c2410_wdt.c           |   10 +++
 5 files changed, 163 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv310/mach-smdkv310-dt.c
 create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts

[PATCH 1/4] ARM: DT: Add SMDKV310 machine with device tree support.

[Thomas Abraham]

This patch adds a new SMDKV310 machine that boots with a minimal
device tree support. The bootargs and memory information is 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-smdkv310-dt.c |  101 ++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv310/mach-smdkv310-dt.c

diff --git a/arch/arm/mach-s5pv310/Kconfig b/arch/arm/mach-s5pv310/Kconfig
index 09c4c21..6c5f0bf 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_SMDKV310_DT
+	bool "SMDKV310 with device tree support"
+	select CPU_S5PV310
+	select USE_OF
+	help
+	  Machine support for Samsung SMDKV310 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..59545aa 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_SMDKV310_DT)	+= mach-smdkv310-dt.o
 obj-$(CONFIG_MACH_UNIVERSAL_C210)	+= mach-universal_c210.o
 
 # device support
diff --git a/arch/arm/mach-s5pv310/mach-smdkv310-dt.c b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
new file mode 100644
index 0000000..367fdb1
--- /dev/null
+++ b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
@@ -0,0 +1,101 @@
+/* linux/arch/arm/mach-s5pv310/mach-smdkv310.c
+ *
+ * Copyright (c) 2010 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>
+
+/* Following are default values for UCON, ULCON and UFCON UART registers */
+#define SMDKV310_UCON_DEFAULT	(S3C2410_UCON_TXILEVEL |	\
+				 S3C2410_UCON_RXILEVEL |	\
+				 S3C2410_UCON_TXIRQMODE |	\
+				 S3C2410_UCON_RXIRQMODE |	\
+				 S3C2410_UCON_RXFIFO_TOI |	\
+				 S3C2443_UCON_RXERR_IRQEN)
+
+#define SMDKV310_ULCON_DEFAULT	S3C2410_LCON_CS8
+
+#define SMDKV310_UFCON_DEFAULT	(S3C2410_UFCON_FIFOMODE |	\
+				 S5PV210_UFCON_TXTRIG4 |	\
+				 S5PV210_UFCON_RXTRIG4)
+
+static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
+	[0] = {
+		.hwport		= 0,
+		.flags		= 0,
+		.ucon		= SMDKV310_UCON_DEFAULT,
+		.ulcon		= SMDKV310_ULCON_DEFAULT,
+		.ufcon		= SMDKV310_UFCON_DEFAULT,
+	},
+	[1] = {
+		.hwport		= 1,
+		.flags		= 0,
+		.ucon		= SMDKV310_UCON_DEFAULT,
+		.ulcon		= SMDKV310_ULCON_DEFAULT,
+		.ufcon		= SMDKV310_UFCON_DEFAULT,
+	},
+	[2] = {
+		.hwport		= 2,
+		.flags		= 0,
+		.ucon		= SMDKV310_UCON_DEFAULT,
+		.ulcon		= SMDKV310_ULCON_DEFAULT,
+		.ufcon		= SMDKV310_UFCON_DEFAULT,
+	},
+	[3] = {
+		.hwport		= 3,
+		.flags		= 0,
+		.ucon		= SMDKV310_UCON_DEFAULT,
+		.ulcon		= SMDKV310_ULCON_DEFAULT,
+		.ufcon		= SMDKV310_UFCON_DEFAULT,
+	},
+};
+
+static struct platform_device *smdkv310_devices[] __initdata = {
+};
+
+static void __init smdkv310_map_io(void)
+{
+	s5p_init_io(NULL, 0, S5P_VA_CHIPID);
+	s3c24xx_init_clocks(24000000);
+	s3c24xx_init_uarts(smdkv310_uartcfgs, ARRAY_SIZE(smdkv310_uartcfgs));
+}
+
+static void __init smdkv310_machine_init(void)
+{
+	platform_add_devices(smdkv310_devices, ARRAY_SIZE(smdkv310_devices));
+}
+
+static char const *smdkv310_dt_compat[] = {
+	"samsung,smdkv310",
+	NULL
+};
+
+DT_MACHINE_START(SMDKV310, "Samsung's SMDKV310 with flattened device tree")
+	/* Maintainer: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
+	/* Maintainer: Changhwan Youn <chaos.youn-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
+	.boot_params	= S5P_PA_SDRAM + 0x100,
+	.init_irq	= s5pv310_init_irq,
+	.map_io		= smdkv310_map_io,
+	.init_machine	= smdkv310_machine_init,
+	.timer		= &s5pv310_timer,
+	.dt_compat	= smdkv310_dt_compat,
+MACHINE_END
-- 
1.6.6.rc2

Re: [PATCH 1/4] ARM: DT: Add SMDKV310 machine with device tree support.

[Grant Likely]

On Sun, Feb 06, 2011 at 06:47:27PM +0530, Thomas Abraham wrote:
> This patch adds a new SMDKV310 machine that boots with a minimal
> device tree support. The bootargs and memory information is 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-smdkv310-dt.c |  101 ++++++++++++++++++++++++++++++
>  3 files changed, 109 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-s5pv310/mach-smdkv310-dt.c
> 
> diff --git a/arch/arm/mach-s5pv310/Kconfig b/arch/arm/mach-s5pv310/Kconfig
> index 09c4c21..6c5f0bf 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_SMDKV310_DT
> +	bool "SMDKV310 with device tree support"

SMDKV310 is the board designation, not the SoC, correct?  Considering
that the point of this is to allow a single machine description to
support many boards, you should consider naming this MACH_S5PV310_DT.
Otherwise, you may as well just add a dt_compat table to the existing
mach-smdkv310.c board support file.

> +	select CPU_S5PV310
> +	select USE_OF
> +	help
> +	  Machine support for Samsung SMDKV310 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..59545aa 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_SMDKV310_DT)	+= mach-smdkv310-dt.o
>  obj-$(CONFIG_MACH_UNIVERSAL_C210)	+= mach-universal_c210.o
>  
>  # device support
> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310-dt.c b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
> new file mode 100644
> index 0000000..367fdb1
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
> @@ -0,0 +1,101 @@
> +/* linux/arch/arm/mach-s5pv310/mach-smdkv310.c
> + *
> + * Copyright (c) 2010 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>
> +
> +/* Following are default values for UCON, ULCON and UFCON UART registers */
> +#define SMDKV310_UCON_DEFAULT	(S3C2410_UCON_TXILEVEL |	\
> +				 S3C2410_UCON_RXILEVEL |	\
> +				 S3C2410_UCON_TXIRQMODE |	\
> +				 S3C2410_UCON_RXIRQMODE |	\
> +				 S3C2410_UCON_RXFIFO_TOI |	\
> +				 S3C2443_UCON_RXERR_IRQEN)
> +
> +#define SMDKV310_ULCON_DEFAULT	S3C2410_LCON_CS8
> +
> +#define SMDKV310_UFCON_DEFAULT	(S3C2410_UFCON_FIFOMODE |	\
> +				 S5PV210_UFCON_TXTRIG4 |	\
> +				 S5PV210_UFCON_RXTRIG4)
> +
> +static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
> +	[0] = {
> +		.hwport		= 0,
> +		.flags		= 0,

'0' initial values don't need to be specified.  .flags could be
dropped from each of these entries.

> +		.ucon		= SMDKV310_UCON_DEFAULT,
> +		.ulcon		= SMDKV310_ULCON_DEFAULT,
> +		.ufcon		= SMDKV310_UFCON_DEFAULT,
> +	},
> +	[1] = {
> +		.hwport		= 1,
> +		.flags		= 0,
> +		.ucon		= SMDKV310_UCON_DEFAULT,
> +		.ulcon		= SMDKV310_ULCON_DEFAULT,
> +		.ufcon		= SMDKV310_UFCON_DEFAULT,
> +	},
> +	[2] = {
> +		.hwport		= 2,
> +		.flags		= 0,
> +		.ucon		= SMDKV310_UCON_DEFAULT,
> +		.ulcon		= SMDKV310_ULCON_DEFAULT,
> +		.ufcon		= SMDKV310_UFCON_DEFAULT,
> +	},
> +	[3] = {
> +		.hwport		= 3,
> +		.flags		= 0,
> +		.ucon		= SMDKV310_UCON_DEFAULT,
> +		.ulcon		= SMDKV310_ULCON_DEFAULT,
> +		.ufcon		= SMDKV310_UFCON_DEFAULT,
> +	},
> +};
> +
> +static struct platform_device *smdkv310_devices[] __initdata = {
> +};

Since this list is empty, it can be removed, and so can the call to
platform_add_devices() below.

> +
> +static void __init smdkv310_map_io(void)
> +{
> +	s5p_init_io(NULL, 0, S5P_VA_CHIPID);
> +	s3c24xx_init_clocks(24000000);
> +	s3c24xx_init_uarts(smdkv310_uartcfgs, ARRAY_SIZE(smdkv310_uartcfgs));
> +}
> +
> +static void __init smdkv310_machine_init(void)
> +{
> +	platform_add_devices(smdkv310_devices, ARRAY_SIZE(smdkv310_devices));
> +}
> +
> +static char const *smdkv310_dt_compat[] = {
> +	"samsung,smdkv310",
> +	NULL
> +};
> +
> +DT_MACHINE_START(SMDKV310, "Samsung's SMDKV310 with flattened device tree")
> +	/* Maintainer: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
> +	/* Maintainer: Changhwan Youn <chaos.youn-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> */
> +	.boot_params	= S5P_PA_SDRAM + 0x100,

.boot_params can be dropped.

> +	.init_irq	= s5pv310_init_irq,
> +	.map_io		= smdkv310_map_io,
> +	.init_machine	= smdkv310_machine_init,
> +	.timer		= &s5pv310_timer,
> +	.dt_compat	= smdkv310_dt_compat,
> +MACHINE_END
> -- 
> 1.6.6.rc2
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: [PATCH 1/4] ARM: DT: Add SMDKV310 machine with device tree support.

[Arnd Bergmann]

On Sunday 06 February 2011, Thomas Abraham wrote:

> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310-dt.c b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
> new file mode 100644
> index 0000000..367fdb1
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
> @@ -0,0 +1,101 @@
> +/* linux/arch/arm/mach-s5pv310/mach-smdkv310.c

The file name in the comment doesn't match. I normally recommend not putting
it in the comment at all so you don't need to change it when moving or
copying the file.

A comment along the lines of "s5pv310 platform with device tree enabled" is
more helpful to the reader anyway.

> +static void __init smdkv310_map_io(void)
> +{
> +	s5p_init_io(NULL, 0, S5P_VA_CHIPID);
> +	s3c24xx_init_clocks(24000000);
> +	s3c24xx_init_uarts(smdkv310_uartcfgs, ARRAY_SIZE(smdkv310_uartcfgs));
> +}

There is very little here that is really platform specific, right?

Is this going to increase? If not, it might be good to read out the
data (S5P_VA_CHIPID, 24000000, smdkv310_uartcfgs) from the device tree
as well before passing them down. No need to do it right away, but it
would be good to know the direction in which we're heading.

Maybe add a comment like /* TODO: read these from the device tree */
or similar.

	Arnd

[PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

[Thomas Abraham]

This patch adds a basic dts file for Samsung's SMDKV310 machine.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
 1 files changed, 38 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..74d80bf
--- /dev/null
+++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
@@ -0,0 +1,38 @@
+/dts-v1/;
+
+/ {
+	model = "smdkv310";
+	compatible = "samsung,smdkv310";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	memory {
+		device_type = "memory";
+		reg = <0x40000000 0x08000000>;
+	};
+
+	chosen {
+		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
+
+		GIC:gic@0x10500000 {
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0x10500000 0x1000>;
+			compatible = "arm,gic";
+		};
+
+		watchdog@0x10060000 {
+			reg = <0x10060000 0x400>;
+			interrupts = <552>;
+			interrupt-parent = <&GIC>;
+			compatible = "samsung,s3c2410-wdt";
+		};
+	};
+};
-- 
1.6.6.rc2

Re: [PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

[David Gibson]

On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
> This patch adds a basic dts file for Samsung's SMDKV310 machine.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>  1 files changed, 38 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..74d80bf
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> @@ -0,0 +1,38 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "smdkv310";
> +	compatible = "samsung,smdkv310";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x40000000 0x08000000>;
> +	};

Uh.. where are the cpus?

> +	chosen {
> +		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";

It's generally a good idea to list the specific soc model before "simple-bus".

> +		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
> +
> +		GIC:gic@0x10500000 {
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0x10500000 0x1000>;
> +			compatible = "arm,gic";
> +		};
> +
> +		watchdog@0x10060000 {
> +			reg = <0x10060000 0x400>;
> +			interrupts = <552>;
> +			interrupt-parent = <&GIC>;
> +			compatible = "samsung,s3c2410-wdt";
> +		};
> +	};
> +};

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

[Rob Herring]

David, Thomas,

On 02/06/2011 06:04 PM, David Gibson wrote:
> On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
>> This patch adds a basic dts file for Samsung's SMDKV310 machine.
>>
>> Signed-off-by: Thomas Abraham<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>   arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>>   1 files changed, 38 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..74d80bf
>> --- /dev/null
>> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>> @@ -0,0 +1,38 @@
>> +/dts-v1/;
>> +
>> +/ {
>> +	model = "smdkv310";
>> +	compatible = "samsung,smdkv310";
>> +	#address-cells =<1>;
>> +	#size-cells =<1>;
>> +
>> +	memory {
>> +		device_type = "memory";
>> +		reg =<0x40000000 0x08000000>;
>> +	};
>
> Uh.. where are the cpus?
>

But for ARM, all the details of the cpu are probe-able. So what would we 
gain by putting cpu info in the DTS?

>> +	chosen {
>> +		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
>> +	};
>> +
>> +	soc {
>> +		#address-cells =<1>;
>> +		#size-cells =<1>;
>> +		compatible = "simple-bus";
>
> It's generally a good idea to list the specific soc model before "simple-bus".
>
>> +		ranges =<0x00000000 0x00000000 0xFFFFFFFF>;

For no translation, you can do just:

ranges;

Rob

Re: [PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

[Meador Inge]

On 02/07/2011 01:24 PM, Rob Herring wrote:
> David, Thomas,
>
> On 02/06/2011 06:04 PM, David Gibson wrote:
>> On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
>>> This patch adds a basic dts file for Samsung's SMDKV310 machine.
>>>
>>> Signed-off-by: Thomas Abraham<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>    arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>>>    1 files changed, 38 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..74d80bf
>>> --- /dev/null
>>> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>>> @@ -0,0 +1,38 @@
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> +	model = "smdkv310";
>>> +	compatible = "samsung,smdkv310";
>>> +	#address-cells =<1>;
>>> +	#size-cells =<1>;
>>> +
>>> +	memory {
>>> +		device_type = "memory";
>>> +		reg =<0x40000000 0x08000000>;
>>> +	};
>>
>> Uh.. where are the cpus?
>>
>
> But for ARM, all the details of the cpu are probe-able. So what would we
> gain by putting cpu info in the DTS?

Perhaps there are use cases where the system designer wants to lock in a 
certain configuration.  Hardware partitioning is one such case.  For 
example, say you have a multicore ARM platform and you only want your 
kernel to know about certain cpus.  One way to do that is by pulling the 
cpus you don't want the kernel to know about out of the device tree.

-- 
Meador Inge     | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software

Re: [PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

[Grant Likely]

On Mon, Feb 07, 2011 at 01:24:12PM -0600, Rob Herring wrote:
> David, Thomas,
> 
> On 02/06/2011 06:04 PM, David Gibson wrote:
> >On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
> >>This patch adds a basic dts file for Samsung's SMDKV310 machine.
> >>
> >>Signed-off-by: Thomas Abraham<thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >>---
> >>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
> >>  1 files changed, 38 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..74d80bf
> >>--- /dev/null
> >>+++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> >>@@ -0,0 +1,38 @@
> >>+/dts-v1/;
> >>+
> >>+/ {
> >>+	model = "smdkv310";
> >>+	compatible = "samsung,smdkv310";
> >>+	#address-cells =<1>;
> >>+	#size-cells =<1>;
> >>+
> >>+	memory {
> >>+		device_type = "memory";
> >>+		reg =<0x40000000 0x08000000>;
> >>+	};
> >
> >Uh.. where are the cpus?
> >
> 
> But for ARM, all the details of the cpu are probe-able. So what
> would we gain by putting cpu info in the DTS?

You can detect what cpus are present, but you cannot necessarily
detect if all cpus are available (for instance in a multicore AMP
system where some cores run a different operating environment.  It is
also always good practice to have the structure of the device tree
fully populated even if the OS doesn't currently use it because it
provides points that (for instance) cpu affinity or non-probeable
configuration can be hung off of.

Re: [PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

[Grant Likely]

On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
> This patch adds a basic dts file for Samsung's SMDKV310 machine.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>  1 files changed, 38 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..74d80bf
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> @@ -0,0 +1,38 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "smdkv310";
> +	compatible = "samsung,smdkv310";

Should include the soc in this compatible list:

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

Also, every 'compatible' value specified in this file should also have
documentation in Documentation/devicetree/bindings.

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

It's usually a good idea to have a "interrupt-parent = <&GIC>;" here
so that all child nodes inherit it as the default interrupt
controller.

Also, missing a 'cpus' node.

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

initrd base address will get passed via the linux,initrd-start and
linux,initrd-end properties which are automatically populated by
u-boot.  Drop initrd from the boot args.

> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";

Should also specify the soc with something like:

		compatible = "samsung,s5pv310-soc", "simple-bus";

> +		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;

You can specify that the entire ranges is translated simply by
specifying an empty ranges property like so:

		ranges;

> +
> +		GIC:gic@0x10500000 {
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0x10500000 0x1000>;
> +			compatible = "arm,gic";

Be specific about the device on all the device nodes, followed by a
list of the compatible devices.  Should be something like:

	compatible = "samsung,s5pv310-soc", "arm,gic";

> +		};
> +
> +		watchdog@0x10060000 {
> +			reg = <0x10060000 0x400>;
> +			interrupts = <552>;
> +			interrupt-parent = <&GIC>;

By putting the interrupt-parent property in the root node, this
interrupt-parent property can be dropped.

> +			compatible = "samsung,s3c2410-wdt";

compatible needs to first specify the exact device
(samsung,s5pv310-wdt?) followed by the compatible device.  I assume
the wdt in the s5pv310 is compatible with the one in the s3c2410,
correct?  Or am I missing something?

> +		};
> +	};
> +};
> -- 
> 1.6.6.rc2
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

[PATCH 3/4] ARM: DT: Add support for probing platform bus on SMDKV310 machine

[Thomas Abraham]

This patch adds support for probing devices from device tree
for Samsung's SMDKV310 machine.

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

diff --git a/arch/arm/mach-s5pv310/mach-smdkv310-dt.c b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
index 367fdb1..f81d88a 100644
--- a/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
+++ b/arch/arm/mach-s5pv310/mach-smdkv310-dt.c
@@ -79,9 +79,15 @@ static void __init smdkv310_map_io(void)
 	s3c24xx_init_uarts(smdkv310_uartcfgs, ARRAY_SIZE(smdkv310_uartcfgs));
 }
 
+static struct of_device_id smdkv310_dt_match_table[] __initdata = {
+	{ .compatible = "simple-bus", },
+	{},
+};
+
 static void __init smdkv310_machine_init(void)
 {
 	platform_add_devices(smdkv310_devices, ARRAY_SIZE(smdkv310_devices));
+	of_platform_bus_probe(NULL, smdkv310_dt_match_table, NULL);
 }
 
 static char const *smdkv310_dt_compat[] = {
-- 
1.6.6.rc2

[PATCH 4/4] 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

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

[Rob Herring]

Thomas,

On 02/06/2011 07:17 AM, 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(-)
>
> 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

The ifdefs aren't necessary if you pick-up Grant's patch in 
devicetree/next branch.

Rob