[RFC PATCH 00/14] Versatile Express device tree port

[RFC PATCH 00/14] Versatile Express device tree port

[Lorenzo Pieralisi]

This patchset provides an initial version of device tree enabled kernel on
an ARM Versatile Express board. The patchset applies to Jeremy Kerr's tree:

	git://kernel.ubuntu.com/jk/dt/linux-2.6.git dtbimage
	commit: 4cb80ac96489220554d28f6fde527aeef83e628b

The patched kernel version is available on my public ARM git tree:

	git://linux-arm.org/linux-2.6-lp.git ve-fdt

It has been tested on HW ARM Versatile Express board, with both static and DT 
configurations.

It contains fixes for generic device tree features and clock configuration,
code to patch Versatile Express peripherals drivers and build system.

Clock names as well as amba device names are temporary waiting for OF bindings
definition (clocks).
The Versatile Express board specific init code has been split into DT and 
non-DT code in order to factor out common code between the two configs. 

A static inline function has been added to the platform bus in order 
to initialize the OF match table and avoid cluttering code with preprocessor
macros.

Drivers ( and GIC, sp804, PL310 specific code) device tree init is an initial
stab at configuring peripherals with device tree data, so some choices 
especially concerning error codes are arguable and require thorough review.

The whole patchset is a request for comments on code and methodology. 

Cheers.

Lorenzo Pieralisi (14):
  ARM: amba device memory allocation fix
  ARM: vexpress: fix clocks definition to comply with new framework
  ARM: fix add instruction to set the flags
  ARM: r1 DT mach id init
  ARM: vexpress: fix typo in addruart
  platform: add function to initialize OF match table
  drivers/smsc911x: add DT support
  ARM: versatile-i2c driver DT port
  ARM: ARM flash driver DT port
  drivers/USB: isp1760 DT platform parsing and binding
  ARM: PMU: add device tree probing
  ARM: vexpress: add board support for DT probing
  ARM: vexpress: Definition of vexpress dts specification
  ARM: vexpress: add device tree build system and dtbuImage

 arch/arm/Kconfig                                  |    4 +-
 arch/arm/Makefile                                 |    2 +-
 arch/arm/boot/Makefile                            |   10 +-
 arch/arm/boot/dt/dtb.S                            |    3 +
 arch/arm/boot/dts/vexpress.dts                    |  199 +++++++++++++++++++++
 arch/arm/include/asm/pmu.h                        |    6 +
 arch/arm/kernel/Makefile                          |    3 +-
 arch/arm/kernel/head.S                            |    2 +-
 arch/arm/kernel/pmu-of.c                          |   30 +++
 arch/arm/kernel/pmu.c                             |   18 ++-
 arch/arm/mach-vexpress/Kconfig                    |    7 +
 arch/arm/mach-vexpress/Makefile                   |    5 +-
 arch/arm/mach-vexpress/core.h                     |   15 ++-
 arch/arm/mach-vexpress/ct-ca9x4-base.c            |  108 +++++++++++
 arch/arm/mach-vexpress/ct-ca9x4-of.c              |  192 ++++++++++++++++++++
 arch/arm/mach-vexpress/ct-ca9x4.c                 |   93 ----------
 arch/arm/mach-vexpress/include/mach/clkdev.h      |    2 +-
 arch/arm/mach-vexpress/include/mach/ct-ca9x4.h    |    2 +
 arch/arm/mach-vexpress/include/mach/debug-macro.S |    2 +-
 arch/arm/mach-vexpress/v2m-base.c                 |  197 ++++++++++++++++++++
 arch/arm/mach-vexpress/v2m-of.c                   |   94 ++++++++++
 arch/arm/mach-vexpress/v2m.c                      |  167 +-----------------
 arch/arm/mm/Kconfig                               |    2 +-
 drivers/amba/bus.c                                |    2 +-
 drivers/i2c/busses/i2c-versatile.c                |    6 +
 drivers/mtd/maps/integrator-flash.c               |    6 +
 drivers/net/Makefile                              |    3 +-
 drivers/net/smsc911x-of.c                         |   53 ++++++
 drivers/net/smsc911x.c                            |   28 ++-
 drivers/net/smsc911x.h                            |    8 +
 drivers/usb/host/Makefile                         |    4 +-
 drivers/usb/host/isp1760-hcd.h                    |    7 +
 drivers/usb/host/isp1760-if.c                     |   43 +++--
 drivers/usb/host/isp1760-of.c                     |   36 ++++
 include/linux/platform_device.h                   |   11 ++
 35 files changed, 1067 insertions(+), 303 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress.dts
 create mode 100644 arch/arm/kernel/pmu-of.c
 create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-base.c
 create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-of.c
 create mode 100644 arch/arm/mach-vexpress/v2m-base.c
 create mode 100644 arch/arm/mach-vexpress/v2m-of.c
 create mode 100644 drivers/net/smsc911x-of.c
 create mode 100644 drivers/usb/host/isp1760-of.c

[RFC PATCH 01/14] ARM: amba device memory allocation fix

[Lorenzo Pieralisi]

When instantiating amba devices dynamically from device tree nodes, the
memory allocation should be carried out using the kzalloc function to make
sure all the members are zero initialized.

This patch replaces the kmalloc call with a kzalloc one.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 drivers/amba/bus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index d73302c..234f4a3 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -421,7 +421,7 @@ static int amba_add_device_from_node(struct device_node *node)
 	const char *name;
 	int i, ret, len;
 
-	dev = kmalloc(sizeof(*dev), GFP_KERNEL);
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
-- 
1.6.3.3

[RFC PATCH 02/14] ARM: vexpress: fix clocks definition to comply with new framework

[Lorenzo Pieralisi]

The new clock infrastructure introduced within the DT series of patches
slightly changed how clocks are declared and initialized. Some clock
definitions become platform specific.

This patch fixes some typos on ARM Versatile Express clock definition
and initialization that caused compile time errors.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/mach-vexpress/include/mach/clkdev.h |    2 +-
 arch/arm/mach-vexpress/v2m.c                 |   13 ++++++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-vexpress/include/mach/clkdev.h b/arch/arm/mach-vexpress/include/mach/clkdev.h
index ad3f6e7..9e23cac 100644
--- a/arch/arm/mach-vexpress/include/mach/clkdev.h
+++ b/arch/arm/mach-vexpress/include/mach/clkdev.h
@@ -9,7 +9,7 @@ struct clk_v2m {
 	u32		config;
 };
 
-const struct clk_ops clk_v2m_ops;
+extern const struct clk_ops clk_v2m_ops;
 
 #define INIT_CLK_V2M(r,c) { .clk = INIT_CLK(clk_v2m_ops), \
 	.rate = r, .config = c }
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index f22e272..06d6914 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -285,12 +285,21 @@ static int clk_v2m_set_rate(struct clk *_clk, unsigned long rate)
 	struct clk_v2m *clk = to_clk_v2m(_clk);
 	int ret;
 
-	ret = v2m_cfg_write(clk->cofig, rate);
+	ret = v2m_cfg_write(clk->config, rate);
 	if (!ret)
 		clk->rate = rate;
 	return ret;
 }
 
+static void clk_v2m_put(struct clk *_clk)
+{
+}
+
+static int clk_v2m_get(struct clk *_clk)
+{
+	return 1;
+}
+
 static unsigned long clk_v2m_get_rate(struct clk *clk)
 {
 	return to_clk_v2m(clk)->rate;
@@ -300,6 +309,8 @@ const struct clk_ops clk_v2m_ops = {
 	.round_rate = clk_v2m_round_rate,
 	.get_rate = clk_v2m_get_rate,
 	.set_rate = clk_v2m_set_rate,
+	.get = clk_v2m_get,
+	.put = clk_v2m_put,
 };
 
 static struct clk_v2m osc1_clk =
-- 
1.6.3.3

[RFC PATCH 03/14] ARM: fix add instruction to set the flags

[Lorenzo Pieralisi]

ARM data processing instructions require the 's' suffix to update
the flags upon execution.

This patch fixes code requiring flags to be updated in order to check
the carry flag for overflow.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/kernel/head.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 7e00565..9879797 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -96,7 +96,7 @@ ENTRY(stext)
 	 * here, as we'll lookup one based on the device tree in setup_arch().
 	 * Also, we don't need to call __vet_atags.
 	 */
-	add	r3, r1, #1
+	adds	r3, r1, #1
 	bcs	1f
 
 	bl	__lookup_machine_type		@ r5=machinfo
-- 
1.6.3.3

[RFC PATCH 04/14] ARM: r1 DT mach id init

[Lorenzo Pieralisi]

Device tree ARM machine desc has been allocated id 0xffffffff temporarily.
This machine id should be passed in r1 upon kernel booting.
Legacy code requires code stubs to update r1 accordingly when
booting from legacy boot loaders that do not necessarily pass DT
mach id on r1.

This patch adds code in the dtb stub to automatically generate and move
the required DT machine id into r1 before jumping to kernel code.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/boot/dt/dtb.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dt/dtb.S b/arch/arm/boot/dt/dtb.S
index e678db2..7d56289 100644
--- a/arch/arm/boot/dt/dtb.S
+++ b/arch/arm/boot/dt/dtb.S
@@ -35,6 +35,9 @@ setup_dtb:
 	cmp	r4, r5
 	blo	2b
 
+	/* update r1 with DT id  */
+	mvn	r1, #0x0
+
 	b	zimage_start
 
 	.ltorg
-- 
1.6.3.3

[RFC PATCH 05/14] ARM: vexpress: fix typo in addruart

[Lorenzo Pieralisi]

The new infrastructure for low level debug in ARM requires the
addruart macro to be defined on a platform specific basis to get
rid of static addresses init in machine descriptors.
Uart physical and virtual addresses should be retrieved through
the addruart macro.

This patch fixes the addruart macro for Versatile Express board
in order to return the proper uart physical address.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/mach-vexpress/include/mach/debug-macro.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-vexpress/include/mach/debug-macro.S b/arch/arm/mach-vexpress/include/mach/debug-macro.S
index e0382fd..050d65e 100644
--- a/arch/arm/mach-vexpress/include/mach/debug-macro.S
+++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S
@@ -15,7 +15,7 @@
 		.macro	addruart,rp,rv
 		mov	\rp, #DEBUG_LL_UART_OFFSET
 		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rv, \rp, #0x10000000	@ physical base
+		orr	\rp, \rp, #0x10000000	@ physical base
 		.endm
 
 #include <asm/hardware/debug-pl01x.S>
-- 
1.6.3.3

[RFC PATCH 06/14] platform: add function to initialize OF match table

[Lorenzo Pieralisi]

If the kernel is configured to enable the Open Firmware device tree
infrastructure, the driver member of struct platform_driver has an
additional member, the match table, that has to be initialized in
order for the kernel to be capable of probing drivers at run-time
using the OF methodology.

This patch adds a static inline function that should be used to
initialize the match table member for platform drivers. When OF is
not enabled, the function is empty and it is just optimized out.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 include/linux/platform_device.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 5417944..bdfe6f3 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -84,6 +84,17 @@ extern struct platform_device *platform_create_bundle(struct platform_driver *dr
 					struct resource *res, unsigned int n_res,
 					const void *data, size_t size);
 
+#ifndef CONFIG_OF
+static inline void platform_init_match(struct platform_driver *pdriver,
+		const struct of_device_id *ids) { }
+#else
+static inline void platform_init_match(struct platform_driver *pdriver,
+		const struct of_device_id *ids)
+{
+	pdriver->driver.of_match_table = ids;
+}
+#endif
+
 /* early platform driver interface */
 struct early_platform_driver {
 	const char *class_str;
-- 
1.6.3.3

[RFC PATCH 07/14] drivers/smsc911x: add DT support

[Lorenzo Pieralisi]

When OF is enabled the device driver should initialize the match table
in order to be probable from the device tree. Furthermore, HW properties
should be retrieved from the device tree node, so a device tree probe
function is required to parse node properties at run-time. To avoid
preprocessor macros, open firmware functions are defined in a separate
C file that is optionally compiled-in.

This patch adds the infrastructure needed to enable device tree parsing
to smsc911 platform driver, inclusive of Makefile and device tree
parsing functions. Driver properties should be defined and retrieved
from the device tree if OF is enabled, otherwise the driver would fall
back to platform data static initialization.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 drivers/net/Makefile      |    3 +-
 drivers/net/smsc911x-of.c |   53 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/smsc911x.c    |   28 ++++++++++++++++-------
 drivers/net/smsc911x.h    |    8 ++++++
 4 files changed, 82 insertions(+), 10 deletions(-)
 create mode 100644 drivers/net/smsc911x-of.c

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 0a0512a..0d2faf9 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -243,7 +243,8 @@ obj-$(CONFIG_VXGE) += vxge/
 obj-$(CONFIG_MYRI10GE) += myri10ge/
 obj-$(CONFIG_SMC91X) += smc91x.o
 obj-$(CONFIG_SMC911X) += smc911x.o
-obj-$(CONFIG_SMSC911X) += smsc911x.o
+smsc911x_of-$(CONFIG_OF) := smsc911x-of.o
+obj-$(CONFIG_SMSC911X) += smsc911x.o $(smsc911x_of-y)
 obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
 obj-$(CONFIG_DM9000) += dm9000.o
 obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
diff --git a/drivers/net/smsc911x-of.c b/drivers/net/smsc911x-of.c
new file mode 100644
index 0000000..b8751cf
--- /dev/null
+++ b/drivers/net/smsc911x-of.c
@@ -0,0 +1,53 @@
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of.h>
+#include <linux/smsc911x.h>
+#include <linux/kernel.h>
+
+#include "smsc911x.h"
+
+
+int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
+		struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	int ret = -ENODEV;
+	const u32 *prop;
+
+	if (!node)
+		goto err;
+
+	prop = of_get_property(node, "flags", NULL);
+	if (!prop)
+		goto err;
+
+	cfg->flags = of_read_number(prop, 1);
+
+	prop = of_get_property(node, "irq-polarity", NULL);
+	if (!prop)
+		goto err;
+
+	cfg->irq_polarity = of_read_number(prop, 1);
+
+	prop = of_get_property(node, "irq-type", NULL);
+	if (!prop)
+		goto err;
+
+	cfg->irq_type = of_read_number(prop, 1);
+
+	prop = of_get_property(node, "phy-interface", NULL);
+	if (!prop)
+		goto err;
+
+	cfg->phy_interface = of_read_number(prop, 1);
+
+	pr_info("%s: Parsed FDT info\n", node->name);
+	pr_info("FLAGS %d\n", cfg->flags);
+	pr_info("IRQ-POLARITY %d\n", cfg->irq_polarity);
+	pr_info("IRQ-TYPE %d\n", cfg->irq_type);
+	pr_info("PHY IF %d\n", cfg->phy_interface);
+	return 0;
+err:
+	return ret;
+}
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index cc55974..c56b8f7 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -50,6 +50,7 @@
 #include <linux/phy.h>
 #include <linux/smsc911x.h>
 #include <linux/device.h>
+
 #include "smsc911x.h"
 
 #define SMSC_CHIPNAME		"smsc911x"
@@ -1945,13 +1946,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 
 	pr_info("%s: Driver version %s.\n", SMSC_CHIPNAME, SMSC_DRV_VERSION);
 
-	/* platform data specifies irq & dynamic bus configuration */
-	if (!pdev->dev.platform_data) {
-		pr_warning("%s: platform_data not provided\n", SMSC_CHIPNAME);
-		retval = -ENODEV;
-		goto out_0;
-	}
-
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 					   "smsc911x-memory");
 	if (!res)
@@ -1988,12 +1982,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 
 	pdata = netdev_priv(dev);
 
+	if (smsc911x_probe_dt(&pdata->config, pdev) == -ENODEV) {
+		/* platform data specifies irq & dynamic bus configuration */
+		if (!pdev->dev.platform_data) {
+			pr_warning("%s: platform_data not provided\n",
+					SMSC_CHIPNAME);
+			retval = -ENODEV;
+			goto out_0;
+		}
+		/* copy config parameters across to pdata */
+		memcpy(&pdata->config, config, sizeof(pdata->config));
+	}
+
 	dev->irq = irq_res->start;
 	irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
 	pdata->ioaddr = ioremap_nocache(res->start, res_size);
 
-	/* copy config parameters across to pdata */
-	memcpy(&pdata->config, config, sizeof(pdata->config));
 
 	pdata->dev = dev;
 	pdata->msg_enable = ((1 << debug) - 1);
@@ -2150,6 +2154,11 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
 #define SMSC911X_PM_OPS NULL
 #endif
 
+static struct of_device_id smsc911x_matches[] = {
+	{ .compatible = "smc,smsc-911"},
+	{},
+};
+
 static struct platform_driver smsc911x_driver = {
 	.probe = smsc911x_drv_probe,
 	.remove = __devexit_p(smsc911x_drv_remove),
@@ -2163,6 +2172,7 @@ static struct platform_driver smsc911x_driver = {
 /* Entry point for loading the module */
 static int __init smsc911x_init_module(void)
 {
+	platform_init_match(&smsc911x_driver, smsc911x_matches);
 	return platform_driver_register(&smsc911x_driver);
 }
 
diff --git a/drivers/net/smsc911x.h b/drivers/net/smsc911x.h
index 016360c..5ec9a8b 100644
--- a/drivers/net/smsc911x.h
+++ b/drivers/net/smsc911x.h
@@ -394,4 +394,12 @@
 #define LPA_PAUSE_ALL			(LPA_PAUSE_CAP | \
 					 LPA_PAUSE_ASYM)
 
+#ifndef CONFIG_OF
+static inline int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
+		struct platform_device *pdev) { return -ENODEV; }
+#else
+extern int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
+		struct platform_device *pdev);
+#endif
+
 #endif				/* __SMSC911X_H__ */
-- 
1.6.3.3

[RFC PATCH 08/14] ARM: versatile-i2c driver DT port

[Lorenzo Pieralisi]

When OF is enabled, each driver should define a match table to
allow the kernel to recognise drivers suitable for a given device.
Initialization is carried out through a static inline at driver init
time. The match-table is declared following OF bindings.

This patch adds the required initialization call to the versatile-i2c
driver.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 drivers/i2c/busses/i2c-versatile.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
index 6055601..ea8e3b5 100644
--- a/drivers/i2c/busses/i2c-versatile.c
+++ b/drivers/i2c/busses/i2c-versatile.c
@@ -133,6 +133,11 @@ static int i2c_versatile_remove(struct platform_device *dev)
 	return 0;
 }
 
+static struct of_device_id versatile_i2c_matches[] = {
+	{ .compatible = "arm,versatile-i2c"},
+	{},
+};
+
 static struct platform_driver i2c_versatile_driver = {
 	.probe		= i2c_versatile_probe,
 	.remove		= i2c_versatile_remove,
@@ -144,6 +149,7 @@ static struct platform_driver i2c_versatile_driver = {
 
 static int __init i2c_versatile_init(void)
 {
+	platform_init_match(&i2c_versatile_driver, versatile_i2c_matches);
 	return platform_driver_register(&i2c_versatile_driver);
 }
 
-- 
1.6.3.3

[RFC PATCH 09/14] ARM: ARM flash driver DT port

[Lorenzo Pieralisi]

When OF is enabled drivers should initialize the driver match table
to allow the kernel to find and recognise drivers suitable for
devices. The initialization is carried out through a static inline
function at driver init time.

This patch adds a match table and relative init code to support
device tree probing methodology for the ARM flash driver.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 drivers/mtd/maps/integrator-flash.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/maps/integrator-flash.c b/drivers/mtd/maps/integrator-flash.c
index 2aac41b..aa9a556 100644
--- a/drivers/mtd/maps/integrator-flash.c
+++ b/drivers/mtd/maps/integrator-flash.c
@@ -291,6 +291,11 @@ static int armflash_remove(struct platform_device *dev)
 	return 0;
 }
 
+static struct of_device_id armflash_matches[] = {
+	{ .compatible = "arm,arm-flash"},
+	{},
+};
+
 static struct platform_driver armflash_driver = {
 	.probe		= armflash_probe,
 	.remove		= armflash_remove,
@@ -302,6 +307,7 @@ static struct platform_driver armflash_driver = {
 
 static int __init armflash_init(void)
 {
+	platform_init_match(&armflash_driver, armflash_matches);
 	return platform_driver_register(&armflash_driver);
 }
 
-- 
1.6.3.3

[RFC PATCH 10/14] drivers/USB: isp1760 DT platform parsing and binding

[Lorenzo Pieralisi]

When OF is enabled, each driver should define a match table to
allow the kernel to recognise drivers suitable for a given device.
Initialization is carried out through a static inline at driver init
time. The match-table is declared following OF bindings.
Driver properties are parsed from the device tree node at run time,
but if the device tree probing fails, the driver falls back to static
driver initialization (platform data).

This patch adds the infrastructure to support device tree probing and
initialization for isp1760 USB host controller. The build infrastructure
changes accordingly in order to have a clean separation between OF and
non-OF init code.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 drivers/usb/host/Makefile      |    4 +-
 drivers/usb/host/isp1760-hcd.h |    7 ++++++
 drivers/usb/host/isp1760-if.c  |   43 ++++++++++++++++++++++++++--------------
 drivers/usb/host/isp1760-of.c  |   36 +++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 17 deletions(-)
 create mode 100644 drivers/usb/host/isp1760-of.c

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index b6315aa..edf6e8a 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -5,8 +5,8 @@
 ifeq ($(CONFIG_USB_DEBUG),y)
 	EXTRA_CFLAGS		+= -DDEBUG
 endif
-
-isp1760-objs := isp1760-hcd.o isp1760-if.o
+isp1760_of-$(CONFIG_OF)		:= isp1760-of.o
+isp1760-objs := isp1760-hcd.o isp1760-if.o $(isp1760_of-y)
 fhci-objs := fhci-hcd.o fhci-hub.o fhci-q.o fhci-mem.o \
 	     fhci-tds.o fhci-sched.o
 ifeq ($(CONFIG_FHCI_DEBUG),y)
diff --git a/drivers/usb/host/isp1760-hcd.h b/drivers/usb/host/isp1760-hcd.h
index 6931ef5..93afabd 100644
--- a/drivers/usb/host/isp1760-hcd.h
+++ b/drivers/usb/host/isp1760-hcd.h
@@ -9,6 +9,13 @@ struct usb_hcd *isp1760_register(phys_addr_t res_start, resource_size_t res_len,
 int init_kmem_once(void);
 void deinit_kmem_cache(void);
 
+#ifndef CONFIG_OF
+static inline int isp1760_plat_probe_dt(int *devflags,
+		struct platform_device *pdev) { return -ENODEV; }
+#else
+extern int isp1760_plat_probe_dt(int *devflags, struct platform_device *pdev);
+#endif
+
 /* EHCI capability registers */
 #define HC_CAPLENGTH		0x00
 #define HC_HCSPARAMS		0x04
diff --git a/drivers/usb/host/isp1760-if.c b/drivers/usb/host/isp1760-if.c
index ec85d0c..3a1839e 100644
--- a/drivers/usb/host/isp1760-if.c
+++ b/drivers/usb/host/isp1760-if.c
@@ -17,7 +17,7 @@
 
 #include "isp1760-hcd.h"
 
-#ifdef CONFIG_PPC_OF
+#if defined(CONFIG_PPC_OF)
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #endif
@@ -305,6 +305,17 @@ static struct pci_driver isp1761_pci_driver = {
 };
 #endif
 
+static const struct of_device_id of_isp1760_match[] = {
+	{
+		.compatible = "nxp,usb-isp1760",
+	},
+	{
+		.compatible = "nxp,usb-isp1761",
+	},
+	{ },
+};
+
+
 static int __devinit isp1760_plat_probe(struct platform_device *pdev)
 {
 	int ret = 0;
@@ -336,21 +347,22 @@ static int __devinit isp1760_plat_probe(struct platform_device *pdev)
 	}
 	irqflags |= irq_res->flags & IRQF_TRIGGER_MASK;
 
-	if (priv) {
-		if (priv->is_isp1761)
-			devflags |= ISP1760_FLAG_ISP1761;
-		if (priv->bus_width_16)
-			devflags |= ISP1760_FLAG_BUS_WIDTH_16;
-		if (priv->port1_otg)
-			devflags |= ISP1760_FLAG_OTG_EN;
-		if (priv->analog_oc)
-			devflags |= ISP1760_FLAG_ANALOG_OC;
-		if (priv->dack_polarity_high)
-			devflags |= ISP1760_FLAG_DACK_POL_HIGH;
-		if (priv->dreq_polarity_high)
-			devflags |= ISP1760_FLAG_DREQ_POL_HIGH;
+	if (isp1760_plat_probe_dt(&devflags, pdev) == -ENODEV) {
+		if (priv) {
+			if (priv->is_isp1761)
+				devflags |= ISP1760_FLAG_ISP1761;
+			if (priv->bus_width_16)
+				devflags |= ISP1760_FLAG_BUS_WIDTH_16;
+			if (priv->port1_otg)
+				devflags |= ISP1760_FLAG_OTG_EN;
+			if (priv->analog_oc)
+				devflags |= ISP1760_FLAG_ANALOG_OC;
+			if (priv->dack_polarity_high)
+				devflags |= ISP1760_FLAG_DACK_POL_HIGH;
+			if (priv->dreq_polarity_high)
+				devflags |= ISP1760_FLAG_DREQ_POL_HIGH;
+		}
 	}
-
 	hcd = isp1760_register(mem_res->start, mem_size, irq_res->start,
 			       irqflags, &pdev->dev, dev_name(&pdev->dev), devflags);
 	if (IS_ERR(hcd)) {
@@ -394,6 +406,7 @@ static int __init isp1760_init(void)
 
 	init_kmem_once();
 
+	platform_init_match(&isp1760_plat_driver, of_isp1760_match);
 	ret = platform_driver_register(&isp1760_plat_driver);
 	if (!ret)
 		any_ret = 0;
diff --git a/drivers/usb/host/isp1760-of.c b/drivers/usb/host/isp1760-of.c
new file mode 100644
index 0000000..c98ee58
--- /dev/null
+++ b/drivers/usb/host/isp1760-of.c
@@ -0,0 +1,36 @@
+
+#include <linux/platform_device.h>
+
+#include <linux/of.h>
+#include "isp1760-hcd.h"
+
+int isp1760_plat_probe_dt(int *devflags, struct platform_device *pdev)
+{
+	struct device_node *dp = pdev->dev.of_node;
+	const u32 *prop;
+
+	if (!dp)
+		return -ENODEV;
+
+	if (of_device_is_compatible(dp, "nxp,usb-isp1761"))
+		*devflags |= ISP1760_FLAG_ISP1761;
+
+	/* Some systems wire up only 16 of the 32 data lines */
+	prop = of_get_property(dp, "bus-width", NULL);
+	if (prop && *prop == 16)
+		*devflags |= ISP1760_FLAG_BUS_WIDTH_16;
+
+	if (of_get_property(dp, "port1-otg", NULL))
+		*devflags |= ISP1760_FLAG_OTG_EN;
+
+	if (of_get_property(dp, "analog-oc", NULL))
+		*devflags |= ISP1760_FLAG_ANALOG_OC;
+
+	if (of_get_property(dp, "dack-polarity", NULL))
+		*devflags |= ISP1760_FLAG_DACK_POL_HIGH;
+
+	if (of_get_property(dp, "dreq-polarity", NULL))
+		*devflags |= ISP1760_FLAG_DREQ_POL_HIGH;
+
+	return 0;
+}
-- 
1.6.3.3

[RFC PATCH 11/14] ARM: PMU: add device tree probing

[Lorenzo Pieralisi]

When OF is enabled, platform drivers are required to define a
match table in order to allow the kernel to find drivers suitable
for a given device. The device tree allows to retrieve resources
from device tree nodes dynamically.

This patch adds device tree support to the ARM PMU driver. This
includes a match table and code to initialize the driver id from
the respective device tree node compatible property.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/include/asm/pmu.h |    6 ++++++
 arch/arm/kernel/Makefile   |    3 ++-
 arch/arm/kernel/pmu-of.c   |   30 ++++++++++++++++++++++++++++++
 arch/arm/kernel/pmu.c      |   18 ++++++++++++------
 4 files changed, 50 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/kernel/pmu-of.c

diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index 8ccea01..d317b64 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h
@@ -17,6 +17,12 @@ enum arm_pmu_type {
 	ARM_NUM_PMU_DEVICES,
 };
 
+#ifndef CONFIG_OF
+static inline int pmu_probe_dt(struct platform_device *pdev) { return -ENODEV; }
+#else
+extern int __devinit pmu_probe_dt(struct platform_device *pdev);
+#endif
+
 #ifdef CONFIG_CPU_HAS_PMU
 
 /**
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 0ace897..b7d846f 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -48,7 +48,8 @@ obj-$(CONFIG_CPU_XSCALE)	+= xscale-cp0.o
 obj-$(CONFIG_CPU_XSC3)		+= xscale-cp0.o
 obj-$(CONFIG_CPU_MOHAWK)	+= xscale-cp0.o
 obj-$(CONFIG_IWMMXT)		+= iwmmxt.o
-obj-$(CONFIG_CPU_HAS_PMU)	+= pmu.o
+pmu_of-$(CONFIG_OF)		:= pmu-of.o
+obj-$(CONFIG_CPU_HAS_PMU)	+= pmu.o $(pmu_of-y)
 obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event.o
 AFLAGS_iwmmxt.o			:= -Wa,-mcpu=iwmmxt
 
diff --git a/arch/arm/kernel/pmu-of.c b/arch/arm/kernel/pmu-of.c
new file mode 100644
index 0000000..5c5ee94
--- /dev/null
+++ b/arch/arm/kernel/pmu-of.c
@@ -0,0 +1,30 @@
+/*
+ *  linux/arch/arm/kernel/pmu-of.c --  PMU DT probe function
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <asm/pmu.h>
+
+int __devinit pmu_probe_dt(struct platform_device *pdev)
+{
+	int ret = -ENODEV;
+	struct device_node *node = pdev->dev.of_node;
+
+	if (of_device_is_compatible(node, "arm,arm-pmu")) {
+		pdev->id = ARM_PMU_DEVICE_CPU;
+		ret = 0;
+	} else {
+		pr_warning("Probing device not recognized "
+				"device %s\n", node->full_name);
+	}
+
+	return ret;
+}
diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index b8af96e..f070b3d 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c
@@ -27,12 +27,12 @@ static struct platform_device *pmu_devices[ARM_NUM_PMU_DEVICES];
 
 static int __devinit pmu_device_probe(struct platform_device *pdev)
 {
-
-	if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
-		pr_warning("received registration request for unknown "
-				"device %d\n", pdev->id);
-		return -EINVAL;
-	}
+	if (pmu_probe_dt(pdev) == -ENODEV)
+		if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
+			pr_warning("received registration request for unknown "
+					"device %d\n", pdev->id);
+			return -EINVAL;
+		}
 
 	if (pmu_devices[pdev->id])
 		pr_warning("registering new PMU device type %d overwrites "
@@ -45,6 +45,11 @@ static int __devinit pmu_device_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id arm_pmu_matches[] = {
+	{ .compatible = "arm,arm-pmu"},
+	{},
+};
+
 static struct platform_driver pmu_driver = {
 	.driver		= {
 		.name	= "arm-pmu",
@@ -54,6 +59,7 @@ static struct platform_driver pmu_driver = {
 
 static int __init register_pmu_driver(void)
 {
+	platform_init_match(&pmu_driver, arm_pmu_matches);
 	return platform_driver_register(&pmu_driver);
 }
 device_initcall(register_pmu_driver);
-- 
1.6.3.3

[RFC PATCH 12/14] ARM: vexpress: add board support for DT probing

[Lorenzo Pieralisi]

Discussion is under way to decide if a set of machine ids is allocated
to ARM platforms based on device tree initialization.
Init code for a given platform should be split in a static init version
and a device tree init version. The Versatile Express ARM board requires
the initialization of motherboard and daughterboard in separate steps,
so the split between static init and DT init is doubled in terms of
files. Common code should be factored out in common C files to avoid code
duplication.

This patch refactors the Versatile Express init code in order
to split it in DT and non-DT versions, compiled in accordingly depending
on kernel configuration options. Clock look-up for the DT version is
still work in progress waiting for a definitive clock binding spec so
device names are not really DT compliant at the moment in the dts.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/mach-vexpress/core.h                  |   15 ++-
 arch/arm/mach-vexpress/ct-ca9x4-base.c         |  108 +++++++++++++
 arch/arm/mach-vexpress/ct-ca9x4-of.c           |  192 +++++++++++++++++++++++
 arch/arm/mach-vexpress/ct-ca9x4.c              |   93 -----------
 arch/arm/mach-vexpress/include/mach/ct-ca9x4.h |    2 +
 arch/arm/mach-vexpress/v2m-base.c              |  197 ++++++++++++++++++++++++
 arch/arm/mach-vexpress/v2m-of.c                |   94 +++++++++++
 arch/arm/mach-vexpress/v2m.c                   |  178 +---------------------
 8 files changed, 610 insertions(+), 269 deletions(-)
 create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-base.c
 create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-of.c
 create mode 100644 arch/arm/mach-vexpress/v2m-base.c
 create mode 100644 arch/arm/mach-vexpress/v2m-of.c

diff --git a/arch/arm/mach-vexpress/core.h b/arch/arm/mach-vexpress/core.h
index 57dd95c..b8430eb 100644
--- a/arch/arm/mach-vexpress/core.h
+++ b/arch/arm/mach-vexpress/core.h
@@ -20,7 +20,20 @@ struct amba_device name##_device = {		\
 
 struct map_desc;
 
-void v2m_map_io(struct map_desc *tile, size_t num);
+extern void v2m_map_io(struct map_desc *tile, size_t num);
+extern void v2m_power_off(void);
+extern void v2m_restart(char str, const char *cmd);
+
 extern struct sys_timer v2m_timer;
+extern struct mmci_platform_data v2m_mmci_data;
+extern struct flash_platform_data v2m_flash_data;
 
 extern void __iomem *gic_cpu_base_addr;
+
+extern const struct clk_ops clk_v2m_ops;
+
+#ifndef CONFIG_OF
+static inline int v2m_timer_probe_dt(void) { return -ENODEV; }
+#else
+extern int v2m_timer_probe_dt(void);
+#endif
diff --git a/arch/arm/mach-vexpress/ct-ca9x4-base.c b/arch/arm/mach-vexpress/ct-ca9x4-base.c
new file mode 100644
index 0000000..44ec683
--- /dev/null
+++ b/arch/arm/mach-vexpress/ct-ca9x4-base.c
@@ -0,0 +1,108 @@
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/amba/bus.h>
+#include <linux/amba/clcd.h>
+
+#include <asm/mach/map.h>
+
+#include <mach/ct-ca9x4.h>
+#include <mach/motherboard.h>
+
+#include "core.h"
+
+
+#define V2M_PA_CS7	0x10000000
+
+static struct map_desc ct_ca9x4_io_desc[] __initdata = {
+	{
+		.virtual	= __MMIO_P2V(CT_CA9X4_MPIC),
+		.pfn		= __phys_to_pfn(CT_CA9X4_MPIC),
+		.length		= SZ_16K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= __MMIO_P2V(CT_CA9X4_SP804_TIMER),
+		.pfn		= __phys_to_pfn(CT_CA9X4_SP804_TIMER),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= __MMIO_P2V(CT_CA9X4_L2CC),
+		.pfn		= __phys_to_pfn(CT_CA9X4_L2CC),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+};
+
+void __init ct_ca9x4_map_io(void)
+{
+	v2m_map_io(ct_ca9x4_io_desc, ARRAY_SIZE(ct_ca9x4_io_desc));
+}
+
+static struct clcd_panel xvga_panel = {
+	.mode		= {
+		.name		= "XVGA",
+		.refresh	= 60,
+		.xres		= 1024,
+		.yres		= 768,
+		.pixclock	= 15384,
+		.left_margin	= 168,
+		.right_margin	= 8,
+		.upper_margin	= 29,
+		.lower_margin	= 3,
+		.hsync_len	= 144,
+		.vsync_len	= 6,
+		.sync		= 0,
+		.vmode		= FB_VMODE_NONINTERLACED,
+	},
+	.width		= -1,
+	.height		= -1,
+	.tim2		= TIM2_BCD | TIM2_IPC,
+	.cntl		= CNTL_LCDTFT | CNTL_BGR | CNTL_LCDVCOMP(1),
+	.bpp		= 16,
+};
+
+static void ct_ca9x4_clcd_enable(struct clcd_fb *fb)
+{
+	v2m_cfg_write(SYS_CFG_MUXFPGA | SYS_CFG_SITE_DB1, 0);
+	v2m_cfg_write(SYS_CFG_DVIMODE | SYS_CFG_SITE_DB1, 2);
+}
+
+static int ct_ca9x4_clcd_setup(struct clcd_fb *fb)
+{
+	unsigned long framesize = 1024 * 768 * 2;
+	dma_addr_t dma;
+
+	fb->panel = &xvga_panel;
+
+	fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, framesize,
+				&dma, GFP_KERNEL);
+	if (!fb->fb.screen_base) {
+		printk(KERN_ERR "CLCD: unable to map frame buffer\n");
+		return -ENOMEM;
+	}
+	fb->fb.fix.smem_start = dma;
+	fb->fb.fix.smem_len = framesize;
+
+	return 0;
+}
+
+static int ct_ca9x4_clcd_mmap(struct clcd_fb *fb, struct vm_area_struct *vma)
+{
+	return dma_mmap_writecombine(&fb->dev->dev, vma, fb->fb.screen_base,
+		fb->fb.fix.smem_start, fb->fb.fix.smem_len);
+}
+
+static void ct_ca9x4_clcd_remove(struct clcd_fb *fb)
+{
+	dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len,
+		fb->fb.screen_base, fb->fb.fix.smem_start);
+}
+
+struct clcd_board ct_ca9x4_clcd_data = {
+	.name		= "CT-CA9X4",
+	.check		= clcdfb_check,
+	.decode		= clcdfb_decode,
+	.enable		= ct_ca9x4_clcd_enable,
+	.setup		= ct_ca9x4_clcd_setup,
+	.mmap		= ct_ca9x4_clcd_mmap,
+	.remove		= ct_ca9x4_clcd_remove,
+};
diff --git a/arch/arm/mach-vexpress/ct-ca9x4-of.c b/arch/arm/mach-vexpress/ct-ca9x4-of.c
new file mode 100644
index 0000000..b259ad2
--- /dev/null
+++ b/arch/arm/mach-vexpress/ct-ca9x4-of.c
@@ -0,0 +1,192 @@
+/*
+ * Versatile Express DT Core Tile Cortex A9x4 Support
+ */
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/amba/bus.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <asm/clkdev.h>
+#include <asm/hardware/arm_timer.h>
+#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/gic.h>
+#include <asm/mach-types.h>
+
+#include <mach/clkdev.h>
+#include <mach/ct-ca9x4.h>
+
+#include <plat/timer-sp.h>
+
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include "core.h"
+
+#include <mach/motherboard.h>
+
+#define V2M_PA_CS7	0x10000000
+
+void __iomem *gic_cpu_base_addr;
+
+static int __init gic_cpu_addr(void)
+{
+	struct device_node *node;
+	struct resource r;
+	int err = -ENODEV;
+
+	node = of_find_compatible_node(NULL, NULL, "arm,"
+			"gic-cpu-interrupt-controller");
+	if (node) {
+		err = of_address_to_resource(node, 0, &r);
+		if (err)
+			pr_warn("git-cpu: Could not get resource for "
+				"device tree node '%s'", node->full_name);
+		else
+			gic_cpu_base_addr = MMIO_P2V(r.start);
+
+		of_node_put(node);
+	}
+
+	return err;
+}
+
+static int __init gic_init_dt(void)
+{
+	struct device_node *node;
+	struct resource r;
+	int err = -ENODEV;
+
+	if (gic_cpu_addr() < 0)
+		goto out;
+
+	node = of_find_compatible_node(NULL, NULL,
+			"arm,gic-dist-interrupt-controller");
+	if (node) {
+		const u32 *prop;
+		int irq_start = 29;
+
+		err = of_address_to_resource(node, 0, &r);
+		if (err) {
+			pr_warn("gic-dist: Could not get resource for "
+				"device tree node '%s'", node->full_name);
+		} else {
+			prop = of_get_property(node, "irq-start", NULL);
+			if (prop)
+				irq_start = of_read_number(prop, 1);
+			else
+				pr_warn("gic-dist: Could not get irq-start "
+					"property" " initialized to default"
+					"%d\n", irq_start);
+			gic_dist_init(0, MMIO_P2V(r.start), irq_start);
+			gic_cpu_init(0, gic_cpu_base_addr);
+		}
+		of_node_put(node);
+	}
+out:
+	return err;
+}
+
+static void __init ct_ca9x4_dt_init_irq(void)
+{
+	if (gic_init_dt() < 0) {
+		gic_cpu_base_addr = MMIO_P2V(A9_MPCORE_GIC_CPU);
+		gic_dist_init(0, MMIO_P2V(A9_MPCORE_GIC_DIST), 29);
+		gic_cpu_init(0, gic_cpu_base_addr);
+	}
+}
+
+
+static struct clk_v2m osc1_clk =
+	INIT_CLK_V2M(24000000, SYS_CFG_OSC | SYS_CFG_SITE_DB1 | 1);
+static struct clk_lookup lookups[] = {
+	{	/* CLCD */
+		.dev_id		= "clcd@0",
+		.clk		= &osc1_clk.clk,
+	},
+};
+
+
+static int __init arm_vexpress_dt_probe(unsigned long dt)
+{
+	if (!of_flat_dt_is_compatible(dt, "arm,versatile-express"))
+		return 0;
+
+	return 1;
+}
+
+static int ca9x4_notifier(struct device *device)
+{
+	struct device_node *dn = device->of_node;
+
+	if (of_device_is_compatible(dn, "arm,clcd-pl11x")) {
+		device->platform_data =	&ct_ca9x4_clcd_data;
+		pr_info("initialized clcd-pl11x platform_data\n");
+	}
+	if (of_device_is_compatible(dn, "arm,mmc-pl18x")) {
+		device->platform_data =	&v2m_mmci_data;
+		pr_info("initialized mmc-pl18x platform_data\n");
+	}
+	if (of_device_is_compatible(dn, "arm,arm-flash")) {
+		device->platform_data =	&v2m_flash_data;
+		pr_info("initialized arm-flash platform_data\n");
+	}
+	return 0;
+}
+
+static int __init l2x0_init_dt(void)
+{
+	struct device_node *node;
+	int err = -ENODEV;
+
+	node = of_find_compatible_node(NULL, NULL, "arm,"
+			"pl310-cache-controller");
+	if (node) {
+		struct resource r;
+
+		err = of_address_to_resource(node, 0, &r);
+		if (err)
+			pr_warn("pl-310: Could not get resource for "
+					"device tree node '%s'",
+					node->full_name);
+		else
+			l2x0_init(MMIO_P2V(r.start), 0x00000000, 0xfe0fffff);
+
+		of_node_put(node);
+	}
+
+	return err;
+}
+
+static void ct_ca9x4_dt_init(void)
+{
+	struct device_node *node;
+#ifdef CONFIG_CACHE_L2X0
+	if (l2x0_init_dt() < 0)
+		l2x0_init(MMIO_P2V(CT_CA9X4_L2CC), 0x00000000, 0xfe0fffff);
+#endif
+	platform_notify = ca9x4_notifier;
+
+	clkdev_add_table(lookups, ARRAY_SIZE(lookups));
+
+	node = of_find_node_by_path("/amba");
+	if (node)
+		amba_parse_dt_node(node);
+
+	of_platform_bus_probe(NULL, NULL, NULL);
+}
+
+DT_MACHINE_START(VEXPRESS, "ARM-Versatile Express CA9x4 DT")
+	.boot_params	= PHYS_OFFSET + 0x00000100,
+	.map_io		= ct_ca9x4_map_io,
+	.init_irq	= ct_ca9x4_dt_init_irq,
+#if 0
+	.timer		= &ct_ca9x4_timer,
+#else
+	.timer		= &v2m_timer,
+#endif
+	.init_machine	= ct_ca9x4_dt_init,
+	.probe_dt	= arm_vexpress_dt_probe,
+MACHINE_END
diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
index 3419da5..97b7b17 100644
--- a/arch/arm/mach-vexpress/ct-ca9x4.c
+++ b/arch/arm/mach-vexpress/ct-ca9x4.c
@@ -23,7 +23,6 @@
 #include <plat/timer-sp.h>
 
 #include <asm/mach/arch.h>
-#include <asm/mach/map.h>
 #include <asm/mach/time.h>
 
 #include "core.h"
@@ -32,29 +31,6 @@
 
 #define V2M_PA_CS7	0x10000000
 
-static struct map_desc ct_ca9x4_io_desc[] __initdata = {
-	{
-		.virtual	= __MMIO_P2V(CT_CA9X4_MPIC),
-		.pfn		= __phys_to_pfn(CT_CA9X4_MPIC),
-		.length		= SZ_16K,
-		.type		= MT_DEVICE,
-	}, {
-		.virtual	= __MMIO_P2V(CT_CA9X4_SP804_TIMER),
-		.pfn		= __phys_to_pfn(CT_CA9X4_SP804_TIMER),
-		.length		= SZ_4K,
-		.type		= MT_DEVICE,
-	}, {
-		.virtual	= __MMIO_P2V(CT_CA9X4_L2CC),
-		.pfn		= __phys_to_pfn(CT_CA9X4_L2CC),
-		.length		= SZ_4K,
-		.type		= MT_DEVICE,
-	},
-};
-
-static void __init ct_ca9x4_map_io(void)
-{
-	v2m_map_io(ct_ca9x4_io_desc, ARRAY_SIZE(ct_ca9x4_io_desc));
-}
 
 void __iomem *gic_cpu_base_addr;
 
@@ -80,75 +56,6 @@ static struct sys_timer ct_ca9x4_timer = {
 };
 #endif
 
-static struct clcd_panel xvga_panel = {
-	.mode		= {
-		.name		= "XVGA",
-		.refresh	= 60,
-		.xres		= 1024,
-		.yres		= 768,
-		.pixclock	= 15384,
-		.left_margin	= 168,
-		.right_margin	= 8,
-		.upper_margin	= 29,
-		.lower_margin	= 3,
-		.hsync_len	= 144,
-		.vsync_len	= 6,
-		.sync		= 0,
-		.vmode		= FB_VMODE_NONINTERLACED,
-	},
-	.width		= -1,
-	.height		= -1,
-	.tim2		= TIM2_BCD | TIM2_IPC,
-	.cntl		= CNTL_LCDTFT | CNTL_BGR | CNTL_LCDVCOMP(1),
-	.bpp		= 16,
-};
-
-static void ct_ca9x4_clcd_enable(struct clcd_fb *fb)
-{
-	v2m_cfg_write(SYS_CFG_MUXFPGA | SYS_CFG_SITE_DB1, 0);
-	v2m_cfg_write(SYS_CFG_DVIMODE | SYS_CFG_SITE_DB1, 2);
-}
-
-static int ct_ca9x4_clcd_setup(struct clcd_fb *fb)
-{
-	unsigned long framesize = 1024 * 768 * 2;
-	dma_addr_t dma;
-
-	fb->panel = &xvga_panel;
-
-	fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, framesize,
-				&dma, GFP_KERNEL);
-	if (!fb->fb.screen_base) {
-		printk(KERN_ERR "CLCD: unable to map frame buffer\n");
-		return -ENOMEM;
-	}
-	fb->fb.fix.smem_start = dma;
-	fb->fb.fix.smem_len = framesize;
-
-	return 0;
-}
-
-static int ct_ca9x4_clcd_mmap(struct clcd_fb *fb, struct vm_area_struct *vma)
-{
-	return dma_mmap_writecombine(&fb->dev->dev, vma, fb->fb.screen_base,
-		fb->fb.fix.smem_start, fb->fb.fix.smem_len);
-}
-
-static void ct_ca9x4_clcd_remove(struct clcd_fb *fb)
-{
-	dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len,
-		fb->fb.screen_base, fb->fb.fix.smem_start);
-}
-
-static struct clcd_board ct_ca9x4_clcd_data = {
-	.name		= "CT-CA9X4",
-	.check		= clcdfb_check,
-	.decode		= clcdfb_decode,
-	.enable		= ct_ca9x4_clcd_enable,
-	.setup		= ct_ca9x4_clcd_setup,
-	.mmap		= ct_ca9x4_clcd_mmap,
-	.remove		= ct_ca9x4_clcd_remove,
-};
 
 static AMBA_DEVICE(clcd, "ct:clcd", CT_CA9X4_CLCDC, &ct_ca9x4_clcd_data);
 static AMBA_DEVICE(dmc, "ct:dmc", CT_CA9X4_DMC, NULL);
diff --git a/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h b/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h
index 8650f04..65870d8 100644
--- a/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h
+++ b/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h
@@ -44,4 +44,6 @@
 #define IRQ_CT_CA9X4_PMU_CPU2	94
 #define IRQ_CT_CA9X4_PMU_CPU3	95
 
+extern struct clcd_board ct_ca9x4_clcd_data;
+extern void __init ct_ca9x4_map_io(void);
 #endif
diff --git a/arch/arm/mach-vexpress/v2m-base.c b/arch/arm/mach-vexpress/v2m-base.c
new file mode 100644
index 0000000..3d24d5b
--- /dev/null
+++ b/arch/arm/mach-vexpress/v2m-base.c
@@ -0,0 +1,197 @@
+/*
+ * Versatile Express V2M Motherboard Support
+ */
+#include <linux/amba/mmci.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/sysdev.h>
+
+#include <asm/clkdev.h>
+#include <asm/sizes.h>
+#include <asm/mach/flash.h>
+#include <asm/mach/map.h>
+#include <asm/mach/time.h>
+#include <asm/hardware/arm_timer.h>
+
+#include <mach/clkdev.h>
+#include <mach/motherboard.h>
+
+#include <plat/timer-sp.h>
+
+#include "core.h"
+
+#define V2M_PA_CS0	0x40000000
+#define V2M_PA_CS1	0x44000000
+#define V2M_PA_CS2	0x48000000
+#define V2M_PA_CS3	0x4c000000
+#define V2M_PA_CS7	0x10000000
+
+static struct map_desc v2m_io_desc[] __initdata = {
+	{
+		.virtual	= __MMIO_P2V(V2M_PA_CS7),
+		.pfn		= __phys_to_pfn(V2M_PA_CS7),
+		.length		= SZ_128K,
+		.type		= MT_DEVICE,
+	},
+};
+
+void __init v2m_map_io(struct map_desc *tile, size_t num)
+{
+	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+	iotable_init(tile, num);
+}
+
+
+static void v2m_timer_init(void)
+{
+	if (v2m_timer_probe_dt() == -ENODEV) {
+
+		writel(0, MMIO_P2V(V2M_TIMER0) + TIMER_CTRL);
+		writel(0, MMIO_P2V(V2M_TIMER1) + TIMER_CTRL);
+
+		sp804_clocksource_init(MMIO_P2V(V2M_TIMER1));
+		sp804_clockevents_init(MMIO_P2V(V2M_TIMER0), IRQ_V2M_TIMER0);
+	}
+}
+
+struct sys_timer v2m_timer = {
+	.init	= v2m_timer_init,
+};
+
+
+static DEFINE_SPINLOCK(v2m_cfg_lock);
+
+int v2m_cfg_write(u32 devfn, u32 data)
+{
+	/* Configuration interface broken? */
+	u32 val;
+
+	printk("%s: writing %08x to %08x\n", __func__, data, devfn);
+
+	devfn |= SYS_CFG_START | SYS_CFG_WRITE;
+
+	spin_lock(&v2m_cfg_lock);
+	val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
+	writel(val & ~SYS_CFG_COMPLETE, MMIO_P2V(V2M_SYS_CFGSTAT));
+
+	writel(data, MMIO_P2V(V2M_SYS_CFGDATA));
+	writel(devfn, MMIO_P2V(V2M_SYS_CFGCTRL));
+
+	do {
+		val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
+	} while (val == 0);
+	spin_unlock(&v2m_cfg_lock);
+
+	return !!(val & SYS_CFG_ERR);
+}
+
+int v2m_cfg_read(u32 devfn, u32 *data)
+{
+	u32 val;
+
+	devfn |= SYS_CFG_START;
+
+	spin_lock(&v2m_cfg_lock);
+	writel(0, MMIO_P2V(V2M_SYS_CFGSTAT));
+	writel(devfn, MMIO_P2V(V2M_SYS_CFGCTRL));
+
+	mb();
+
+	do {
+		cpu_relax();
+		val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
+	} while (val == 0);
+
+	*data = readl(MMIO_P2V(V2M_SYS_CFGDATA));
+	spin_unlock(&v2m_cfg_lock);
+
+	return !!(val & SYS_CFG_ERR);
+}
+
+
+static unsigned int v2m_mmci_status(struct device *dev)
+{
+	return !(readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0));
+}
+
+struct mmci_platform_data v2m_mmci_data = {
+	.ocr_mask	= MMC_VDD_32_33|MMC_VDD_33_34,
+	.status		= v2m_mmci_status,
+};
+
+static int v2m_flash_init(void)
+{
+	writel(0, MMIO_P2V(V2M_SYS_FLASH));
+	return 0;
+}
+
+static void v2m_flash_exit(void)
+{
+	writel(0, MMIO_P2V(V2M_SYS_FLASH));
+}
+
+static void v2m_flash_set_vpp(int on)
+{
+	writel(on != 0, MMIO_P2V(V2M_SYS_FLASH));
+}
+
+struct flash_platform_data v2m_flash_data = {
+	.map_name	= "cfi_probe",
+	.width		= 4,
+	.init		= v2m_flash_init,
+	.exit		= v2m_flash_exit,
+	.set_vpp	= v2m_flash_set_vpp,
+};
+
+#define to_clk_v2m(c) (container_of(c, struct clk_v2m, clk))
+
+static long clk_v2m_round_rate(struct clk *clk, unsigned long rate)
+{
+	return rate;
+}
+
+static int clk_v2m_set_rate(struct clk *_clk, unsigned long rate)
+{
+	struct clk_v2m *clk = to_clk_v2m(_clk);
+	int ret;
+
+	ret = v2m_cfg_write(clk->config, rate);
+	if (!ret)
+		clk->rate = rate;
+	return ret;
+}
+
+static void clk_v2m_put(struct clk *_clk)
+{
+}
+
+static int clk_v2m_get(struct clk *_clk)
+{
+	return 1;
+}
+
+static unsigned long clk_v2m_get_rate(struct clk *clk)
+{
+	return to_clk_v2m(clk)->rate;
+}
+
+const struct clk_ops clk_v2m_ops = {
+	.round_rate = clk_v2m_round_rate,
+	.get_rate = clk_v2m_get_rate,
+	.set_rate = clk_v2m_set_rate,
+	.get = clk_v2m_get,
+	.put = clk_v2m_put,
+};
+
+void v2m_power_off(void)
+{
+	if (v2m_cfg_write(SYS_CFG_SHUTDOWN | SYS_CFG_SITE_MB, 0))
+		printk(KERN_EMERG "Unable to shutdown\n");
+}
+
+void v2m_restart(char str, const char *cmd)
+{
+	if (v2m_cfg_write(SYS_CFG_REBOOT | SYS_CFG_SITE_MB, 0))
+		printk(KERN_EMERG "Unable to reboot\n");
+}
diff --git a/arch/arm/mach-vexpress/v2m-of.c b/arch/arm/mach-vexpress/v2m-of.c
new file mode 100644
index 0000000..acc058c
--- /dev/null
+++ b/arch/arm/mach-vexpress/v2m-of.c
@@ -0,0 +1,94 @@
+/*
+ * Versatile Express V2M Motherboard DT init
+ */
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#include <asm/clkdev.h>
+#include <asm/hardware/arm_timer.h>
+
+#include <mach/clkdev.h>
+#include <mach/motherboard.h>
+
+#include <plat/timer-sp.h>
+
+#include "core.h"
+
+static inline void v2m_timer_init_ce(void __iomem *va, unsigned int irq)
+{
+	if (irq != NO_IRQ)
+		sp804_clockevents_init(va, irq);
+	else
+		sp804_clocksource_init(va);
+}
+
+int v2m_timer_probe_dt(void)
+{
+	struct device_node *node;
+	int irq, err = -ENODEV;
+
+	for_each_compatible_node(node, NULL, "arm,arm-sp804") {
+		struct resource r[2];
+
+		err = of_address_to_resource(node, 0, &r[0]);
+		if (err) {
+			pr_warn("Could not get " "resource for device tree"
+					"node '%s'", node->full_name);
+			goto put_node;
+		} else {
+			writel(0, MMIO_P2V(r[0].start) + TIMER_CTRL);
+			irq = of_irq_to_resource(node, 0, &r[1]);
+			v2m_timer_init_ce(MMIO_P2V(r[0].start), irq);
+			pr_info("Initializing timer %s start @0x%x irq %d\n",
+					node->name, r[0].start, irq);
+		}
+	}
+
+	return err;
+put_node:
+	of_node_put(node);
+	return err;
+}
+
+static struct clk_v2m osc1_clk =
+	INIT_CLK_V2M(24000000, SYS_CFG_OSC | SYS_CFG_SITE_MB | 1);
+
+static struct clk_fixed osc2_clk = INIT_CLK_FIXED(24000000);
+
+static struct clk_lookup v2m_lookups[] = {
+	{	/* FDT uart 0 */
+		.dev_id		= "uart@0",
+		.clk		= &osc2_clk.clk,
+	}, {	/* FDT uart 1 */
+		.dev_id		= "uart@1",
+		.clk		= &osc2_clk.clk,
+	}, {	/* FDT kbd    */
+		.dev_id		= "kbd@0",
+		.clk		= &osc2_clk.clk,
+	}, {	/* FDT mouse  */
+		.dev_id		= "ps2@0",
+		.clk		= &osc2_clk.clk,
+	}, {	/* FDT mouse  */
+		.dev_id		= "mmci@0",
+		.clk		= &osc2_clk.clk,
+	}, {	/* FDT mb clcd  */
+		.dev_id		= "clcd@1",
+		.clk		= &osc1_clk.clk,
+	}
+};
+
+static int __init v2m_init_dt(void)
+{
+	clkdev_add_table(v2m_lookups, ARRAY_SIZE(v2m_lookups));
+
+	pm_power_off = v2m_power_off;
+	arm_pm_restart = v2m_restart;
+
+	return 0;
+}
+arch_initcall(v2m_init_dt);
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index 06d6914..b9e46cd 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -1,29 +1,21 @@
 /*
- * Versatile Express V2M Motherboard Support
+ * Versatile Express V2M Motherboard static init
  */
 #include <linux/device.h>
 #include <linux/amba/bus.h>
-#include <linux/amba/mmci.h>
-#include <linux/io.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/smsc911x.h>
-#include <linux/spinlock.h>
 #include <linux/sysdev.h>
 #include <linux/usb/isp1760.h>
 
 #include <asm/clkdev.h>
 #include <asm/sizes.h>
 #include <asm/mach/flash.h>
-#include <asm/mach/map.h>
-#include <asm/mach/time.h>
-#include <asm/hardware/arm_timer.h>
 
 #include <mach/clkdev.h>
 #include <mach/motherboard.h>
 
-#include <plat/timer-sp.h>
-
 #include "core.h"
 
 #define V2M_PA_CS0	0x40000000
@@ -32,86 +24,6 @@
 #define V2M_PA_CS3	0x4c000000
 #define V2M_PA_CS7	0x10000000
 
-static struct map_desc v2m_io_desc[] __initdata = {
-	{
-		.virtual	= __MMIO_P2V(V2M_PA_CS7),
-		.pfn		= __phys_to_pfn(V2M_PA_CS7),
-		.length		= SZ_128K,
-		.type		= MT_DEVICE,
-	},
-};
-
-void __init v2m_map_io(struct map_desc *tile, size_t num)
-{
-	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
-	iotable_init(tile, num);
-}
-
-
-static void v2m_timer_init(void)
-{
-	writel(0, MMIO_P2V(V2M_TIMER0) + TIMER_CTRL);
-	writel(0, MMIO_P2V(V2M_TIMER1) + TIMER_CTRL);
-
-	sp804_clocksource_init(MMIO_P2V(V2M_TIMER1));
-	sp804_clockevents_init(MMIO_P2V(V2M_TIMER0), IRQ_V2M_TIMER0);
-}
-
-struct sys_timer v2m_timer = {
-	.init	= v2m_timer_init,
-};
-
-
-static DEFINE_SPINLOCK(v2m_cfg_lock);
-
-int v2m_cfg_write(u32 devfn, u32 data)
-{
-	/* Configuration interface broken? */
-	u32 val;
-
-	printk("%s: writing %08x to %08x\n", __func__, data, devfn);
-
-	devfn |= SYS_CFG_START | SYS_CFG_WRITE;
-
-	spin_lock(&v2m_cfg_lock);
-	val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
-	writel(val & ~SYS_CFG_COMPLETE, MMIO_P2V(V2M_SYS_CFGSTAT));
-
-	writel(data, MMIO_P2V(V2M_SYS_CFGDATA));
-	writel(devfn, MMIO_P2V(V2M_SYS_CFGCTRL));
-
-	do {
-		val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
-	} while (val == 0);
-	spin_unlock(&v2m_cfg_lock);
-
-	return !!(val & SYS_CFG_ERR);
-}
-
-int v2m_cfg_read(u32 devfn, u32 *data)
-{
-	u32 val;
-
-	devfn |= SYS_CFG_START;
-
-	spin_lock(&v2m_cfg_lock);
-	writel(0, MMIO_P2V(V2M_SYS_CFGSTAT));
-	writel(devfn, MMIO_P2V(V2M_SYS_CFGCTRL));
-
-	mb();
-
-	do {
-		cpu_relax();
-		val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
-	} while (val == 0);
-
-	*data = readl(MMIO_P2V(V2M_SYS_CFGDATA));
-	spin_unlock(&v2m_cfg_lock);
-
-	return !!(val & SYS_CFG_ERR);
-}
-
-
 static struct resource v2m_pcie_i2c_resource = {
 	.start	= V2M_SERIAL_BUS_PCI,
 	.end	= V2M_SERIAL_BUS_PCI + SZ_4K - 1,
@@ -194,29 +106,6 @@ static struct platform_device v2m_usb_device = {
 	.dev.platform_data = &v2m_usb_config,
 };
 
-static int v2m_flash_init(void)
-{
-	writel(0, MMIO_P2V(V2M_SYS_FLASH));
-	return 0;
-}
-
-static void v2m_flash_exit(void)
-{
-	writel(0, MMIO_P2V(V2M_SYS_FLASH));
-}
-
-static void v2m_flash_set_vpp(int on)
-{
-	writel(on != 0, MMIO_P2V(V2M_SYS_FLASH));
-}
-
-static struct flash_platform_data v2m_flash_data = {
-	.map_name	= "cfi_probe",
-	.width		= 4,
-	.init		= v2m_flash_init,
-	.exit		= v2m_flash_exit,
-	.set_vpp	= v2m_flash_set_vpp,
-};
 
 static struct resource v2m_flash_resources[] = {
 	{
@@ -239,15 +128,6 @@ static struct platform_device v2m_flash_device = {
 };
 
 
-static unsigned int v2m_mmci_status(struct device *dev)
-{
-	return !(readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0));
-}
-
-static struct mmci_platform_data v2m_mmci_data = {
-	.ocr_mask	= MMC_VDD_32_33|MMC_VDD_33_34,
-	.status		= v2m_mmci_status,
-};
 
 static AMBA_DEVICE(aaci,  "mb:aaci",  V2M_AACI, NULL);
 static AMBA_DEVICE(mmci,  "mb:mmci",  V2M_MMCI, &v2m_mmci_data);
@@ -273,46 +153,6 @@ static struct amba_device *v2m_amba_devs[] __initdata = {
 	&rtc_device,
 };
 
-#define to_clk_v2m(c) (container_of(c, struct clk_v2m, clk))
-
-static long clk_v2m_round_rate(struct clk *clk, unsigned long rate)
-{
-	return rate;
-}
-
-static int clk_v2m_set_rate(struct clk *_clk, unsigned long rate)
-{
-	struct clk_v2m *clk = to_clk_v2m(_clk);
-	int ret;
-
-	ret = v2m_cfg_write(clk->config, rate);
-	if (!ret)
-		clk->rate = rate;
-	return ret;
-}
-
-static void clk_v2m_put(struct clk *_clk)
-{
-}
-
-static int clk_v2m_get(struct clk *_clk)
-{
-	return 1;
-}
-
-static unsigned long clk_v2m_get_rate(struct clk *clk)
-{
-	return to_clk_v2m(clk)->rate;
-}
-
-const struct clk_ops clk_v2m_ops = {
-	.round_rate = clk_v2m_round_rate,
-	.get_rate = clk_v2m_get_rate,
-	.set_rate = clk_v2m_set_rate,
-	.get = clk_v2m_get,
-	.put = clk_v2m_put,
-};
-
 static struct clk_v2m osc1_clk =
 	INIT_CLK_V2M(24000000, SYS_CFG_OSC | SYS_CFG_SITE_MB | 1);
 
@@ -346,24 +186,10 @@ static struct clk_lookup v2m_lookups[] = {
 	},
 };
 
-static void v2m_power_off(void)
-{
-	if (v2m_cfg_write(SYS_CFG_SHUTDOWN | SYS_CFG_SITE_MB, 0))
-		printk(KERN_EMERG "Unable to shutdown\n");
-}
-
-static void v2m_restart(char str, const char *cmd)
-{
-	if (v2m_cfg_write(SYS_CFG_REBOOT | SYS_CFG_SITE_MB, 0))
-		printk(KERN_EMERG "Unable to reboot\n");
-}
-
 static int __init v2m_init(void)
 {
 	int i;
 
-	clkdev_add_table(v2m_lookups, ARRAY_SIZE(v2m_lookups));
-
 	platform_device_register(&v2m_pcie_i2c_device);
 	platform_device_register(&v2m_ddc_i2c_device);
 	platform_device_register(&v2m_flash_device);
@@ -373,6 +199,8 @@ static int __init v2m_init(void)
 	for (i = 0; i < ARRAY_SIZE(v2m_amba_devs); i++)
 		amba_device_register(v2m_amba_devs[i], &iomem_resource);
 
+	clkdev_add_table(v2m_lookups, ARRAY_SIZE(v2m_lookups));
+
 	pm_power_off = v2m_power_off;
 	arm_pm_restart = v2m_restart;
 
-- 
1.6.3.3

[RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Lorenzo Pieralisi]

The device tree methodology requires a dts file to be defined per
platform in order to describe the platform address space and topology,
in short the device tree in source format.

This patch adds the dts file for the Versatile Express board.
Some device names are still temporary and non-compliant waiting for the
definition of proper clock bindings.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/boot/dts/vexpress.dts |  199 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 199 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress.dts

diff --git a/arch/arm/boot/dts/vexpress.dts b/arch/arm/boot/dts/vexpress.dts
new file mode 100644
index 0000000..cc58603
--- /dev/null
+++ b/arch/arm/boot/dts/vexpress.dts
@@ -0,0 +1,199 @@
+/dts-v1/;
+
+/ {
+	model = "versatile-express";
+	compatible = "arm,versatile-express";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	#interrupt-cells = <1>;
+	interrupt-parent = <&intc>;
+
+	memory {
+		name = "memory";
+		device_type = "memory";
+		reg = <0x60000000 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "rdinit=/bin/ash console=ttyAMA0 vga=0x311 mem=512M debug earlyprintk";
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		device_type = "soc";
+		compatible = "simple-bus";
+		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
+
+		gic-cpu {
+			#address-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0x1e000100 0x1000>;
+			compatible = "arm,gic-cpu-interrupt-controller";
+			device_type = "gic";
+		};
+
+		intc: gic-dist {
+			#address-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0x1e001000 0x1000>;
+			compatible = "arm,gic-dist-interrupt-controller";
+			device_type = "gic";
+			irq-start = <29>;
+		};
+
+		smsc@4e000000 {
+			compatible = "smc,smsc-911";
+			 reg = <0x4e000000 0x1000>;
+			interrupts = <47>;
+			flags = <2>;
+			irq-polarity = <1>;
+			irq-type = <1>;
+			phy-interface = <0>;
+		};
+
+		flash@40000000 {
+			compatible = "arm,arm-flash";
+			reg = <0x40000000 0x4000000
+			       0x44000000 0x4000000>;
+		};
+
+		isp@4f000000 {
+			compatible = "nxp,usb-isp1761";
+			reg = <0x4f000000 0x20000>;
+			interrupts = <48>;
+			port1_otg;
+		};
+
+		ddc@10016000 {
+			compatible = "arm,versatile-i2c";
+			reg = <0x10016000 0x1000>;
+		};
+
+		pmu@0 {
+			compatible = "arm,arm-pmu";
+			interrupts = <92 93 94 95>;
+		};
+
+		timer@10011000 {
+			compatible = "arm,arm-sp804";
+			reg = <0x10011000 0x1000>;
+			interrupts = <34>;
+		};
+
+		timer@10012000 {
+			compatible = "arm,arm-sp804";
+			reg = <0x10012000 0x1000>;
+		};
+
+	};
+
+	amba {
+		compatible = "arm,amba";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		uart@0 {
+			compatible = "arm,amba-device";
+			reg = <0x10009000 0x1000>;
+			arm,amba-deviceid = <0x00141011>;
+			interrupts = <37>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		uart@1 {
+			compatible = "arm,amba-device";
+			reg = <0x1000a000 0x1000>;
+			arm,amba-deviceid = <0x00141011>;
+			interrupts = <38>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		kbd@0 {
+			compatible = "arm,amba-device";
+			reg = <0x10006000 0x1000>;
+			arm,amba-deviceid = <0x00041050>;
+			interrupts = <44>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		aaci@0 {
+			compatible = "arm,amba-device";
+			reg = <0x10004000 0x1000>;
+			arm,amba-deviceid = <0x00041041>;
+			interrupts = <43>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		ps2@0 {
+			compatible = "arm,amba-device";
+			reg = <0x10007000 0x1000>;
+			arm,amba-deviceid = <0x00041050>;
+			interrupts = <45>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		rtc@0 {
+			compatible = "arm,amba-device";
+			reg = <0x10017000 0x1000>;
+			arm,amba-deviceid = <0x00041031>;
+			interrupts = <36>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		wdt@0 {
+			compatible = "arm,amba-device";
+			reg = <0x1000f000 0x1000>;
+			arm,amba-deviceid = <0x00041085>;
+			interrupts = <32>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		clcd@0 {
+			compatible = "arm,clcd-pl11x","arm,amba-device";
+			reg = <0x10020000 0x1000>;
+			arm,amba-deviceid = <0x00041111>;
+			interrupts = <76>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		mmci@0 {
+			compatible = "arm,mmc-pl18x","arm,amba-device";
+			reg = <0x10005000 0x1000>;
+			interrupts = <41 42>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		dmc@0 {
+			compatible = "arm,dmc","arm,amba-device";
+			reg = <0x100e0000 0x1000>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		smc@0 {
+			compatible = "arm,smc","arm,amba-device";
+			reg = <0x100e1000 0x1000>;
+			interrupts = <77 78>;
+			clock-map = <&ref24_clk 0>;
+		};
+
+		gpio@0 {
+			compatible = "arm,gpio","arm,amba-device";
+			reg = <0x100e8000 0x1000>;
+			interrupts = <82>;
+			clock-map = <&ref24_clk 0>;
+		};
+	};
+
+	clocks {
+		ref24_clk: clock@0 {
+			device_type = "clock";
+			compatible = "fixed-clock";
+			clock-frequency = <24000000>;
+			clock-output-names = "ref";
+		};
+	};
+
+};
-- 
1.6.3.3

[RFC PATCH 14/14] ARM: vexpress: add device tree build system and dtbuImage

[Lorenzo Pieralisi]

The introduction of device tree in ARM Linux kernel tree requires the
definition of a new class of machines. Board init files should be split
in DT and non-DT (static) and the Kernel build system and config files
should be updated accordingly. To allow u-boot to recognize and boot
images with a device tree embedded stub, a new image is required
(dtbuImage), which is a u-boot compressed image with an assembly stub
containing the compiled flattened device tree.

This patch modifies the kernel build and config system to take into
account the Versatile Express device tree machine port and adds a new
image file suitable for booting a DT kernel with u-boot.
An appropriate machine config is added encompassing all config
parameters needed to compile in C units for DT booting on Versatile
Express.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/Kconfig                |    4 ++--
 arch/arm/Makefile               |    2 +-
 arch/arm/boot/Makefile          |   10 +++++++++-
 arch/arm/mach-vexpress/Kconfig  |    7 +++++++
 arch/arm/mach-vexpress/Makefile |    5 +++--
 arch/arm/mm/Kconfig             |    2 +-
 6 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 4ebecee..a63f7c2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1104,10 +1104,10 @@ config SMP
 	bool "Symmetric Multi-Processing (EXPERIMENTAL)"
 	depends on EXPERIMENTAL && (REALVIEW_EB_ARM11MP || REALVIEW_EB_A9MP ||\
 		 MACH_REALVIEW_PB11MP || MACH_REALVIEW_PBX || ARCH_OMAP4 ||\
-		 ARCH_U8500 || ARCH_VEXPRESS_CA9X4)
+		 ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_VEXPRESS_CA9X4_DT)
 	depends on GENERIC_CLOCKEVENTS
 	select USE_GENERIC_SMP_HELPERS
-	select HAVE_ARM_SCU if (ARCH_REALVIEW || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4)
+	select HAVE_ARM_SCU if (ARCH_REALVIEW || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_VEXPRESS_CA9X4_DT)
 	help
 	  This enables support for systems with more than one CPU. If you have
 	  a system with only one CPU, like most personal computers, say N. If
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index dab066a..3ce1751 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -264,7 +264,7 @@ archprepare:
 # Convert bzImage to zImage
 bzImage: zImage
 
-zImage Image xipImage bootpImage uImage dtbImage: vmlinux
+zImage Image xipImage bootpImage uImage dtbImage dtbuImage: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
 
 zinstall install: vmlinux
diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index c608c98..63200f1 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -27,7 +27,7 @@ INITRD_PHYS := $(initrd_phys-y)
 
 export ZRELADDR INITRD_PHYS PARAMS_PHYS
 
-targets := Image zImage xipImage bootpImage uImage dtbImage
+targets := Image zImage xipImage bootpImage uImage dtbuImage
 
 ifeq ($(CONFIG_XIP_KERNEL),y)
 
@@ -66,14 +66,18 @@ quiet_cmd_uimage = UIMAGE  $@
 
 ifeq ($(CONFIG_ZBOOT_ROM),y)
 $(obj)/uImage: LOADADDR=$(CONFIG_ZBOOT_ROM_TEXT)
+$(obj)/dtbuImage: LOADADDR=$(CONFIG_ZBOOT_ROM_TEXT)
 else
 $(obj)/uImage: LOADADDR=$(ZRELADDR)
+$(obj)/dtbuImage: LOADADDR=$(ZRELADDR)
 endif
 
 ifeq ($(CONFIG_THUMB2_KERNEL),y)
 # Set bit 0 to 1 so that "mov pc, rx" switches to Thumb-2 mode
 $(obj)/uImage: STARTADDR=$(shell echo $(LOADADDR) | sed -e "s/.$$/1/")
+$(obj)/dtbuImage: STARTADDR=$(shell echo $(LOADADDR) | sed -e "s/.$$/1/")
 else
+$(obj)/dtbuImage: STARTADDR=$(LOADADDR)
 $(obj)/uImage: STARTADDR=$(LOADADDR)
 endif
 
@@ -81,6 +85,10 @@ $(obj)/uImage:	$(obj)/zImage FORCE
 	$(call if_changed,uimage)
 	@echo '  Image $@ is ready'
 
+$(obj)/dtbuImage:	$(obj)/dtbImage FORCE
+	$(call if_changed,uimage)
+	@echo '  Image $@ is ready'
+
 $(obj)/bootp/bootp: $(obj)/zImage initrd FORCE
 	$(Q)$(MAKE) $(build)=$(obj)/bootp $@
 	@:
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 3f19b66..1d35a11 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -6,4 +6,11 @@ config ARCH_VEXPRESS_CA9X4
 	select CPU_V7
 	select ARM_GIC
 
+config ARCH_VEXPRESS_CA9X4_DT
+	bool "Versatile Express Cortex-A9x4 tile - DT enabled"
+	select CPU_V7
+	select ARM_GIC
+	select OF
+	select OF_FLATTREE
+
 endmenu
diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
index 1b71b77..c6bc159 100644
--- a/arch/arm/mach-vexpress/Makefile
+++ b/arch/arm/mach-vexpress/Makefile
@@ -2,7 +2,8 @@
 # Makefile for the linux kernel.
 #
 
-obj-y					:= v2m.o
-obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
+obj-y					:= v2m-base.o ct-ca9x4-base.o
+obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o v2m.o
+obj-$(CONFIG_ARCH_VEXPRESS_CA9X4_DT)	+= ct-ca9x4-of.o v2m-of.o
 obj-$(CONFIG_SMP)			+= platsmp.o headsmp.o
 obj-$(CONFIG_LOCAL_TIMERS)		+= localtimer.o
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 101105e..97bc19e 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -782,7 +782,7 @@ config CACHE_L2X0
 	bool "Enable the L2x0 outer cache controller"
 	depends on REALVIEW_EB_ARM11MP || MACH_REALVIEW_PB11MP || MACH_REALVIEW_PB1176 || \
 		   REALVIEW_EB_A9MP || ARCH_MX35 || ARCH_MX31 || MACH_REALVIEW_PBX || \
-		   ARCH_NOMADIK || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4
+		   ARCH_NOMADIK || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_VEXPRESS_CA9X4_DT
 	default y
 	select OUTER_CACHE
 	select OUTER_CACHE_SYNC
-- 
1.6.3.3

Re: [RFC PATCH 14/14] ARM: vexpress: add device tree build system and dtbuImage

[Grant Likely]

On Wed, Aug 18, 2010 at 1:00 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> The introduction of device tree in ARM Linux kernel tree requires the
> definition of a new class of machines. Board init files should be split
> in DT and non-DT (static) and the Kernel build system and config files
> should be updated accordingly. To allow u-boot to recognize and boot
> images with a device tree embedded stub, a new image is required
> (dtbuImage), which is a u-boot compressed image with an assembly stub
> containing the compiled flattened device tree.
>
> This patch modifies the kernel build and config system to take into
> account the Versatile Express device tree machine port and adds a new
> image file suitable for booting a DT kernel with u-boot.
> An appropriate machine config is added encompassing all config
> parameters needed to compile in C units for DT booting on Versatile
> Express.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm/Kconfig                |    4 ++--
>  arch/arm/Makefile               |    2 +-
>  arch/arm/boot/Makefile          |   10 +++++++++-
>  arch/arm/mach-vexpress/Kconfig  |    7 +++++++
>  arch/arm/mach-vexpress/Makefile |    5 +++--
>  arch/arm/mm/Kconfig             |    2 +-
>  6 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 4ebecee..a63f7c2 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1104,10 +1104,10 @@ config SMP
>        bool "Symmetric Multi-Processing (EXPERIMENTAL)"
>        depends on EXPERIMENTAL && (REALVIEW_EB_ARM11MP || REALVIEW_EB_A9MP ||\
>                 MACH_REALVIEW_PB11MP || MACH_REALVIEW_PBX || ARCH_OMAP4 ||\
> -                ARCH_U8500 || ARCH_VEXPRESS_CA9X4)
> +                ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_VEXPRESS_CA9X4_DT)
>        depends on GENERIC_CLOCKEVENTS
>        select USE_GENERIC_SMP_HELPERS
> -       select HAVE_ARM_SCU if (ARCH_REALVIEW || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4)
> +       select HAVE_ARM_SCU if (ARCH_REALVIEW || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_VEXPRESS_CA9X4_DT)

This of course is not sustainable.  There should be a common config
symbol which these boards select.  ARCH_VEXPRESS_CA9X4 and
ARCH_VEXPRESS_CA9X4_DT should certainly not be tested for separately.

>        help
>          This enables support for systems with more than one CPU. If you have
>          a system with only one CPU, like most personal computers, say N. If
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index dab066a..3ce1751 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -264,7 +264,7 @@ archprepare:
>  # Convert bzImage to zImage
>  bzImage: zImage
>
> -zImage Image xipImage bootpImage uImage dtbImage: vmlinux
> +zImage Image xipImage bootpImage uImage dtbImage dtbuImage: vmlinux
>        $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
>
>  zinstall install: vmlinux
> diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
> index c608c98..63200f1 100644
> --- a/arch/arm/boot/Makefile
> +++ b/arch/arm/boot/Makefile
> @@ -27,7 +27,7 @@ INITRD_PHYS := $(initrd_phys-y)
>
>  export ZRELADDR INITRD_PHYS PARAMS_PHYS
>
> -targets := Image zImage xipImage bootpImage uImage dtbImage
> +targets := Image zImage xipImage bootpImage uImage dtbuImage
>
>  ifeq ($(CONFIG_XIP_KERNEL),y)
>
> @@ -66,14 +66,18 @@ quiet_cmd_uimage = UIMAGE  $@
>
>  ifeq ($(CONFIG_ZBOOT_ROM),y)
>  $(obj)/uImage: LOADADDR=$(CONFIG_ZBOOT_ROM_TEXT)
> +$(obj)/dtbuImage: LOADADDR=$(CONFIG_ZBOOT_ROM_TEXT)
>  else
>  $(obj)/uImage: LOADADDR=$(ZRELADDR)
> +$(obj)/dtbuImage: LOADADDR=$(ZRELADDR)
>  endif
>
>  ifeq ($(CONFIG_THUMB2_KERNEL),y)
>  # Set bit 0 to 1 so that "mov pc, rx" switches to Thumb-2 mode
>  $(obj)/uImage: STARTADDR=$(shell echo $(LOADADDR) | sed -e "s/.$$/1/")
> +$(obj)/dtbuImage: STARTADDR=$(shell echo $(LOADADDR) | sed -e "s/.$$/1/")
>  else
> +$(obj)/dtbuImage: STARTADDR=$(LOADADDR)
>  $(obj)/uImage: STARTADDR=$(LOADADDR)
>  endif
>
> @@ -81,6 +85,10 @@ $(obj)/uImage:       $(obj)/zImage FORCE
>        $(call if_changed,uimage)
>        @echo '  Image $@ is ready'
>
> +$(obj)/dtbuImage:      $(obj)/dtbImage FORCE
> +       $(call if_changed,uimage)
> +       @echo '  Image $@ is ready'
> +
>  $(obj)/bootp/bootp: $(obj)/zImage initrd FORCE
>        $(Q)$(MAKE) $(build)=$(obj)/bootp $@
>        @:
> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index 3f19b66..1d35a11 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -6,4 +6,11 @@ config ARCH_VEXPRESS_CA9X4
>        select CPU_V7
>        select ARM_GIC
>
> +config ARCH_VEXPRESS_CA9X4_DT
> +       bool "Versatile Express Cortex-A9x4 tile - DT enabled"
> +       select CPU_V7
> +       select ARM_GIC
> +       select OF
> +       select OF_FLATTREE
> +
>  endmenu
> diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> index 1b71b77..c6bc159 100644
> --- a/arch/arm/mach-vexpress/Makefile
> +++ b/arch/arm/mach-vexpress/Makefile
> @@ -2,7 +2,8 @@
>  # Makefile for the linux kernel.
>  #
>
> -obj-y                                  := v2m.o
> -obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)      += ct-ca9x4.o
> +obj-y                                  := v2m-base.o ct-ca9x4-base.o
> +obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)      += ct-ca9x4.o v2m.o
> +obj-$(CONFIG_ARCH_VEXPRESS_CA9X4_DT)   += ct-ca9x4-of.o v2m-of.o
>  obj-$(CONFIG_SMP)                      += platsmp.o headsmp.o
>  obj-$(CONFIG_LOCAL_TIMERS)             += localtimer.o
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 101105e..97bc19e 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -782,7 +782,7 @@ config CACHE_L2X0
>        bool "Enable the L2x0 outer cache controller"
>        depends on REALVIEW_EB_ARM11MP || MACH_REALVIEW_PB11MP || MACH_REALVIEW_PB1176 || \
>                   REALVIEW_EB_A9MP || ARCH_MX35 || ARCH_MX31 || MACH_REALVIEW_PBX || \
> -                  ARCH_NOMADIK || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4
> +                  ARCH_NOMADIK || ARCH_OMAP4 || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_VEXPRESS_CA9X4_DT
>        default y
>        select OUTER_CACHE
>        select OUTER_CACHE_SYNC
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> The device tree methodology requires a dts file to be defined per
> platform in order to describe the platform address space and topology,
> in short the device tree in source format.
>
> This patch adds the dts file for the Versatile Express board.
> Some device names are still temporary and non-compliant waiting for the
> definition of proper clock bindings.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm/boot/dts/vexpress.dts |  199 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 199 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/boot/dts/vexpress.dts
>
> diff --git a/arch/arm/boot/dts/vexpress.dts b/arch/arm/boot/dts/vexpress.dts
> new file mode 100644
> index 0000000..cc58603
> --- /dev/null
> +++ b/arch/arm/boot/dts/vexpress.dts
> @@ -0,0 +1,199 @@
> +/dts-v1/;
> +
> +/ {
> +       model = "versatile-express";
> +       compatible = "arm,versatile-express";
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       #interrupt-cells = <1>;

#interrupt-cells does not belong in the root node.  You can remove this.

> +       interrupt-parent = <&intc>;
> +
> +       memory {
> +               name = "memory";
> +               device_type = "memory";
> +               reg = <0x60000000 0x20000000>;
> +       };
> +
> +       chosen {
> +               bootargs = "rdinit=/bin/ash console=ttyAMA0 vga=0x311 mem=512M debug earlyprintk";
> +       };
> +
> +       soc {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               device_type = "soc";

Remove device_type property.  It only has meaning when real
OpenFirmware is present on the board.

> +               compatible = "simple-bus";
> +               ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
> +
> +               gic-cpu {
> +                       #address-cells = <0>;
> +                       #interrupt-cells = <1>;
> +                       interrupt-controller;
> +                       reg = <0x1e000100 0x1000>;
> +                       compatible = "arm,gic-cpu-interrupt-controller";
> +                       device_type = "gic";

Ditto on device_type

> +               };
> +
> +               intc: gic-dist {
> +                       #address-cells = <0>;
> +                       #interrupt-cells = <1>;
> +                       interrupt-controller;
> +                       reg = <0x1e001000 0x1000>;
> +                       compatible = "arm,gic-dist-interrupt-controller";
> +                       device_type = "gic";

Ditto

> +                       irq-start = <29>;

This ends up encoding Linux-kernel implementation details into the
device tree.  Ideally the Linux irq numbers are to be dynamically
assigned by the kernel for each interrupt controller, but I haven't
ported the support code for that yet.  Shouldn't be too hard to do
though.

> +               };
> +
> +               smsc@4e000000 {
> +                       compatible = "smc,smsc-911";
> +                        reg = <0x4e000000 0x1000>;
> +                       interrupts = <47>;

I suspect this interrupts property is wrong.  An interrupts specifier
is specific to the interrupt controller node; so this should always
specify the interrupt input on a specific controller (either gic-cpu
or gic-dist in this system).  It looks like '47' is the global irq
number.  Am I correct?

> +                       flags = <2>;
> +                       irq-polarity = <1>;
> +                       irq-type = <1>;
> +                       phy-interface = <0>;

As mentioned in my comments on the smsc911x driver changes, there
already is a binding for the phy which should be used, and the irq
flags should be specified as a second cell in the 'interrupts'
property.  This also means that the interrupt controller nodes should
probably use #interrupt-cells = <2>;

> +               };
> +
> +               flash@40000000 {
> +                       compatible = "arm,arm-flash";

Use "cfi-flash" which the physmap_of.c driver will bind against.

> +                       reg = <0x40000000 0x4000000
> +                              0x44000000 0x4000000>;
> +               };
> +
> +               isp@4f000000 {
> +                       compatible = "nxp,usb-isp1761";
> +                       reg = <0x4f000000 0x20000>;
> +                       interrupts = <48>;
> +                       port1_otg;
> +               };
> +
> +               ddc@10016000 {
> +                       compatible = "arm,versatile-i2c";
> +                       reg = <0x10016000 0x1000>;
> +               };
> +
> +               pmu@0 {

If there is no 'reg' property, then there should not be a @0 component
to the node name.

> +                       compatible = "arm,arm-pmu";
> +                       interrupts = <92 93 94 95>;
> +               };
> +
> +               timer@10011000 {
> +                       compatible = "arm,arm-sp804";
> +                       reg = <0x10011000 0x1000>;
> +                       interrupts = <34>;
> +               };
> +
> +               timer@10012000 {
> +                       compatible = "arm,arm-sp804";
> +                       reg = <0x10012000 0x1000>;
> +               };
> +
> +       };
> +
> +       amba {
> +               compatible = "arm,amba";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +
> +               uart@0 {

uart@10009000

The @<id> portion of the node name consists of the first
#address-cells values of the reg property separated by commas.  If
#address-cells = <1>, then it is just a single number like in this
case.

> +                       compatible = "arm,amba-device";
> +                       reg = <0x10009000 0x1000>;
> +                       arm,amba-deviceid = <0x00141011>;
> +                       interrupts = <37>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               uart@1 {

ditto (and so on down the file)

> +                       compatible = "arm,amba-device";
> +                       reg = <0x1000a000 0x1000>;
> +                       arm,amba-deviceid = <0x00141011>;
> +                       interrupts = <38>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               kbd@0 {
> +                       compatible = "arm,amba-device";
> +                       reg = <0x10006000 0x1000>;
> +                       arm,amba-deviceid = <0x00041050>;
> +                       interrupts = <44>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               aaci@0 {
> +                       compatible = "arm,amba-device";
> +                       reg = <0x10004000 0x1000>;
> +                       arm,amba-deviceid = <0x00041041>;
> +                       interrupts = <43>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               ps2@0 {
> +                       compatible = "arm,amba-device";
> +                       reg = <0x10007000 0x1000>;
> +                       arm,amba-deviceid = <0x00041050>;
> +                       interrupts = <45>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               rtc@0 {
> +                       compatible = "arm,amba-device";
> +                       reg = <0x10017000 0x1000>;
> +                       arm,amba-deviceid = <0x00041031>;
> +                       interrupts = <36>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               wdt@0 {
> +                       compatible = "arm,amba-device";
> +                       reg = <0x1000f000 0x1000>;
> +                       arm,amba-deviceid = <0x00041085>;
> +                       interrupts = <32>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               clcd@0 {
> +                       compatible = "arm,clcd-pl11x","arm,amba-device";
> +                       reg = <0x10020000 0x1000>;
> +                       arm,amba-deviceid = <0x00041111>;
> +                       interrupts = <76>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               mmci@0 {
> +                       compatible = "arm,mmc-pl18x","arm,amba-device";
> +                       reg = <0x10005000 0x1000>;
> +                       interrupts = <41 42>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               dmc@0 {
> +                       compatible = "arm,dmc","arm,amba-device";
> +                       reg = <0x100e0000 0x1000>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               smc@0 {
> +                       compatible = "arm,smc","arm,amba-device";
> +                       reg = <0x100e1000 0x1000>;
> +                       interrupts = <77 78>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +
> +               gpio@0 {
> +                       compatible = "arm,gpio","arm,amba-device";
> +                       reg = <0x100e8000 0x1000>;
> +                       interrupts = <82>;
> +                       clock-map = <&ref24_clk 0>;
> +               };
> +       };
> +
> +       clocks {
> +               ref24_clk: clock@0 {
> +                       device_type = "clock";
> +                       compatible = "fixed-clock";
> +                       clock-frequency = <24000000>;
> +                       clock-output-names = "ref";

This will of course need to be updated to the clock binding that
Jeremy and I have settled on, but you already knew that.  :-)

Cheers,
g.

RE: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Lorenzo Pieralisi]

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> Likely
> Sent: 18 August 2010 22:55
> To: Lorenzo Pieralisi
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Philippe Robin; nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org;
> linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org; Catalin Marinas; jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
> Subject: Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts
> specification
> 
> On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> > The device tree methodology requires a dts file to be defined per
> > platform in order to describe the platform address space and topology,
> > in short the device tree in source format.
> >
> > This patch adds the dts file for the Versatile Express board.
> > Some device names are still temporary and non-compliant waiting for the
> > definition of proper clock bindings.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/vexpress.dts |  199
> ++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 199 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/arm/boot/dts/vexpress.dts
> >
> > diff --git a/arch/arm/boot/dts/vexpress.dts
> b/arch/arm/boot/dts/vexpress.dts
> > new file mode 100644
> > index 0000000..cc58603
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/vexpress.dts
> > @@ -0,0 +1,199 @@
> > +/dts-v1/;
> > +
> > +/ {
> > +       model = "versatile-express";
> > +       compatible = "arm,versatile-express";
> > +       #address-cells = <1>;
> > +       #size-cells = <1>;
> > +       #interrupt-cells = <1>;
> 
> #interrupt-cells does not belong in the root node.  You can remove this.
> 
> > +       interrupt-parent = <&intc>;
> > +
> > +       memory {
> > +               name = "memory";
> > +               device_type = "memory";
> > +               reg = <0x60000000 0x20000000>;
> > +       };
> > +
> > +       chosen {
> > +               bootargs = "rdinit=/bin/ash console=ttyAMA0 vga=0x311
> mem=512M debug earlyprintk";
> > +       };
> > +
> > +       soc {
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               device_type = "soc";
> 
> Remove device_type property.  It only has meaning when real
> OpenFirmware is present on the board.

OK.

> 
> > +               compatible = "simple-bus";
> > +               ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
> > +
> > +               gic-cpu {
> > +                       #address-cells = <0>;
> > +                       #interrupt-cells = <1>;
> > +                       interrupt-controller;
> > +                       reg = <0x1e000100 0x1000>;
> > +                       compatible = "arm,gic-cpu-interrupt-controller";
> > +                       device_type = "gic";
> 
> Ditto on device_type
>

OK.
 
> > +               };
> > +
> > +               intc: gic-dist {
> > +                       #address-cells = <0>;
> > +                       #interrupt-cells = <1>;
> > +                       interrupt-controller;
> > +                       reg = <0x1e001000 0x1000>;
> > +                       compatible = "arm,gic-dist-interrupt-controller";
> > +                       device_type = "gic";
> 
> Ditto
> 
> > +                       irq-start = <29>;
> 
> This ends up encoding Linux-kernel implementation details into the
> device tree.  Ideally the Linux irq numbers are to be dynamically
> assigned by the kernel for each interrupt controller, but I haven't
> ported the support code for that yet.  Shouldn't be too hard to do
> though.
> 

Ok, let's keep in sync on this, see below.

> > +               };
> > +
> > +               smsc@4e000000 {
> > +                       compatible = "smc,smsc-911";
> > +                        reg = <0x4e000000 0x1000>;
> > +                       interrupts = <47>;
> 
> I suspect this interrupts property is wrong.  An interrupts specifier
> is specific to the interrupt controller node; so this should always
> specify the interrupt input on a specific controller (either gic-cpu
> or gic-dist in this system).  It looks like '47' is the global irq
> number.  Am I correct?
>

Strictly related to the previous point.

When you say input Grant you mean input HW pin or interrupt HW ID ?

I think you mean the irq ID specific to a given interrupt controller, 
because that is what SW sees and controls.

47 is the eth interrupt ID on the gic-dist, that in this particular
case becomes the global IRQ number as well if I am not mistaken. 
(it is retrieved through the platform device resource).
I think it is correct as it is, but I will countercheck.

> > +                       flags = <2>;
> > +                       irq-polarity = <1>;
> > +                       irq-type = <1>;
> > +                       phy-interface = <0>;
> 
> As mentioned in my comments on the smsc911x driver changes, there
> already is a binding for the phy which should be used, and the irq
> flags should be specified as a second cell in the 'interrupts'
> property.  This also means that the interrupt controller nodes should
> probably use #interrupt-cells = <2>;
> 

Ok, I will patch the dts accordingly.

> > +               };
> > +
> > +               flash@40000000 {
> > +                       compatible = "arm,arm-flash";
> 
> Use "cfi-flash" which the physmap_of.c driver will bind against.
> 

Ok, I will follow Catalin's reply up.

> > +                       reg = <0x40000000 0x4000000
> > +                              0x44000000 0x4000000>;
> > +               };
> > +
> > +               isp@4f000000 {
> > +                       compatible = "nxp,usb-isp1761";
> > +                       reg = <0x4f000000 0x20000>;
> > +                       interrupts = <48>;
> > +                       port1_otg;
> > +               };
> > +
> > +               ddc@10016000 {
> > +                       compatible = "arm,versatile-i2c";
> > +                       reg = <0x10016000 0x1000>;
> > +               };
> > +
> > +               pmu@0 {
> 
> If there is no 'reg' property, then there should not be a @0 component
> to the node name.
>

Ok.
 
> > +                       compatible = "arm,arm-pmu";
> > +                       interrupts = <92 93 94 95>;
> > +               };
> > +
> > +               timer@10011000 {
> > +                       compatible = "arm,arm-sp804";
> > +                       reg = <0x10011000 0x1000>;
> > +                       interrupts = <34>;
> > +               };
> > +
> > +               timer@10012000 {
> > +                       compatible = "arm,arm-sp804";
> > +                       reg = <0x10012000 0x1000>;
> > +               };
> > +
> > +       };
> > +
> > +       amba {
> > +               compatible = "arm,amba";
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +
> > +               uart@0 {
> 
> uart@10009000
> 
> The @<id> portion of the node name consists of the first
> #address-cells values of the reg property separated by commas.  If
> #address-cells = <1>, then it is just a single number like in this
> case.
>

Yes Grant, consider it done, I left them like that because the clock 
look-up uses the name (before merging your changes) and I avoided recoding
the clock LUT before posting the patchset.
 
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x10009000 0x1000>;
> > +                       arm,amba-deviceid = <0x00141011>;
> > +                       interrupts = <37>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               uart@1 {
> 
> ditto (and so on down the file)
> 
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x1000a000 0x1000>;
> > +                       arm,amba-deviceid = <0x00141011>;
> > +                       interrupts = <38>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               kbd@0 {
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x10006000 0x1000>;
> > +                       arm,amba-deviceid = <0x00041050>;
> > +                       interrupts = <44>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               aaci@0 {
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x10004000 0x1000>;
> > +                       arm,amba-deviceid = <0x00041041>;
> > +                       interrupts = <43>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               ps2@0 {
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x10007000 0x1000>;
> > +                       arm,amba-deviceid = <0x00041050>;
> > +                       interrupts = <45>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               rtc@0 {
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x10017000 0x1000>;
> > +                       arm,amba-deviceid = <0x00041031>;
> > +                       interrupts = <36>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               wdt@0 {
> > +                       compatible = "arm,amba-device";
> > +                       reg = <0x1000f000 0x1000>;
> > +                       arm,amba-deviceid = <0x00041085>;
> > +                       interrupts = <32>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               clcd@0 {
> > +                       compatible = "arm,clcd-pl11x","arm,amba-device";
> > +                       reg = <0x10020000 0x1000>;
> > +                       arm,amba-deviceid = <0x00041111>;
> > +                       interrupts = <76>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               mmci@0 {
> > +                       compatible = "arm,mmc-pl18x","arm,amba-device";
> > +                       reg = <0x10005000 0x1000>;
> > +                       interrupts = <41 42>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               dmc@0 {
> > +                       compatible = "arm,dmc","arm,amba-device";
> > +                       reg = <0x100e0000 0x1000>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               smc@0 {
> > +                       compatible = "arm,smc","arm,amba-device";
> > +                       reg = <0x100e1000 0x1000>;
> > +                       interrupts = <77 78>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +
> > +               gpio@0 {
> > +                       compatible = "arm,gpio","arm,amba-device";
> > +                       reg = <0x100e8000 0x1000>;
> > +                       interrupts = <82>;
> > +                       clock-map = <&ref24_clk 0>;
> > +               };
> > +       };
> > +
> > +       clocks {
> > +               ref24_clk: clock@0 {
> > +                       device_type = "clock";
> > +                       compatible = "fixed-clock";
> > +                       clock-frequency = <24000000>;
> > +                       clock-output-names = "ref";
> 
> This will of course need to be updated to the clock binding that
> Jeremy and I have settled on, but you already knew that.  :-)
> 

Yes, again, you are right I knew that but I wanted to post a version 
I completely tested before applying the new clock bindings when rebasing.
I will update the code for the next respin.

> Cheers,
> g.

Thanks a lot,
Lorenzo

Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Grant Likely]

Hi Lorenzo,

On Fri, Aug 20, 2010 at 11:51 AM, Lorenzo Pieralisi
<Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
>> > +               };
>> > +
>> > +               smsc@4e000000 {
>> > +                       compatible = "smc,smsc-911";
>> > +                        reg = <0x4e000000 0x1000>;
>> > +                       interrupts = <47>;
>>
>> I suspect this interrupts property is wrong.  An interrupts specifier
>> is specific to the interrupt controller node; so this should always
>> specify the interrupt input on a specific controller (either gic-cpu
>> or gic-dist in this system).  It looks like '47' is the global irq
>> number.  Am I correct?
>>
>
> Strictly related to the previous point.
>
> When you say input Grant you mean input HW pin or interrupt HW ID ?
>
> I think you mean the irq ID specific to a given interrupt controller,
> because that is what SW sees and controls.

I mean HW pin, but see discussion below to make sure we're talking
about the same thing.

> 47 is the eth interrupt ID on the gic-dist, that in this particular
> case becomes the global IRQ number as well if I am not mistaken.
> (it is retrieved through the platform device resource).
> I think it is correct as it is, but I will countercheck.

Okay.  Just to make sure I understand... I see two interrupt
controllers in this device tree; gic-cpu and gic-dist.  I'm assuming
that each irq controller has N interrupt inputs numbered 0..N-1.  My
expectation would be that each device is wired to a single interrupt
pin on one of the two controllers.  I also would expect that (unless
there are two interrupt input pins on the CPU) one of the irq
controllers is cascaded to the other.  Am I correct so far?

So, if 47 was the hardware input number, that would mean irq-dist had
least 48 discrete interrupt inputs (this is actually what had me
suspicious; I assumed that that are only 32 inputs on the gic
controller but I didn't go and check).

>From the Linux perspective, the hw irq numbers for each controller
needs to be mapped onto the flat Linux irq space.  So if gic-cpu has N
irqs and gic-dist has M irqs, then the mapping might look something
like this:

linux irq
(0)   == gic-cpu:0
(1)   == gic-cpu:1
...
(N-2) == gpi-cpu:N-2
(N-1) == gpi-cpu:N-1
(N)   == gic-dist:0
(N+1) == gic-dist:1
...
(N+M-2) == gpi-dist:M-2
(N+M-1) == gpi-dist:M-1

Am I still correct, or have I misunderstood the versatile interrupt
controller architecture?

Regardless though, the linux mapping doesn't actually have any bearing
on the device tree because the mapping from HW to linux irq number is
all internal to the kernel.  The kernel could choose to start the
mapping at 42 or somewhere else and it would all still work.  The
of_irq support code has all the information it needs to translate an
interrupt specifier to an interrupt controller, and then map the
number to the linux irq number.

[...]
>> > +       clocks {
>> > +               ref24_clk: clock@0 {
>> > +                       device_type = "clock";
>> > +                       compatible = "fixed-clock";
>> > +                       clock-frequency = <24000000>;
>> > +                       clock-output-names = "ref";
>>
>> This will of course need to be updated to the clock binding that
>> Jeremy and I have settled on, but you already knew that.  :-)
>>
>
> Yes, again, you are right I knew that but I wanted to post a version
> I completely tested before applying the new clock bindings when rebasing.
> I will update the code for the next respin.


Cool, I look forward to seeing it.  Thanks!

g.

Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[M. Warner Losh]

In message: <AANLkTi=WiHd06=e4d_WdCe+HMfFhj-Q=xRS_4f-TzGUs-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
            Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> writes:
: Regardless though, the linux mapping doesn't actually have any bearing
: on the device tree because the mapping from HW to linux irq number is
: all internal to the kernel.  The kernel could choose to start the
: mapping at 42 or somewhere else and it would all still work.

Some (all?) PowerPC platforms start their interrupt numbering at 16,
since interrupts 0-15 are reserved for old-school ISA interrupts from
the Super I/O chip that may (or may not) be present on these systems.
It's all handled inside the kernel, so is invisible to the
device-tree, as Grant is saying.  Also, more OSes than Linux use
device trees, so hard-coding Linux IRQ numbers would be displeasing
from that point of view.

Just to give one concrete example.

Warner

Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Grant Likely]

On Fri, Aug 20, 2010 at 3:03 PM, M. Warner Losh <imp-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org> wrote:
> In message: <AANLkTi=WiHd06=e4d_WdCe+HMfFhj-Q=xRS_4f-TzGUs-JsoAwUIsXos@public.gmane.orgil.com>
>            Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> writes:
> : Regardless though, the linux mapping doesn't actually have any bearing
> : on the device tree because the mapping from HW to linux irq number is
> : all internal to the kernel.  The kernel could choose to start the
> : mapping at 42 or somewhere else and it would all still work.
>
> Some (all?) PowerPC platforms start their interrupt numbering at 16,
> since interrupts 0-15 are reserved for old-school ISA interrupts from
> the Super I/O chip that may (or may not) be present on these systems.
> It's all handled inside the kernel, so is invisible to the
> device-tree, as Grant is saying.  Also, more OSes than Linux use
> device trees, so hard-coding Linux IRQ numbers would be displeasing
> from that point of view.

Yes, good point Warner.  In fact, that 16 offset has nothing to do
with the hardware, but rather to legacy drivers which have their irq
numbers hard coded in the 0-15 range.

g.

RE: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Lorenzo Pieralisi]

Hi Grant,

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> Likely
> Sent: 20 August 2010 21:32
> To: Lorenzo Pieralisi
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Philippe Robin; nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org;
> linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org; Catalin Marinas; jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
> Subject: Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts
> specification
> 
> Hi Lorenzo,
> 
> On Fri, Aug 20, 2010 at 11:51 AM, Lorenzo Pieralisi
> <Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> >> > +               };
> >> > +
> >> > +               smsc@4e000000 {
> >> > +                       compatible = "smc,smsc-911";
> >> > +                        reg = <0x4e000000 0x1000>;
> >> > +                       interrupts = <47>;
> >>
> >> I suspect this interrupts property is wrong.  An interrupts specifier
> >> is specific to the interrupt controller node; so this should always
> >> specify the interrupt input on a specific controller (either gic-cpu
> >> or gic-dist in this system).  It looks like '47' is the global irq
> >> number.  Am I correct?
> >>
> >
> > Strictly related to the previous point.
> >
> > When you say input Grant you mean input HW pin or interrupt HW ID ?
> >
> > I think you mean the irq ID specific to a given interrupt controller,
> > because that is what SW sees and controls.
> 
> I mean HW pin, but see discussion below to make sure we're talking
> about the same thing.
> 
> > 47 is the eth interrupt ID on the gic-dist, that in this particular
> > case becomes the global IRQ number as well if I am not mistaken.
> > (it is retrieved through the platform device resource).
> > I think it is correct as it is, but I will countercheck.
> 
> Okay.  Just to make sure I understand... I see two interrupt
> controllers in this device tree; gic-cpu and gic-dist.  I'm assuming
> that each irq controller has N interrupt inputs numbered 0..N-1.  My
> expectation would be that each device is wired to a single interrupt
> pin on one of the two controllers.  I also would expect that (unless
> there are two interrupt input pins on the CPU) one of the irq
> controllers is cascaded to the other.  Am I correct so far?
>

Well, it is the fact that I coded them as two controllers which is the
source of confusion, I apologise but it is good to start discussing 
a proper binding. 
The gic-cpu and gic-dist are two subcomponents of a single 
interrupt controller called GIC. Devices are wired to the gic-dist which
is in charge of distributing interrupts to the gic-cpu (per cpu IRQ IF). 
 
> So, if 47 was the hardware input number, that would mean irq-dist had
> least 48 discrete interrupt inputs (this is actually what had me
> suspicious; I assumed that that are only 32 inputs on the gic
> controller but I didn't go and check).

The GIC deals with interrupts through ID numbers, and yes the gic-dist 
can have more than 48 discrete interrupts lines.
The point is that HW input pin numbers or wires map to irq ID through a shift
(e.g. HW ID 37 means interrupt pin 5, because the first 32 irq IDs are
used for SW IRQs and private (to one core) peripheral IRQs).
That's why I asked if  "interrupts" should represent the HW input pin/line.
Ethernet interrupt GIC HW ID is 47 (vexpress), but that does not mean 
the eth device irq line is connected to the 47th irq input line in 
the gic-dist because interrupt ID for peripherals start at ID 16 
(and there are two kinds of peripherals, private to a core (ID 16..31) 
or common to all cores, ID 32 onwards).
TO be honest I do not think we should think in HW interrupt pin terms, in
this context it is the GIC HW ID that should go into "interrupts" (which in
turn means we may well end up having two identical "interrupts" values
on different GICs (cascaded), and that's perfectly fine because
they are relative to a given controller).

> 
> From the Linux perspective, the hw irq numbers for each controller
> needs to be mapped onto the flat Linux irq space.  So if gic-cpu has N
> irqs and gic-dist has M irqs, then the mapping might look something
> like this:
> 
> linux irq
> (0)   == gic-cpu:0
> (1)   == gic-cpu:1
> ...
> (N-2) == gpi-cpu:N-2
> (N-1) == gpi-cpu:N-1
> (N)   == gic-dist:0
> (N+1) == gic-dist:1
> ...
> (N+M-2) == gpi-dist:M-2
> (N+M-1) == gpi-dist:M-1
> 
> Am I still correct, or have I misunderstood the versatile interrupt
> controller architecture?

The two components have to be seen as one interrupt controller.
It is possible we need two "reg" properties since the offset between
gic-cpu registers and gic-dist ones may differ.
If there is a board/SoC with two cascaded GICs your reasoning
above applies, agreed, sorry for having split the single GIC in two
interrupt controllers, that caused confusion (but it is a RFC
to define a proper binding 8-) ).

> 
> Regardless though, the linux mapping doesn't actually have any bearing
> on the device tree because the mapping from HW to linux irq number is
> all internal to the kernel.  The kernel could choose to start the
> mapping at 42 or somewhere else and it would all still work.  The
> of_irq support code has all the information it needs to translate an
> interrupt specifier to an interrupt controller, and then map the
> number to the linux irq number.
> 

Agreed, and took your point and Warner's. I did not want to code the
Linux global number in the interrupts property. IMHO that should define
the GIC HW irq ID and it is not Linux specific, see above.
But due to the current mapping 47 ends up being the Linux IRQ number 
as well.
It is time I started writing a GIC binding since it is likely to be
reused for different boards (and gic init code should be factored out
of board specific dirs).

I hope it helps.

Thanks,
Lorenzo

Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts specification

[Grant Likely]

On Mon, Aug 23, 2010 at 9:47 AM, Lorenzo Pieralisi
<Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi Grant,
>
>> -----Original Message-----
>> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
>> Likely
>> Sent: 20 August 2010 21:32
>> To: Lorenzo Pieralisi
>> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Philippe Robin; nico-vtqb6HGKxmxNg+MwTxZMZA@public.gmane.orgt;
>> linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org; Catalin Marinas; jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
>> Subject: Re: [RFC PATCH 13/14] ARM: vexpress: Definition of vexpress dts
>> specification
>>
>> Hi Lorenzo,
>>
>> On Fri, Aug 20, 2010 at 11:51 AM, Lorenzo Pieralisi
>> <Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
>> >> > +               };
>> >> > +
>> >> > +               smsc@4e000000 {
>> >> > +                       compatible = "smc,smsc-911";
>> >> > +                        reg = <0x4e000000 0x1000>;
>> >> > +                       interrupts = <47>;
>> >>
>> >> I suspect this interrupts property is wrong.  An interrupts specifier
>> >> is specific to the interrupt controller node; so this should always
>> >> specify the interrupt input on a specific controller (either gic-cpu
>> >> or gic-dist in this system).  It looks like '47' is the global irq
>> >> number.  Am I correct?
>> >>
>> >
>> > Strictly related to the previous point.
>> >
>> > When you say input Grant you mean input HW pin or interrupt HW ID ?
>> >
>> > I think you mean the irq ID specific to a given interrupt controller,
>> > because that is what SW sees and controls.
>>
>> I mean HW pin, but see discussion below to make sure we're talking
>> about the same thing.
>>
>> > 47 is the eth interrupt ID on the gic-dist, that in this particular
>> > case becomes the global IRQ number as well if I am not mistaken.
>> > (it is retrieved through the platform device resource).
>> > I think it is correct as it is, but I will countercheck.
>>
>> Okay.  Just to make sure I understand... I see two interrupt
>> controllers in this device tree; gic-cpu and gic-dist.  I'm assuming
>> that each irq controller has N interrupt inputs numbered 0..N-1.  My
>> expectation would be that each device is wired to a single interrupt
>> pin on one of the two controllers.  I also would expect that (unless
>> there are two interrupt input pins on the CPU) one of the irq
>> controllers is cascaded to the other.  Am I correct so far?
>>
>
> Well, it is the fact that I coded them as two controllers which is the
> source of confusion, I apologise but it is good to start discussing
> a proper binding.
> The gic-cpu and gic-dist are two subcomponents of a single
> interrupt controller called GIC. Devices are wired to the gic-dist which
> is in charge of distributing interrupts to the gic-cpu (per cpu IRQ IF).

Ah, so this is really just one device.  It probably makes sense to
describe it with only one interrupt-controller node then.  (Or if it
is a couple of nodes, only one should be declared as an
interrupt-controller.

>> So, if 47 was the hardware input number, that would mean irq-dist had
>> least 48 discrete interrupt inputs (this is actually what had me
>> suspicious; I assumed that that are only 32 inputs on the gic
>> controller but I didn't go and check).
>
> The GIC deals with interrupts through ID numbers, and yes the gic-dist
> can have more than 48 discrete interrupts lines.
> The point is that HW input pin numbers or wires map to irq ID through a shift
> (e.g. HW ID 37 means interrupt pin 5, because the first 32 irq IDs are
> used for SW IRQs and private (to one core) peripheral IRQs).
> That's why I asked if  "interrupts" should represent the HW input pin/line.
> Ethernet interrupt GIC HW ID is 47 (vexpress), but that does not mean
> the eth device irq line is connected to the 47th irq input line in
> the gic-dist because interrupt ID for peripherals start at ID 16
> (and there are two kinds of peripherals, private to a core (ID 16..31)
> or common to all cores, ID 32 onwards).

In general, using the HW input pin number is a good idea because the
device tree data is supposed to describe the hardware.

> TO be honest I do not think we should think in HW interrupt pin terms, in
> this context it is the GIC HW ID that should go into "interrupts"

Hmmm.  You've lost me here.  I'm not catching how HW ID is lining up
with HW interrupt pin number, and so having trouble determining which
is the better value to specify.

> (which in
> turn means we may well end up having two identical "interrupts" values
> on different GICs (cascaded), and that's perfectly fine because
> they are relative to a given controller).

Correct.

>> From the Linux perspective, the hw irq numbers for each controller
>> needs to be mapped onto the flat Linux irq space.  So if gic-cpu has N
>> irqs and gic-dist has M irqs, then the mapping might look something
>> like this:
>>
>> linux irq
>> (0)   == gic-cpu:0
>> (1)   == gic-cpu:1
>> ...
>> (N-2) == gpi-cpu:N-2
>> (N-1) == gpi-cpu:N-1
>> (N)   == gic-dist:0
>> (N+1) == gic-dist:1
>> ...
>> (N+M-2) == gpi-dist:M-2
>> (N+M-1) == gpi-dist:M-1
>>
>> Am I still correct, or have I misunderstood the versatile interrupt
>> controller architecture?
>
> The two components have to be seen as one interrupt controller.
> It is possible we need two "reg" properties since the offset between
> gic-cpu registers and gic-dist ones may differ.
> If there is a board/SoC with two cascaded GICs your reasoning
> above applies, agreed, sorry for having split the single GIC in two
> interrupt controllers, that caused confusion (but it is a RFC
> to define a proper binding 8-) ).

:-)

>> Regardless though, the linux mapping doesn't actually have any bearing
>> on the device tree because the mapping from HW to linux irq number is
>> all internal to the kernel.  The kernel could choose to start the
>> mapping at 42 or somewhere else and it would all still work.  The
>> of_irq support code has all the information it needs to translate an
>> interrupt specifier to an interrupt controller, and then map the
>> number to the linux irq number.
>>
>
> Agreed, and took your point and Warner's. I did not want to code the
> Linux global number in the interrupts property. IMHO that should define
> the GIC HW irq ID and it is not Linux specific, see above.
> But due to the current mapping 47 ends up being the Linux IRQ number
> as well.

"accidental" congruity is perfectly fine; as long as the software
doesn't depend on the congruity continuing to exist in the future.

> It is time I started writing a GIC binding since it is likely to be
> reused for different boards (and gic init code should be factored out
> of board specific dirs).
>
> I hope it helps.

I think so; but I'm still a little confused.  HW documentation would help.

g.

Re: [RFC PATCH 12/14] ARM: vexpress: add board support for DT probing

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> Discussion is under way to decide if a set of machine ids is allocated
> to ARM platforms based on device tree initialization.
> Init code for a given platform should be split in a static init version
> and a device tree init version. The Versatile Express ARM board requires
> the initialization of motherboard and daughterboard in separate steps,
> so the split between static init and DT init is doubled in terms of
> files. Common code should be factored out in common C files to avoid code
> duplication.
>
> This patch refactors the Versatile Express init code in order
> to split it in DT and non-DT versions, compiled in accordingly depending
> on kernel configuration options. Clock look-up for the DT version is
> still work in progress waiting for a definitive clock binding spec so
> device names are not really DT compliant at the moment in the dts.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>

Hi Lorenzo,

This ends up looking like quite an imposing patch for something that
should be relatively simple.  At the very least, moving the common
functions should be performed in a separate patch so that the -dt
changes can be reviewed independently of the code reorganization.
I'll need to dig into the detail of this one a bit later when I've got
some time.

Cheers,
g.

> ---
>  arch/arm/mach-vexpress/core.h                  |   15 ++-
>  arch/arm/mach-vexpress/ct-ca9x4-base.c         |  108 +++++++++++++
>  arch/arm/mach-vexpress/ct-ca9x4-of.c           |  192 +++++++++++++++++++++++
>  arch/arm/mach-vexpress/ct-ca9x4.c              |   93 -----------
>  arch/arm/mach-vexpress/include/mach/ct-ca9x4.h |    2 +
>  arch/arm/mach-vexpress/v2m-base.c              |  197 ++++++++++++++++++++++++
>  arch/arm/mach-vexpress/v2m-of.c                |   94 +++++++++++
>  arch/arm/mach-vexpress/v2m.c                   |  178 +---------------------
>  8 files changed, 610 insertions(+), 269 deletions(-)
>  create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-base.c
>  create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-of.c
>  create mode 100644 arch/arm/mach-vexpress/v2m-base.c
>  create mode 100644 arch/arm/mach-vexpress/v2m-of.c
>

Re: [RFC PATCH 12/14] ARM: vexpress: add board support for DT probing

[Mitch Bradley]

The sequence "of_get_property() .. of_read_number()" is sufficiently 
common to justify a function like

   error = of_read_numeric_property(node, name, default_value, &value)

Mitch

Re: [RFC PATCH 12/14] ARM: vexpress: add board support for DT probing

[Grant Likely]

On Wed, Aug 18, 2010 at 5:56 PM, Mitch Bradley <wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> wrote:
> The sequence "of_get_property() .. of_read_number()" is sufficiently common
> to justify a function like
>
>  error = of_read_numeric_property(node, name, default_value, &value)

Agreed.

g.

Re: [RFC PATCH 11/14] ARM: PMU: add device tree probing

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> When OF is enabled, platform drivers are required to define a
> match table in order to allow the kernel to find drivers suitable
> for a given device. The device tree allows to retrieve resources
> from device tree nodes dynamically.
>
> This patch adds device tree support to the ARM PMU driver. This
> includes a match table and code to initialize the driver id from
> the respective device tree node compatible property.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm/include/asm/pmu.h |    6 ++++++
>  arch/arm/kernel/Makefile   |    3 ++-
>  arch/arm/kernel/pmu-of.c   |   30 ++++++++++++++++++++++++++++++
>  arch/arm/kernel/pmu.c      |   18 ++++++++++++------
>  4 files changed, 50 insertions(+), 7 deletions(-)
>  create mode 100644 arch/arm/kernel/pmu-of.c
>
> diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
> index 8ccea01..d317b64 100644
> --- a/arch/arm/include/asm/pmu.h
> +++ b/arch/arm/include/asm/pmu.h
> @@ -17,6 +17,12 @@ enum arm_pmu_type {
>        ARM_NUM_PMU_DEVICES,
>  };
>
> +#ifndef CONFIG_OF
> +static inline int pmu_probe_dt(struct platform_device *pdev) { return -ENODEV; }
> +#else
> +extern int __devinit pmu_probe_dt(struct platform_device *pdev);
> +#endif
> +
>  #ifdef CONFIG_CPU_HAS_PMU
>
>  /**
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index 0ace897..b7d846f 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -48,7 +48,8 @@ obj-$(CONFIG_CPU_XSCALE)      += xscale-cp0.o
>  obj-$(CONFIG_CPU_XSC3)         += xscale-cp0.o
>  obj-$(CONFIG_CPU_MOHAWK)       += xscale-cp0.o
>  obj-$(CONFIG_IWMMXT)           += iwmmxt.o
> -obj-$(CONFIG_CPU_HAS_PMU)      += pmu.o
> +pmu_of-$(CONFIG_OF)            := pmu-of.o
> +obj-$(CONFIG_CPU_HAS_PMU)      += pmu.o $(pmu_of-y)
>  obj-$(CONFIG_HW_PERF_EVENTS)   += perf_event.o
>  AFLAGS_iwmmxt.o                        := -Wa,-mcpu=iwmmxt
>
> diff --git a/arch/arm/kernel/pmu-of.c b/arch/arm/kernel/pmu-of.c
> new file mode 100644
> index 0000000..5c5ee94
> --- /dev/null
> +++ b/arch/arm/kernel/pmu-of.c
> @@ -0,0 +1,30 @@
> +/*
> + *  linux/arch/arm/kernel/pmu-of.c --  PMU DT probe function
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/pmu.h>
> +
> +int __devinit pmu_probe_dt(struct platform_device *pdev)
> +{
> +       int ret = -ENODEV;
> +       struct device_node *node = pdev->dev.of_node;
> +
> +       if (of_device_is_compatible(node, "arm,arm-pmu")) {
> +               pdev->id = ARM_PMU_DEVICE_CPU;
> +               ret = 0;
> +       } else {
> +               pr_warning("Probing device not recognized "
> +                               "device %s\n", node->full_name);

Do you really want the warning on an OF-enabled kernel when booting on
an non-OF board?

> +       }
> +
> +       return ret;
> +}

How about:
{
        if (!of_match_node(arm_pmu_matches, node))
                return -ENODEV;

        pdev->id = ARM_PMU_DEVICE_CPU;
        return 0;
}

Clearer and more concise, no?

Also, (as you've brought up before) the pdev->id issue needs to be
solved.  I don't believe it is allowed to change the id field after
the platform_device is registered.  I'd rather see the id dynamically
assigned, which generally works in the OF use-case because
interconnections between devices are described in the tree and
specific ids aren't really needed.  However, it could be that the best
thing to do is retrieve the system-wide unique id from the aliases
node.

... and ditto on my comment that this will be simpler if it lives
directly in pmu.c.  :-)

> diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
> index b8af96e..f070b3d 100644
> --- a/arch/arm/kernel/pmu.c
> +++ b/arch/arm/kernel/pmu.c
> @@ -27,12 +27,12 @@ static struct platform_device *pmu_devices[ARM_NUM_PMU_DEVICES];
>
>  static int __devinit pmu_device_probe(struct platform_device *pdev)
>  {
> -
> -       if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
> -               pr_warning("received registration request for unknown "
> -                               "device %d\n", pdev->id);
> -               return -EINVAL;
> -       }
> +       if (pmu_probe_dt(pdev) == -ENODEV)
> +               if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
> +                       pr_warning("received registration request for unknown "
> +                                       "device %d\n", pdev->id);
> +                       return -EINVAL;
> +               }
>
>        if (pmu_devices[pdev->id])
>                pr_warning("registering new PMU device type %d overwrites "
> @@ -45,6 +45,11 @@ static int __devinit pmu_device_probe(struct platform_device *pdev)
>        return 0;
>  }
>
> +static struct of_device_id arm_pmu_matches[] = {
> +       { .compatible = "arm,arm-pmu"},
> +       {},
> +};
> +
>  static struct platform_driver pmu_driver = {
>        .driver         = {
>                .name   = "arm-pmu",
> @@ -54,6 +59,7 @@ static struct platform_driver pmu_driver = {
>
>  static int __init register_pmu_driver(void)
>  {
> +       platform_init_match(&pmu_driver, arm_pmu_matches);
>        return platform_driver_register(&pmu_driver);
>  }
>  device_initcall(register_pmu_driver);
> --
> 1.6.3.3
>
>



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

RE: [RFC PATCH 11/14] ARM: PMU: add device tree probing

[Lorenzo Pieralisi]

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> Likely
> Sent: 18 August 2010 22:37
> To: Lorenzo Pieralisi
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Philippe Robin; nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org;
> linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org; Catalin Marinas; jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
> Subject: Re: [RFC PATCH 11/14] ARM: PMU: add device tree probing
> 
> On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> > When OF is enabled, platform drivers are required to define a
> > match table in order to allow the kernel to find drivers suitable
> > for a given device. The device tree allows to retrieve resources
> > from device tree nodes dynamically.
> >
> > This patch adds device tree support to the ARM PMU driver. This
> > includes a match table and code to initialize the driver id from
> > the respective device tree node compatible property.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > ---
> >  arch/arm/include/asm/pmu.h |    6 ++++++
> >  arch/arm/kernel/Makefile   |    3 ++-
> >  arch/arm/kernel/pmu-of.c   |   30 ++++++++++++++++++++++++++++++
> >  arch/arm/kernel/pmu.c      |   18 ++++++++++++------
> >  4 files changed, 50 insertions(+), 7 deletions(-)
> >  create mode 100644 arch/arm/kernel/pmu-of.c
> >
> > diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
> > index 8ccea01..d317b64 100644
> > --- a/arch/arm/include/asm/pmu.h
> > +++ b/arch/arm/include/asm/pmu.h
> > @@ -17,6 +17,12 @@ enum arm_pmu_type {
> >        ARM_NUM_PMU_DEVICES,
> >  };
> >
> > +#ifndef CONFIG_OF
> > +static inline int pmu_probe_dt(struct platform_device *pdev) { return -
> ENODEV; }
> > +#else
> > +extern int __devinit pmu_probe_dt(struct platform_device *pdev);
> > +#endif
> > +
> >  #ifdef CONFIG_CPU_HAS_PMU
> >
> >  /**
> > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> > index 0ace897..b7d846f 100644
> > --- a/arch/arm/kernel/Makefile
> > +++ b/arch/arm/kernel/Makefile
> > @@ -48,7 +48,8 @@ obj-$(CONFIG_CPU_XSCALE)      += xscale-cp0.o
> >  obj-$(CONFIG_CPU_XSC3)         += xscale-cp0.o
> >  obj-$(CONFIG_CPU_MOHAWK)       += xscale-cp0.o
> >  obj-$(CONFIG_IWMMXT)           += iwmmxt.o
> > -obj-$(CONFIG_CPU_HAS_PMU)      += pmu.o
> > +pmu_of-$(CONFIG_OF)            := pmu-of.o
> > +obj-$(CONFIG_CPU_HAS_PMU)      += pmu.o $(pmu_of-y)
> >  obj-$(CONFIG_HW_PERF_EVENTS)   += perf_event.o
> >  AFLAGS_iwmmxt.o                        := -Wa,-mcpu=iwmmxt
> >
> > diff --git a/arch/arm/kernel/pmu-of.c b/arch/arm/kernel/pmu-of.c
> > new file mode 100644
> > index 0000000..5c5ee94
> > --- /dev/null
> > +++ b/arch/arm/kernel/pmu-of.c
> > @@ -0,0 +1,30 @@
> > +/*
> > + *  linux/arch/arm/kernel/pmu-of.c --  PMU DT probe function
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <asm/pmu.h>
> > +
> > +int __devinit pmu_probe_dt(struct platform_device *pdev)
> > +{
> > +       int ret = -ENODEV;
> > +       struct device_node *node = pdev->dev.of_node;
> > +
> > +       if (of_device_is_compatible(node, "arm,arm-pmu")) {
> > +               pdev->id = ARM_PMU_DEVICE_CPU;
> > +               ret = 0;
> > +       } else {
> > +               pr_warning("Probing device not recognized "
> > +                               "device %s\n", node->full_name);
> 
> Do you really want the warning on an OF-enabled kernel when booting on
> an non-OF board?
>

Good point. I would answer no. Valid in other contexts as well.
I will take that into account.
 
> > +       }
> > +
> > +       return ret;
> > +}
> 
> How about:
> {
>         if (!of_match_node(arm_pmu_matches, node))
>                 return -ENODEV;
> 
>         pdev->id = ARM_PMU_DEVICE_CPU;
>         return 0;
> }
> 
> Clearer and more concise, no?
> 

Definitely.

> Also, (as you've brought up before) the pdev->id issue needs to be
> solved.  I don't believe it is allowed to change the id field after
> the platform_device is registered.  I'd rather see the id dynamically
> assigned, which generally works in the OF use-case because
> interconnections between devices are described in the tree and
> specific ids aren't really needed.  However, it could be that the best
> thing to do is retrieve the system-wide unique id from the aliases
> node.
> 
> ... and ditto on my comment that this will be simpler if it lives
> directly in pmu.c.  :-)

When you say "retrieve the system-wide id from the aliases node" you mean
at platform_device register time  in the OF layer (ie of_device_register), 
right ? 

> 
> > diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
> > index b8af96e..f070b3d 100644
> > --- a/arch/arm/kernel/pmu.c
> > +++ b/arch/arm/kernel/pmu.c
> > @@ -27,12 +27,12 @@ static struct platform_device
> *pmu_devices[ARM_NUM_PMU_DEVICES];
> >
> >  static int __devinit pmu_device_probe(struct platform_device *pdev)
> >  {
> > -
> > -       if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
> > -               pr_warning("received registration request for unknown "
> > -                               "device %d\n", pdev->id);
> > -               return -EINVAL;
> > -       }
> > +       if (pmu_probe_dt(pdev) == -ENODEV)
> > +               if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
> > +                       pr_warning("received registration request for
> unknown "
> > +                                       "device %d\n", pdev->id);
> > +                       return -EINVAL;
> > +               }
> >
> >        if (pmu_devices[pdev->id])
> >                pr_warning("registering new PMU device type %d overwrites
> "
> > @@ -45,6 +45,11 @@ static int __devinit pmu_device_probe(struct
> platform_device *pdev)
> >        return 0;
> >  }
> >
> > +static struct of_device_id arm_pmu_matches[] = {
> > +       { .compatible = "arm,arm-pmu"},
> > +       {},
> > +};
> > +
> >  static struct platform_driver pmu_driver = {
> >        .driver         = {
> >                .name   = "arm-pmu",
> > @@ -54,6 +59,7 @@ static struct platform_driver pmu_driver = {
> >
> >  static int __init register_pmu_driver(void)
> >  {
> > +       platform_init_match(&pmu_driver, arm_pmu_matches);
> >        return platform_driver_register(&pmu_driver);
> >  }
> >  device_initcall(register_pmu_driver);
> > --
> > 1.6.3.3
> >
> >
> 
> 
> 
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.

Re: [RFC PATCH 11/14] ARM: PMU: add device tree probing

[Grant Likely]

On Fri, Aug 20, 2010 at 4:56 AM, Lorenzo Pieralisi
<Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
>> Also, (as you've brought up before) the pdev->id issue needs to be
>> solved.  I don't believe it is allowed to change the id field after
>> the platform_device is registered.  I'd rather see the id dynamically
>> assigned, which generally works in the OF use-case because
>> interconnections between devices are described in the tree and
>> specific ids aren't really needed.  However, it could be that the best
>> thing to do is retrieve the system-wide unique id from the aliases
>> node.
>>
>> ... and ditto on my comment that this will be simpler if it lives
>> directly in pmu.c.  :-)
>
> When you say "retrieve the system-wide id from the aliases node" you mean
> at platform_device register time  in the OF layer (ie of_device_register),
> right ?

Not necessarily.  Where possible, I'd rather modify the drivers to not
strictly enforce a 1:1 relationship between the pdev->id, and the
driver's internal enumeration.  Hmmm, that was a little dense, so let
me try again...  In the specific case where id == -1, I'd like to
driver to be able to handle it gracefully and allow the probe routine
to either dynamically assign, or to deduce an appropriate enumeration
value from the aliases node.

I don't have an example of the implementation to show you though.  We
should probably discuss on IRC or have a conf call.  There are a
number of subtleties and details that need to be agreed on here.

Cheers,
g.

Re: [RFC PATCH 10/14] drivers/USB: isp1760 DT platform parsing and binding

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> When OF is enabled, each driver should define a match table to
> allow the kernel to recognise drivers suitable for a given device.
> Initialization is carried out through a static inline at driver init
> time. The match-table is declared following OF bindings.
> Driver properties are parsed from the device tree node at run time,
> but if the device tree probing fails, the driver falls back to static
> driver initialization (platform data).
>
> This patch adds the infrastructure to support device tree probing and
> initialization for isp1760 USB host controller. The build infrastructure
> changes accordingly in order to have a clean separation between OF and
> non-OF init code.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/usb/host/Makefile      |    4 +-
>  drivers/usb/host/isp1760-hcd.h |    7 ++++++
>  drivers/usb/host/isp1760-if.c  |   43 ++++++++++++++++++++++++++--------------
>  drivers/usb/host/isp1760-of.c  |   36 +++++++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/usb/host/isp1760-of.c
>
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index b6315aa..edf6e8a 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -5,8 +5,8 @@
>  ifeq ($(CONFIG_USB_DEBUG),y)
>        EXTRA_CFLAGS            += -DDEBUG
>  endif
> -
> -isp1760-objs := isp1760-hcd.o isp1760-if.o
> +isp1760_of-$(CONFIG_OF)                := isp1760-of.o
> +isp1760-objs := isp1760-hcd.o isp1760-if.o $(isp1760_of-y)
>  fhci-objs := fhci-hcd.o fhci-hub.o fhci-q.o fhci-mem.o \
>             fhci-tds.o fhci-sched.o
>  ifeq ($(CONFIG_FHCI_DEBUG),y)
> diff --git a/drivers/usb/host/isp1760-hcd.h b/drivers/usb/host/isp1760-hcd.h
> index 6931ef5..93afabd 100644
> --- a/drivers/usb/host/isp1760-hcd.h
> +++ b/drivers/usb/host/isp1760-hcd.h
> @@ -9,6 +9,13 @@ struct usb_hcd *isp1760_register(phys_addr_t res_start, resource_size_t res_len,
>  int init_kmem_once(void);
>  void deinit_kmem_cache(void);
>
> +#ifndef CONFIG_OF
> +static inline int isp1760_plat_probe_dt(int *devflags,
> +               struct platform_device *pdev) { return -ENODEV; }
> +#else
> +extern int isp1760_plat_probe_dt(int *devflags, struct platform_device *pdev);
> +#endif
> +
>  /* EHCI capability registers */
>  #define HC_CAPLENGTH           0x00
>  #define HC_HCSPARAMS           0x04
> diff --git a/drivers/usb/host/isp1760-if.c b/drivers/usb/host/isp1760-if.c
> index ec85d0c..3a1839e 100644
> --- a/drivers/usb/host/isp1760-if.c
> +++ b/drivers/usb/host/isp1760-if.c
> @@ -17,7 +17,7 @@
>
>  #include "isp1760-hcd.h"
>
> -#ifdef CONFIG_PPC_OF
> +#if defined(CONFIG_PPC_OF)
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #endif
> @@ -305,6 +305,17 @@ static struct pci_driver isp1761_pci_driver = {
>  };
>  #endif
>
> +static const struct of_device_id of_isp1760_match[] = {
> +       {
> +               .compatible = "nxp,usb-isp1760",
> +       },
> +       {
> +               .compatible = "nxp,usb-isp1761",
> +       },
> +       { },
> +};
> +
> +
>  static int __devinit isp1760_plat_probe(struct platform_device *pdev)
>  {
>        int ret = 0;
> @@ -336,21 +347,22 @@ static int __devinit isp1760_plat_probe(struct platform_device *pdev)
>        }
>        irqflags |= irq_res->flags & IRQF_TRIGGER_MASK;
>
> -       if (priv) {
> -               if (priv->is_isp1761)
> -                       devflags |= ISP1760_FLAG_ISP1761;
> -               if (priv->bus_width_16)
> -                       devflags |= ISP1760_FLAG_BUS_WIDTH_16;
> -               if (priv->port1_otg)
> -                       devflags |= ISP1760_FLAG_OTG_EN;
> -               if (priv->analog_oc)
> -                       devflags |= ISP1760_FLAG_ANALOG_OC;
> -               if (priv->dack_polarity_high)
> -                       devflags |= ISP1760_FLAG_DACK_POL_HIGH;
> -               if (priv->dreq_polarity_high)
> -                       devflags |= ISP1760_FLAG_DREQ_POL_HIGH;
> +       if (isp1760_plat_probe_dt(&devflags, pdev) == -ENODEV) {
> +               if (priv) {
> +                       if (priv->is_isp1761)
> +                               devflags |= ISP1760_FLAG_ISP1761;
> +                       if (priv->bus_width_16)
> +                               devflags |= ISP1760_FLAG_BUS_WIDTH_16;
> +                       if (priv->port1_otg)
> +                               devflags |= ISP1760_FLAG_OTG_EN;
> +                       if (priv->analog_oc)
> +                               devflags |= ISP1760_FLAG_ANALOG_OC;
> +                       if (priv->dack_polarity_high)
> +                               devflags |= ISP1760_FLAG_DACK_POL_HIGH;
> +                       if (priv->dreq_polarity_high)
> +                               devflags |= ISP1760_FLAG_DREQ_POL_HIGH;
> +               }
>        }
> -
>        hcd = isp1760_register(mem_res->start, mem_size, irq_res->start,
>                               irqflags, &pdev->dev, dev_name(&pdev->dev), devflags);
>        if (IS_ERR(hcd)) {
> @@ -394,6 +406,7 @@ static int __init isp1760_init(void)
>
>        init_kmem_once();
>
> +       platform_init_match(&isp1760_plat_driver, of_isp1760_match);
>        ret = platform_driver_register(&isp1760_plat_driver);
>        if (!ret)
>                any_ret = 0;
> diff --git a/drivers/usb/host/isp1760-of.c b/drivers/usb/host/isp1760-of.c
> new file mode 100644
> index 0000000..c98ee58
> --- /dev/null
> +++ b/drivers/usb/host/isp1760-of.c
> @@ -0,0 +1,36 @@
> +
> +#include <linux/platform_device.h>
> +
> +#include <linux/of.h>
> +#include "isp1760-hcd.h"
> +
> +int isp1760_plat_probe_dt(int *devflags, struct platform_device *pdev)
> +{
> +       struct device_node *dp = pdev->dev.of_node;
> +       const u32 *prop;
> +
> +       if (!dp)
> +               return -ENODEV;
> +
> +       if (of_device_is_compatible(dp, "nxp,usb-isp1761"))
> +               *devflags |= ISP1760_FLAG_ISP1761;
> +
> +       /* Some systems wire up only 16 of the 32 data lines */
> +       prop = of_get_property(dp, "bus-width", NULL);
> +       if (prop && *prop == 16)
> +               *devflags |= ISP1760_FLAG_BUS_WIDTH_16;
> +
> +       if (of_get_property(dp, "port1-otg", NULL))
> +               *devflags |= ISP1760_FLAG_OTG_EN;
> +
> +       if (of_get_property(dp, "analog-oc", NULL))
> +               *devflags |= ISP1760_FLAG_ANALOG_OC;
> +
> +       if (of_get_property(dp, "dack-polarity", NULL))
> +               *devflags |= ISP1760_FLAG_DACK_POL_HIGH;
> +
> +       if (of_get_property(dp, "dreq-polarity", NULL))
> +               *devflags |= ISP1760_FLAG_DREQ_POL_HIGH;

All of these properties need to be documented before a driver is
merged.  To start, because these are chip-specific properties, they
should be prefixed with "nxp," to avoid future namespace collisions.
You can write a draft binding on devicetree.org.

> +
> +       return 0;
> +}

Ditto to my previous comment on the smsc911x network driver, my
preference is to see this function in the main .c file.

> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 09/14] ARM: ARM flash driver DT port

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> When OF is enabled drivers should initialize the driver match table
> to allow the kernel to find and recognise drivers suitable for
> devices. The initialization is carried out through a static inline
> function at driver init time.
>
> This patch adds a match table and relative init code to support
> device tree probing methodology for the ARM flash driver.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/mtd/maps/integrator-flash.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/maps/integrator-flash.c b/drivers/mtd/maps/integrator-flash.c
> index 2aac41b..aa9a556 100644
> --- a/drivers/mtd/maps/integrator-flash.c
> +++ b/drivers/mtd/maps/integrator-flash.c
> @@ -291,6 +291,11 @@ static int armflash_remove(struct platform_device *dev)
>        return 0;
>  }
>
> +static struct of_device_id armflash_matches[] = {
> +       { .compatible = "arm,arm-flash"},

(Assuming I'm reading the integrator-flash driver correctly...) When
using the device tree, the entire flash map should be encoded in the
tree, and this mapping driver is no longer needed.  See
mtd/maps/physmap_of.c.

> +       {},
> +};
> +
>  static struct platform_driver armflash_driver = {
>        .probe          = armflash_probe,
>        .remove         = armflash_remove,
> @@ -302,6 +307,7 @@ static struct platform_driver armflash_driver = {
>
>  static int __init armflash_init(void)
>  {
> +       platform_init_match(&armflash_driver, armflash_matches);
>        return platform_driver_register(&armflash_driver);
>  }
>
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 09/14] ARM: ARM flash driver DT port

[Catalin Marinas]

On Wed, 2010-08-18 at 22:22 +0100, Grant Likely wrote:
> On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> > When OF is enabled drivers should initialize the driver match table
> > to allow the kernel to find and recognise drivers suitable for
> > devices. The initialization is carried out through a static inline
> > function at driver init time.
> >
> > This patch adds a match table and relative init code to support
> > device tree probing methodology for the ARM flash driver.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > ---
> >  drivers/mtd/maps/integrator-flash.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mtd/maps/integrator-flash.c b/drivers/mtd/maps/integrator-flash.c
> > index 2aac41b..aa9a556 100644
> > --- a/drivers/mtd/maps/integrator-flash.c
> > +++ b/drivers/mtd/maps/integrator-flash.c
> > @@ -291,6 +291,11 @@ static int armflash_remove(struct platform_device *dev)
> >        return 0;
> >  }
> >
> > +static struct of_device_id armflash_matches[] = {
> > +       { .compatible = "arm,arm-flash"},
> 
> (Assuming I'm reading the integrator-flash driver correctly...) When
> using the device tree, the entire flash map should be encoded in the
> tree, and this mapping driver is no longer needed.  See
> mtd/maps/physmap_of.c.

BTW, I managed to get the RealView boards working with physmap.c instead
of the integrator flash (same functionality, just different drivers). We
could do the same for Versatile Express and get rid of the integrator
flash entirely.

-- 
Catalin

Re: [RFC PATCH 09/14] ARM: ARM flash driver DT port

[Grant Likely]

On Fri, Aug 20, 2010 at 4:29 AM, Catalin Marinas
<catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, 2010-08-18 at 22:22 +0100, Grant Likely wrote:
>> On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
>> > When OF is enabled drivers should initialize the driver match table
>> > to allow the kernel to find and recognise drivers suitable for
>> > devices. The initialization is carried out through a static inline
>> > function at driver init time.
>> >
>> > This patch adds a match table and relative init code to support
>> > device tree probing methodology for the ARM flash driver.
>> >
>> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
>> > ---
>> >  drivers/mtd/maps/integrator-flash.c |    6 ++++++
>> >  1 files changed, 6 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/mtd/maps/integrator-flash.c b/drivers/mtd/maps/integrator-flash.c
>> > index 2aac41b..aa9a556 100644
>> > --- a/drivers/mtd/maps/integrator-flash.c
>> > +++ b/drivers/mtd/maps/integrator-flash.c
>> > @@ -291,6 +291,11 @@ static int armflash_remove(struct platform_device *dev)
>> >        return 0;
>> >  }
>> >
>> > +static struct of_device_id armflash_matches[] = {
>> > +       { .compatible = "arm,arm-flash"},
>>
>> (Assuming I'm reading the integrator-flash driver correctly...) When
>> using the device tree, the entire flash map should be encoded in the
>> tree, and this mapping driver is no longer needed.  See
>> mtd/maps/physmap_of.c.
>
> BTW, I managed to get the RealView boards working with physmap.c instead
> of the integrator flash (same functionality, just different drivers). We
> could do the same for Versatile Express and get rid of the integrator
> flash entirely.

Nice!  It should just drop in then (although there may be some endian
issues to tidy up).

g.

Re: [RFC PATCH 08/14] ARM: versatile-i2c driver DT port

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> When OF is enabled, each driver should define a match table to
> allow the kernel to recognise drivers suitable for a given device.
> Initialization is carried out through a static inline at driver init
> time. The match-table is declared following OF bindings.
>
> This patch adds the required initialization call to the versatile-i2c
> driver.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-versatile.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
> index 6055601..ea8e3b5 100644
> --- a/drivers/i2c/busses/i2c-versatile.c
> +++ b/drivers/i2c/busses/i2c-versatile.c
> @@ -133,6 +133,11 @@ static int i2c_versatile_remove(struct platform_device *dev)
>        return 0;
>  }
>
> +static struct of_device_id versatile_i2c_matches[] = {
> +       { .compatible = "arm,versatile-i2c"},
> +       {},
> +};
> +
>  static struct platform_driver i2c_versatile_driver = {
>        .probe          = i2c_versatile_probe,
>        .remove         = i2c_versatile_remove,
> @@ -144,6 +149,7 @@ static struct platform_driver i2c_versatile_driver = {
>
>  static int __init i2c_versatile_init(void)
>  {
> +       platform_init_match(&i2c_versatile_driver, versatile_i2c_matches);

Ditto to my comments on the last driver.  I'd rather see the pointer
statically assigned instead of set at runtime.

>        return platform_driver_register(&i2c_versatile_driver);
>  }
>
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 07/14] drivers/smsc911x: add DT support

[Grant Likely]

[cc'ing networking maintainer David Miller to keep him in the loop]

Hi Lorenzo,

A number of comments below and some rework required, but in general
the approach looks good and impact on the core driver is limited to
the start of the .probe() routine.

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> When OF is enabled the device driver should initialize the match table
> in order to be probable from the device tree. Furthermore, HW properties
> should be retrieved from the device tree node, so a device tree probe
> function is required to parse node properties at run-time. To avoid
> preprocessor macros, open firmware functions are defined in a separate
> C file that is optionally compiled-in.
>
> This patch adds the infrastructure needed to enable device tree parsing
> to smsc911 platform driver, inclusive of Makefile and device tree
> parsing functions. Driver properties should be defined and retrieved
> from the device tree if OF is enabled, otherwise the driver would fall
> back to platform data static initialization.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/net/Makefile      |    3 +-
>  drivers/net/smsc911x-of.c |   53 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/net/smsc911x.c    |   28 ++++++++++++++++-------
>  drivers/net/smsc911x.h    |    8 ++++++
>  4 files changed, 82 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/net/smsc911x-of.c
>
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 0a0512a..0d2faf9 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -243,7 +243,8 @@ obj-$(CONFIG_VXGE) += vxge/
>  obj-$(CONFIG_MYRI10GE) += myri10ge/
>  obj-$(CONFIG_SMC91X) += smc91x.o
>  obj-$(CONFIG_SMC911X) += smc911x.o
> -obj-$(CONFIG_SMSC911X) += smsc911x.o
> +smsc911x_of-$(CONFIG_OF) := smsc911x-of.o
> +obj-$(CONFIG_SMSC911X) += smsc911x.o $(smsc911x_of-y)
>  obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
>  obj-$(CONFIG_DM9000) += dm9000.o
>  obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
> diff --git a/drivers/net/smsc911x-of.c b/drivers/net/smsc911x-of.c
> new file mode 100644
> index 0000000..b8751cf
> --- /dev/null
> +++ b/drivers/net/smsc911x-of.c
> @@ -0,0 +1,53 @@
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of.h>
> +#include <linux/smsc911x.h>
> +#include <linux/kernel.h>
> +
> +#include "smsc911x.h"
> +
> +
> +int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
> +               struct platform_device *pdev)
> +{
> +       struct device_node *node = pdev->dev.of_node;
> +       int ret = -ENODEV;
> +       const u32 *prop;
> +
> +       if (!node)
> +               goto err;

Nothing is allocated or needs to be undone in this routine in the
event of failure.  Instead of "goto err", you can just "return
-ENODEV" in each error case.

> +
> +       prop = of_get_property(node, "flags", NULL);
> +       if (!prop)
> +               goto err;
> +
> +       cfg->flags = of_read_number(prop, 1);

Looks like Linux-specific implementation details are being encoded
into the device tree.  What is the intended meaning for the "flags"
property?

> +
> +       prop = of_get_property(node, "irq-polarity", NULL);
> +       if (!prop)
> +               goto err;
> +
> +       cfg->irq_polarity = of_read_number(prop, 1);
> +
> +       prop = of_get_property(node, "irq-type", NULL);
> +       if (!prop)
> +               goto err;
> +
> +       cfg->irq_type = of_read_number(prop, 1);

Most interrupt controller bindings include an extra cell in the
interrupt specifier to encode flags like irq polarity and type.
Encoding these outside of the interrupt specifier makes things less
clear.

> +
> +       prop = of_get_property(node, "phy-interface", NULL);
> +       if (!prop)
> +               goto err;
> +
> +       cfg->phy_interface = of_read_number(prop, 1);

There is a binding phy devices with support routines.  This network
device should use those instead of inventing something new.

> +
> +       pr_info("%s: Parsed FDT info\n", node->name);
> +       pr_info("FLAGS %d\n", cfg->flags);
> +       pr_info("IRQ-POLARITY %d\n", cfg->irq_polarity);
> +       pr_info("IRQ-TYPE %d\n", cfg->irq_type);
> +       pr_info("PHY IF %d\n", cfg->phy_interface);

I prefer to see the dev_info() macro used for per-device-instance messages.

> +       return 0;
> +err:
> +       return ret;
> +}

I don't think it is necessary to have a separate .c file for this one
function.  I don't see any problem with it living in
drivers/net/smsc911x.c.  I'd rather have everything contained in a
single place.

> diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> index cc55974..c56b8f7 100644
> --- a/drivers/net/smsc911x.c
> +++ b/drivers/net/smsc911x.c
> @@ -50,6 +50,7 @@
>  #include <linux/phy.h>
>  #include <linux/smsc911x.h>
>  #include <linux/device.h>
> +
>  #include "smsc911x.h"

Unrelated whitespace change.

>
>  #define SMSC_CHIPNAME          "smsc911x"
> @@ -1945,13 +1946,6 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
>
>        pr_info("%s: Driver version %s.\n", SMSC_CHIPNAME, SMSC_DRV_VERSION);
>
> -       /* platform data specifies irq & dynamic bus configuration */
> -       if (!pdev->dev.platform_data) {
> -               pr_warning("%s: platform_data not provided\n", SMSC_CHIPNAME);
> -               retval = -ENODEV;
> -               goto out_0;
> -       }
> -
>        res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>                                           "smsc911x-memory");
>        if (!res)
> @@ -1988,12 +1982,22 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
>
>        pdata = netdev_priv(dev);
>
> +       if (smsc911x_probe_dt(&pdata->config, pdev) == -ENODEV) {
> +               /* platform data specifies irq & dynamic bus configuration */
> +               if (!pdev->dev.platform_data) {
> +                       pr_warning("%s: platform_data not provided\n",
> +                                       SMSC_CHIPNAME);
> +                       retval = -ENODEV;
> +                       goto out_0;
> +               }
> +               /* copy config parameters across to pdata */
> +               memcpy(&pdata->config, config, sizeof(pdata->config));
> +       }
> +
>        dev->irq = irq_res->start;
>        irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
>        pdata->ioaddr = ioremap_nocache(res->start, res_size);
>
> -       /* copy config parameters across to pdata */
> -       memcpy(&pdata->config, config, sizeof(pdata->config));
>
>        pdata->dev = dev;
>        pdata->msg_enable = ((1 << debug) - 1);
> @@ -2150,6 +2154,11 @@ static const struct dev_pm_ops smsc911x_pm_ops = {
>  #define SMSC911X_PM_OPS NULL
>  #endif
>
> +static struct of_device_id smsc911x_matches[] = {
> +       { .compatible = "smc,smsc-911"},
> +       {},
> +};
> +
>  static struct platform_driver smsc911x_driver = {
>        .probe = smsc911x_drv_probe,
>        .remove = __devexit_p(smsc911x_drv_remove),
> @@ -2163,6 +2172,7 @@ static struct platform_driver smsc911x_driver = {
>  /* Entry point for loading the module */
>  static int __init smsc911x_init_module(void)
>  {
> +       platform_init_match(&smsc911x_driver, smsc911x_matches);

the match table should be statically initialized into the driver
structure.  Right now that does require wrapping the line with #ifdef
CONFIG_OF which is a little stinky, but I'm working on a solution for
that.  Regardless, I don't like having to use a macro to set the
pointer at probe time when it can be done statically.

>        return platform_driver_register(&smsc911x_driver);
>  }
>
> diff --git a/drivers/net/smsc911x.h b/drivers/net/smsc911x.h
> index 016360c..5ec9a8b 100644
> --- a/drivers/net/smsc911x.h
> +++ b/drivers/net/smsc911x.h
> @@ -394,4 +394,12 @@
>  #define LPA_PAUSE_ALL                  (LPA_PAUSE_CAP | \
>                                         LPA_PAUSE_ASYM)
>
> +#ifndef CONFIG_OF
> +static inline int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
> +               struct platform_device *pdev) { return -ENODEV; }
> +#else
> +extern int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
> +               struct platform_device *pdev);
> +#endif
> +

This hunk will of course also get merged into the main .c file if the
OF helper function is rolled into the main driver.

Cheers,
g.

>  #endif                         /* __SMSC911X_H__ */
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 07/14] drivers/smsc911x: add DT support

[Grant Likely]

On Wed, Aug 18, 2010 at 3:17 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> +static struct of_device_id smsc911x_matches[] = {
>> +       { .compatible = "smc,smsc-911"},
>> +       {},
>> +};
>> +

Oh, and this should be the actual part number instead of an
abbreviation.  So, something like "smsc,lan91c111".

g.

RE: [RFC PATCH 07/14] drivers/smsc911x: add DT support

[Lorenzo Pieralisi]

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of
> Grant Likely
> Sent: 18 August 2010 22:17
> To: Lorenzo Pieralisi
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Philippe Robin;
> nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org; linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org; Catalin Marinas;
> jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org; David Miller
> Subject: Re: [RFC PATCH 07/14] drivers/smsc911x: add DT support
> 
> [cc'ing networking maintainer David Miller to keep him in the loop]
> 
> Hi Lorenzo,
> 
> A number of comments below and some rework required, but in general
> the approach looks good and impact on the core driver is limited to
> the start of the .probe() routine.
> 
> On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> > When OF is enabled the device driver should initialize the match
> table
> > in order to be probable from the device tree. Furthermore, HW
> properties
> > should be retrieved from the device tree node, so a device tree probe
> > function is required to parse node properties at run-time. To avoid
> > preprocessor macros, open firmware functions are defined in a
> separate
> > C file that is optionally compiled-in.
> >
> > This patch adds the infrastructure needed to enable device tree
> parsing
> > to smsc911 platform driver, inclusive of Makefile and device tree
> > parsing functions. Driver properties should be defined and retrieved
> > from the device tree if OF is enabled, otherwise the driver would
> fall
> > back to platform data static initialization.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > ---
> >  drivers/net/Makefile      |    3 +-
> >  drivers/net/smsc911x-of.c |   53
> +++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/net/smsc911x.c    |   28 ++++++++++++++++-------
> >  drivers/net/smsc911x.h    |    8 ++++++
> >  4 files changed, 82 insertions(+), 10 deletions(-)
> >  create mode 100644 drivers/net/smsc911x-of.c
> >
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index 0a0512a..0d2faf9 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -243,7 +243,8 @@ obj-$(CONFIG_VXGE) += vxge/
> >  obj-$(CONFIG_MYRI10GE) += myri10ge/
> >  obj-$(CONFIG_SMC91X) += smc91x.o
> >  obj-$(CONFIG_SMC911X) += smc911x.o
> > -obj-$(CONFIG_SMSC911X) += smsc911x.o
> > +smsc911x_of-$(CONFIG_OF) := smsc911x-of.o
> > +obj-$(CONFIG_SMSC911X) += smsc911x.o $(smsc911x_of-y)
> >  obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
> >  obj-$(CONFIG_DM9000) += dm9000.o
> >  obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
> > diff --git a/drivers/net/smsc911x-of.c b/drivers/net/smsc911x-of.c
> > new file mode 100644
> > index 0000000..b8751cf
> > --- /dev/null
> > +++ b/drivers/net/smsc911x-of.c
> > @@ -0,0 +1,53 @@
> > +#include <linux/device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of.h>
> > +#include <linux/smsc911x.h>
> > +#include <linux/kernel.h>
> > +
> > +#include "smsc911x.h"
> > +
> > +
> > +int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
> > +               struct platform_device *pdev)
> > +{
> > +       struct device_node *node = pdev->dev.of_node;
> > +       int ret = -ENODEV;
> > +       const u32 *prop;
> > +
> > +       if (!node)
> > +               goto err;
> 
> Nothing is allocated or needs to be undone in this routine in the
> event of failure.  Instead of "goto err", you can just "return
> -ENODEV" in each error case.

Right, it is just because all those "return -ENODEV;" do not look nice 
either, it is clear there is no point in jumping to just pop out.
But I agree, I will stick to the return approach.
It may be worthwhile to code a loop with a struct containing the set 
of string properties, that's just cosmetic changes.

> 
> > +
> > +       prop = of_get_property(node, "flags", NULL);
> > +       if (!prop)
> > +               goto err;
> > +
> > +       cfg->flags = of_read_number(prop, 1);
> 
> Looks like Linux-specific implementation details are being encoded
> into the device tree.  What is the intended meaning for the "flags"
> property?
> 

Well, looks like you are right. It is to define if a 16-bit bus or 
32-bit bus (to force atomicity of burst of write/read) is used to 
access the device, plus flags to force external or internal phy and
an option to save the MAC address.

What's the approach to be followed in this case ?

> > +
> > +       prop = of_get_property(node, "irq-polarity", NULL);
> > +       if (!prop)
> > +               goto err;
> > +
> > +       cfg->irq_polarity = of_read_number(prop, 1);
> > +
> > +       prop = of_get_property(node, "irq-type", NULL);
> > +       if (!prop)
> > +               goto err;
> > +
> > +       cfg->irq_type = of_read_number(prop, 1);
> 
> Most interrupt controller bindings include an extra cell in the
> interrupt specifier to encode flags like irq polarity and type.
> Encoding these outside of the interrupt specifier makes things less
> clear.
> 

Point taken.

> > +
> > +       prop = of_get_property(node, "phy-interface", NULL);
> > +       if (!prop)
> > +               goto err;
> > +
> > +       cfg->phy_interface = of_read_number(prop, 1);
> 
> There is a binding phy devices with support routines.  This network
> device should use those instead of inventing something new.
> 

Ditto.

> > +
> > +       pr_info("%s: Parsed FDT info\n", node->name);
> > +       pr_info("FLAGS %d\n", cfg->flags);
> > +       pr_info("IRQ-POLARITY %d\n", cfg->irq_polarity);
> > +       pr_info("IRQ-TYPE %d\n", cfg->irq_type);
> > +       pr_info("PHY IF %d\n", cfg->phy_interface);
> 
> I prefer to see the dev_info() macro used for per-device-instance
> messages.
>

 
Ok.

> > +       return 0;
> > +err:
> > +       return ret;
> > +}
> 
> I don't think it is necessary to have a separate .c file for this one
> function.  I don't see any problem with it living in
> drivers/net/smsc911x.c.  I'd rather have everything contained in a
> single place.

The point Grant is that the dt probe function requires access to CONFIG_OF 
dependent members (of_node). If the need for #ifdef CONFIG_OF will go 
I am more than happy to just merge it all in one file. 
This is valid for all the drivers and the match table init macro as well.
 
> 
> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
> > index cc55974..c56b8f7 100644
> > --- a/drivers/net/smsc911x.c
> > +++ b/drivers/net/smsc911x.c
> > @@ -50,6 +50,7 @@
> >  #include <linux/phy.h>
> >  #include <linux/smsc911x.h>
> >  #include <linux/device.h>
> > +
> >  #include "smsc911x.h"
> 
> Unrelated whitespace change.
> 

Ok.

> >
> {
> >  /* Entry point for loading the module */
> >  static int __init smsc911x_init_module(void)
> >  {
> > +       platform_init_match(&smsc911x_driver, smsc911x_matches);
> 
> the match table should be statically initialized into the driver
> structure.  Right now that does require wrapping the line with #ifdef
> CONFIG_OF which is a little stinky, but I'm working on a solution for
> that.  Regardless, I don't like having to use a macro to set the
> pointer at probe time when it can be done statically.
> 

Point taken, but to be precise it is not set at probe time, it is set at 
init time (insmod or initcalls). Again, it is just to avoid ifdefs,
if the #ifdefs go away, I will do it statically with nary a problem.

> >        return platform_driver_register(&smsc911x_driver);
> >  }
> >
> > diff --git a/drivers/net/smsc911x.h b/drivers/net/smsc911x.h
> > index 016360c..5ec9a8b 100644
> > --- a/drivers/net/smsc911x.h
> > +++ b/drivers/net/smsc911x.h
> > @@ -394,4 +394,12 @@
> >  #define LPA_PAUSE_ALL                  (LPA_PAUSE_CAP | \
> >                                         LPA_PAUSE_ASYM)
> >
> > +#ifndef CONFIG_OF
> > +static inline int smsc911x_probe_dt(struct smsc911x_platform_config
> *cfg,
> > +               struct platform_device *pdev) { return -ENODEV; }
> > +#else
> > +extern int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
> > +               struct platform_device *pdev);
> > +#endif
> > +
> 
> This hunk will of course also get merged into the main .c file if the
> OF helper function is rolled into the main driver.

Ok.

> 
> Cheers,
> g.

Many thanks, that helps a lot.
Lorenzo

> 
> >  #endif                         /* __SMSC911X_H__ */
> > --
> > 1.6.3.3
> >
> >
> 
> 
> 
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.

Re: [RFC PATCH 07/14] drivers/smsc911x: add DT support

[Grant Likely]

On Fri, Aug 20, 2010 at 4:24 AM, Lorenzo Pieralisi
<Lorenzo.Pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
>> -----Original Message-----
>> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of
>> Grant Likely
>> Sent: 18 August 2010 22:17
>> To: Lorenzo Pieralisi
>> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; Philippe Robin;
>> nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org; linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org; Catalin Marinas;
>> jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org; David Miller
>> Subject: Re: [RFC PATCH 07/14] drivers/smsc911x: add DT support
>>
>> [cc'ing networking maintainer David Miller to keep him in the loop]
>>
>> Hi Lorenzo,
>>
>> A number of comments below and some rework required, but in general
>> the approach looks good and impact on the core driver is limited to
>> the start of the .probe() routine.
>>
>> On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
>> > When OF is enabled the device driver should initialize the match
>> table
>> > in order to be probable from the device tree. Furthermore, HW
>> properties
>> > should be retrieved from the device tree node, so a device tree probe
>> > function is required to parse node properties at run-time. To avoid
>> > preprocessor macros, open firmware functions are defined in a
>> separate
>> > C file that is optionally compiled-in.
>> >
>> > This patch adds the infrastructure needed to enable device tree
>> parsing
>> > to smsc911 platform driver, inclusive of Makefile and device tree
>> > parsing functions. Driver properties should be defined and retrieved
>> > from the device tree if OF is enabled, otherwise the driver would
>> fall
>> > back to platform data static initialization.
>> >
>> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
>> > ---
>> >  drivers/net/Makefile      |    3 +-
>> >  drivers/net/smsc911x-of.c |   53
>> +++++++++++++++++++++++++++++++++++++++++++++
>> >  drivers/net/smsc911x.c    |   28 ++++++++++++++++-------
>> >  drivers/net/smsc911x.h    |    8 ++++++
>> >  4 files changed, 82 insertions(+), 10 deletions(-)
>> >  create mode 100644 drivers/net/smsc911x-of.c
>> >
>> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>> > index 0a0512a..0d2faf9 100644
>> > --- a/drivers/net/Makefile
>> > +++ b/drivers/net/Makefile
>> > @@ -243,7 +243,8 @@ obj-$(CONFIG_VXGE) += vxge/
>> >  obj-$(CONFIG_MYRI10GE) += myri10ge/
>> >  obj-$(CONFIG_SMC91X) += smc91x.o
>> >  obj-$(CONFIG_SMC911X) += smc911x.o
>> > -obj-$(CONFIG_SMSC911X) += smsc911x.o
>> > +smsc911x_of-$(CONFIG_OF) := smsc911x-of.o
>> > +obj-$(CONFIG_SMSC911X) += smsc911x.o $(smsc911x_of-y)
>> >  obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
>> >  obj-$(CONFIG_DM9000) += dm9000.o
>> >  obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
>> > diff --git a/drivers/net/smsc911x-of.c b/drivers/net/smsc911x-of.c
>> > new file mode 100644
>> > index 0000000..b8751cf
>> > --- /dev/null
>> > +++ b/drivers/net/smsc911x-of.c
>> > @@ -0,0 +1,53 @@
>> > +#include <linux/device.h>
>> > +#include <linux/platform_device.h>
>> > +#include <linux/of_platform.h>
>> > +#include <linux/of.h>
>> > +#include <linux/smsc911x.h>
>> > +#include <linux/kernel.h>
>> > +
>> > +#include "smsc911x.h"
>> > +
>> > +
>> > +int smsc911x_probe_dt(struct smsc911x_platform_config *cfg,
>> > +               struct platform_device *pdev)
>> > +{
>> > +       struct device_node *node = pdev->dev.of_node;
>> > +       int ret = -ENODEV;
>> > +       const u32 *prop;
>> > +
>> > +       if (!node)
>> > +               goto err;
>>
>> Nothing is allocated or needs to be undone in this routine in the
>> event of failure.  Instead of "goto err", you can just "return
>> -ENODEV" in each error case.
>
> Right, it is just because all those "return -ENODEV;" do not look nice
> either,

Heh, I happen to think it looks *better* when all that needs to be
done is return, or at least easier to parse.  :-)  There's no
accounting for some tastes.

> it is clear there is no point in jumping to just pop out.
> But I agree, I will stick to the return approach.
> It may be worthwhile to code a loop with a struct containing the set
> of string properties, that's just cosmetic changes.

There is also Mitch's good suggestion to have a helper function for
reading simple single-cell properties.  That would simplify the code
too.  Eventually I'll get around to implementing that (unless someone
beats me to it).

>
>>
>> > +
>> > +       prop = of_get_property(node, "flags", NULL);
>> > +       if (!prop)
>> > +               goto err;
>> > +
>> > +       cfg->flags = of_read_number(prop, 1);
>>
>> Looks like Linux-specific implementation details are being encoded
>> into the device tree.  What is the intended meaning for the "flags"
>> property?
>>
>
> Well, looks like you are right. It is to define if a 16-bit bus or
> 32-bit bus (to force atomicity of burst of write/read) is used to
> access the device, plus flags to force external or internal phy and
> an option to save the MAC address.
>
> What's the approach to be followed in this case ?

Usually there is a separate node for the MDIO bus, and then a node on
the MDIO bus for each PHY.  A phy-device property is used to link the
MAC to the PHY node.  Search for mdio in arch/powerpc/boot/dts/* for
examples. For the internal PHY, that might not use MDIO at all, so it
may be appropriate to have a device-specific property in that use
case.

>> > +       return 0;
>> > +err:
>> > +       return ret;
>> > +}
>>
>> I don't think it is necessary to have a separate .c file for this one
>> function.  I don't see any problem with it living in
>> drivers/net/smsc911x.c.  I'd rather have everything contained in a
>> single place.
>
> The point Grant is that the dt probe function requires access to CONFIG_OF
> dependent members (of_node). If the need for #ifdef CONFIG_OF will go
> I am more than happy to just merge it all in one file.
> This is valid for all the drivers and the match table init macro as well.

The of_device_id match table already need to be wrapped with #ifdef
CONFIG_OF, so I think it is no worse to also include the decode
function inside that #ifdef block.  As for the setting .of_node, I
have been wrapping it with #ifdef, just like CONFIG_PM is often done,
but I agree it is stinky.  I'll probably make a macro that can be used
in the static initializer.

>
>>
>> > diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
>> > index cc55974..c56b8f7 100644
>> > --- a/drivers/net/smsc911x.c
>> > +++ b/drivers/net/smsc911x.c
>> > @@ -50,6 +50,7 @@
>> >  #include <linux/phy.h>
>> >  #include <linux/smsc911x.h>
>> >  #include <linux/device.h>
>> > +
>> >  #include "smsc911x.h"
>>
>> Unrelated whitespace change.
>>
>
> Ok.
>
>> >
>> {
>> >  /* Entry point for loading the module */
>> >  static int __init smsc911x_init_module(void)
>> >  {
>> > +       platform_init_match(&smsc911x_driver, smsc911x_matches);
>>
>> the match table should be statically initialized into the driver
>> structure.  Right now that does require wrapping the line with #ifdef
>> CONFIG_OF which is a little stinky, but I'm working on a solution for
>> that.  Regardless, I don't like having to use a macro to set the
>> pointer at probe time when it can be done statically.
>>
>
> Point taken, but to be precise it is not set at probe time, it is set at
> init time (insmod or initcalls). Again, it is just to avoid ifdefs,
> if the #ifdefs go away, I will do it statically with nary a problem.

Heh; typo.  sorry.

Cheers,
g.

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

Re: [RFC PATCH 05/14] ARM: vexpress: fix typo in addruart

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> The new infrastructure for low level debug in ARM requires the
> addruart macro to be defined on a platform specific basis to get
> rid of static addresses init in machine descriptors.
> Uart physical and virtual addresses should be retrieved through
> the addruart macro.
>
> This patch fixes the addruart macro for Versatile Express board
> in order to return the proper uart physical address.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm/mach-vexpress/include/mach/debug-macro.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>

I'll leave this one to Jeremy.  I've not picked up any of the debug
macro patches into my tree.

g.

> diff --git a/arch/arm/mach-vexpress/include/mach/debug-macro.S b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> index e0382fd..050d65e 100644
> --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S
> +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> @@ -15,7 +15,7 @@
>                .macro  addruart,rp,rv
>                mov     \rp, #DEBUG_LL_UART_OFFSET
>                orr     \rv, \rp, #0xf8000000   @ virtual base
> -               orr     \rv, \rp, #0x10000000   @ physical base
> +               orr     \rp, \rp, #0x10000000   @ physical base
>                .endm
>
>  #include <asm/hardware/debug-pl01x.S>
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 04/14] ARM: r1 DT mach id init

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> Device tree ARM machine desc has been allocated id 0xffffffff temporarily.
> This machine id should be passed in r1 upon kernel booting.
> Legacy code requires code stubs to update r1 accordingly when
> booting from legacy boot loaders that do not necessarily pass DT
> mach id on r1.
>
> This patch adds code in the dtb stub to automatically generate and move
> the required DT machine id into r1 before jumping to kernel code.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>

Fixed in test-devicetree.  You'll be able to drop this one.

> ---
>  arch/arm/boot/dt/dtb.S |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dt/dtb.S b/arch/arm/boot/dt/dtb.S
> index e678db2..7d56289 100644
> --- a/arch/arm/boot/dt/dtb.S
> +++ b/arch/arm/boot/dt/dtb.S
> @@ -35,6 +35,9 @@ setup_dtb:
>        cmp     r4, r5
>        blo     2b
>
> +       /* update r1 with DT id  */
> +       mvn     r1, #0x0
> +
>        b       zimage_start
>
>        .ltorg
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 03/14] ARM: fix add instruction to set the flags

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> ARM data processing instructions require the 's' suffix to update
> the flags upon execution.
>
> This patch fixes code requiring flags to be updated in order to check
> the carry flag for overflow.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>

Fixed in test-devicetree.  You'll be able to drop this one when you rebase.

g.

> ---
>  arch/arm/kernel/head.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 7e00565..9879797 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -96,7 +96,7 @@ ENTRY(stext)
>         * here, as we'll lookup one based on the device tree in setup_arch().
>         * Also, we don't need to call __vet_atags.
>         */
> -       add     r3, r1, #1
> +       adds    r3, r1, #1
>        bcs     1f
>
>        bl      __lookup_machine_type           @ r5=machinfo
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 02/14] ARM: vexpress: fix clocks definition to comply with new framework

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> The new clock infrastructure introduced within the DT series of patches
> slightly changed how clocks are declared and initialized. Some clock
> definitions become platform specific.
>
> This patch fixes some typos on ARM Versatile Express clock definition
> and initialization that caused compile time errors.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>

I'll pick this one up and squash it into Jeremy's original versatile patch.

> ---
>  arch/arm/mach-vexpress/include/mach/clkdev.h |    2 +-
>  arch/arm/mach-vexpress/v2m.c                 |   13 ++++++++++++-
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-vexpress/include/mach/clkdev.h b/arch/arm/mach-vexpress/include/mach/clkdev.h
> index ad3f6e7..9e23cac 100644
> --- a/arch/arm/mach-vexpress/include/mach/clkdev.h
> +++ b/arch/arm/mach-vexpress/include/mach/clkdev.h
> @@ -9,7 +9,7 @@ struct clk_v2m {
>        u32             config;
>  };
>
> -const struct clk_ops clk_v2m_ops;
> +extern const struct clk_ops clk_v2m_ops;
>
>  #define INIT_CLK_V2M(r,c) { .clk = INIT_CLK(clk_v2m_ops), \
>        .rate = r, .config = c }
> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> index f22e272..06d6914 100644
> --- a/arch/arm/mach-vexpress/v2m.c
> +++ b/arch/arm/mach-vexpress/v2m.c
> @@ -285,12 +285,21 @@ static int clk_v2m_set_rate(struct clk *_clk, unsigned long rate)
>        struct clk_v2m *clk = to_clk_v2m(_clk);
>        int ret;
>
> -       ret = v2m_cfg_write(clk->cofig, rate);
> +       ret = v2m_cfg_write(clk->config, rate);
>        if (!ret)
>                clk->rate = rate;
>        return ret;
>  }
>
> +static void clk_v2m_put(struct clk *_clk)
> +{
> +}
> +
> +static int clk_v2m_get(struct clk *_clk)
> +{
> +       return 1;
> +}
> +
>  static unsigned long clk_v2m_get_rate(struct clk *clk)
>  {
>        return to_clk_v2m(clk)->rate;
> @@ -300,6 +309,8 @@ const struct clk_ops clk_v2m_ops = {
>        .round_rate = clk_v2m_round_rate,
>        .get_rate = clk_v2m_get_rate,
>        .set_rate = clk_v2m_set_rate,
> +       .get = clk_v2m_get,
> +       .put = clk_v2m_put,
>  };
>
>  static struct clk_v2m osc1_clk =
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 01/14] ARM: amba device memory allocation fix

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> When instantiating amba devices dynamically from device tree nodes, the
> memory allocation should be carried out using the kzalloc function to make
> sure all the members are zero initialized.
>
> This patch replaces the kmalloc call with a kzalloc one.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>

Fixed in my tree.  You'll be able to drop this one when you rebase.

g.

> ---
>  drivers/amba/bus.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index d73302c..234f4a3 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -421,7 +421,7 @@ static int amba_add_device_from_node(struct device_node *node)
>        const char *name;
>        int i, ret, len;
>
> -       dev = kmalloc(sizeof(*dev), GFP_KERNEL);
> +       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>        if (!dev)
>                return -ENOMEM;
>
> --
> 1.6.3.3
>
>



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

Re: [RFC PATCH 00/14] Versatile Express device tree port

[Grant Likely]

On Wed, Aug 18, 2010 at 12:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> This patchset provides an initial version of device tree enabled kernel on
> an ARM Versatile Express board. The patchset applies to Jeremy Kerr's tree:
>
>        git://kernel.ubuntu.com/jk/dt/linux-2.6.git dtbimage
>        commit: 4cb80ac96489220554d28f6fde527aeef83e628b
>
> The patched kernel version is available on my public ARM git tree:
>
>        git://linux-arm.org/linux-2.6-lp.git ve-fdt
>
> It has been tested on HW ARM Versatile Express board, with both static and DT
> configurations.

w00t!  Thanks Lorenzo.  I'll look through and make comments.  There
have been quite few changes since Jeremy's dtbimage branch.  In
particular the amba bus clock changes are taking a bit to rework.
I've also cherry picked most of Jeremy's patches (everything except
imx51) and have also made a number of changes.  For the next spin it
would be a good idea to use my test-devicetree branch as the base.

I'm currently reworking the device tree breakage caused by amba clock
support, and then I'll be pushing out my branch updated to 2.6.36-rc1
later this afternoon.

Cheers,
g.

> It contains fixes for generic device tree features and clock configuration,
> code to patch Versatile Express peripherals drivers and build system.
>
> Clock names as well as amba device names are temporary waiting for OF bindings
> definition (clocks).
> The Versatile Express board specific init code has been split into DT and
> non-DT code in order to factor out common code between the two configs.
>
> A static inline function has been added to the platform bus in order
> to initialize the OF match table and avoid cluttering code with preprocessor
> macros.
>
> Drivers ( and GIC, sp804, PL310 specific code) device tree init is an initial
> stab at configuring peripherals with device tree data, so some choices
> especially concerning error codes are arguable and require thorough review.
>
> The whole patchset is a request for comments on code and methodology.
>
> Cheers.
>
> Lorenzo Pieralisi (14):
>  ARM: amba device memory allocation fix
>  ARM: vexpress: fix clocks definition to comply with new framework
>  ARM: fix add instruction to set the flags
>  ARM: r1 DT mach id init
>  ARM: vexpress: fix typo in addruart
>  platform: add function to initialize OF match table
>  drivers/smsc911x: add DT support
>  ARM: versatile-i2c driver DT port
>  ARM: ARM flash driver DT port
>  drivers/USB: isp1760 DT platform parsing and binding
>  ARM: PMU: add device tree probing
>  ARM: vexpress: add board support for DT probing
>  ARM: vexpress: Definition of vexpress dts specification
>  ARM: vexpress: add device tree build system and dtbuImage
>
>  arch/arm/Kconfig                                  |    4 +-
>  arch/arm/Makefile                                 |    2 +-
>  arch/arm/boot/Makefile                            |   10 +-
>  arch/arm/boot/dt/dtb.S                            |    3 +
>  arch/arm/boot/dts/vexpress.dts                    |  199 +++++++++++++++++++++
>  arch/arm/include/asm/pmu.h                        |    6 +
>  arch/arm/kernel/Makefile                          |    3 +-
>  arch/arm/kernel/head.S                            |    2 +-
>  arch/arm/kernel/pmu-of.c                          |   30 +++
>  arch/arm/kernel/pmu.c                             |   18 ++-
>  arch/arm/mach-vexpress/Kconfig                    |    7 +
>  arch/arm/mach-vexpress/Makefile                   |    5 +-
>  arch/arm/mach-vexpress/core.h                     |   15 ++-
>  arch/arm/mach-vexpress/ct-ca9x4-base.c            |  108 +++++++++++
>  arch/arm/mach-vexpress/ct-ca9x4-of.c              |  192 ++++++++++++++++++++
>  arch/arm/mach-vexpress/ct-ca9x4.c                 |   93 ----------
>  arch/arm/mach-vexpress/include/mach/clkdev.h      |    2 +-
>  arch/arm/mach-vexpress/include/mach/ct-ca9x4.h    |    2 +
>  arch/arm/mach-vexpress/include/mach/debug-macro.S |    2 +-
>  arch/arm/mach-vexpress/v2m-base.c                 |  197 ++++++++++++++++++++
>  arch/arm/mach-vexpress/v2m-of.c                   |   94 ++++++++++
>  arch/arm/mach-vexpress/v2m.c                      |  167 +-----------------
>  arch/arm/mm/Kconfig                               |    2 +-
>  drivers/amba/bus.c                                |    2 +-
>  drivers/i2c/busses/i2c-versatile.c                |    6 +
>  drivers/mtd/maps/integrator-flash.c               |    6 +
>  drivers/net/Makefile                              |    3 +-
>  drivers/net/smsc911x-of.c                         |   53 ++++++
>  drivers/net/smsc911x.c                            |   28 ++-
>  drivers/net/smsc911x.h                            |    8 +
>  drivers/usb/host/Makefile                         |    4 +-
>  drivers/usb/host/isp1760-hcd.h                    |    7 +
>  drivers/usb/host/isp1760-if.c                     |   43 +++--
>  drivers/usb/host/isp1760-of.c                     |   36 ++++
>  include/linux/platform_device.h                   |   11 ++
>  35 files changed, 1067 insertions(+), 303 deletions(-)
>  create mode 100644 arch/arm/boot/dts/vexpress.dts
>  create mode 100644 arch/arm/kernel/pmu-of.c
>  create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-base.c
>  create mode 100644 arch/arm/mach-vexpress/ct-ca9x4-of.c
>  create mode 100644 arch/arm/mach-vexpress/v2m-base.c
>  create mode 100644 arch/arm/mach-vexpress/v2m-of.c
>  create mode 100644 drivers/net/smsc911x-of.c
>  create mode 100644 drivers/usb/host/isp1760-of.c
>
>



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

Re: [RFC PATCH 00/14] Versatile Express device tree port

[Jeremy Kerr]

Hi Lorenzo,

> This patchset provides an initial version of device tree enabled kernel on
> an ARM Versatile Express board.

Awesome, nice work. As Grant has said, most of my patches are now in his
tree, and that's probably the best place to work from. There shouldn't
be much change required to do a rebase.

> Clock names as well as amba device names are temporary waiting for OF bindings
> definition (clocks).

Grant - I saw you weren't too concerned with the mixed phandle,string
format. What do you think about the binding - should we continue using
the clock name in the property name?

> Lorenzo Pieralisi (14):
>   ARM: amba device memory allocation fix
>   ARM: vexpress: fix clocks definition to comply with new framework
>   ARM: fix add instruction to set the flags
>   ARM: r1 DT mach id init

Sounds like these are (or will be) in Grant's tree already

>   ARM: vexpress: fix typo in addruart

I'd suggest posting this as a separate fix to linux-arm-kernel.

>   platform: add function to initialize OF match table
>   drivers/smsc911x: add DT support
>   ARM: versatile-i2c driver DT port
>   ARM: ARM flash driver DT port
>   drivers/USB: isp1760 DT platform parsing and binding
>   ARM: PMU: add device tree probing
>   ARM: vexpress: add board support for DT probing
>   ARM: vexpress: Definition of vexpress dts specification
>   ARM: vexpress: add device tree build system and dtbuImage

Cheers,

Jeremy

Re: [RFC PATCH 00/14] Versatile Express device tree port

[Jeremy Kerr]

Hi Lorenzo,

> >   ARM: vexpress: fix typo in addruart
> 
> I'd suggest posting this as a separate fix to linux-arm-kernel.

On second thoughts, I'm about to re-post the addruart series, so I've
rolled this change into the original patch.

Cheers,


Jeremy