[PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[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 <sr-ynQEQJNshbs@public.gmane.org>
Cc: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 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

diff --git a/Documentation/devicetree/bindings/arm/spear.txt b/Documentation/devicetree/bindings/arm/spear.txt
new file mode 100644
index 0000000..8e9f83e
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/spear.txt
@@ -0,0 +1,6 @@
+ST SPEAr Platforms Device Tree Bindings
+---------------------------------------
+
+SPEAr600 EVB (Evaluation Board)
+Required root node properties:
+    - compatible = "st,spear600-evb";
diff --git a/arch/arm/boot/dts/spear600-evb.dts b/arch/arm/boot/dts/spear600-evb.dts
new file mode 100644
index 0000000..f92d099
--- /dev/null
+++ b/arch/arm/boot/dts/spear600-evb.dts
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *
+ * Copyright 2012 Stefan Roese <sr-ynQEQJNshbs@public.gmane.org>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+/include/ "spear600.dtsi"
+
+/ {
+	model = "ST SPEAr600 Evaluation Board";
+	compatible = "st,spear600-evb";
+	#address-cells = <1>;
+	#size-cells = <1>;
+};
diff --git a/arch/arm/boot/dts/spear600.dtsi b/arch/arm/boot/dts/spear600.dtsi
new file mode 100644
index 0000000..82b086d
--- /dev/null
+++ b/arch/arm/boot/dts/spear600.dtsi
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *
+ * Copyright 2012 Stefan Roese <sr-ynQEQJNshbs@public.gmane.org>
+ *
+ * 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"
+
+/ {
+	cpus {
+		cpu@0 {
+			compatible = "arm,arm926ejs";
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0 0>; /* Filled by U-Boot */
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges;
+
+		vic0: interrupt-controller@f1100000 {
+			compatible = "arm,pl190-vic";
+			interrupt-controller;
+			reg = <0xf1100000 0x1000>;
+			#interrupt-cells = <1>;
+		};
+
+		vic1: interrupt-controller@f1000000 {
+			compatible = "arm,pl190-vic";
+			interrupt-controller;
+			reg = <0xf1000000 0x1000>;
+			#interrupt-cells = <1>;
+		};
+	};
+};
diff --git a/arch/arm/mach-spear6xx/Kconfig b/arch/arm/mach-spear6xx/Kconfig
index ff4ae5b..7777f72 100644
--- a/arch/arm/mach-spear6xx/Kconfig
+++ b/arch/arm/mach-spear6xx/Kconfig
@@ -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
diff --git a/arch/arm/mach-spear6xx/Makefile b/arch/arm/mach-spear6xx/Makefile
index cc1a4d8..e2d79b8 100644
--- a/arch/arm/mach-spear6xx/Makefile
+++ b/arch/arm/mach-spear6xx/Makefile
@@ -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
diff --git a/arch/arm/mach-spear6xx/board-dt.c b/arch/arm/mach-spear6xx/board-dt.c
new file mode 100644
index 0000000..ee4ff33
--- /dev/null
+++ b/arch/arm/mach-spear6xx/board-dt.c
@@ -0,0 +1,75 @@
+/*
+ * arch/arm/mach-spear6xx/board-dt.c
+ *
+ * Generic SPEAr600 platform support
+ *
+ * Copyright (C) 2009 ST Microelectronics
+ * Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>
+ *
+ * Copyright 2012 Stefan Roese <sr-ynQEQJNshbs@public.gmane.org>
+ *
+ * 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>
+#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 */ }
+};
+
+static void __init spear6xx_dt_init_irq(void)
+{
+	of_irq_init(vic_of_match);
+}
+
+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
-- 
1.7.9.2

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Tuesday 13 March 2012, Stefan Roese wrote:
> 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 <sr@denx.de>
> Cc: Viresh Kumar <viresh.kumar@st.com>

Hi Stefan,

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

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

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

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

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

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

> +}
> +
> +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);
> +}
> +
> +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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/13/2012 10:14 PM, Arnd Bergmann wrote:
> 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. :)

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 14 March 2012, Viresh Kumar wrote:
> On 3/13/2012 10:14 PM, Arnd Bergmann wrote:
> > 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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/14/2012 3:28 PM, Arnd Bergmann wrote:
> 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.

You pointed exactly to the doubt i had.
That was very helpful. I really appreciate it.

Thanks a lot.

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Stefan Roese]

Hi Arnd,

On Tuesday 13 March 2012 17:44:11 Arnd Bergmann wrote:
> On Tuesday 13 March 2012, Stefan Roese wrote:
> > 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 <sr-ynQEQJNshbs@public.gmane.org>
> > Cc: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
> 
> Hi Stefan,
> 
> This is very cool, welcome onboard for the DT conversion with spear600!
> 
> > +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?
 
> > +static struct platform_device *plat_devs[] __initdata = {
> > +};
> 
> This could be dropped right away, because you would never
> add anything here.

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

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

Okay.

> > +}
> > +
> > +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);
> > +}
> > +
> > +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.

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

Thanks,
Stefan

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 14 March 2012, Stefan Roese wrote:

> > This is very cool, welcome onboard for the DT conversion with spear600!
> > 
> > > +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?

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.

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

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Stefan Roese]

On Wednesday 14 March 2012 10:48:44 Arnd Bergmann wrote:
> > > 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.

Ahh, I see. Let me see, if I can get this working...
 
> 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.
> 
> > > 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.

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

Thanks,
Stefan

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 14 March 2012, Stefan Roese wrote:
> On Wednesday 14 March 2012 10:48:44 Arnd Bergmann wrote:
> > > > 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.
> 
> 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.

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

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Stefan Roese]

On Wednesday 14 March 2012 14:27:00 Arnd Bergmann wrote:
> > > 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...

Okay. Finally got it working. The main problem was the clock device name 
matching. Solved it via OF_DEV_AUXDATA. New patch will follow soon...
 
> 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.
 
> > > 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.

Okay, I'll add all of what I got locally. And enable only the ones that should 
be fine.

Thanks,
Stefan

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 14 March 2012, Stefan Roese wrote:
> > 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.
>  

Could be. I haven't actually looked at the series myself, just saw the
email topic earlier today and thought of you.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Rob Herring]

On 03/14/2012 08:27 AM, Arnd Bergmann wrote:
> On Wednesday 14 March 2012, Stefan Roese wrote:
>> On Wednesday 14 March 2012 10:48:44 Arnd Bergmann wrote:
>>>>> 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.
>>

They do need to have "arm,primecell" for things to work as the type of
h/w is probeable, but the location of it is not. There should also be a
compatible property for the specific module (i.e. arm,pl011), but Linux
doesn't look at it. There's support for overriding the hw id in the DT
as well which some spear platforms need IIRC.

I thought I had added documentation for some of the primecells and Dave
Martin was going to do the rest, but seems it got dropped on the
floor... Off to dig in my git tree.

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

This is just interrupt support. The pl061 driver already supports DT.
And so do pl011, pl03x, pl022, pl330 (memory to memory only).

Rob

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

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

Other than Arnd's comments:

On 3/13/2012 8:17 PM, Stefan Roese wrote:

> diff --git a/arch/arm/boot/dts/spear600-evb.dts b/arch/arm/boot/dts/spear600-evb.dts
> new file mode 100644
> index 0000000..f92d099
> --- /dev/null
> +++ b/arch/arm/boot/dts/spear600-evb.dts
> @@ -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?

> + * Copyright 2012 Stefan Roese <sr-ynQEQJNshbs@public.gmane.org>
> + *
> + * 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
> + */
> +


> diff --git a/arch/arm/mach-spear6xx/Kconfig b/arch/arm/mach-spear6xx/Kconfig
> index ff4ae5b..7777f72 100644
> --- a/arch/arm/mach-spear6xx/Kconfig
> +++ b/arch/arm/mach-spear6xx/Kconfig
> @@ -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
> diff --git a/arch/arm/mach-spear6xx/Makefile b/arch/arm/mach-spear6xx/Makefile
> index cc1a4d8..e2d79b8 100644
> --- a/arch/arm/mach-spear6xx/Makefile
> +++ b/arch/arm/mach-spear6xx/Makefile
> @@ -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
> diff --git a/arch/arm/mach-spear6xx/board-dt.c b/arch/arm/mach-spear6xx/board-dt.c
> new file mode 100644
> index 0000000..ee4ff33
> --- /dev/null
> +++ b/arch/arm/mach-spear6xx/board-dt.c
> @@ -0,0 +1,75 @@
> +/*
> + * arch/arm/mach-spear6xx/board-dt.c
> + *
> + * Generic SPEAr600 platform support
> + *
> + * Copyright (C) 2009 ST Microelectronics
> + * Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>

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

> + *
> + * Copyright 2012 Stefan Roese <sr-ynQEQJNshbs@public.gmane.org>
> + *
> + * 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>

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

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

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

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

Thanks for the patch. :)

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Stefan Roese]

On Wednesday 14 March 2012 08:05:05 Viresh Kumar wrote:
> Other than Arnd's comments:
> 
> On 3/13/2012 8:17 PM, Stefan Roese wrote:
> > diff --git a/arch/arm/boot/dts/spear600-evb.dts
> > b/arch/arm/boot/dts/spear600-evb.dts new file mode 100644
> > index 0000000..f92d099
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/spear600-evb.dts
> > @@ -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?

I cloned this file from another dtsi file, where those copyright lines were 
present. Not sure if I need to preserve them in the new file.
 
> > + * Copyright 2012 Stefan Roese <sr@denx.de>
> > + *
> > + * 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
> > + */
> > +
> > 
> > 
> > diff --git a/arch/arm/mach-spear6xx/Kconfig
> > b/arch/arm/mach-spear6xx/Kconfig index ff4ae5b..7777f72 100644
> > --- a/arch/arm/mach-spear6xx/Kconfig
> > +++ b/arch/arm/mach-spear6xx/Kconfig
> > @@ -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
> > 
> > diff --git a/arch/arm/mach-spear6xx/Makefile
> > b/arch/arm/mach-spear6xx/Makefile index cc1a4d8..e2d79b8 100644
> > --- a/arch/arm/mach-spear6xx/Makefile
> > +++ b/arch/arm/mach-spear6xx/Makefile
> > @@ -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
> > diff --git a/arch/arm/mach-spear6xx/board-dt.c
> > b/arch/arm/mach-spear6xx/board-dt.c new file mode 100644
> > index 0000000..ee4ff33
> > --- /dev/null
> > +++ b/arch/arm/mach-spear6xx/board-dt.c
> > @@ -0,0 +1,75 @@
> > +/*
> > + * arch/arm/mach-spear6xx/board-dt.c
> > + *
> > + * Generic SPEAr600 platform support
> > + *
> > + * Copyright (C) 2009 ST Microelectronics
> > + * Viresh Kumar<viresh.kumar@st.com>
> 
>                   ^
> Thanks for this. Can add space here between r & <.

Okay.
 
> > + *
> > + * Copyright 2012 Stefan Roese <sr@denx.de>
> > + *
> > + * 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>
> 
> keeping header files in alphabetical order makes life easy at
> later stages. :)

Yes.
 
> > +#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 */ }
> > +};
> > +
> > +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.

Let me check this.
 
> > +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.

This file should be a generic SPEAr600 file. Not specific for the EVB. Future 
SPEAr600 board can also use this file. The differences between the boards 
should be encapsulated in the board specific dts files.
 
Thanks,
Stefan

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Stefan Roese]

Viresh,

On Wednesday 14 March 2012 08:05:05 Viresh Kumar wrote:
> > +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);
> > +}
> > +
> 
> 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.

Thanks,
Stefan

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/15/2012 2:18 PM, Stefan Roese wrote:
>>> > > +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);
>>> > > +}
>>> > > +
>> > 
>> > 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.

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.

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Stefan Roese]

On Thursday 15 March 2012 10:00:27 Viresh Kumar wrote:
> On 3/15/2012 2:18 PM, Stefan Roese wrote:
> >>> > > +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);
> >>> > > +}
> >>> > > +
> >> > 
> >> > 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.
> 
> 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".

Thanks,
Stefan

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/15/2012 4:08 PM, Stefan Roese wrote:
> I still don't think its worth it. But please feel free to send an add-on 
> patch, to "clean this up".

Ok. I will do it.

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Thursday 15 March 2012, Stefan Roese wrote:
> On Thursday 15 March 2012 10:00:27 Viresh Kumar wrote:
> > On 3/15/2012 2:18 PM, Stefan Roese wrote:
> > >>> > > +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);
> > >>> > > +}
> > >>> > > +
> > >> > 
> > >> > 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.
> > 
> > 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".

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

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/15/2012 7:09 PM, Arnd Bergmann wrote:
> On Thursday 15 March 2012, Stefan Roese wrote:
>> On Thursday 15 March 2012 10:00:27 Viresh Kumar wrote:
>>> On 3/15/2012 2:18 PM, Stefan Roese wrote:
>>>>>>>> +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);
>>>>>>>> +}
>>>>>>>> +
>>>>>>
>>>>>> 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.
>>>
>>> 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".
> 
> 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.

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 21 March 2012, Viresh Kumar wrote:
> > 
> > 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.

Ok, sounds good.

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. I just now saw that
you were not on Cc on the submission of the patch nor on my reply, but you
had given your Ack on the 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.

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

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

On Wed, Mar 21, 2012 at 6:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.

Ya. I already know that. :)

> 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?

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

--
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 21 March 2012, viresh kumar wrote:
> On Wed, Mar 21, 2012 at 6:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 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 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.

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

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

I'm sending it to Linus now and you can start using it, but it may still
change in a number of ways based on the requirements of new platforms that
start migrate to it.

> 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've looked at this earlier today out of curiosity and found:

debug_macro.h, gpio.h, hardware.h, timex.h, uncompress.h:
	 identical already
clock.c: almost identical, can be easily merged, or both copies
         kept around during transition.
misc_regs.h: can be merged into clock.c
spear.h: can merged into spear6xx.c but mostly go away
generic.h, irqs.h: not needed any more for spear6xx

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.

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

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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

On Wed, Mar 21, 2012 at 7:34 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 21 March 2012, viresh kumar wrote:
>> - 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.

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

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

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

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

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

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

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

Sure.

--
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 21 March 2012, viresh kumar wrote:
> On Wed, Mar 21, 2012 at 7:34 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > On Wednesday 21 March 2012, viresh kumar wrote:
> >> - 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.
> 
> So, i can add stuff in board file for pinmux for now?

Probably yes. It depends on the scale though: if you add more code than
what is needed to convert to the pinctrl framework, we might decide that
it's not worth it.

Note that moving to the pinctrl subsystem can be done independent from
doing the device tree conversion, and it does not depend on the
pinctrl dt binding even if you already have DT support.

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

Ok, I see.

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

Right, I was only looking at the stuff that is actually being used in mainline.
However I think this file should not be needed at all -- any contents that
are specific to one driver can just live in that driver, and stuff that is
shared across multiple drivers generally looks like it is also shared between
3xx and 6xx.

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

There is not all that much that can be shared here:

* clock.c would go into drivers/clk/ for spear13xx and possibly for the others
  too.
* padmux.c would go into drivers/pinctrl
* time.c would go into drivers/clocksource
* shirq.c is potentially shared, but we've discussed in the past about
  creating a new drivers/irqchip where this can live.
* restart.c is very small, there is no harm in duplicating this.
* The header files will have to go elsewhere, too, as we move towards
  have a single multiplatform zImage.

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

Good idea. As Stefan has shown, it's not really all that hard to get a lot
done for the older platforms, and spear13xx will likely be much harder
because of the increased hardware complexity.

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

Yes, you can do updates in there that are logical and keep the current
basic structure or improve it. I would only object if you do a more or
less complete rewrite of those files but don't use the common infrastructure
in the process.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 21 March 2012, Arnd Bergmann wrote:
> 
> On Wednesday 21 March 2012, viresh kumar wrote:
> > On Wed, Mar 21, 2012 at 6:06 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > > 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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

On Wed, Mar 21, 2012 at 11:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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. :(

--
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Thursday 22 March 2012, viresh kumar wrote:
> On Wed, Mar 21, 2012 at 11:56 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > 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. :(

No problem. Just apply them all to one tree and send me pull
requests when you think they are stable enough and should get into
arm-soc. Once they are in there, I'm not rebasing the patches
any more, but you are free to rebase and reorder the patches
before you send them to me.

If you would like me to take patches that are still under work
and may have to get changed, that's fine too, but tell me in advance
so we can put them into a staging branch that will be part of 
the linux-next kernel but can still get rebased. I'm not sending
staging branches to Linus though, unless you tell me to turn
them into stable branches before the merge window.

Also, please base all your pull requests on an -rc release from
Linus, or (if you have special dependencies) on a stable branch
from another person that then needs to get sent to Linus before
I send your changes. The pull requests should be split up by
topic, e.g. 'general cleanup', 'consolidation of SPEAr3xx and
SPEAr600', 'device tree conversion', 'board specific changes'
etc. so we can combine them with similar topics from other
platform maintainers when sending them out to Linus.

There are two bug fix branches, one for the current kernel (now
3.4) and one for the 'next' kernel (will be 3.5). Any important
changes should go into the 'current' fixes branch, and patches
that also apply to older kernels should get marked 'Cc:
stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org' so that Greg will automatically grab
them and apply them to his stable kernel series.

When you send a pull request, please send it to arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
so it reaches both Olof and me, as we are taking turns in
applying them to the arm-soc tree.

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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

[-- Attachment #1.1: Type: text/plain, Size: 343 bytes --]

On Mar 22, 2012 1:40 PM, "Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>

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

Everything is clear. Thanks.

--
Viresh

[-- Attachment #1.2: Type: text/html, Size: 492 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

[-- Attachment #1.1: Type: text/plain, Size: 507 bytes --]

On Mar 21, 2012 6:09 PM, "Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> 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.

Hi Arnd,

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?

--
Viresh

[-- Attachment #1.2: Type: text/html, Size: 659 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Thursday 22 March 2012, viresh kumar wrote:
> On Mar 21, 2012 6:09 PM, "Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > 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.

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.

For instance, in case of your clock driver, I would suggest you use
the "compatible" property of the clock device node, and do

/* null terminated array, turn the lookups into null termination as well */
struct of_device_id spear_clock_match = {
	{ "st,spear300-clock", &spear300_clk_lookups },
	{ "st,spear310-clock", &spear310_clk_lookups },
	{ "st,spear320-clock", &spear320_clk_lookups },
	{ },
};

void __init spear3xx_of_clk_init(void)
{
	struct device_node *np;
	struct of_device_id *match;
	struct clk_lookup *lookup;

	for_each_matching_node(np, &spear_clock_match)
		match = of_match_node(&spear_clock_match, np);

	for (lookup = match->data; lookup->clk; lookup++)
		clk_register(lookup);

	for (lookup = spear_clk_lookups; lookup->clk; lookup++)
		clk_register(lookup);
}

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

This is for the transition phase where you want to be able to boot both
using ATAG and using DT.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

[-- Attachment #1.1: Type: text/plain, Size: 270 bytes --]

On Mar 22, 2012 7:50 PM, "Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>
> 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.

Thanks.

[-- Attachment #1.2: Type: text/html, Size: 391 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/22/2012 7:50 PM, Arnd Bergmann wrote:
> 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?

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Tuesday 27 March 2012, Viresh Kumar wrote:
> On 3/22/2012 7:50 PM, Arnd Bergmann wrote:
> > 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?

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.

void __init spear3xx_clk_init(void)
{
	int i;
	struct clk_lookup *lookups;

	for (i = 0; i < ARRAY_SIZE(spear3xx_clk_lookups); i++) 
		clk_register(&spear3xx_clk_lookups[i])
}

void __init spear300_clk_init(void)
{
	int i;
	struct clk_lookup *lookups;

	spear3xx_clk_init();

	for (i = 0; i < ARRAY_SIZE(spear300_clk_lookups); i++) 
		clk_register(&spear300_clk_lookups[i])

	clk_init();
}

void __init spear300_map_io(void)
{
	spear3xx_map_io();
	spear300_clk_init();
}

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 the long run, I would hope that a lot of the differences go away anyway
as information that is now hardcoded just moves into the device tree.
At that point, you should no longer need to care which soc you are running
on. I realize that there is still significant amount of work to be done
until you get there.

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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/27/2012 4:45 PM, Arnd Bergmann wrote:
> 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?

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

DT should be up but, nodes would be up only once we call
of_platform_populate() from this init routine. And i had following code in
my patch:

static void __init spear3xx_dt_init(void)
{
...
	if (of_machine_is_compatible("st,spear300"))
		of_platform_populate(NULL, of_default_bus_match_table,
				spear300_auxdata_lookup, NULL);
	else if (of_machine_is_compatible("st,spear310"))
		of_platform_populate(NULL, of_default_bus_match_table,
				spear310_auxdata_lookup, NULL);
	else if (of_machine_is_compatible("st,spear320"))
		of_platform_populate(NULL, of_default_bus_match_table,
				spear320_auxdata_lookup, NULL);

...
}

And so these checks are not working here. :(

> 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!!

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/27/2012 4:57 PM, Viresh Kumar wrote:
> DT should be up but, nodes would be up only once we call
> of_platform_populate() from this init routine.

I was wrong here atleast :(
Can use these routines here before calling populate().

But question regarding multiple DT_MACHINE_START() is still valid.

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Tuesday 27 March 2012, Viresh Kumar wrote:
> On 3/27/2012 4:45 PM, Arnd Bergmann wrote:
> > 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?

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.

> > 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!!

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.

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[viresh kumar]

[-- Attachment #1.1: Type: text/plain, Size: 1766 bytes --]

On Mar 27, 2012 7:07 PM, "Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>
> On Tuesday 27 March 2012, Viresh Kumar wrote:
> > On 3/27/2012 4:45 PM, Arnd Bergmann wrote:
> > > 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?
>
> 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.

> > > 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!!
>
> 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?

I haven't.

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

--
Viresh

[-- Attachment #1.2: Type: text/html, Size: 2188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Tuesday 27 March 2012, viresh kumar wrote:
> On Mar 27, 2012 7:07 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
> >
> > On Tuesday 27 March 2012, Viresh Kumar wrote:
> > > On 3/27/2012 4:45 PM, Arnd Bergmann wrote:
> > > > 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?
> >
> > 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.

Right. I mean you can feel free to do whichever works best for you. If it's easy
to combine the DT_MACHINE_START sections, then do, otherwise don't.

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

	Arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Viresh Kumar]

On 3/27/2012 7:29 PM, Arnd Bergmann wrote:
> 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?

-- 
viresh

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Arnd Bergmann]

On Wednesday 28 March 2012, Viresh Kumar wrote:
> 
> On 3/27/2012 7:29 PM, Arnd Bergmann wrote:
> > 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.

I don't know what others do to get around this, this must be a fairly
common dependency, but there is no need to worry about it now. It might
all change if you device to port it to the common clk infrastructure.
 
> So, i believe i must have DT_MACHINE_START per SoC.

Ok.

> 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?

That's ok, yes.

	arnd

Re: [PATCH] ARM: SPEAr600: Add device-tree support to SPEAr600 boards

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:47 Tue 13 Mar     , Stefan Roese wrote:
> 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 <sr@denx.de>
> Cc: Viresh Kumar <viresh.kumar@st.com>
> ---
>  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
> 
> diff --git a/Documentation/devicetree/bindings/arm/spear.txt b/Documentation/devicetree/bindings/arm/spear.txt
> new file mode 100644
> index 0000000..8e9f83e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/spear.txt
> @@ -0,0 +1,6 @@
> +ST SPEAr Platforms Device Tree Bindings
> +---------------------------------------
> +
> +SPEAr600 EVB (Evaluation Board)
> +Required root node properties:
> +    - compatible = "st,spear600-evb";
> diff --git a/arch/arm/boot/dts/spear600-evb.dts b/arch/arm/boot/dts/spear600-evb.dts
> new file mode 100644
> index 0000000..f92d099
> --- /dev/null
> +++ b/arch/arm/boot/dts/spear600-evb.dts
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011 Linaro Ltd.
freescale for a ST SoC ??
> + *
> + * Copyright 2012 Stefan Roese <sr@denx.de>
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/dts-v1/;
> +/include/ "spear600.dtsi"
> +
> +/ {
> +	model = "ST SPEAr600 Evaluation Board";
> +	compatible = "st,spear600-evb";
put the soc too
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +};

put the board memory
> diff --git a/arch/arm/boot/dts/spear600.dtsi b/arch/arm/boot/dts/spear600.dtsi
> new file mode 100644
> index 0000000..82b086d
> --- /dev/null
> +++ b/arch/arm/boot/dts/spear600.dtsi
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011 Linaro Ltd.
> + *
> + * Copyright 2012 Stefan Roese <sr@denx.de>
> + *
> + * 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"
> +
> +/ {
> +	cpus {
> +		cpu@0 {
> +			compatible = "arm,arm926ejs";
> +		};
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0 0>; /* Filled by U-Boot */
here the max and do not expect the bootloader to do it
put the min of the board
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		vic0: interrupt-controller@f1100000 {
> +			compatible = "arm,pl190-vic";
> +			interrupt-controller;
> +			reg = <0xf1100000 0x1000>;
> +			#interrupt-cells = <1>;
> +		};
> +
> +		vic1: interrupt-controller@f1000000 {
> +			compatible = "arm,pl190-vic";
> +			interrupt-controller;
> +			reg = <0xf1000000 0x1000>;
> +			#interrupt-cells = <1>;
> +		};
> +	};
> +};
> diff --git a/arch/arm/mach-spear6xx/Kconfig b/arch/arm/mach-spear6xx/Kconfig
> index ff4ae5b..7777f72 100644
> --- a/arch/arm/mach-spear6xx/Kconfig
> +++ b/arch/arm/mach-spear6xx/Kconfig
> @@ -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
> diff --git a/arch/arm/mach-spear6xx/Makefile b/arch/arm/mach-spear6xx/Makefile
> index cc1a4d8..e2d79b8 100644
> --- a/arch/arm/mach-spear6xx/Makefile
> +++ b/arch/arm/mach-spear6xx/Makefile
> @@ -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
> diff --git a/arch/arm/mach-spear6xx/board-dt.c b/arch/arm/mach-spear6xx/board-dt.c
> new file mode 100644
> index 0000000..ee4ff33
> --- /dev/null
> +++ b/arch/arm/mach-spear6xx/board-dt.c
> @@ -0,0 +1,75 @@
> +/*
> + * arch/arm/mach-spear6xx/board-dt.c
> + *
> + * Generic SPEAr600 platform support
> + *
> + * Copyright (C) 2009 ST Microelectronics
> + * Viresh Kumar<viresh.kumar@st.com>
> + *
> + * Copyright 2012 Stefan Roese <sr@denx.de>
> + *
> + * 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>
> +#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],
> +};
there is no bining?
> +
> +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
> +};
put the soc instead of the list of bord

Best Regards,
J.