ARM: SPEAr600: Add device-tree support to SPEAr600 boards

This patch adds a generic target for SPEAr600 board that can be configured via the device-tree. Currently only interrupts are configured via device-tree. Other peripheral devices (e.g. ethernet, I2C, SMI flash, FSMC NAND flash etc) will follow in later patches.

Only the spear600-evb is currently supported. Other SPEAr600 based boards will follow later.

Signed-off-by Stefan Roese <[email protected]>

Cc Viresh Kumar <[email protected]>

arch/arm/mach-spear6xx/board-dt.c new file +16 -0

+}
+
+static const char *spear600_dt_board_compat[] = {
+	"st,spear600-evb",
+	NULL
+};
+
+static const struct of_device_id vic_of_match[] __initconst = {
+	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
+	{ /* Sentinel */ }
+};
+
+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}

I think you should add all the code from this file to spear6xx.c instead of adding a new file. We can then delete the spear600.c and spear600_evb.c files once the DT support has matured.

Quoted 3 lines · Arnd Bergmann

I think you should add all the code from this file to spear6xx.c instead of adding a new file. We can then delete the spear600.c and spear600_evb.c files once the DT support has matured.

But what if have another board in future, and this is what Stefan is going to do soon, as they have another board.

So we may actually move this code to spear600_evb.c instead.

Sorry for any bad suggestions, i am new to DT. :)

No problem, we are all learning things as we are working on them.

The idea with DT is that you no longer need any board files because all of the information in those files is now passed as a data structure to the kernel at boot time. Platforms that are fully converted to DT don't have any board files but just have one DT_MACHINE_START entry for all of them in the common platform files. Since spear600 has very little board specific contents at the moment, it is very easy to get to this point now, and that will actually help support other boards without having to do more patches to add board files.

When this work is completed, we might actually be able to extend the spear6xx DT code to also support spear3xx and spear13xx, renaming it to mach-spear. In that case, we would probably need one DT_MACHINE_START entry for each family and would not be able to actually build 13xx together with the other ones, but I guess a lot of the infrastructure could be shared.

I would suggest you read Documentation/devicetree/booting-without-of.txt to get a better understanding of what this is all about.

Quoted 11 lines · Stefan Roese

This patch adds a generic target for SPEAr600 board that can be configured via the device-tree. Currently only interrupts are configured via device-tree. Other peripheral devices (e.g. ethernet, I2C, SMI flash, FSMC NAND flash etc) will follow in later patches.

Only the spear600-evb is currently supported. Other SPEAr600 based boards will follow later.

Signed-off-by Stefan Roese <[email protected]>

Cc Viresh Kumar <[email protected]>

Hi Stefan,

This is very cool, welcome onboard for the DT conversion with spear600!

arch/arm/mach-spear6xx/board-dt.c line 32

Quoted 7 lines

arch/arm/mach-spear6xx/board-dt.c new file +7 -0

+static struct amba_device *amba_devs[] __initdata = {
+	&gpio_device[0],
+	&gpio_device[1],
+	&gpio_device[2],
+	&uart_device[0],
+	&uart_device[1],
+};

I would suggest you convert these to DT next so you can remove the amba_devs list. Which devices are these? If they are pl061 and pl010/pl011, the binding should be really easy to do.

Review

Quoted 2 lines

(quoted diff) +2 -0

+static struct platform_device *plat_devs[] __initdata = {
+};

This could be dropped right away, because you would never add anything here.

arch/arm/mach-spear6xx/board-dt.c line 42

Quoted 6 lines

arch/arm/mach-spear6xx/board-dt.c new file +6 -0

+static void __init spear600_dt_init(void)
+{
+	unsigned int i;
+
+	/* call spear600 machine init function */
+	spear600_init();

spear600_init currently is an empty function, I think you can drop that one too.

arch/arm/mach-spear6xx/board-dt.c line 49

Quoted 6 lines

arch/arm/mach-spear6xx/board-dt.c new file +6 -0

+	/* Add Platform Devices */
+	platform_add_devices(plat_devs, ARRAY_SIZE(plat_devs));
+
+	/* Add Amba Devices */
+	for (i = 0; i < ARRAY_SIZE(amba_devs); i++)
+		amba_device_register(amba_devs[i], &iomem_resource);

So although all of this can go away soon, you will have to add a call to of_platform_populate() here in order to add the devices from the device tree.

Quoted 16 lines

arch/arm/mach-spear6xx/board-dt.c new file +16 -0

+}
+
+static const char *spear600_dt_board_compat[] = {
+	"st,spear600-evb",
+	NULL
+};
+
+static const struct of_device_id vic_of_match[] __initconst = {
+	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
+	{ /* Sentinel */ }
+};
+
+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}

arch/arm/mach-spear6xx/board-dt.c line 75

Quoted 10 lines

arch/arm/mach-spear6xx/board-dt.c new file +10 -0

+
+DT_MACHINE_START(SPEAR600_DT, "ST-SPEAR600-DT")
+	.map_io		=	spear6xx_map_io,
+	.init_irq	=	spear6xx_dt_init_irq,
+	.handle_irq	=	vic_handle_irq,
+	.timer		=	&spear6xx_timer,
+	.init_machine	=	spear600_dt_init,
+	.restart	=	spear_restart,
+	.dt_compat	=	spear600_dt_board_compat,
+MACHINE_END

Since there is only one upstream board file and that is for the same board, we can soon collapse all of it into the base platform support.

I think you should add all the code from this file to spear6xx.c instead of adding a new file. We can then delete the spear600.c and spear600_evb.c files once the DT support has matured.

Quoted 13 lines

This is very cool, welcome onboard for the DT conversion with spear600!

arch/arm/mach-spear6xx/board-dt.c line 32

Quoted 7 lines

arch/arm/mach-spear6xx/board-dt.c new file +7 -0

+static struct amba_device *amba_devs[] __initdata = {
+	&gpio_device[0],
+	&gpio_device[1],
+	&gpio_device[2],
+	&uart_device[0],
+	&uart_device[1],
+};

I would suggest you convert these to DT next so you can remove the amba_devs list. Which devices are these? If they are pl061 and pl010/pl011, the binding should be really easy to do.

Yes. I really wanted to do that. But from a quick look at the pl011 driver (drivers/tty/serial/amba-pl011.c), this driver doesn't support DT probing. I might have missed something here though. And ideas?

Quoted 6 lines

Since there is only one upstream board file and that is for the same board, we can soon collapse all of it into the base platform support.

I think you should add all the code from this file to spear6xx.c instead of adding a new file. We can then delete the spear600.c and spear600_evb.c files once the DT support has matured.

Sounds like a plan. I'll rework the patch soon.

Quoted 7 lines

Quoted 3 lines

I would suggest you convert these to DT next so you can remove the amba_devs list. Which devices are these? If they are pl061 and pl010/pl011, the binding should be really easy to do.

Yes. I really wanted to do that. But from a quick look at the pl011 driver (drivers/tty/serial/amba-pl011.c), this driver doesn't support DT probing. I might have missed something here though. And ideas?

amba primecell devices don't actually need to register to a "compatible" property, they are probed using the primecell ID, and the device tree is just used to tell the system about memory and IRQ resources.

The pl061 driver has support for setting the gc.base and irq_base using device tree instead of the platform data and that might be enough.

Quoted 8 lines

Quoted 6 lines

Since there is only one upstream board file and that is for the same board, we can soon collapse all of it into the base platform support.

I think you should add all the code from this file to spear6xx.c instead of adding a new file. We can then delete the spear600.c and spear600_evb.c files once the DT support has matured.

Sounds like a plan. I'll rework the patch soon.

Great!

Don't worry about the the gpio and uart devices if they are not in the first initial version. I do think that they should be done fairly soon though, before we get into most of the other devices.

It's ok if you put a lot of the other devices in the dts file though, like the ethernet device, it gives a better overview of what is actually there, even if the driver does not actually use it yet.

Quoted 8 lines · On Wednesday 14 March 2012 10:48:44 Arnd Bergmann wrote:

Quoted 7 lines

Quoted 3 lines

I would suggest you convert these to DT next so you can remove the amba_devs list. Which devices are these? If they are pl061 and pl010/pl011, the binding should be really easy to do.

Yes. I really wanted to do that. But from a quick look at the pl011 driver (drivers/tty/serial/amba-pl011.c), this driver doesn't support DT probing. I might have missed something here though. And ideas?

Quoted 3 lines

amba primecell devices don't actually need to register to a "compatible" property, they are probed using the primecell ID, and the device tree is just used to tell the system about memory and IRQ resources.

Ahh, I see. Let me see, if I can get this working...

Quoted 7 lines

Don't worry about the the gpio and uart devices if they are not in the first initial version. I do think that they should be done fairly soon though, before we get into most of the other devices.

It's ok if you put a lot of the other devices in the dts file though, like the ethernet device, it gives a better overview of what is actually there, even if the driver does not actually use it yet.

My initial idea was to push the devices once their DT support is accepted. Otherwise the bindings are still not settled.

Quoted 6 lines

Quoted 4 lines

amba primecell devices don't actually need to register to a "compatible" property, they are probed using the primecell ID, and the device tree is just used to tell the system about memory and IRQ resources.

Ahh, I see. Let me see, if I can get this working...

I just saw that there is a patch series for pl061 that Rob Herring did at

git://sources.calxeda.com/kernel/linux.git irqdomain-for-grant

Please have a look at that branch first.

Quoted 10 lines

Quoted 7 lines

Don't worry about the the gpio and uart devices if they are not in the first initial version. I do think that they should be done fairly soon though, before we get into most of the other devices.

It's ok if you put a lot of the other devices in the dts file though, like the ethernet device, it gives a better overview of what is actually there, even if the driver does not actually use it yet.

My initial idea was to push the devices once their DT support is accepted. Otherwise the bindings are still not settled.

Right, that makes sense. However, you can add a property containing

	status = "disabled;

so that the device is visible in the source but no platform device gets generated. That would have no consequences at run time but can be helpful for review.

Quoted 5 lines

I just saw that there is a patch series for pl061 that Rob Herring did at

git://sources.calxeda.com/kernel/linux.git irqdomain-for-grant

Please have a look at that branch first.

That's for GPIO interrupt support, right? I'll postpone this GPIO interrupt support to a later patch. This one will "only" support the plain GPIO's.

Quoted 5 lines · Stefan Roese

Quoted 4 lines · On Wednesday 14 March 2012 10:48:44 Arnd Bergmann wrote:

Quoted 4 lines

Quoted 3 lines

I would suggest you convert these to DT next so you can remove the amba_devs list. Which devices are these? If they are pl061 and pl010/pl011, the binding should be really easy to do.

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

Yes. I really wanted to do that. But from a quick look at the pl011 driver (drivers/tty/serial/amba-pl011.c), this driver doesn't support DT probing. I might have missed something here though. And ideas?

amba primecell devices don't actually need to register to a "compatible" property, they are probed using the primecell ID, and the device tree is just used to tell the system about memory and IRQ resources.

Quoted 1 line

Ahh, I see. Let me see, if I can get this working...

I just saw that there is a patch series for pl061 that Rob Herring did at

git://sources.calxeda.com/kernel/linux.git irqdomain-for-grant

Please have a look at that branch first.

Quoted 10 lines

Quoted 7 lines

Don't worry about the the gpio and uart devices if they are not in the first initial version. I do think that they should be done fairly soon though, before we get into most of the other devices.

It's ok if you put a lot of the other devices in the dts file though, like the ethernet device, it gives a better overview of what is actually there, even if the driver does not actually use it yet.

My initial idea was to push the devices once their DT support is accepted. Otherwise the bindings are still not settled.

Right, that makes sense. However, you can add a property containing

	status = "disabled;

so that the device is visible in the source but no platform device gets generated. That would have no consequences at run time but can be helpful for review.

Arnd

arch/arm/boot/dts/spear600-evb.dts new file +10 -0

+ * Copyright 2012 Stefan Roese <[email protected]>
+ *
+ * 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
+ */
+

arch/arm/mach-spear6xx/Kconfig +7 -0

index ff4ae5b..7777f72 100644
@@ -11,6 +11,13 @@ config BOARD_SPEAR600_EVB
 	help
 	  Supports ST SPEAr600 Evaluation Board
 
+config BOARD_SPEAR600_DT
+	bool "SPEAr600 generic board configured via device-tree"
+	select MACH_SPEAR600
+	select USE_OF
+	help
+	  Supports ST SPEAr600 boards configured via the device-tree
+
 endmenu
 
 config MACH_SPEAR600

arch/arm/mach-spear6xx/Makefile +1 -0

index cc1a4d8..e2d79b8 100644
@@ -10,3 +10,4 @@ obj-$(CONFIG_MACH_SPEAR600) += spear600.o
 
 # spear600 boards files
 obj-$(CONFIG_BOARD_SPEAR600_EVB) += spear600_evb.o
+obj-$(CONFIG_BOARD_SPEAR600_DT) += board-dt.o

arch/arm/mach-spear6xx/board-dt.c new file +0 -0

new file mode 100644
index 0000000..ee4ff33

arch/arm/mach-spear6xx/board-dt.c new file +37 -0

+#include <asm/hardware/vic.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+#include <mach/generic.h>
+#include <mach/hardware.h>
+
+static struct amba_device *amba_devs[] __initdata = {
+	&gpio_device[0],
+	&gpio_device[1],
+	&gpio_device[2],
+	&uart_device[0],
+	&uart_device[1],
+};
+
+static struct platform_device *plat_devs[] __initdata = {
+};
+
+static void __init spear600_dt_init(void)
+{
+	unsigned int i;
+
+	/* call spear600 machine init function */
+	spear600_init();
+
+	/* Add Platform Devices */
+	platform_add_devices(plat_devs, ARRAY_SIZE(plat_devs));
+
+	/* Add Amba Devices */
+	for (i = 0; i < ARRAY_SIZE(amba_devs); i++)
+		amba_device_register(amba_devs[i], &iomem_resource);
+}
+
+static const char *spear600_dt_board_compat[] = {
+	"st,spear600-evb",
+	NULL
+};
+

Other than Arnd's comments:

arch/arm/boot/dts/spear600-evb.dts line 4

Quoted 10 lines · Stefan Roese

(quoted diff) +0 -0

b/arch/arm/boot/dts/spear600-evb.dts new file mode 100644 index 0000000..f92d099

arch/arm/boot/dts/spear600-evb.dts new file +4 -0

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *

Just for knowledge, why are above two here? Is including them suggested for all dts files?

Quoted 50 lines

arch/arm/boot/dts/spear600-evb.dts new file +10 -0

+ * Copyright 2012 Stefan Roese <[email protected]>
+ *
+ * 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
+ */
+

(quoted diff) +0 -0

b/arch/arm/mach-spear6xx/Kconfig index ff4ae5b..7777f72 100644

arch/arm/mach-spear6xx/Kconfig +7 -0

@@ -11,6 +11,13 @@ config BOARD_SPEAR600_EVB
 
 	help
 
 	  Supports ST SPEAr600 Evaluation Board
 
+config BOARD_SPEAR600_DT
+	bool "SPEAr600 generic board configured via device-tree"
+	select MACH_SPEAR600
+	select USE_OF
+	help
+	  Supports ST SPEAr600 boards configured via the device-tree
+
 

endmenu

config MACH_SPEAR600

(quoted diff) +0 -0

b/arch/arm/mach-spear6xx/Makefile index cc1a4d8..e2d79b8 100644

arch/arm/mach-spear6xx/Makefile +1 -0

@@ -10,3 +10,4 @@ obj-$(CONFIG_MACH_SPEAR600) += spear600.o
 
 # spear600 boards files
 obj-$(CONFIG_BOARD_SPEAR600_EVB) += spear600_evb.o
 
+obj-$(CONFIG_BOARD_SPEAR600_DT) += board-dt.o

(quoted diff) +0 -0

b/arch/arm/mach-spear6xx/board-dt.c new file mode 100644 index 0000000..ee4ff33

arch/arm/mach-spear6xx/board-dt.c line 7

Quoted 7 lines

arch/arm/mach-spear6xx/board-dt.c new file +7 -0

@@ -0,0 +1,75 @@
+/*
+ * arch/arm/mach-spear6xx/board-dt.c
+ *
+ * Generic SPEAr600 platform support
+ *
+ * Copyright (C) 2009 ST Microelectronics
+ * Viresh Kumar<[email protected]>

                  ^

Thanks for this. Can add space here between r & <.

Quoted 7 lines

arch/arm/mach-spear6xx/board-dt.c new file +7 -0

+ *
+ * Copyright 2012 Stefan Roese <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */

arch/arm/mach-spear6xx/board-dt.c line 19

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>

keeping header files in alphabetical order makes life easy at later stages. :)

Quoted 42 lines

arch/arm/mach-spear6xx/board-dt.c new file +42 -0

+#include <asm/hardware/vic.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+#include <mach/generic.h>
+#include <mach/hardware.h>
+
+static struct amba_device *amba_devs[] __initdata = {
+	&gpio_device[0],
+	&gpio_device[1],
+	&gpio_device[2],
+	&uart_device[0],
+	&uart_device[1],
+};
+
+static struct platform_device *plat_devs[] __initdata = {
+};
+
+static void __init spear600_dt_init(void)
+{
+	unsigned int i;
+
+	/* call spear600 machine init function */
+	spear600_init();
+
+	/* Add Platform Devices */
+	platform_add_devices(plat_devs, ARRAY_SIZE(plat_devs));
+
+	/* Add Amba Devices */
+	for (i = 0; i < ARRAY_SIZE(amba_devs); i++)
+		amba_device_register(amba_devs[i], &iomem_resource);
+}
+
+static const char *spear600_dt_board_compat[] = {
+	"st,spear600-evb",
+	NULL
+};
+
+static const struct of_device_id vic_of_match[] __initconst = {
+	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
+	{ /* Sentinel */ }
+};
+

arch/arm/mach-spear6xx/board-dt.c line 66

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}
+

What about adding this routine in vic.c file, which can then be used by all platforms, instead of replicating this code.

arch/arm/mach-spear6xx/board-dt.c line 67

Quoted 1 line

arch/arm/mach-spear6xx/board-dt.c new file +1 -0

+DT_MACHINE_START(SPEAR600_DT, "ST-SPEAR600-DT")

It looks this is not a generic file for all boards, but for a single board? And that would be evb only?

If above is true, we may name this file and internal stuff more appropriately.

Quoted 10 lines

arch/arm/mach-spear6xx/board-dt.c new file +10 -0

+static const char *spear600_dt_board_compat[] = {
+	"st,spear600-evb",
+	NULL
+};
+
+static const struct of_device_id vic_of_match[] __initconst = {
+	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
+	{ /* Sentinel */ }
+};
+

arch/arm/mach-spear6xx/board-dt.c line 66

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}
+

What about adding this routine in vic.c file, which can then be used by all platforms, instead of replicating this code.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static const struct of_device_id vic_of_match[] __initconst = {
+	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
+	{ /* Sentinel */ }
+};
+

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c line 66

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}
+

What about adding this routine in vic.c file, which can then be used by all platforms, instead of replicating this code.

We can't move vic_of_match to the vic.c, since there are differences between the boards using it. And only moving spear6xx_dt_init_irq() (renamed of course) doesn't seem worth it.

Quoted 14 lines · Stefan Roese

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static const struct of_device_id vic_of_match[] __initconst = {
+	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
+	{ /* Sentinel */ }
+};
+

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c line 66

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}
+

What about adding this routine in vic.c file, which can then be used by all platforms, instead of replicating this code.

Quoted 3 lines

We can't move vic_of_match to the vic.c, since there are differences between the boards using it. And only moving spear6xx_dt_init_irq() (renamed of course) doesn't seem worth it.

I knew this. Actually my thought was, we have two compatible strings for
vic: pl190 and pl192.

So, create two vic_of_match* arrays and two vic*_dt_init_irq() routines. That will solve the issue i believe.

I still don't think its worth it. But please feel free to send an add-on patch, to "clean this up".

Quoted 19 lines · On Thursday 15 March 2012 10:00:27 Viresh Kumar wrote:

Quoted 17 lines · Stefan Roese

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static const struct of_device_id vic_of_match[] __initconst = {
+     { .compatible = "arm,pl190-vic", .data = vic_of_init, },
+     { /* Sentinel */ }
+};
+

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c line 66

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static void __init spear6xx_dt_init_irq(void)
+{
+     of_irq_init(vic_of_match);
+}
+

What about adding this routine in vic.c file, which can then be used by all platforms, instead of replicating this code.

We can't move vic_of_match to the vic.c, since there are differences between the boards using it. And only moving spear6xx_dt_init_irq() (renamed of course) doesn't seem worth it.

Quoted 5 lines

I knew this. Actually my thought was, we have two compatible strings for
vic: pl190 and pl192.

So, create two vic_of_match* arrays and two vic*_dt_init_irq() routines. That will solve the issue i believe.

I still don't think its worth it. But please feel free to send an add-on patch, to "clean this up".

Quoted 23 lines · Stefan Roese

Quoted 22 lines · On Thursday 15 March 2012 10:00:27 Viresh Kumar wrote:

Quoted 14 lines · Stefan Roese

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static const struct of_device_id vic_of_match[] __initconst = {
+     { .compatible = "arm,pl190-vic", .data = vic_of_init, },
+     { /* Sentinel */ }
+};
+

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c line 66

Quoted 5 lines

arch/arm/mach-spear6xx/board-dt.c new file +5 -0

+static void __init spear6xx_dt_init_irq(void)
+{
+     of_irq_init(vic_of_match);
+}
+

What about adding this routine in vic.c file, which can then be used by all platforms, instead of replicating this code.

Quoted 3 lines

We can't move vic_of_match to the vic.c, since there are differences between the boards using it. And only moving spear6xx_dt_init_irq() (renamed of course) doesn't seem worth it.

I knew this. Actually my thought was, we have two compatible strings for
vic: pl190 and pl192.

Quoted 5 lines

Quoted 2 lines

So, create two vic_of_match* arrays and two vic*_dt_init_irq() routines. That will solve the issue i believe.

I still don't think its worth it. But please feel free to send an add-on patch, to "clean this up".

What's wrong with one vic_dt_init_irq() function and an array with two entries?

Quoted 2 lines

What's wrong with one vic_dt_init_irq() function and an array with two entries?

Hey Arnd,

Actually i was doing 3xx DT porting and again came to this thing. I believe what you suggested should be fine too as the actual vic model/name is going to be passed from .dts file.

If this is acceptable to you, i will add patch for both vic & gic in my patchset.

Let's make sure we're on the same page with the process though. I hope you're aware that I took Stefan's spear6xx v5 patch.

Just let me know how you want to handle
these things in the future -- the normal way would be to always let you forward the patches to me, rather than having them applied to arm-soc directly.

I think spear is simple and clean enough that it can serve as an example for others doing DT conversion. Do you also plan to unify 3xx and 6xx? I think there is hardly anything left in mach-spear6xx that justifies having a separate platform, especially since with the DT conversion done, we can actually remove most of the header file contents as well.

Quoted 4 lines · Arnd Bergmann

Just let me know how you want to handle
these things in the future -- the normal way would be to always let you forward the patches to me, rather than having them applied to arm-soc directly.

Hmmm. I don't know how exactly it works, but maybe you can add me as an custodian for SPEAr, with a git repo somewhere in kernel.org (?). So that i can apply patches directly to my branch, you pull it. What do you say?

Quoted 5 lines

I think spear is simple and clean enough that it can serve as an example for others doing DT conversion. Do you also plan to unify 3xx and 6xx? I think there is hardly anything left in mach-spear6xx that justifies having a separate platform, especially since with the DT conversion done, we can actually remove most of the header file contents as well.

I issue with us was that we were not able to push stuff mainline in time and were too busy supporting customers.

We have lot of patches locally, that we would like to upstream. But it can't happen
without updating it with new expectations of list. I actually wanted to ask you few
things:

- DT: Obviously we would have this in our patches
- Pinmux: I would remove our padmux and use pinmux, but what about its DT support.
I know stephen had few patches for it, but don't know there
progress. I don't want to
do stuff twice. First add support for it in board and the *.dts files.

- Clock Framework: Is common clock framework ready for use or can we update our local clock framework?

I will see if merging 3xx & 6xx is feasible. If yes, then i will surely move in that direction.
I always like cleaning stuff. :)

I would be adding 13xx (cortex-A9) patches again later, once done with 3xx.

Quoted 5 lines · viresh kumar

- Pinmux: I would remove our padmux and use pinmux, but what about its DT support.
? I know stephen had few patches for it, but don't know there progress. I don't want to
? do stuff twice. First add support for it in board and the *.dts files.

AFAICT, the pinmux DT binding should be ready for v3.5, I don't think it made it into v3.4.

I've looked at this earlier today out of curiosity and found:

clock.c: almost identical, can be easily merged, or both copies ? ? ? ? kept around during transition.

misc_regs.h: can be merged into clock.c

Once you have taken care of these, files, you can enable building 3xx and 6xx together, and move the contents of mach-spear3xx, mach-spear6xx and plat-spear into one directory.

Ok, excellent. Note that there is no requirement to do it all in the order you line out above. You can always get support for new stuff into the existing platform without cleaning up all of it right away, as long as you have a good balance between cleanups and new functionality.

In particular, you don't have to start using pinctrl and the common clk immediately for spear3xx, although you might find that it helps you.

For spear13xx, the platform being completely new, I would however ask you to use those from the start.

Quoted 7 lines · Arnd Bergmann

Quoted 5 lines · viresh kumar

- Pinmux: I would remove our padmux and use pinmux, but what about its DT support.
I know stephen had few patches for it, but don't know there
progress. I don't want to
do stuff twice. First add support for it in board and the *.dts files.

Quoted 2 lines

AFAICT, the pinmux DT binding should be ready for v3.5, I don't think it made it into v3.4.

So, i can add stuff in board file for pinmux for now?

Quoted 4 lines

I've looked at this earlier today out of curiosity and found:

clock.c: almost identical, can be easily merged, or both copies
kept around during transition.

Would make it very confusing if we merge these two. Some clock structures are same, but others are not. Mostly differences are in the macros (or bit positions in registers), rather than hierarchy. So, i would like to keep them separate. But still have a look again to see if they can really be merged.

Quoted 1 line

misc_regs.h: can be merged into clock.c

it has more than clock stuff. it is used in pinmux and some other places too, that may not be in mainline for now.

Quoted 3 lines

Once you have taken care of these, files, you can enable building 3xx and 6xx together, and move the contents of mach-spear3xx, mach-spear6xx and plat-spear into one directory.

plat-spear would be required for 13xx too. So, merging 3xx and 6xx only would be possible.

Quoted 4 lines

Ok, excellent. Note that there is no requirement to do it all in the order you line out above. You can always get support for new stuff into the existing platform without cleaning up all of it right away, as long as you have a good balance between cleanups and new functionality.

Ya. I just wanted to get some hands on experience with this new stuff, before touching 13xx.

Quoted 2 lines

In particular, you don't have to start using pinctrl and the common clk immediately for spear3xx, although you might find that it helps you.

But, can i update padmux and clock framework of SPEAr platform? Because without the latest code for both padmux and clock, other code for SPEAr might not be that effective or usable.

Quoted 10 lines · viresh kumar

Quoted 4 lines · Arnd Bergmann

Just let me know how you want to handle
these things in the future -- the normal way would be to always let you forward the patches to me, rather than having them applied to arm-soc directly.

Hmmm. I don't know how exactly it works, but maybe you can add me as an custodian for SPEAr, with a git repo somewhere in kernel.org (?). So that i can apply patches directly to my branch, you pull it. What do you say?

Yes, that would be the ideal way. You are already listed in the MAINTAINERS file, so nothing would change in theory. If you don't have an account on kernel.org yet, see http://www.kernel.org/faq/#account for how to get one. This will require that you have your gpg keys signed by at least one person in the kernel web of trust, so you may have to find people locally who can sign yours and are already signed.

I just noticed that you have git trees on http://git.stlinux.com/,
which I would personally prefer you to use actually. git.kernel.org is most useful for people that don't have their own git servers.

Quoted 3 lines · Arnd Bergmann

I just noticed that you have git trees on http://git.stlinux.com/,
which I would personally prefer you to use actually. git.kernel.org is most useful for people that don't have their own git servers.

Ah, that's even better for me too. I don't really have to manage two trees.

Just to understand the flow, how does this work now? There can be two sources of patches:
- From ST internal developers
- From external developers

Should i apply them all to my tree and then send you a pull request? Or you automatically pull everyday from some specific branch?

Sorry i am new to this. :(

Don't hesitate to ask if you have further process questions, I know this can be confusing and we're doing things much stricter than other maintainters in order to keep up with ~600 patches per merge window.

Regarding the spear3xx patches, I'm looking forward to your patches. I think spear is simple and clean enough that it can serve as an example for others doing DT conversion.

Quoted 3 lines · Arnd Bergmann

Regarding the spear3xx patches, I'm looking forward to your patches. I think spear is simple and clean enough that it can serve as an example for others doing DT conversion.

I was doing DT stuff for 3xx and have a doubt
regarding DT. How are boards identified at runtime now? Earlier we had machine_is_*() to do that.

Also why are both DT_MACHINE_START and MACHINE_START present on some SoC's?

The direct replacement is of_machine_is_compatible(), but there are a lot of cases where it's better to have a local property in the device node that a driver is using.

The direct replacement is of_machine_is_compatible(), but there are a lot of cases where it's better to have a local property in the device node that a driver is using.

Quoted 3 lines · Arnd Bergmann

The direct replacement is of_machine_is_compatible(), but there are a lot of cases where it's better to have a local property in the device node that a driver is using.

Hey Arnd,

I used of_machine_is_compatible() at several places, where it is not working :( Actually all these usages are before a call to of_platform_populate() and it looks the tree is not up by this time.

So, of_machine_is_compatible() always fails. The places where i am using this routine are:
- spear3xx_dt_init(): to call SoC specific of_platform_populate()
- spear3xx_clk_init(): to call SoC specific clk_register()

Can you please suggest what should i do here?

On Tuesday 27 March 2012, Viresh Kumar wrote:
The normal way is to turn around the logic so you don't have to include this test at all. Just have one soc-specific init_machine and map_io function, that calls both soc-specific and shared soc functions, e.g.

The other option would be to try to move stuff to a later point, e.g. don't initialize the clocks until the basic device tree is set up. of_machine_is_compatible() should work at init_early() time, but not at map_io() time. Other platforms set up the clocks at init_early() time. The spear3xx_dt_init function should be called from init_machine(), which happens much later, so I'm pretty sure you can use of_machine_is_compatible() if you can't just avoid it.

In case of the clocks, I think you could already merge all the clk_lookup arrays into one, which would result in a larger kernel image but should do no harm otherwise.

DT should be up but, nodes would be up only once we call of_platform_populate() from this init routine.

Quoted 4 lines · Arnd Bergmann

On Tuesday 27 March 2012, Viresh Kumar wrote:
The normal way is to turn around the logic so you don't have to include this test at all. Just have one soc-specific init_machine and map_io function, that calls both soc-specific and shared soc functions, e.g.

But with that, i need multiple DT_MACHINE_START(). Isn't it? Is this advisable?

Quoted 3 lines

In case of the clocks, I think you could already merge all the clk_lookup arrays into one, which would result in a larger kernel image but should do no harm otherwise.

Actually we can't do it. :(
If i boot 300 then i will also get clocks of 310 and 320 in my clock list. And once i go through this clock list to create clock tree (parent-child relationship), i will try to access hardware registers of 310 & 320, which are just not valid for 300. Kernel Crash!!

Quoted 3 lines · Viresh Kumar

Quoted 2 lines · Arnd Bergmann

On Tuesday 27 March 2012, Viresh Kumar wrote:
The normal way is to turn around the logic so you don't have to

Quoted 1 line

Quoted 1 line

at all. Just have one soc-specific init_machine and map_io function,

Quoted 4 lines

Quoted 1 line

both soc-specific and shared soc functions, e.g.

But with that, i need multiple DT_MACHINE_START(). Isn't it? Is this advisable?

Having one DT_MACHINE_START per soc is ok if it helps you. Of course if

have multiple ones that are basically identical, it's simpler to just combine them.

Quoted 1 line

Quoted 1 line

In case of the clocks, I think you could already merge all the

Quoted 1 line

Quoted 1 line

arrays into one, which would result in a larger kernel image but

Quoted 4 lines

Quoted 1 line

do no harm otherwise.

Actually we can't do it. :(
If i boot 300 then i will also get clocks of 310 and 320 in my clock

Quoted 3 lines

And once i go through this clock list to create clock tree (parent-child relationship), i will try to access hardware registers of 310 & 320, which are just not valid for 300. Kernel Crash!!

Ok, I see. BTW, have you tried what I first recommended to you, which is to check the compatible property of the clock node instead of the machine?

Obviously that only works when you initialize the clocks from init_early() instead of from map_io(), but I think that would be the cleanest choice if you want to have single function with run-time dependencies.

Quoted 11 lines · Arnd Bergmann

Quoted 8 lines · Viresh Kumar

Quoted 4 lines · Arnd Bergmann

On Tuesday 27 March 2012, Viresh Kumar wrote:
The normal way is to turn around the logic so you don't have to include this test at all. Just have one soc-specific init_machine and map_io function, that calls both soc-specific and shared soc functions, e.g.

But with that, i need multiple DT_MACHINE_START(). Isn't it? Is this advisable?

Quoted 3 lines

Having one DT_MACHINE_START per soc is ok if it helps you. Of course if you have multiple ones that are basically identical, it's simpler to just combine them.

That's what i have done in V1. And then only i came to The first problem i mentioned.

Quoted 3 lines

Obviously that only works when you initialize the clocks from init_early() instead of from map_io(), but I think that would be the cleanest choice if you want to have single function with run-time dependencies.

But then why not use machine_init() for doing this too.

That would be even better. I think we should generally try do initialization as late as possible, ideally from initcalls. The init_early() function should only be used for code that absolutely has to be run from setup_arch() and no later, while the map_io() function should ideally only be used to set up the static mappings and nothing else.

Quoted 5 lines · Arnd Bergmann

That would be even better. I think we should generally try do initialization as late as possible, ideally from initcalls. The init_early() function should only be used for code that absolutely has to be run from setup_arch() and no later, while the map_io() function should ideally only be used to set up the static mappings and nothing else.

I remember now why we do it in map_io().
Timers need clock support and they are up before machine_init() is called.

So, i believe i must have DT_MACHINE_START per SoC.

I will post V2 for 3xx DT support today, but can i have initial level of review from you, so that i can fixup minor issues in V2 only?

This patch adds a generic target for SPEAr600 board that can be configured via the device-tree. Currently only interrupts are configured via device-tree. Other peripheral devices (e.g. ethernet, I2C, SMI flash, FSMC NAND flash etc) will follow in later patches.

Only the spear600-evb is currently supported. Other SPEAr600 based boards will follow later.

Signed-off-by Stefan Roese <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 Documentation/devicetree/bindings/arm/spear.txt |    6 ++
 arch/arm/boot/dts/spear600-evb.dts              |   23 +++++++
 arch/arm/boot/dts/spear600.dtsi                 |   49 +++++++++++++++
 arch/arm/mach-spear6xx/Kconfig                  |    7 +++
 arch/arm/mach-spear6xx/Makefile                 |    1 +
 arch/arm/mach-spear6xx/board-dt.c               |   75 +++++++++++++++++++++++
 6 files changed, 161 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/spear.txt
 create mode 100644 arch/arm/boot/dts/spear600-evb.dts
 create mode 100644 arch/arm/boot/dts/spear600.dtsi
 create mode 100644 arch/arm/mach-spear6xx/board-dt.c

Documentation/devicetree/bindings/arm/spear.txt new file +6 -0

new file mode 100644
index 0000000..8e9f83e
@@ -0,0 +1,6 @@
+ST SPEAr Platforms Device Tree Bindings
+---------------------------------------
+
+SPEAr600 EVB (Evaluation Board)
+Required root node properties:
+    - compatible = "st,spear600-evb";

arch/arm/boot/dts/spear600-evb.dts new file +0 -0

new file mode 100644
index 0000000..f92d099

arch/arm/boot/dts/spear600-evb.dts new file +7 -0

+ *
+ * Copyright 2012 Stefan Roese <[email protected]>
+ *
+ * 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:
+ *

arch/arm/boot/dts/spear600.dtsi new file +16 -0

new file mode 100644
index 0000000..82b086d
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *
+ * Copyright 2012 Stefan Roese <[email protected]>
+ *
+ * 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/ "skeleton.dtsi"
+

arch/arm/boot/dts/spear600.dtsi new file +23 -0

+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges;
+
+		vic0: interrupt-controller at f1100000 {
+			compatible = "arm,pl190-vic";
+			interrupt-controller;
+			reg = <0xf1100000 0x1000>;
+			#interrupt-cells = <1>;
+		};
+
+		vic1: interrupt-controller at f1000000 {
+			compatible = "arm,pl190-vic";
+			interrupt-controller;
+			reg = <0xf1000000 0x1000>;
+			#interrupt-cells = <1>;
+		};
+	};
+};

arch/arm/mach-spear6xx/Kconfig +7 -0

index ff4ae5b..7777f72 100644
@@ -11,6 +11,13 @@ config BOARD_SPEAR600_EVB
 	help
 	  Supports ST SPEAr600 Evaluation Board
 
+config BOARD_SPEAR600_DT
+	bool "SPEAr600 generic board configured via device-tree"
+	select MACH_SPEAR600
+	select USE_OF
+	help
+	  Supports ST SPEAr600 boards configured via the device-tree
+
 endmenu
 
 config MACH_SPEAR600

arch/arm/mach-spear6xx/Makefile +1 -0

index cc1a4d8..e2d79b8 100644
@@ -10,3 +10,4 @@ obj-$(CONFIG_MACH_SPEAR600) += spear600.o
 
 # spear600 boards files
 obj-$(CONFIG_BOARD_SPEAR600_EVB) += spear600_evb.o
+obj-$(CONFIG_BOARD_SPEAR600_DT) += board-dt.o

arch/arm/mach-spear6xx/board-dt.c new file +22 -0

new file mode 100644
index 0000000..ee4ff33
@@ -0,0 +1,75 @@
+/*
+ * arch/arm/mach-spear6xx/board-dt.c
+ *
+ * Generic SPEAr600 platform support
+ *
+ * Copyright (C) 2009 ST Microelectronics
+ * Viresh Kumar<[email protected]>
+ *
+ * Copyright 2012 Stefan Roese <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <asm/hardware/vic.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>

arch/arm/mach-spear6xx/board-dt.c new file +13 -0

+
+static struct platform_device *plat_devs[] __initdata = {
+};
+
+static void __init spear600_dt_init(void)
+{
+	unsigned int i;
+
+	/* call spear600 machine init function */
+	spear600_init();
+
+	/* Add Platform Devices */
+	platform_add_devices(plat_devs, ARRAY_SIZE(plat_devs));