[PATCH v4 0/4] amba bus devicetree probing

[PATCH v4 0/4] amba bus devicetree probing

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

This is updated amba bus DT probe support. Amba devices are now handled as a
special case in the device tree bus scanning code. Found devices with a
"arm,amba-device" compatible property will be created by of_amba_device_create.

The first 2 patches are fixes for of_platform_populate.

Applies to devicetree/next on git://git.secretlab.ca/git/linux-2.6

Compile tested with DT enabled and AMBA bus enabled or disabled. 

Rob

Rob Herring (4):
  dt: check root nodes for match on of_platform_populate
  dt: add EXPORT_SYMBOL to of_platform_populate
  drivers/amba: create devices from device tree
  dt: add amba device creation to platform bus scan

 Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
 drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
 drivers/of/platform.c                          |    9 ++++
 include/linux/amba/bus.h                       |   18 ++++++++
 4 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amba.txt

-- 
1.7.4.1

[PATCH v4 0/4] amba bus devicetree probing

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

This is updated amba bus DT probe support. Amba devices are now handled as a
special case in the device tree bus scanning code. Found devices with a
"arm,amba-device" compatible property will be created by of_amba_device_create.

The first 2 patches are fixes for of_platform_populate.

Applies to devicetree/next on git://git.secretlab.ca/git/linux-2.6

Compile tested with DT enabled and AMBA bus enabled or disabled. 

Rob

Rob Herring (4):
  dt: check root nodes for match on of_platform_populate
  dt: add EXPORT_SYMBOL to of_platform_populate
  drivers/amba: create devices from device tree
  dt: add amba device creation to platform bus scan

 Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
 drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
 drivers/of/platform.c                          |    9 ++++
 include/linux/amba/bus.h                       |   18 ++++++++
 4 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amba.txt

-- 
1.7.4.1

[PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

of_platform_populate will create platform devices for the top-level nodes
if they have a compatible string, but regardless of a match or not.

Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 drivers/of/platform.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9b785be..dc56a77 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
 		return -EINVAL;
 
 	for_each_child_of_node(root, child) {
+		if (!of_match_node(matches, child))
+			continue;
 		rc = of_platform_bus_create(child, matches, parent, true);
 		if (rc)
 			break;
-- 
1.7.4.1

[PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

of_platform_populate will create platform devices for the top-level nodes
if they have a compatible string, but regardless of a match or not.

Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/platform.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9b785be..dc56a77 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
 		return -EINVAL;
 
 	for_each_child_of_node(root, child) {
+		if (!of_match_node(matches, child))
+			continue;
 		rc = of_platform_bus_create(child, matches, parent, true);
 		if (rc)
 			break;
-- 
1.7.4.1

[PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

of_platform_populate is missing an EXPORT_SYMBOL, so add it.

Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 drivers/of/platform.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index dc56a77..8d0f477 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
 	of_node_put(root);
 	return rc;
 }
+EXPORT_SYMBOL(of_platform_populate);
 #endif /* !CONFIG_SPARC */
-- 
1.7.4.1

[PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

of_platform_populate is missing an EXPORT_SYMBOL, so add it.

Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/platform.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index dc56a77..8d0f477 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
 	of_node_put(root);
 	return rc;
 }
+EXPORT_SYMBOL(of_platform_populate);
 #endif /* !CONFIG_SPARC */
-- 
1.7.4.1

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Add a function to create amba bus devices (i.e. primecell peripherals) from
device tree nodes. The device tree scanning is done by
of_platform_probe/populate functions which can call of_amba_device_create
based on a match table entry.

Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
value.

Based on the original work by Jeremy Kerr.

Cc: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org
Cc: arnd-r2nGTMty4D4@public.gmane.org
Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
 drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
 include/linux/amba/bus.h                       |   18 ++++++++
 3 files changed, 90 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amba.txt

diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
new file mode 100644
index 0000000..23fde7f
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/amba.txt
@@ -0,0 +1,21 @@
+* ARM Primecell Peripherals
+
+ARM, Ltd. Primecell peripherals have a standard id register that can be used to
+identify the peripheral type, vendor, and revision. This value can be used for
+driver matching. 
+
+Required properties:
+
+- compatible : should be a specific value for peripheral and "arm,amba-device"
+
+Optional properties:
+
+- arm,amba-deviceid : Value to override the h/w value with
+
+Example:
+
+serial@fff36000 {
+	compatible = "arm,pl011", "arm,amba-device";
+	arm,amba-deviceid = <0x00341011>;
+};
+
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index d74926e..19f712b 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -13,6 +13,11 @@
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/amba/bus.h>
@@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
 EXPORT_SYMBOL(amba_find_device);
 EXPORT_SYMBOL(amba_request_regions);
 EXPORT_SYMBOL(amba_release_regions);
+
+#ifdef CONFIG_OF
+int of_amba_device_create(struct device_node *node, struct device *parent)
+{
+	struct amba_device *dev;
+	const void *prop;
+	int i, ret;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	/* setup generic device info */
+	dev->dev.coherent_dma_mask = ~0;
+	dev->dev.of_node = node;
+	dev->dev.parent = parent;
+	of_device_make_bus_id(&dev->dev);
+
+	/* setup amba-specific device info */
+	dev->dma_mask = ~0;
+
+	/* Allow the arm,amba-deviceid value to override the h/w value */
+	prop = of_get_property(node, "arm,amba-deviceid", NULL);
+	if (prop)
+		dev->periphid = of_read_ulong(prop, 1);
+
+	/* Decode the IRQs and address ranges */
+	for (i = 0; i < AMBA_NR_IRQS; i++)
+		dev->irq[i] = irq_of_parse_and_map(node, i);
+
+	ret = of_address_to_resource(node, 0, &dev->res);
+	if (ret)
+		goto err_free;
+
+	ret = amba_device_register(dev, &iomem_resource);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+err_free:
+	kfree(dev);
+	return ret;
+}
+
+#endif /* CONFIG_OF */
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index fcbbe71..0177274 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -24,6 +24,7 @@
 #define AMBA_CID	0xb105f00d
 
 struct clk;
+struct device_node;
 
 struct amba_device {
 	struct device		dev;
@@ -63,6 +64,7 @@ extern struct bus_type amba_bustype;
 #define amba_get_drvdata(d)	dev_get_drvdata(&d->dev)
 #define amba_set_drvdata(d,p)	dev_set_drvdata(&d->dev, p)
 
+#ifdef CONFIG_ARM_AMBA
 int amba_driver_register(struct amba_driver *);
 void amba_driver_unregister(struct amba_driver *);
 int amba_device_register(struct amba_device *, struct resource *);
@@ -94,4 +96,20 @@ void amba_release_regions(struct amba_device *);
 #define amba_manf(d)	AMBA_MANF_BITS((d)->periphid)
 #define amba_part(d)	AMBA_PART_BITS((d)->periphid)
 
+#ifdef CONFIG_OF
+int of_amba_device_create(struct device_node *node, struct device *parent);
+#endif
+
+#else
+
+#ifdef CONFIG_OF
+static inline int of_amba_device_create(struct device_node *node, 
+					struct device *parent)
+{
+	return 0;
+}
+#endif
+
+#endif /* CONFIG_ARM_AMBA */
+
 #endif
-- 
1.7.4.1

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

Add a function to create amba bus devices (i.e. primecell peripherals) from
device tree nodes. The device tree scanning is done by
of_platform_probe/populate functions which can call of_amba_device_create
based on a match table entry.

Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
value.

Based on the original work by Jeremy Kerr.

Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: linux at arm.linux.org.uk
Cc: arnd at arndb.de
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
 drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
 include/linux/amba/bus.h                       |   18 ++++++++
 3 files changed, 90 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amba.txt

diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
new file mode 100644
index 0000000..23fde7f
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/amba.txt
@@ -0,0 +1,21 @@
+* ARM Primecell Peripherals
+
+ARM, Ltd. Primecell peripherals have a standard id register that can be used to
+identify the peripheral type, vendor, and revision. This value can be used for
+driver matching. 
+
+Required properties:
+
+- compatible : should be a specific value for peripheral and "arm,amba-device"
+
+Optional properties:
+
+- arm,amba-deviceid : Value to override the h/w value with
+
+Example:
+
+serial at fff36000 {
+	compatible = "arm,pl011", "arm,amba-device";
+	arm,amba-deviceid = <0x00341011>;
+};
+
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index d74926e..19f712b 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -13,6 +13,11 @@
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/amba/bus.h>
@@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
 EXPORT_SYMBOL(amba_find_device);
 EXPORT_SYMBOL(amba_request_regions);
 EXPORT_SYMBOL(amba_release_regions);
+
+#ifdef CONFIG_OF
+int of_amba_device_create(struct device_node *node, struct device *parent)
+{
+	struct amba_device *dev;
+	const void *prop;
+	int i, ret;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	/* setup generic device info */
+	dev->dev.coherent_dma_mask = ~0;
+	dev->dev.of_node = node;
+	dev->dev.parent = parent;
+	of_device_make_bus_id(&dev->dev);
+
+	/* setup amba-specific device info */
+	dev->dma_mask = ~0;
+
+	/* Allow the arm,amba-deviceid value to override the h/w value */
+	prop = of_get_property(node, "arm,amba-deviceid", NULL);
+	if (prop)
+		dev->periphid = of_read_ulong(prop, 1);
+
+	/* Decode the IRQs and address ranges */
+	for (i = 0; i < AMBA_NR_IRQS; i++)
+		dev->irq[i] = irq_of_parse_and_map(node, i);
+
+	ret = of_address_to_resource(node, 0, &dev->res);
+	if (ret)
+		goto err_free;
+
+	ret = amba_device_register(dev, &iomem_resource);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+err_free:
+	kfree(dev);
+	return ret;
+}
+
+#endif /* CONFIG_OF */
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index fcbbe71..0177274 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -24,6 +24,7 @@
 #define AMBA_CID	0xb105f00d
 
 struct clk;
+struct device_node;
 
 struct amba_device {
 	struct device		dev;
@@ -63,6 +64,7 @@ extern struct bus_type amba_bustype;
 #define amba_get_drvdata(d)	dev_get_drvdata(&d->dev)
 #define amba_set_drvdata(d,p)	dev_set_drvdata(&d->dev, p)
 
+#ifdef CONFIG_ARM_AMBA
 int amba_driver_register(struct amba_driver *);
 void amba_driver_unregister(struct amba_driver *);
 int amba_device_register(struct amba_device *, struct resource *);
@@ -94,4 +96,20 @@ void amba_release_regions(struct amba_device *);
 #define amba_manf(d)	AMBA_MANF_BITS((d)->periphid)
 #define amba_part(d)	AMBA_PART_BITS((d)->periphid)
 
+#ifdef CONFIG_OF
+int of_amba_device_create(struct device_node *node, struct device *parent);
+#endif
+
+#else
+
+#ifdef CONFIG_OF
+static inline int of_amba_device_create(struct device_node *node, 
+					struct device *parent)
+{
+	return 0;
+}
+#endif
+
+#endif /* CONFIG_ARM_AMBA */
+
 #endif
-- 
1.7.4.1

[PATCH v4 4/4] dt: add amba device creation to platform bus scan

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Add support to the platform bus scanning to call custom device creation
function for amba devices.

Cc: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: arnd-r2nGTMty4D4@public.gmane.org
Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 drivers/of/platform.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8d0f477..1712f22 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -21,6 +21,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/amba/bus.h>
 
 static int of_dev_node_match(struct device *dev, void *data)
 {
@@ -234,6 +235,11 @@ static int of_platform_bus_create(struct device_node *bus,
 		return 0;
 	}
 
+	if (of_device_is_compatible(bus, "arm,amba-device")) {
+		of_amba_device_create(bus, parent);
+		return 0;
+	}
+	
 	dev = of_platform_device_create(bus, NULL, parent);
 	if (!dev || !of_match_node(matches, bus))
 		return 0;
-- 
1.7.4.1

[PATCH v4 4/4] dt: add amba device creation to platform bus scan

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

Add support to the platform bus scanning to call custom device creation
function for amba devices.

Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: arnd at arndb.de
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/platform.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8d0f477..1712f22 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -21,6 +21,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/amba/bus.h>
 
 static int of_dev_node_match(struct device *dev, void *data)
 {
@@ -234,6 +235,11 @@ static int of_platform_bus_create(struct device_node *bus,
 		return 0;
 	}
 
+	if (of_device_is_compatible(bus, "arm,amba-device")) {
+		of_amba_device_create(bus, parent);
+		return 0;
+	}
+	
 	dev = of_platform_device_create(bus, NULL, parent);
 	if (!dev || !of_match_node(matches, bus))
 		return 0;
-- 
1.7.4.1

Re: [PATCH v4 4/4] dt: add amba device creation to platform bus scan

[Arnd Bergmann]

On Friday 10 June 2011 22:48:43 Rob Herring wrote:
> 
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> Add support to the platform bus scanning to call custom device creation
> function for amba devices.
> 
> Cc: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: arnd-r2nGTMty4D4@public.gmane.org
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Reviewed-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Looks so trivial now ;-)

[PATCH v4 4/4] dt: add amba device creation to platform bus scan

[Arnd Bergmann]

On Friday 10 June 2011 22:48:43 Rob Herring wrote:
> 
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add support to the platform bus scanning to call custom device creation
> function for amba devices.
> 
> Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: arnd at arndb.de
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Looks so trivial now ;-)

Re: [PATCH v4 3/4] drivers/amba: create devices from device tree

[Arnd Bergmann]

On Friday 10 June 2011 22:48:42 Rob Herring wrote:
> 
> +#ifdef CONFIG_OF
> +int of_amba_device_create(struct device_node *node, struct device *parent);
> +#endif
> +
> +#else
> +
> +#ifdef CONFIG_OF
> +static inline int of_amba_device_create(struct device_node *node, 
> +                                       struct device *parent)
> +{
> +       return 0;
> +}
> +#endif
> +

We normally don't use #ifdef around declarations like this, it doesn't
hurt to have them even when the code is not there. Otherwise the patch
looks good.

	Arnd

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Arnd Bergmann]

On Friday 10 June 2011 22:48:42 Rob Herring wrote:
> 
> +#ifdef CONFIG_OF
> +int of_amba_device_create(struct device_node *node, struct device *parent);
> +#endif
> +
> +#else
> +
> +#ifdef CONFIG_OF
> +static inline int of_amba_device_create(struct device_node *node, 
> +                                       struct device *parent)
> +{
> +       return 0;
> +}
> +#endif
> +

We normally don't use #ifdef around declarations like this, it doesn't
hurt to have them even when the code is not there. Otherwise the patch
looks good.

	Arnd

Re: [PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Arnd Bergmann]

On Friday 10 June 2011 22:48:40 Rob Herring wrote:
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9b785be..dc56a77 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
>                 return -EINVAL;
>  
>         for_each_child_of_node(root, child) {
> +               if (!of_match_node(matches, child))
> +                       continue;
>                 rc = of_platform_bus_create(child, matches, parent, true);
>                 if (rc)
>                         break;


Why doesn't of_platform_populate just call of_platform_bus_create on the
root node? That should normally take care of matching all children.

	Arnd

[PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Arnd Bergmann]

On Friday 10 June 2011 22:48:40 Rob Herring wrote:
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9b785be..dc56a77 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
>                 return -EINVAL;
>  
>         for_each_child_of_node(root, child) {
> +               if (!of_match_node(matches, child))
> +                       continue;
>                 rc = of_platform_bus_create(child, matches, parent, true);
>                 if (rc)
>                         break;


Why doesn't of_platform_populate just call of_platform_bus_create on the
root node? That should normally take care of matching all children.

	Arnd

Re: [PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 11:06:48PM +0200, Arnd Bergmann wrote:
> On Friday 10 June 2011 22:48:40 Rob Herring wrote:
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 9b785be..dc56a77 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
> >                 return -EINVAL;
> >  
> >         for_each_child_of_node(root, child) {
> > +               if (!of_match_node(matches, child))
> > +                       continue;
> >                 rc = of_platform_bus_create(child, matches, parent, true);
> >                 if (rc)
> >                         break;
> 
> 
> Why doesn't of_platform_populate just call of_platform_bus_create on the
> root node? That should normally take care of matching all children.

of_platform_bus_create creates a device for the 'bus' node, where
of_platform_populate() creates children as direct children of the
'parent' device; which I think is the behaviour wanted here.

Also, of_platform_bus_create checks for a compatible property on the bus
node, whereas of_platform_populate does not, and should not.

g.

[PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 11:06:48PM +0200, Arnd Bergmann wrote:
> On Friday 10 June 2011 22:48:40 Rob Herring wrote:
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 9b785be..dc56a77 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
> >                 return -EINVAL;
> >  
> >         for_each_child_of_node(root, child) {
> > +               if (!of_match_node(matches, child))
> > +                       continue;
> >                 rc = of_platform_bus_create(child, matches, parent, true);
> >                 if (rc)
> >                         break;
> 
> 
> Why doesn't of_platform_populate just call of_platform_bus_create on the
> root node? That should normally take care of matching all children.

of_platform_bus_create creates a device for the 'bus' node, where
of_platform_populate() creates children as direct children of the
'parent' device; which I think is the behaviour wanted here.

Also, of_platform_bus_create checks for a compatible property on the bus
node, whereas of_platform_populate does not, and should not.

g.

Re: [PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:40PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> of_platform_populate will create platform devices for the top-level nodes
> if they have a compatible string, but regardless of a match or not.
> 
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/of/platform.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9b785be..dc56a77 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
>  		return -EINVAL;
>  
>  	for_each_child_of_node(root, child) {
> +		if (!of_match_node(matches, child))
> +			continue;

This doesn't look right.  The point of of_platform_populate() is to
register platform_devices for all the child nodes, regardless of
whether or not they are in the match table.

of_platform_bus_create() does the correct behaviour to check for
whether or not to register grandchildren.

>  		rc = of_platform_bus_create(child, matches, parent, true);
>  		if (rc)
>  			break;
> -- 
> 1.7.4.1
> 

[PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:40PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> of_platform_populate will create platform devices for the top-level nodes
> if they have a compatible string, but regardless of a match or not.
> 
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  drivers/of/platform.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9b785be..dc56a77 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -318,6 +318,8 @@ int of_platform_populate(struct device_node *root,
>  		return -EINVAL;
>  
>  	for_each_child_of_node(root, child) {
> +		if (!of_match_node(matches, child))
> +			continue;

This doesn't look right.  The point of of_platform_populate() is to
register platform_devices for all the child nodes, regardless of
whether or not they are in the match table.

of_platform_bus_create() does the correct behaviour to check for
whether or not to register grandchildren.

>  		rc = of_platform_bus_create(child, matches, parent, true);
>  		if (rc)
>  			break;
> -- 
> 1.7.4.1
> 

Re: [PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Arnd Bergmann]

On Friday 10 June 2011 23:11:19 Grant Likely wrote:
> > Why doesn't of_platform_populate just call of_platform_bus_create on the
> > root node? That should normally take care of matching all children.
> 
> of_platform_bus_create creates a device for the 'bus' node, where
> of_platform_populate() creates children as direct children of the
> 'parent' device; which I think is the behaviour wanted here.
> 
> Also, of_platform_bus_create checks for a compatible property on the bus
> node, whereas of_platform_populate does not, and should not.

Ok, makes sense.

Thanks,

	Arnd

[PATCH v4 1/4] dt: check root nodes for match on of_platform_populate

[Arnd Bergmann]

On Friday 10 June 2011 23:11:19 Grant Likely wrote:
> > Why doesn't of_platform_populate just call of_platform_bus_create on the
> > root node? That should normally take care of matching all children.
> 
> of_platform_bus_create creates a device for the 'bus' node, where
> of_platform_populate() creates children as direct children of the
> 'parent' device; which I think is the behaviour wanted here.
> 
> Also, of_platform_bus_create checks for a compatible property on the bus
> node, whereas of_platform_populate does not, and should not.

Ok, makes sense.

Thanks,

	Arnd

Re: [PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:41PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> of_platform_populate is missing an EXPORT_SYMBOL, so add it.
> 
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/of/platform.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index dc56a77..8d0f477 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
>  	of_node_put(root);
>  	return rc;
>  }
> +EXPORT_SYMBOL(of_platform_populate);

I hadn't done this because I hadn't seen a need for calling it from
module code.  Do you have a driver that uses it?

g.

[PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:41PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> of_platform_populate is missing an EXPORT_SYMBOL, so add it.
> 
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  drivers/of/platform.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index dc56a77..8d0f477 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
>  	of_node_put(root);
>  	return rc;
>  }
> +EXPORT_SYMBOL(of_platform_populate);

I hadn't done this because I hadn't seen a need for calling it from
module code.  Do you have a driver that uses it?

g.

Re: [PATCH v4 3/4] drivers/amba: create devices from device tree

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:42PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> Add a function to create amba bus devices (i.e. primecell peripherals) from
> device tree nodes. The device tree scanning is done by
> of_platform_probe/populate functions which can call of_amba_device_create
> based on a match table entry.
> 
> Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
> value.
> 
> Based on the original work by Jeremy Kerr.
> 
> Cc: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org
> Cc: arnd-r2nGTMty4D4@public.gmane.org
> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
>  drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
>  include/linux/amba/bus.h                       |   18 ++++++++
>  3 files changed, 90 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/amba.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
> new file mode 100644
> index 0000000..23fde7f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/amba.txt
> @@ -0,0 +1,21 @@
> +* ARM Primecell Peripherals
> +
> +ARM, Ltd. Primecell peripherals have a standard id register that can be used to
> +identify the peripheral type, vendor, and revision. This value can be used for
> +driver matching. 
> +
> +Required properties:
> +
> +- compatible : should be a specific value for peripheral and "arm,amba-device"

After recent conversations, I'd like to change this to
"arm,primecell" to better reflect the kind of devices it models.

> +
> +Optional properties:
> +
> +- arm,amba-deviceid : Value to override the h/w value with

Similarly, based on the primecell device documents, this should
probabaly be arm,primecell-periphid.

> +
> +Example:
> +
> +serial@fff36000 {
> +	compatible = "arm,pl011", "arm,amba-device";
> +	arm,amba-deviceid = <0x00341011>;
> +};
> +
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index d74926e..19f712b 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -13,6 +13,11 @@
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/amba/bus.h>
> @@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
>  EXPORT_SYMBOL(amba_find_device);
>  EXPORT_SYMBOL(amba_request_regions);
>  EXPORT_SYMBOL(amba_release_regions);
> +
> +#ifdef CONFIG_OF
> +int of_amba_device_create(struct device_node *node, struct device *parent)
> +{
> +	struct amba_device *dev;
> +	const void *prop;
> +	int i, ret;
> +
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	/* setup generic device info */
> +	dev->dev.coherent_dma_mask = ~0;
> +	dev->dev.of_node = node;
> +	dev->dev.parent = parent;
> +	of_device_make_bus_id(&dev->dev);
> +
> +	/* setup amba-specific device info */
> +	dev->dma_mask = ~0;
> +
> +	/* Allow the arm,amba-deviceid value to override the h/w value */
> +	prop = of_get_property(node, "arm,amba-deviceid", NULL);
> +	if (prop)
> +		dev->periphid = of_read_ulong(prop, 1);
> +
> +	/* Decode the IRQs and address ranges */
> +	for (i = 0; i < AMBA_NR_IRQS; i++)
> +		dev->irq[i] = irq_of_parse_and_map(node, i);
> +
> +	ret = of_address_to_resource(node, 0, &dev->res);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = amba_device_register(dev, &iomem_resource);
> +	if (ret)
> +		goto err_free;
> +
> +	return 0;
> +
> +err_free:
> +	kfree(dev);
> +	return ret;
> +}
> +

Yeah, I think this looks like the right thing to do.  I'll pick it up
into devicetree/test and probably move it to devicetree/arm in the
near future after testing a bit.

> +#endif /* CONFIG_OF */
> diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
> index fcbbe71..0177274 100644
> --- a/include/linux/amba/bus.h
> +++ b/include/linux/amba/bus.h
> @@ -24,6 +24,7 @@
>  #define AMBA_CID	0xb105f00d
>  
>  struct clk;
> +struct device_node;
>  
>  struct amba_device {
>  	struct device		dev;
> @@ -63,6 +64,7 @@ extern struct bus_type amba_bustype;
>  #define amba_get_drvdata(d)	dev_get_drvdata(&d->dev)
>  #define amba_set_drvdata(d,p)	dev_set_drvdata(&d->dev, p)
>  
> +#ifdef CONFIG_ARM_AMBA
>  int amba_driver_register(struct amba_driver *);
>  void amba_driver_unregister(struct amba_driver *);
>  int amba_device_register(struct amba_device *, struct resource *);
> @@ -94,4 +96,20 @@ void amba_release_regions(struct amba_device *);
>  #define amba_manf(d)	AMBA_MANF_BITS((d)->periphid)
>  #define amba_part(d)	AMBA_PART_BITS((d)->periphid)
>  
> +#ifdef CONFIG_OF
> +int of_amba_device_create(struct device_node *node, struct device *parent);
> +#endif
> +
> +#else
> +
> +#ifdef CONFIG_OF
> +static inline int of_amba_device_create(struct device_node *node, 
> +					struct device *parent)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* CONFIG_ARM_AMBA */
> +
>  #endif
> -- 
> 1.7.4.1
> 

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:42PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add a function to create amba bus devices (i.e. primecell peripherals) from
> device tree nodes. The device tree scanning is done by
> of_platform_probe/populate functions which can call of_amba_device_create
> based on a match table entry.
> 
> Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
> value.
> 
> Based on the original work by Jeremy Kerr.
> 
> Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: linux at arm.linux.org.uk
> Cc: arnd at arndb.de
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
>  drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
>  include/linux/amba/bus.h                       |   18 ++++++++
>  3 files changed, 90 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/amba.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
> new file mode 100644
> index 0000000..23fde7f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/amba.txt
> @@ -0,0 +1,21 @@
> +* ARM Primecell Peripherals
> +
> +ARM, Ltd. Primecell peripherals have a standard id register that can be used to
> +identify the peripheral type, vendor, and revision. This value can be used for
> +driver matching. 
> +
> +Required properties:
> +
> +- compatible : should be a specific value for peripheral and "arm,amba-device"

After recent conversations, I'd like to change this to
"arm,primecell" to better reflect the kind of devices it models.

> +
> +Optional properties:
> +
> +- arm,amba-deviceid : Value to override the h/w value with

Similarly, based on the primecell device documents, this should
probabaly be arm,primecell-periphid.

> +
> +Example:
> +
> +serial at fff36000 {
> +	compatible = "arm,pl011", "arm,amba-device";
> +	arm,amba-deviceid = <0x00341011>;
> +};
> +
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index d74926e..19f712b 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -13,6 +13,11 @@
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/amba/bus.h>
> @@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
>  EXPORT_SYMBOL(amba_find_device);
>  EXPORT_SYMBOL(amba_request_regions);
>  EXPORT_SYMBOL(amba_release_regions);
> +
> +#ifdef CONFIG_OF
> +int of_amba_device_create(struct device_node *node, struct device *parent)
> +{
> +	struct amba_device *dev;
> +	const void *prop;
> +	int i, ret;
> +
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	/* setup generic device info */
> +	dev->dev.coherent_dma_mask = ~0;
> +	dev->dev.of_node = node;
> +	dev->dev.parent = parent;
> +	of_device_make_bus_id(&dev->dev);
> +
> +	/* setup amba-specific device info */
> +	dev->dma_mask = ~0;
> +
> +	/* Allow the arm,amba-deviceid value to override the h/w value */
> +	prop = of_get_property(node, "arm,amba-deviceid", NULL);
> +	if (prop)
> +		dev->periphid = of_read_ulong(prop, 1);
> +
> +	/* Decode the IRQs and address ranges */
> +	for (i = 0; i < AMBA_NR_IRQS; i++)
> +		dev->irq[i] = irq_of_parse_and_map(node, i);
> +
> +	ret = of_address_to_resource(node, 0, &dev->res);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = amba_device_register(dev, &iomem_resource);
> +	if (ret)
> +		goto err_free;
> +
> +	return 0;
> +
> +err_free:
> +	kfree(dev);
> +	return ret;
> +}
> +

Yeah, I think this looks like the right thing to do.  I'll pick it up
into devicetree/test and probably move it to devicetree/arm in the
near future after testing a bit.

> +#endif /* CONFIG_OF */
> diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
> index fcbbe71..0177274 100644
> --- a/include/linux/amba/bus.h
> +++ b/include/linux/amba/bus.h
> @@ -24,6 +24,7 @@
>  #define AMBA_CID	0xb105f00d
>  
>  struct clk;
> +struct device_node;
>  
>  struct amba_device {
>  	struct device		dev;
> @@ -63,6 +64,7 @@ extern struct bus_type amba_bustype;
>  #define amba_get_drvdata(d)	dev_get_drvdata(&d->dev)
>  #define amba_set_drvdata(d,p)	dev_set_drvdata(&d->dev, p)
>  
> +#ifdef CONFIG_ARM_AMBA
>  int amba_driver_register(struct amba_driver *);
>  void amba_driver_unregister(struct amba_driver *);
>  int amba_device_register(struct amba_device *, struct resource *);
> @@ -94,4 +96,20 @@ void amba_release_regions(struct amba_device *);
>  #define amba_manf(d)	AMBA_MANF_BITS((d)->periphid)
>  #define amba_part(d)	AMBA_PART_BITS((d)->periphid)
>  
> +#ifdef CONFIG_OF
> +int of_amba_device_create(struct device_node *node, struct device *parent);
> +#endif
> +
> +#else
> +
> +#ifdef CONFIG_OF
> +static inline int of_amba_device_create(struct device_node *node, 
> +					struct device *parent)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* CONFIG_ARM_AMBA */
> +
>  #endif
> -- 
> 1.7.4.1
> 

Re: [PATCH v4 3/4] drivers/amba: create devices from device tree

[Grant Likely]

On Fri, Jun 10, 2011 at 11:04:06PM +0200, Arnd Bergmann wrote:
> On Friday 10 June 2011 22:48:42 Rob Herring wrote:
> > 
> > +#ifdef CONFIG_OF
> > +int of_amba_device_create(struct device_node *node, struct device *parent);

Actually, it would probably be good to have of_amba_device_create()
return a pointer to the created amba_device.

> > +#endif
> > +
> > +#else
> > +
> > +#ifdef CONFIG_OF
> > +static inline int of_amba_device_create(struct device_node *node, 
> > +                                       struct device *parent)
> > +{
> > +       return 0;
> > +}
> > +#endif
> > +
> 
> We normally don't use #ifdef around declarations like this, it doesn't
> hurt to have them even when the code is not there. Otherwise the patch
> looks good.

Yes, this hook will never get called or referenced by anything if
CONFIG_OF is not selected.  The only real user of it will be
drivers/of/platform.c

g.

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Grant Likely]

On Fri, Jun 10, 2011 at 11:04:06PM +0200, Arnd Bergmann wrote:
> On Friday 10 June 2011 22:48:42 Rob Herring wrote:
> > 
> > +#ifdef CONFIG_OF
> > +int of_amba_device_create(struct device_node *node, struct device *parent);

Actually, it would probably be good to have of_amba_device_create()
return a pointer to the created amba_device.

> > +#endif
> > +
> > +#else
> > +
> > +#ifdef CONFIG_OF
> > +static inline int of_amba_device_create(struct device_node *node, 
> > +                                       struct device *parent)
> > +{
> > +       return 0;
> > +}
> > +#endif
> > +
> 
> We normally don't use #ifdef around declarations like this, it doesn't
> hurt to have them even when the code is not there. Otherwise the patch
> looks good.

Yes, this hook will never get called or referenced by anything if
CONFIG_OF is not selected.  The only real user of it will be
drivers/of/platform.c

g.

Re: [PATCH v4 3/4] drivers/amba: create devices from device tree

[Rob Herring]

On 06/10/2011 05:22 PM, Grant Likely wrote:
> On Fri, Jun 10, 2011 at 03:48:42PM -0500, Rob Herring wrote:
>> From: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>
>> Add a function to create amba bus devices (i.e. primecell peripherals) from
>> device tree nodes. The device tree scanning is done by
>> of_platform_probe/populate functions which can call of_amba_device_create
>> based on a match table entry.
>>
>> Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
>> value.
>>
>> Based on the original work by Jeremy Kerr.
>>
>> Cc: Jeremy Kerr<jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> Cc: Grant Likely<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org
>> Cc: arnd-r2nGTMty4D4@public.gmane.org
>> Acked-by: Linus Walleij<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
>>   drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
>>   include/linux/amba/bus.h                       |   18 ++++++++
>>   3 files changed, 90 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/arm/amba.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
>> new file mode 100644
>> index 0000000..23fde7f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/amba.txt
>> @@ -0,0 +1,21 @@
>> +* ARM Primecell Peripherals
>> +
>> +ARM, Ltd. Primecell peripherals have a standard id register that can be used to
>> +identify the peripheral type, vendor, and revision. This value can be used for
>> +driver matching.
>> +
>> +Required properties:
>> +
>> +- compatible : should be a specific value for peripheral and "arm,amba-device"
>
> After recent conversations, I'd like to change this to
> "arm,primecell" to better reflect the kind of devices it models.

That did cross my mind as I was writing the doc...

>
>> +
>> +Optional properties:
>> +
>> +- arm,amba-deviceid : Value to override the h/w value with
>
> Similarly, based on the primecell device documents, this should
> probabaly be arm,primecell-periphid.
>
>> +
>> +Example:
>> +
>> +serial@fff36000 {
>> +	compatible = "arm,pl011", "arm,amba-device";
>> +	arm,amba-deviceid =<0x00341011>;
>> +};
>> +
>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>> index d74926e..19f712b 100644
>> --- a/drivers/amba/bus.c
>> +++ b/drivers/amba/bus.c
>> @@ -13,6 +13,11 @@
>>   #include<linux/string.h>
>>   #include<linux/slab.h>
>>   #include<linux/io.h>
>> +#include<linux/of.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/of_address.h>
>> +#include<linux/of_device.h>
>> +#include<linux/of_platform.h>
>>   #include<linux/pm.h>
>>   #include<linux/pm_runtime.h>
>>   #include<linux/amba/bus.h>
>> @@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
>>   EXPORT_SYMBOL(amba_find_device);
>>   EXPORT_SYMBOL(amba_request_regions);
>>   EXPORT_SYMBOL(amba_release_regions);
>> +
>> +#ifdef CONFIG_OF
>> +int of_amba_device_create(struct device_node *node, struct device *parent)
>> +{
>> +	struct amba_device *dev;
>> +	const void *prop;
>> +	int i, ret;
>> +
>> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	/* setup generic device info */
>> +	dev->dev.coherent_dma_mask = ~0;
>> +	dev->dev.of_node = node;
>> +	dev->dev.parent = parent;
>> +	of_device_make_bus_id(&dev->dev);
>> +
>> +	/* setup amba-specific device info */
>> +	dev->dma_mask = ~0;
>> +
>> +	/* Allow the arm,amba-deviceid value to override the h/w value */
>> +	prop = of_get_property(node, "arm,amba-deviceid", NULL);
>> +	if (prop)
>> +		dev->periphid = of_read_ulong(prop, 1);
>> +
>> +	/* Decode the IRQs and address ranges */
>> +	for (i = 0; i<  AMBA_NR_IRQS; i++)
>> +		dev->irq[i] = irq_of_parse_and_map(node, i);
>> +
>> +	ret = of_address_to_resource(node, 0,&dev->res);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	ret = amba_device_register(dev,&iomem_resource);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	return 0;
>> +
>> +err_free:
>> +	kfree(dev);
>> +	return ret;
>> +}
>> +
>
> Yeah, I think this looks like the right thing to do.  I'll pick it up
> into devicetree/test and probably move it to devicetree/arm in the
> near future after testing a bit.

Okay, I'll respin with your and Arnd's comments.

Rob

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Rob Herring]

On 06/10/2011 05:22 PM, Grant Likely wrote:
> On Fri, Jun 10, 2011 at 03:48:42PM -0500, Rob Herring wrote:
>> From: Rob Herring<rob.herring@calxeda.com>
>>
>> Add a function to create amba bus devices (i.e. primecell peripherals) from
>> device tree nodes. The device tree scanning is done by
>> of_platform_probe/populate functions which can call of_amba_device_create
>> based on a match table entry.
>>
>> Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
>> value.
>>
>> Based on the original work by Jeremy Kerr.
>>
>> Cc: Jeremy Kerr<jeremy.kerr@canonical.com>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: linux at arm.linux.org.uk
>> Cc: arnd at arndb.de
>> Acked-by: Linus Walleij<linus.walleij@linaro.org>
>> Signed-off-by: Rob Herring<rob.herring@calxeda.com>
>> ---
>>   Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
>>   drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
>>   include/linux/amba/bus.h                       |   18 ++++++++
>>   3 files changed, 90 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/arm/amba.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
>> new file mode 100644
>> index 0000000..23fde7f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/amba.txt
>> @@ -0,0 +1,21 @@
>> +* ARM Primecell Peripherals
>> +
>> +ARM, Ltd. Primecell peripherals have a standard id register that can be used to
>> +identify the peripheral type, vendor, and revision. This value can be used for
>> +driver matching.
>> +
>> +Required properties:
>> +
>> +- compatible : should be a specific value for peripheral and "arm,amba-device"
>
> After recent conversations, I'd like to change this to
> "arm,primecell" to better reflect the kind of devices it models.

That did cross my mind as I was writing the doc...

>
>> +
>> +Optional properties:
>> +
>> +- arm,amba-deviceid : Value to override the h/w value with
>
> Similarly, based on the primecell device documents, this should
> probabaly be arm,primecell-periphid.
>
>> +
>> +Example:
>> +
>> +serial at fff36000 {
>> +	compatible = "arm,pl011", "arm,amba-device";
>> +	arm,amba-deviceid =<0x00341011>;
>> +};
>> +
>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>> index d74926e..19f712b 100644
>> --- a/drivers/amba/bus.c
>> +++ b/drivers/amba/bus.c
>> @@ -13,6 +13,11 @@
>>   #include<linux/string.h>
>>   #include<linux/slab.h>
>>   #include<linux/io.h>
>> +#include<linux/of.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/of_address.h>
>> +#include<linux/of_device.h>
>> +#include<linux/of_platform.h>
>>   #include<linux/pm.h>
>>   #include<linux/pm_runtime.h>
>>   #include<linux/amba/bus.h>
>> @@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
>>   EXPORT_SYMBOL(amba_find_device);
>>   EXPORT_SYMBOL(amba_request_regions);
>>   EXPORT_SYMBOL(amba_release_regions);
>> +
>> +#ifdef CONFIG_OF
>> +int of_amba_device_create(struct device_node *node, struct device *parent)
>> +{
>> +	struct amba_device *dev;
>> +	const void *prop;
>> +	int i, ret;
>> +
>> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	/* setup generic device info */
>> +	dev->dev.coherent_dma_mask = ~0;
>> +	dev->dev.of_node = node;
>> +	dev->dev.parent = parent;
>> +	of_device_make_bus_id(&dev->dev);
>> +
>> +	/* setup amba-specific device info */
>> +	dev->dma_mask = ~0;
>> +
>> +	/* Allow the arm,amba-deviceid value to override the h/w value */
>> +	prop = of_get_property(node, "arm,amba-deviceid", NULL);
>> +	if (prop)
>> +		dev->periphid = of_read_ulong(prop, 1);
>> +
>> +	/* Decode the IRQs and address ranges */
>> +	for (i = 0; i<  AMBA_NR_IRQS; i++)
>> +		dev->irq[i] = irq_of_parse_and_map(node, i);
>> +
>> +	ret = of_address_to_resource(node, 0,&dev->res);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	ret = amba_device_register(dev,&iomem_resource);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	return 0;
>> +
>> +err_free:
>> +	kfree(dev);
>> +	return ret;
>> +}
>> +
>
> Yeah, I think this looks like the right thing to do.  I'll pick it up
> into devicetree/test and probably move it to devicetree/arm in the
> near future after testing a bit.

Okay, I'll respin with your and Arnd's comments.

Rob

Re: [PATCH v4 4/4] dt: add amba device creation to platform bus scan

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:43PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> Add support to the platform bus scanning to call custom device creation
> function for amba devices.
> 
> Cc: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: arnd-r2nGTMty4D4@public.gmane.org
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/of/platform.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 8d0f477..1712f22 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/amba/bus.h>
>  
>  static int of_dev_node_match(struct device *dev, void *data)
>  {
> @@ -234,6 +235,11 @@ static int of_platform_bus_create(struct device_node *bus,
>  		return 0;
>  	}
>  
> +	if (of_device_is_compatible(bus, "arm,amba-device")) {
> +		of_amba_device_create(bus, parent);
> +		return 0;
> +	}
> +	

Yeah, this is the right thing to do.  I'll pick it up.

>  	dev = of_platform_device_create(bus, NULL, parent);
>  	if (!dev || !of_match_node(matches, bus))
>  		return 0;
> -- 
> 1.7.4.1
> 

[PATCH v4 4/4] dt: add amba device creation to platform bus scan

[Grant Likely]

On Fri, Jun 10, 2011 at 03:48:43PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add support to the platform bus scanning to call custom device creation
> function for amba devices.
> 
> Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: arnd at arndb.de
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  drivers/of/platform.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 8d0f477..1712f22 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/amba/bus.h>
>  
>  static int of_dev_node_match(struct device *dev, void *data)
>  {
> @@ -234,6 +235,11 @@ static int of_platform_bus_create(struct device_node *bus,
>  		return 0;
>  	}
>  
> +	if (of_device_is_compatible(bus, "arm,amba-device")) {
> +		of_amba_device_create(bus, parent);
> +		return 0;
> +	}
> +	

Yeah, this is the right thing to do.  I'll pick it up.

>  	dev = of_platform_device_create(bus, NULL, parent);
>  	if (!dev || !of_match_node(matches, bus))
>  		return 0;
> -- 
> 1.7.4.1
> 

Re: [PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Rob Herring]

On 06/10/2011 04:37 PM, Grant Likely wrote:
> On Fri, Jun 10, 2011 at 03:48:41PM -0500, Rob Herring wrote:
>> From: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>
>> of_platform_populate is missing an EXPORT_SYMBOL, so add it.
>>
>> Cc: Grant Likely<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Signed-off-by: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> ---
>>   drivers/of/platform.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index dc56a77..8d0f477 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
>>   	of_node_put(root);
>>   	return rc;
>>   }
>> +EXPORT_SYMBOL(of_platform_populate);
>
> I hadn't done this because I hadn't seen a need for calling it from
> module code.  Do you have a driver that uses it?
>

No, but then it doesn't appear that other functions in this file are 
used by modules either. Only of_platform_device_create is used by a 
module (drivers/macintosh) AFAICT.

Either way, it doesn't really matter to me.

Rob

[PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Rob Herring]

On 06/10/2011 04:37 PM, Grant Likely wrote:
> On Fri, Jun 10, 2011 at 03:48:41PM -0500, Rob Herring wrote:
>> From: Rob Herring<rob.herring@calxeda.com>
>>
>> of_platform_populate is missing an EXPORT_SYMBOL, so add it.
>>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Signed-off-by: Rob Herring<rob.herring@calxeda.com>
>> ---
>>   drivers/of/platform.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index dc56a77..8d0f477 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
>>   	of_node_put(root);
>>   	return rc;
>>   }
>> +EXPORT_SYMBOL(of_platform_populate);
>
> I hadn't done this because I hadn't seen a need for calling it from
> module code.  Do you have a driver that uses it?
>

No, but then it doesn't appear that other functions in this file are 
used by modules either. Only of_platform_device_create is used by a 
module (drivers/macintosh) AFAICT.

Either way, it doesn't really matter to me.

Rob

Re: [PATCH v4 3/4] drivers/amba: create devices from device tree

[Grant Likely]

On Fri, Jun 10, 2011 at 05:37:37PM -0500, Rob Herring wrote:
> On 06/10/2011 05:22 PM, Grant Likely wrote:
> >On Fri, Jun 10, 2011 at 03:48:42PM -0500, Rob Herring wrote:
> >>From: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >>
> >>Add a function to create amba bus devices (i.e. primecell peripherals) from
> >>device tree nodes. The device tree scanning is done by
> >>of_platform_probe/populate functions which can call of_amba_device_create
> >>based on a match table entry.
> >>
> >>Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
> >>value.
> >>
> >>Based on the original work by Jeremy Kerr.
> >>
> >>Cc: Jeremy Kerr<jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> >>Cc: Grant Likely<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> >>Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org
> >>Cc: arnd-r2nGTMty4D4@public.gmane.org
> >>Acked-by: Linus Walleij<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >>Signed-off-by: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >>---
> >>  Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
> >>  drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
> >>  include/linux/amba/bus.h                       |   18 ++++++++
> >>  3 files changed, 90 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/arm/amba.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
> >>new file mode 100644
> >>index 0000000..23fde7f
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/arm/amba.txt
> >>@@ -0,0 +1,21 @@
> >>+* ARM Primecell Peripherals
> >>+
> >>+ARM, Ltd. Primecell peripherals have a standard id register that can be used to
> >>+identify the peripheral type, vendor, and revision. This value can be used for
> >>+driver matching.
> >>+
> >>+Required properties:
> >>+
> >>+- compatible : should be a specific value for peripheral and "arm,amba-device"
> >
> >After recent conversations, I'd like to change this to
> >"arm,primecell" to better reflect the kind of devices it models.
> 
> That did cross my mind as I was writing the doc...
> 
> >
> >>+
> >>+Optional properties:
> >>+
> >>+- arm,amba-deviceid : Value to override the h/w value with
> >
> >Similarly, based on the primecell device documents, this should
> >probabaly be arm,primecell-periphid.
> >
> >>+
> >>+Example:
> >>+
> >>+serial@fff36000 {
> >>+	compatible = "arm,pl011", "arm,amba-device";
> >>+	arm,amba-deviceid =<0x00341011>;
> >>+};
> >>+
> >>diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> >>index d74926e..19f712b 100644
> >>--- a/drivers/amba/bus.c
> >>+++ b/drivers/amba/bus.c
> >>@@ -13,6 +13,11 @@
> >>  #include<linux/string.h>
> >>  #include<linux/slab.h>
> >>  #include<linux/io.h>
> >>+#include<linux/of.h>
> >>+#include<linux/of_irq.h>
> >>+#include<linux/of_address.h>
> >>+#include<linux/of_device.h>
> >>+#include<linux/of_platform.h>
> >>  #include<linux/pm.h>
> >>  #include<linux/pm_runtime.h>
> >>  #include<linux/amba/bus.h>
> >>@@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
> >>  EXPORT_SYMBOL(amba_find_device);
> >>  EXPORT_SYMBOL(amba_request_regions);
> >>  EXPORT_SYMBOL(amba_release_regions);
> >>+
> >>+#ifdef CONFIG_OF
> >>+int of_amba_device_create(struct device_node *node, struct device *parent)
> >>+{
> >>+	struct amba_device *dev;
> >>+	const void *prop;
> >>+	int i, ret;
> >>+
> >>+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >>+	if (!dev)
> >>+		return -ENOMEM;
> >>+
> >>+	/* setup generic device info */
> >>+	dev->dev.coherent_dma_mask = ~0;
> >>+	dev->dev.of_node = node;
> >>+	dev->dev.parent = parent;
> >>+	of_device_make_bus_id(&dev->dev);
> >>+
> >>+	/* setup amba-specific device info */
> >>+	dev->dma_mask = ~0;
> >>+
> >>+	/* Allow the arm,amba-deviceid value to override the h/w value */
> >>+	prop = of_get_property(node, "arm,amba-deviceid", NULL);
> >>+	if (prop)
> >>+		dev->periphid = of_read_ulong(prop, 1);
> >>+
> >>+	/* Decode the IRQs and address ranges */
> >>+	for (i = 0; i<  AMBA_NR_IRQS; i++)
> >>+		dev->irq[i] = irq_of_parse_and_map(node, i);
> >>+
> >>+	ret = of_address_to_resource(node, 0,&dev->res);
> >>+	if (ret)
> >>+		goto err_free;
> >>+
> >>+	ret = amba_device_register(dev,&iomem_resource);
> >>+	if (ret)
> >>+		goto err_free;
> >>+
> >>+	return 0;
> >>+
> >>+err_free:
> >>+	kfree(dev);
> >>+	return ret;
> >>+}
> >>+
> >
> >Yeah, I think this looks like the right thing to do.  I'll pick it up
> >into devicetree/test and probably move it to devicetree/arm in the
> >near future after testing a bit.
> 
> Okay, I'll respin with your and Arnd's comments.

Actually, hold off.  I'm working on it right now so I can get it into
my tree.  I'll take care of the respin.

g.

[PATCH v4 3/4] drivers/amba: create devices from device tree

[Grant Likely]

On Fri, Jun 10, 2011 at 05:37:37PM -0500, Rob Herring wrote:
> On 06/10/2011 05:22 PM, Grant Likely wrote:
> >On Fri, Jun 10, 2011 at 03:48:42PM -0500, Rob Herring wrote:
> >>From: Rob Herring<rob.herring@calxeda.com>
> >>
> >>Add a function to create amba bus devices (i.e. primecell peripherals) from
> >>device tree nodes. The device tree scanning is done by
> >>of_platform_probe/populate functions which can call of_amba_device_create
> >>based on a match table entry.
> >>
> >>Nodes with a "arm,amba-deviceid" property can override the h/w peripheral id
> >>value.
> >>
> >>Based on the original work by Jeremy Kerr.
> >>
> >>Cc: Jeremy Kerr<jeremy.kerr@canonical.com>
> >>Cc: Grant Likely<grant.likely@secretlab.ca>
> >>Cc: linux at arm.linux.org.uk
> >>Cc: arnd at arndb.de
> >>Acked-by: Linus Walleij<linus.walleij@linaro.org>
> >>Signed-off-by: Rob Herring<rob.herring@calxeda.com>
> >>---
> >>  Documentation/devicetree/bindings/arm/amba.txt |   21 ++++++++++
> >>  drivers/amba/bus.c                             |   51 ++++++++++++++++++++++++
> >>  include/linux/amba/bus.h                       |   18 ++++++++
> >>  3 files changed, 90 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/arm/amba.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/arm/amba.txt b/Documentation/devicetree/bindings/arm/amba.txt
> >>new file mode 100644
> >>index 0000000..23fde7f
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/arm/amba.txt
> >>@@ -0,0 +1,21 @@
> >>+* ARM Primecell Peripherals
> >>+
> >>+ARM, Ltd. Primecell peripherals have a standard id register that can be used to
> >>+identify the peripheral type, vendor, and revision. This value can be used for
> >>+driver matching.
> >>+
> >>+Required properties:
> >>+
> >>+- compatible : should be a specific value for peripheral and "arm,amba-device"
> >
> >After recent conversations, I'd like to change this to
> >"arm,primecell" to better reflect the kind of devices it models.
> 
> That did cross my mind as I was writing the doc...
> 
> >
> >>+
> >>+Optional properties:
> >>+
> >>+- arm,amba-deviceid : Value to override the h/w value with
> >
> >Similarly, based on the primecell device documents, this should
> >probabaly be arm,primecell-periphid.
> >
> >>+
> >>+Example:
> >>+
> >>+serial at fff36000 {
> >>+	compatible = "arm,pl011", "arm,amba-device";
> >>+	arm,amba-deviceid =<0x00341011>;
> >>+};
> >>+
> >>diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> >>index d74926e..19f712b 100644
> >>--- a/drivers/amba/bus.c
> >>+++ b/drivers/amba/bus.c
> >>@@ -13,6 +13,11 @@
> >>  #include<linux/string.h>
> >>  #include<linux/slab.h>
> >>  #include<linux/io.h>
> >>+#include<linux/of.h>
> >>+#include<linux/of_irq.h>
> >>+#include<linux/of_address.h>
> >>+#include<linux/of_device.h>
> >>+#include<linux/of_platform.h>
> >>  #include<linux/pm.h>
> >>  #include<linux/pm_runtime.h>
> >>  #include<linux/amba/bus.h>
> >>@@ -785,3 +790,49 @@ EXPORT_SYMBOL(amba_device_unregister);
> >>  EXPORT_SYMBOL(amba_find_device);
> >>  EXPORT_SYMBOL(amba_request_regions);
> >>  EXPORT_SYMBOL(amba_release_regions);
> >>+
> >>+#ifdef CONFIG_OF
> >>+int of_amba_device_create(struct device_node *node, struct device *parent)
> >>+{
> >>+	struct amba_device *dev;
> >>+	const void *prop;
> >>+	int i, ret;
> >>+
> >>+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >>+	if (!dev)
> >>+		return -ENOMEM;
> >>+
> >>+	/* setup generic device info */
> >>+	dev->dev.coherent_dma_mask = ~0;
> >>+	dev->dev.of_node = node;
> >>+	dev->dev.parent = parent;
> >>+	of_device_make_bus_id(&dev->dev);
> >>+
> >>+	/* setup amba-specific device info */
> >>+	dev->dma_mask = ~0;
> >>+
> >>+	/* Allow the arm,amba-deviceid value to override the h/w value */
> >>+	prop = of_get_property(node, "arm,amba-deviceid", NULL);
> >>+	if (prop)
> >>+		dev->periphid = of_read_ulong(prop, 1);
> >>+
> >>+	/* Decode the IRQs and address ranges */
> >>+	for (i = 0; i<  AMBA_NR_IRQS; i++)
> >>+		dev->irq[i] = irq_of_parse_and_map(node, i);
> >>+
> >>+	ret = of_address_to_resource(node, 0,&dev->res);
> >>+	if (ret)
> >>+		goto err_free;
> >>+
> >>+	ret = amba_device_register(dev,&iomem_resource);
> >>+	if (ret)
> >>+		goto err_free;
> >>+
> >>+	return 0;
> >>+
> >>+err_free:
> >>+	kfree(dev);
> >>+	return ret;
> >>+}
> >>+
> >
> >Yeah, I think this looks like the right thing to do.  I'll pick it up
> >into devicetree/test and probably move it to devicetree/arm in the
> >near future after testing a bit.
> 
> Okay, I'll respin with your and Arnd's comments.

Actually, hold off.  I'm working on it right now so I can get it into
my tree.  I'll take care of the respin.

g.

Re: [PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 05:44:47PM -0500, Rob Herring wrote:
> On 06/10/2011 04:37 PM, Grant Likely wrote:
> >On Fri, Jun 10, 2011 at 03:48:41PM -0500, Rob Herring wrote:
> >>From: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >>
> >>of_platform_populate is missing an EXPORT_SYMBOL, so add it.
> >>
> >>Cc: Grant Likely<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> >>Signed-off-by: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >>---
> >>  drivers/of/platform.c |    1 +
> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> >>index dc56a77..8d0f477 100644
> >>--- a/drivers/of/platform.c
> >>+++ b/drivers/of/platform.c
> >>@@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
> >>  	of_node_put(root);
> >>  	return rc;
> >>  }
> >>+EXPORT_SYMBOL(of_platform_populate);
> >
> >I hadn't done this because I hadn't seen a need for calling it from
> >module code.  Do you have a driver that uses it?
> >
> 
> No, but then it doesn't appear that other functions in this file are
> used by modules either. Only of_platform_device_create is used by a
> module (drivers/macintosh) AFAICT.
> 
> Either way, it doesn't really matter to me.

Okay, let's leave this out for the time being.  It is a trivial patch
to add if someone needs it.

g.

[PATCH v4 2/4] dt: add EXPORT_SYMBOL to of_platform_populate

[Grant Likely]

On Fri, Jun 10, 2011 at 05:44:47PM -0500, Rob Herring wrote:
> On 06/10/2011 04:37 PM, Grant Likely wrote:
> >On Fri, Jun 10, 2011 at 03:48:41PM -0500, Rob Herring wrote:
> >>From: Rob Herring<rob.herring@calxeda.com>
> >>
> >>of_platform_populate is missing an EXPORT_SYMBOL, so add it.
> >>
> >>Cc: Grant Likely<grant.likely@secretlab.ca>
> >>Signed-off-by: Rob Herring<rob.herring@calxeda.com>
> >>---
> >>  drivers/of/platform.c |    1 +
> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> >>index dc56a77..8d0f477 100644
> >>--- a/drivers/of/platform.c
> >>+++ b/drivers/of/platform.c
> >>@@ -328,4 +328,5 @@ int of_platform_populate(struct device_node *root,
> >>  	of_node_put(root);
> >>  	return rc;
> >>  }
> >>+EXPORT_SYMBOL(of_platform_populate);
> >
> >I hadn't done this because I hadn't seen a need for calling it from
> >module code.  Do you have a driver that uses it?
> >
> 
> No, but then it doesn't appear that other functions in this file are
> used by modules either. Only of_platform_device_create is used by a
> module (drivers/macintosh) AFAICT.
> 
> Either way, it doesn't really matter to me.

Okay, let's leave this out for the time being.  It is a trivial patch
to add if someone needs it.

g.