[PATCH 0/3] DT bindings Cortex A9 peripherals

[PATCH 0/3] DT bindings Cortex A9 peripherals

[Rob Herring]

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

This series adds DT based initialization and bindings for the pmu, gic and l2x0
found on Cortex-A9's.

Rob

Rob Herring (3):
  ARM: pmu: add OF probing support
  ARM: gic: add OF based initialization
  ARM: l2x0: Add OF based initialization

 arch/arm/common/gic.c                      |   35 +++++++++++++++++++++++++++
 arch/arm/include/asm/hardware/cache-l2x0.h |    1 +
 arch/arm/include/asm/hardware/gic.h        |    1 +
 arch/arm/kernel/pmu.c                      |   23 ++++++++++++++----
 arch/arm/mm/cache-l2x0.c                   |   36 ++++++++++++++++++++++++++++
 5 files changed, 91 insertions(+), 5 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] ARM: pmu: add OF probing support

[Rob Herring]

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

Add OF match table to enable OF style driver binding. The dts entry is like
this:

pmu {
	compatible = "arm,cortex-a9-pmu";
	interrupts = <100 101>;
};

The use of pdev->id as an index breaks with OF device binding, so set the type
based on the OF compatible string.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/kernel/pmu.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index 2c79eec..41bc972 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c
@@ -27,27 +27,40 @@ static struct platform_device *pmu_devices[ARM_NUM_PMU_DEVICES];
 
 static int __devinit pmu_device_probe(struct platform_device *pdev)
 {
+	enum arm_pmu_type type = pdev->id;
 
-	if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
+	if (pdev->dev.of_node)
+		type = ARM_PMU_DEVICE_CPU;
+
+	if (type < 0 || type >= ARM_NUM_PMU_DEVICES) {
 		pr_warning("received registration request for unknown "
 				"device %d\n", pdev->id);
 		return -EINVAL;
 	}
 
-	if (pmu_devices[pdev->id])
+	if (pmu_devices[type])
 		pr_warning("registering new PMU device type %d overwrites "
-				"previous registration!\n", pdev->id);
+				"previous registration!\n", type);
 	else
 		pr_info("registered new PMU device of type %d\n",
-				pdev->id);
+				type);
 
-	pmu_devices[pdev->id] = pdev;
+	pmu_devices[type] = pdev;
 	return 0;
 }
 
+static struct of_device_id pmu_device_ids[] = {
+	{ .compatible = "arm,cortex-a9-pmu" },
+	{ .compatible = "arm,cortex-a8-pmu" },
+	{ .compatible = "arm,arm1136-pmu" },
+	{ .compatible = "arm,arm1176-pmu" },
+	{},
+};
+
 static struct platform_driver pmu_driver = {
 	.driver		= {
 		.name	= "arm-pmu",
+		.of_match_table = pmu_device_ids,
 	},
 	.probe		= pmu_device_probe,
 };
-- 
1.7.4.1

[PATCH 2/3] ARM: gic: add OF based initialization

[Rob Herring]

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

This adds gic initialization using device tree data. An example device tree
binding looks like this:

intc: interrupt-controller at fff11000 {
        compatible = "arm,cortex-a9-gic";
        #interrupt-cells = <1>;
        interrupt-controller;
        reg = <0xfff11000 0x1000>,
              <0xfff10100 0x100>;
        irq-start = <29>;
};

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/common/gic.c               |   35 +++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/hardware/gic.h |    1 +
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4ddd0a6..d44ca5b 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,6 +28,8 @@
 #include <linux/smp.h>
 #include <linux/cpumask.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
@@ -401,3 +403,36 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	writel_relaxed(map << 16 | irq, gic_data[0].dist_base + GIC_DIST_SOFTINT);
 }
 #endif
+
+#ifdef CONFIG_OF
+static struct of_device_id gic_ids[] __initdata = {
+	{ .compatible = "arm,cortex-a9-gic" },
+};
+
+void __init gic_of_init(void)
+{
+	struct device_node *np;
+	void __iomem *cpu_base;
+	void __iomem *dist_base;
+	__u32 irq_start = 16;
+	const __be32 *val;
+
+	np = of_find_matching_node(NULL, gic_ids);
+	if (!np)
+		panic("unable to find compatible gic node in dtb\n");
+
+	dist_base = of_iomap(np, 0);
+	if (!dist_base)
+		panic("unable to map gic dist registers\n");
+
+	cpu_base = of_iomap(np, 1);
+	if (!cpu_base)
+		panic("unable to map gic cpu registers\n");
+
+	if ((val = of_get_property(np, "irq-start", NULL)) != NULL)
+		irq_start = of_read_ulong(val, 1);
+	of_node_put(np);
+
+	gic_init(0, irq_start, dist_base, cpu_base);
+}
+#endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 0691f9d..954a08e 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -37,6 +37,7 @@ extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
 void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
+void gic_of_init(void);
 void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-- 
1.7.4.1

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Rob Herring]

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

This adds probing for pl310 cache controller via device tree. An example
binding looks like this:

L2: l2-cache {
	compatible = "arm,pl310-cache", "cache";
	reg = <0xfff12000 0x1000>;
	aux-value = <0>;
	aux-mask = <0xffffffff>;
	cache-unified;
	cache-level = <2>;
};

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/include/asm/hardware/cache-l2x0.h |    1 +
 arch/arm/mm/cache-l2x0.c                   |   36 ++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 16bd480..1d36632 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -74,6 +74,7 @@
 
 #ifndef __ASSEMBLY__
 extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
+extern int l2x0_of_init(void);
 #endif
 
 #endif
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index ef59099..6dd521c 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -16,9 +16,12 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
+#include <linux/err.h>
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -344,3 +347,36 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 	printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
 			ways, cache_id, aux, l2x0_size);
 }
+
+#ifdef CONFIG_OF
+static struct of_device_id l2x0_ids[] __initdata = {
+	{ .compatible = "arm,pl310-cache" },
+	{ .compatible = "arm,l220-cache" },
+	{ .compatible = "arm,l210-cache" },
+};
+
+int __init l2x0_of_init(void)
+{
+	struct device_node *np;
+	void __iomem *l2_base;
+	__u32 aux_val = 0;
+	__u32 aux_mask = ~0UL;
+	const __be32 *val;
+
+	np = of_find_matching_node(NULL, l2x0_ids);
+	if (!np)
+		return -ENODEV;
+	l2_base = of_iomap(np, 0);
+	if (!l2_base)
+		return -ENOMEM;
+
+	if ((val = of_get_property(np, "aux-value", NULL)) != NULL);
+		aux_val = of_read_ulong(val, 1);
+
+	if ((val = of_get_property(np, "aux-mask", NULL)) != NULL);
+		aux_mask = of_read_ulong(val, 1);
+
+	l2x0_init(l2_base, aux_val, aux_mask);
+	return 0;
+}
+#endif
-- 
1.7.4.1

[PATCH 1/3] ARM: pmu: add OF probing support

[Olof Johansson]

On Wed, Jun 1, 2011 at 9:37 AM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Add OF match table to enable OF style driver binding. The dts entry is like
> this:
>
> pmu {
> ? ? ? ?compatible = "arm,cortex-a9-pmu";
> ? ? ? ?interrupts = <100 101>;
> };
>
> The use of pdev->id as an index breaks with OF device binding, so set the type
> based on the OF compatible string.

Even though they're simple, new bindings like these need to be
documented. Current location for that is
Documentation/devicetree/bindings/.

Can you start an ARM directory there and add corresponding docs for the this?


Thanks!

-Olof

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Olof Johansson]

On Wed, Jun 1, 2011 at 9:37 AM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> This adds probing for pl310 cache controller via device tree. An example
> binding looks like this:
>
> L2: l2-cache {
> ? ? ? ?compatible = "arm,pl310-cache", "cache";

"cache" is too generic to be useful. "arm,pl2x0-cache" would be a more
meaningful fallback, I think?

(Same comment as for 1/3: The bindings need to be documented. Same
applies to 2/3, obviousy).

-Olof

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Rob Herring]

On 06/01/2011 01:40 PM, Olof Johansson wrote:
> On Wed, Jun 1, 2011 at 9:37 AM, Rob Herring<robherring2@gmail.com>  wrote:
>> From: Rob Herring<rob.herring@calxeda.com>
>>
>> This adds probing for pl310 cache controller via device tree. An example
>> binding looks like this:
>>
>> L2: l2-cache {
>>         compatible = "arm,pl310-cache", "cache";
>
> "cache" is too generic to be useful. "arm,pl2x0-cache" would be a more
> meaningful fallback, I think?

I agree. This is what ePAPR says should be present for caches along with 
a more specific string.

Based on the prior discussion on pmu naming, it should be the specific 
model. There is no such thing as a pl2x0. The models of L2 controllers 
the cache-l2x0.c code supports are:

PL310
L220
L210

And my compatible strings reflect that.

>
> (Same comment as for 1/3: The bindings need to be documented. Same
> applies to 2/3, obviousy).
>

Will do.

Rob

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Rob Herring]

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

This adds probing for pl310 cache controller via device tree. An example
binding looks like this:

L2: l2-cache {
	compatible = "arm,pl310-cache", "cache";
	reg = <0xfff12000 0x1000>;
	aux-value = <0>;
	aux-mask = <0xffffffff>;
	cache-unified;
	cache-level = <2>;
};

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |   35 ++++++++++++++++++++++
 arch/arm/include/asm/hardware/cache-l2x0.h     |    1 +
 arch/arm/mm/cache-l2x0.c                       |   38 ++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/l2cc.txt

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
new file mode 100644
index 0000000..17082be
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -0,0 +1,35 @@
+* ARM L2 Cache Controller
+
+ARM cores often have a separate level 2 cache controller. There are various
+implementations of the L2 cache controller with compatible programming models.
+The ARM L2 cache representation in the device tree should be done as under:-
+
+Required properties:
+
+- compatible : should be one of
+	"arm,pl310-cache"
+	"arm,l220-cache"
+	"arm,l210-cache"
+- cache-unified : Specifies the cache is a unified cache.
+- cache-level : Should be set to 2 for a level 2 cache.
+- reg : Physical base address and size of cache controller's memory mapped
+  registers.
+
+Optional properties:
+
+- aux-value : Value to set the Auxillary Control register to. Setting masked
+  bits is undefined. Default value is 0.
+- aux-mask : Mask of bits to preserve in the Auxillary Control register.
+  Default value is 0xffffffff.
+
+Example:
+
+L2: l2-cache {
+        compatible = "arm,pl310-cache", "cache";
+        reg = <0xfff12000 0x1000>;
+        aux-value = <0>;
+        aux-mask = <0xffffffff>;
+        cache-unified;
+        cache-level = <2>;
+};
+
diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 16bd480..1d36632 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -74,6 +74,7 @@
 
 #ifndef __ASSEMBLY__
 extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
+extern int l2x0_of_init(void);
 #endif
 
 #endif
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index ef59099..910f530 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -16,9 +16,12 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
+#include <linux/err.h>
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -344,3 +347,38 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 	printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",
 			ways, cache_id, aux, l2x0_size);
 }
+
+#ifdef CONFIG_OF
+static struct of_device_id l2x0_ids[] __initdata = {
+	{ .compatible = "arm,pl310-cache" },
+	{ .compatible = "arm,l220-cache" },
+	{ .compatible = "arm,l210-cache" },
+};
+
+int __init l2x0_of_init(void)
+{
+	struct device_node *np;
+	void __iomem *l2_base;
+	__u32 aux_val = 0;
+	__u32 aux_mask = ~0UL;
+	const __be32 *val;
+
+	np = of_find_matching_node(NULL, l2x0_ids);
+	if (!np)
+		return -ENODEV;
+	l2_base = of_iomap(np, 0);
+	if (!l2_base)
+		return -ENOMEM;
+
+	val = of_get_property(np, "aux-value", NULL);
+	if (val != NULL)
+		aux_val = of_read_ulong(val, 1);
+
+	val = of_get_property(np, "aux-mask", NULL);
+	if (val != NULL)
+		aux_mask = of_read_ulong(val, 1);
+
+	l2x0_init(l2_base, aux_val, aux_mask);
+	return 0;
+}
+#endif
-- 
1.7.4.1

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Olof Johansson]

On Tue, Jun 7, 2011 at 7:22 AM, Rob Herring <robherring2@gmail.com> wrote:

> +- aux-value : Value to set the Auxillary Control register to. Setting masked
> + ?bits is undefined. Default value is 0.
> +- aux-mask : Mask of bits to preserve in the Auxillary Control register.
> + ?Default value is 0xffffffff.

The device tree should describe the hardware, not the way the linux
kernel drives the hardware. In the case of the AUX register, it's
mostly a collection of options that are either turned on or off. I
don't think they should necessarily be described as an opaque 32-bit
word, but instead as separate attributes.

At least the geometry should be specified that way.

For feature enable bits, it depends on what features should be enabled
and from the kernel side. I'm not sure that belongs in the device tree
at all.


-Olof

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Rob Herring]

Olof,

On 06/07/2011 11:20 AM, Olof Johansson wrote:
> On Tue, Jun 7, 2011 at 7:22 AM, Rob Herring<robherring2@gmail.com>  wrote:
>
>> +- aux-value : Value to set the Auxillary Control register to. Setting masked
>> +  bits is undefined. Default value is 0.
>> +- aux-mask : Mask of bits to preserve in the Auxillary Control register.
>> +  Default value is 0xffffffff.
>
> The device tree should describe the hardware, not the way the linux
> kernel drives the hardware. In the case of the AUX register, it's
> mostly a collection of options that are either turned on or off. I
> don't think they should necessarily be described as an opaque 32-bit
> word, but instead as separate attributes.

Unfortunately, the meaning of the bits depends on the model and there 
doesn't seem to be any commonality or subset of options as to what each 
platform is setting up.

>
> At least the geometry should be specified that way.

That's probably the only part that doesn't get changed. :)

>
> For feature enable bits, it depends on what features should be enabled
> and from the kernel side. I'm not sure that belongs in the device tree
> at all.

I don't think it can be decided what the kernel needs to setup by the 
device tree. The main reason it's needed in the kernel at all is 
probably because the bootloader didn't setup aux_ctrl or set it up 
incorrectly.

Perhaps the magic values should just be left in the platforms and 
continue to be passed into the OF init function:

l2x0_of_init(__u32 aux_val, __u32 aux_mask);

Rob

[PATCH 3/3] ARM: l2x0: Add OF based initialization

[Olof Johansson]

Hi,

On Tue, Jun 7, 2011 at 9:54 AM, Rob Herring <robherring2@gmail.com> wrote:
> Olof,
>
> On 06/07/2011 11:20 AM, Olof Johansson wrote:
>>
>> On Tue, Jun 7, 2011 at 7:22 AM, Rob Herring<robherring2@gmail.com> ?wrote:
>>
>>> +- aux-value : Value to set the Auxillary Control register to. Setting
>>> masked
>>> + ?bits is undefined. Default value is 0.
>>> +- aux-mask : Mask of bits to preserve in the Auxillary Control register.
>>> + ?Default value is 0xffffffff.
>>
>> The device tree should describe the hardware, not the way the linux
>> kernel drives the hardware. In the case of the AUX register, it's
>> mostly a collection of options that are either turned on or off. I
>> don't think they should necessarily be described as an opaque 32-bit
>> word, but instead as separate attributes.
>
> Unfortunately, the meaning of the bits depends on the model and there
> doesn't seem to be any commonality or subset of options as to what each
> platform is setting up.

Luckily, the meaning of the bits can be derived from the most-specific
compatible string, but yeah, it seems like many SoC vendors choose to
enable different settings. Likely because of either what has been
verified to work well, or what worked well for some benchmarks that
they cared about enough to optimize the default case for.

>>
>> At least the geometry should be specified that way.
>
> That's probably the only part that doesn't get changed. :)

Yep, it should ideally still be described though.


>> For feature enable bits, it depends on what features should be enabled
>> and from the kernel side. I'm not sure that belongs in the device tree
>> at all.
>
> I don't think it can be decided what the kernel needs to setup by the device
> tree. The main reason it's needed in the kernel at all is probably because
> the bootloader didn't setup aux_ctrl or set it up incorrectly.
>
> Perhaps the magic values should just be left in the platforms and continue
> to be passed into the OF init function:
>
> l2x0_of_init(__u32 aux_val, __u32 aux_mask);

This might be the least painful way to do it, agreed.

If a specific platform wants to have firmware pass in the preferred
values, they could do so through /chosen or similar.


-Olof