RE: [RFC PATCH 5/7] ARM: s3c64xx: Add board file for boot using Device Tree

Tomasz Figa wrote:

and

+ * at some point, the drivers should be capable of parsing all the

platform

+ * data from the device tree.
+ */
+static const struct of_dev_auxdata s3c64xx_auxdata_lookup[] __initconst =
{
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART0,
+				"s3c6400-uart.0", NULL),
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART1,
+				"s3c6400-uart.1", NULL),
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART2,
+				"s3c6400-uart.2", NULL),
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART3,
+				"s3c6400-uart.3", NULL),

I'd prefer to use 0x7F005000 instead of S3C_PA_UART0 so that we can remove
the static definitions at the map.h

+	{},
+};
+
+static void __init s3c64xx_dt_map_io(void)
+{
+	s3c64xx_init_io(NULL, 0);
+	s3c24xx_init_clocks(12000000);

The value of xtal clock for s3c24xx_init_clocks() should be defined via dt
according to board condition _next_time_  because it can be different on
each board, it's same on current s3c64xx boards though.

+}
+
+static void __init s3c64xx_dt_machine_init(void)
+{
+	of_platform_populate(NULL, of_default_bus_match_table,
+				s3c64xx_auxdata_lookup, NULL);
+}
+
+static char const *s3c6400_dt_compat[] __initdata = {
+	"samsung,s3c6400",
+	NULL
+};
+
+DT_MACHINE_START(S3C6400_DT, "Samsung S3C6400 (Flattened Device
Tree)")
+	/* Maintainer: Tomasz Figa [off-list ref] */
+	.init_irq	= s3c6400_init_irq,
+	.map_io		= s3c64xx_dt_map_io,
+	.handle_irq	= vic_handle_irq,
+	.init_machine	= s3c64xx_dt_machine_init,
+	.init_late	= s3c64xx_init_late,
+	.timer		= &s3c24xx_timer,
+	.dt_compat	= s3c6400_dt_compat,
+	.restart        = s3c64xx_restart,
+MACHINE_END
+
+static char const *s3c6410_dt_compat[] __initdata = {
+	"samsung,s3c6410",
+	NULL
+};

I think, we don't need separation.

+static char const *s3c64xx_dt_compat[] __initdata = {
+	"samsung,s3c6400",
+	"samsung,s3c6410",
+	NULL
+};

+
+DT_MACHINE_START(S3C6410_DT, "Samsung S3C6410 (Flattened Device
Tree)")
+	/* Maintainer: Tomasz Figa [off-list ref] */
+	.init_irq	= s3c6410_init_irq,
+	.map_io		= s3c64xx_dt_map_io,
+	.handle_irq	= vic_handle_irq,
+	.init_machine	= s3c64xx_dt_machine_init,
+	.init_late	= s3c64xx_init_late,
+	.timer		= &s3c24xx_timer,
+	.dt_compat	= s3c6410_dt_compat,

+	.dt_compat	= s3c64xx_dt_compat,

+	.restart        = s3c64xx_restart,
+MACHINE_END
--
1.8.1

Hi Kukjin,

On Wednesday 09 of January 2013 16:24:42 Kukjin Kim wrote:

Tomasz Figa wrote:

quoted

This patch adds board file that will be used to boot S3C64xx-based
boards using Device Tree.

Signed-off-by: Tomasz Figa <redacted>
---

 arch/arm/mach-s3c64xx/Kconfig           | 13 +++++
 arch/arm/mach-s3c64xx/Makefile          |  1 +
 arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c | 93

+++++++++++++++++++++++++++++++++

 3 files changed, 107 insertions(+)
 create mode 100644 arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c

diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-

s3c64xx/Kconfig
index 131c862..a43b3f7 100644

--- a/arch/arm/mach-s3c64xx/Kconfig
+++ b/arch/arm/mach-s3c64xx/Kconfig

@@ -309,3 +309,16 @@ config MACH_WLF_CRAGG_6410

 	select SAMSUNG_GPIO_EXTRA128
 	help
 	
 	  Machine support for the Wolfson Cragganmore S3C6410 variant.

+
+config MACH_S3C64XX_DT
+	bool "Samsung S3C6400/S3C6410 machine using Device Tree"
+	select CPU_S3C6400
+	select CPU_S3C6410
+	select USE_OF
+	help
+	  Machine support for Samsung S3C6400/S3C6410 machines with

Device Tree
+	  enabled.
+	  Select this if a fdt blob is available for your S3C64XX SoC based
+	  board.
+	  Note: This is under development and not all peripherals can be
+	  supported with this machine file.

diff --git a/arch/arm/mach-s3c64xx/Makefile b/arch/arm/mach-

s3c64xx/Makefile
index f9ce1dc..59b3d06 100644

--- a/arch/arm/mach-s3c64xx/Makefile
+++ b/arch/arm/mach-s3c64xx/Makefile

@@ -58,3 +58,4 @@ obj-$(CONFIG_MACH_SMARTQ7)		+= mach-

smartq7.o

 obj-$(CONFIG_MACH_SMDK6400)		+= mach-smdk6400.o
 obj-$(CONFIG_MACH_SMDK6410)		+= mach-smdk6410.o
 obj-$(CONFIG_MACH_WLF_CRAGG_6410)	+= mach-crag6410.o mach-

crag6410-module.o
+obj-$(CONFIG_MACH_S3C64XX_DT)		+= mach-s3c64xx-dt.o

diff --git a/arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c b/arch/arm/mach-

s3c64xx/mach-s3c64xx-dt.c
new file mode 100644
index 0000000..f5725ce

--- /dev/null
+++ b/arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c

@@ -0,0 +1,93 @@
+/*
+ * Samsung's S3C64XX flattened device tree enabled machine
+ *
+ * Copyright (c) 2012 Tomasz Figa <tomasz.figa@gmail.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/of_platform.h>
+#include <linux/serial_core.h>
+
+#include <asm/mach/arch.h>
+#include <asm/hardware/vic.h>
+#include <mach/map.h>
+
+#include <plat/cpu.h>
+#include <plat/regs-serial.h>
+#include <plat/s5p-time.h>
+
+#include "common.h"
+
+/*
+ * The following lookup table is used to override device names when
devices
+ * are registered from device tree. This is temporarily added to
enable + * device tree support addition for the Exynos4 architecture.
+ *
+ * For drivers that require platform data to be provided from the
machine + * file, a platform data pointer can also be supplied along
with the + * devices names. Usually, the platform data elements that
cannot be parsed
+ * from the device tree by the drivers (example: function pointers)
are + * supplied. But it should be noted that this is a temporary
mechanism

and

quoted

+ * at some point, the drivers should be capable of parsing all the

platform

quoted

+ * data from the device tree.
+ */
+static const struct of_dev_auxdata s3c64xx_auxdata_lookup[]
__initconst = {
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART0,
+				"s3c6400-uart.0", NULL),
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART1,
+				"s3c6400-uart.1", NULL),
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART2,
+				"s3c6400-uart.2", NULL),
+	OF_DEV_AUXDATA("samsung,s3c6400-uart", S3C_PA_UART3,
+				"s3c6400-uart.3", NULL),

I'd prefer to use 0x7F005000 instead of S3C_PA_UART0 so that we can
remove the static definitions at the map.h

Ideally, I would like to completely get rid of this and start cleanly with 
common clock framework, but support for it would be probably based on 
Thomas Abraham's code for Exynos, which would have to be merged first.

So for now I will replace symbolic addresses with numeric ones, as you 
suggested.

quoted

+	{},
+};
+
+static void __init s3c64xx_dt_map_io(void)
+{
+	s3c64xx_init_io(NULL, 0);
+	s3c24xx_init_clocks(12000000);

The value of xtal clock for s3c24xx_init_clocks() should be defined via
dt according to board condition _next_time_  because it can be
different on each board, it's same on current s3c64xx boards though.

This is yet another issue that will be solved by using Common 
Clock Framework.

quoted

+}
+
+static void __init s3c64xx_dt_machine_init(void)
+{
+	of_platform_populate(NULL, of_default_bus_match_table,
+				s3c64xx_auxdata_lookup, NULL);
+}
+
+static char const *s3c6400_dt_compat[] __initdata = {
+	"samsung,s3c6400",
+	NULL
+};
+
+DT_MACHINE_START(S3C6400_DT, "Samsung S3C6400 (Flattened Device
Tree)")
+	/* Maintainer: Tomasz Figa [off-list ref] */
+	.init_irq	= s3c6400_init_irq,
+	.map_io		= s3c64xx_dt_map_io,
+	.handle_irq	= vic_handle_irq,
+	.init_machine	= s3c64xx_dt_machine_init,
+	.init_late	= s3c64xx_init_late,
+	.timer		= &s3c24xx_timer,
+	.dt_compat	= s3c6400_dt_compat,
+	.restart        = s3c64xx_restart,
+MACHINE_END
+
+static char const *s3c6410_dt_compat[] __initdata = {
+	"samsung,s3c6410",
+	NULL
+};

I think, we don't need separation.

These two differ in init_irq callback, but now as I look at it, all the 
data are already being parsed from device tree, so I will just add 
s3c64xx_of_init_irq and make just one DT machine.

Best regards,
Tomasz Figa