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

Mark,

On 06/08/2011 10:54 AM, Mark Rutland wrote:

Hi,

quoted

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

I don't think this is the best way to handle the type when we've got an FDT
description:

* release_pmu hasn't been updated to match the type logic here, so it might do
   anything when handed a platform_device initialised by FDT code.

* the warning message for an invalid registration still uses pdev->id rather
   than type. This can't currently be reached when the PMU was handed to us via
   FDT, but it may confuse refactoring later on.

* If we want to add a new PMU type, we'll have to add more logic to
   pmu_device_probe. Given that work is going on to add support for system PMUs,
   this doesn't seem particularly brilliant.

quoted

+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,
  };

This all seems fine for handling CPU PMUs.

I think that a better strategy would be to separate the type logic from the
registration. I have a patch for this:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052455.html

With it, you won't need to change pmu_device_probe, and adding FDT support
should just be a matter of adding the of_match_table.

Okay. I'll rebase mine on top of your changes.

Rob

Hi Rob,

I've Addded Jamie on Cc here as he was interested in the possibility of
adding platform_device_id tables. Hopefully it'll be easy to discuss
{of,platform}_id_tables in one thread.

As Jamie pointed out to me the existence of platform_device_id tables,
I took a look around and noticed that of_device_id tables also seem to
provide support for driver-specific parameters (I saw an example of
usage in arch/sparc/kernel/pci_schizo.c).

I've had a go at getting {of,platform}_device_id tables to provide the
PMU type, so they can be used similarly (the macros make entries look
identical apart from the {plat,of} prefix).

I don't have any entries currently for the platform_device_id table,
but it'd be useful for system pmus, if we were to add an L2 Cache
controller PMU driver, it might have a binding like:

PLAT_MATCH("arm,pl310-pmu", ARM_PMU_TYPE_L2CC),

How does the following series look to you? The first 2 patches are the
ones I mentioned before, unchanged (apart from additional acks).

Mark.


Mark Rutland (4):
  ARM: pmu: refactor reservation
  ARM: pmu: reject duplicate PMU registrations
  ARM: pmu: add OF probing support
  ARM: pmu: add platform_device_id table support

 Documentation/devicetree/bindings/arm/pmu.txt |   22 ++++++
 arch/arm/include/asm/pmu.h                    |    2 +-
 arch/arm/kernel/perf_event.c                  |    4 +-
 arch/arm/kernel/pmu.c                         |   91 ++++++++++++++++++++-----
 4 files changed, 99 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/pmu.txt

Currently, PMU platform_device reservation relies on some minor abuse
of the platform_device::id field for determining the type of PMU. This
is problematic for device tree based probing, where the ID cannot be
controlled.

This patch removes reliance on the id field, and depends on each PMU's
platform driver to figure out which type it is. As all PMUs handled by
the current platform_driver name "arm-pmu" are CPU PMUs, this
convention is hardcoded. New PMU types can be supported through the use
of {of,platform}_device_id tables

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Jamie Iles <redacted>
Acked-by: Will Deacon <redacted>
Cc: Rob Herring <redacted>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 arch/arm/include/asm/pmu.h   |    2 +-
 arch/arm/kernel/perf_event.c |    4 ++--
 arch/arm/kernel/pmu.c        |   33 +++++++++++++++++++--------------
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index 7544ce6..67c70a3 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h

@@ -52,7 +52,7 @@ reserve_pmu(enum arm_pmu_type device);
  * a cookie.
  */
 extern int
-release_pmu(struct platform_device *pdev);
+release_pmu(enum arm_pmu_type type);
 
 /**
  * init_pmu() - Initialise the PMU.

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index d53c0ab..a6c643f 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c

@@ -435,7 +435,7 @@ armpmu_reserve_hardware(void)
 			if (irq >= 0)
 				free_irq(irq, NULL);
 		}
-		release_pmu(pmu_device);
+		release_pmu(ARM_PMU_DEVICE_CPU);
 		pmu_device = NULL;
 	}
 

@@ -454,7 +454,7 @@ armpmu_release_hardware(void)
 	}
 	armpmu->stop();
 
-	release_pmu(pmu_device);
+	release_pmu(ARM_PMU_DEVICE_CPU);
 	pmu_device = NULL;
 }
 

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index 2c79eec..87942b9 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c

@@ -25,36 +25,41 @@ static volatile long pmu_lock;
 
 static struct platform_device *pmu_devices[ARM_NUM_PMU_DEVICES];
 
-static int __devinit pmu_device_probe(struct platform_device *pdev)
+static int __devinit pmu_register(struct platform_device *pdev,
+					enum arm_pmu_type type)
 {
-
-	if (pdev->id < 0 || pdev->id >= ARM_NUM_PMU_DEVICES) {
+	if (type < 0 || type >= ARM_NUM_PMU_DEVICES) {
 		pr_warning("received registration request for unknown "
-				"device %d\n", pdev->id);
+				"device %d\n", type);
 		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 platform_driver pmu_driver = {
+static int __devinit armpmu_device_probe(struct platform_device *pdev)
+{
+	return pmu_register(pdev, ARM_PMU_DEVICE_CPU);
+}
+
+static struct platform_driver armpmu_driver = {
 	.driver		= {
 		.name	= "arm-pmu",
 	},
-	.probe		= pmu_device_probe,
+	.probe		= armpmu_device_probe,
 };
 
 static int __init register_pmu_driver(void)
 {
-	return platform_driver_register(&pmu_driver);
+	return platform_driver_register(&armpmu_driver);
 }
 device_initcall(register_pmu_driver);
 

@@ -77,11 +82,11 @@ reserve_pmu(enum arm_pmu_type device)
 EXPORT_SYMBOL_GPL(reserve_pmu);
 
 int
-release_pmu(struct platform_device *pdev)
+release_pmu(enum arm_pmu_type device)
 {
-	if (WARN_ON(pdev != pmu_devices[pdev->id]))
+	if (WARN_ON(!pmu_devices[device]))
 		return -EINVAL;
-	clear_bit_unlock(pdev->id, &pmu_lock);
+	clear_bit_unlock(device, &pmu_lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(release_pmu);

-- 
1.7.0.4

Currently, the PMU reservation framework allows for multiple PMUs of
the same type to register themselves. This can lead to a bug with the
sequence:

register_pmu(pmu1);
reserve_pmu(pmu_type);
register_pmu(pmu2);
release_pmu(pmu1);

Here, pmu1 cannot be released, and pmu2 cannot be reserved.

This patch modifies register_pmu to reject registrations where a PMU is
already present, preventing this problem. PMUs which can have multiple
instances should not use the PMU reservation framework.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-By: Jamie Iles <redacted>
Acked-By: Will Deacon <redacted>
---
 arch/arm/kernel/pmu.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index 87942b9..de6b1b0 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c

@@ -34,13 +34,13 @@ static int __devinit pmu_register(struct platform_device *pdev,
 		return -EINVAL;
 	}
 
-	if (pmu_devices[type])
-		pr_warning("registering new PMU device type %d overwrites "
-				"previous registration!\n", type);
-	else
-		pr_info("registered new PMU device of type %d\n",
-				type);
+	if (pmu_devices[type]) {
+		pr_warning("rejecting duplicate registration of PMU device "
+			"type %d.", type);
+		return -ENOSPC;
+	}
 
+	pr_info("registered new PMU device of type %d\n", type);
 	pmu_devices[type] = pdev;
 	return 0;
 }

-- 
1.7.0.4

This is based on an earlier patch from Rob Herring [off-list ref]

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.

This modification sets the PMU hardware type based on data embedded in the
binding, allowing easy addition of new PMU types in future.

Support for new PMU types not provided by devicetree can be added later using
platform_device_id tables in a similar fashion.

Cc: Jamie Iles <redacted>
Cc: Rob Herring <redacted>
Cc: Will Deacon <redacted>
---
 
 Rob: would you be happy to merge this into your tree? or would you
 rather I sent it through Russell?

 Documentation/devicetree/bindings/arm/pmu.txt |   22 +++++++++++++++
 arch/arm/kernel/pmu.c                         |   35 ++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/pmu.txt

diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt
new file mode 100644
index 0000000..739d00c
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/pmu.txt

@@ -0,0 +1,22 @@
+* ARM Performance Monitor Units
+
+ARM cores often have a PMU for counting cpu and cache events like cache misses
+and hits. The interface to the PMU is part of the ARM ARM. The ARM PMU
+representation in the device tree should be done as under:-
+
+Required properties:
+
+- compatible : should be one of
+	"arm,cortex-a9-pmu"
+	"arm,cortex-a8-pmu"
+	"arm,arm1176-pmu"
+	"arm,arm1136-pmu"
+- interrupts : 1 combined interrupt or 1 per core.
+
+Example:
+
+pmu {
+        compatible = "arm,cortex-a9-pmu";
+        interrupts = <100 101>;
+};
+

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index de6b1b0..d34cf88 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c

@@ -17,6 +17,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 
 #include <asm/pmu.h>

@@ -45,14 +46,46 @@ static int __devinit pmu_register(struct platform_device *pdev,
 	return 0;
 }
 
+#define OF_MATCH_PMU(name, type) {	\
+	.compatible = name,		\
+	.data = (void *) type,	\
+}
+
+#define OF_MATCH_CPU(name)	OF_MATCH_PMU(name, ARM_PMU_DEVICE_CPU)
+
+static struct of_device_id armpmu_of_device_ids[] = {
+	/* None for now. */
+	OF_MATCH_CPU("arm,cortex-a9-pmu"),
+	OF_MATCH_CPU("arm,cortex-a8-pmu"),
+	OF_MATCH_CPU("arm,arm1136-pmu"),
+	OF_MATCH_CPU("arm,arm1176-pmu"),
+	{},
+};
+
+enum arm_pmu_type armpmu_device_type(struct platform_device *pdev)
+{
+	const struct of_device_id	*of_id;
+
+	/* provided by of_device_id table */
+	if (pdev->dev.of_node) {
+		of_id = of_match_device(armpmu_of_device_ids, &pdev->dev);
+		BUG_ON(!of_id);
+		return (enum arm_pmu_type) of_id->data;
+	}
+
+	/* Provided by a 'legacy' platform_device */
+	return ARM_PMU_DEVICE_CPU;
+}
+
 static int __devinit armpmu_device_probe(struct platform_device *pdev)
 {
-	return pmu_register(pdev, ARM_PMU_DEVICE_CPU);
+	return pmu_register(pdev, armpmu_device_type(pdev));
 }
 
 static struct platform_driver armpmu_driver = {
 	.driver		= {
 		.name	= "arm-pmu",
+		.of_match_table = armpmu_of_device_ids,
 	},
 	.probe		= armpmu_device_probe,
 };

-- 
1.7.0.4

This patch adds support for platform_device_id tables, allowing new
PMU types to be registered with the correct type, without requiring
new platform_driver shims to provide the type.

Macros matching functionality of the of_device_id table macros are
provided for convenience.

Cc: Jamie Iles <redacted>
Cc: Will Deacon <redacted>
---
 arch/arm/kernel/pmu.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index d34cf88..23c3af2 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c

@@ -62,9 +62,22 @@ static struct of_device_id armpmu_of_device_ids[] = {
 	{},
 };
 
+#define PLAT_MATCH_PMU(name, type) {	\
+	.name = name,		\
+	.driver_data = (void *) type,	\
+}
+
+#define PLAT_MATCH_CPU(name)	PLAT_MATCH_PMU(name, ARM_PMU_DEVICE_CPU)
+
+static struct platform_device_id armpmu_plat_device_ids[] = {
+	/* None for now */
+	{},
+};
+
 enum arm_pmu_type armpmu_device_type(struct platform_device *pdev)
 {
 	const struct of_device_id	*of_id;
+	const struct platform_device_id *pdev_id;
 
 	/* provided by of_device_id table */
 	if (pdev->dev.of_node) {

@@ -73,6 +86,11 @@ enum arm_pmu_type armpmu_device_type(struct platform_device *pdev)
 		return (enum arm_pmu_type) of_id->data;
 	}
 
+	/* Provided by platform_device_id table */
+	if ((pdev_id = platform_get_device_id(pdev))) {
+		return (enum arm_pmu_type) pdev_id->driver_data;
+	}
+
 	/* Provided by a 'legacy' platform_device */
 	return ARM_PMU_DEVICE_CPU;
 }

@@ -88,6 +106,7 @@ static struct platform_driver armpmu_driver = {
 		.of_match_table = armpmu_of_device_ids,
 	},
 	.probe		= armpmu_device_probe,
+	.id_table	= armpmu_plat_device_ids,
 };
 
 static int __init register_pmu_driver(void)

-- 
1.7.0.4

Hello.

On 13-06-2011 13:35, Mark Rutland wrote:

This patch adds support for platform_device_id tables, allowing new
PMU types to be registered with the correct type, without requiring
new platform_driver shims to provide the type.

Macros matching functionality of the of_device_id table macros are
provided for convenience.

Cc: Jamie Iles<redacted>
Cc: Will Deacon<redacted>
---
  arch/arm/kernel/pmu.c |   19 +++++++++++++++++++
  1 files changed, 19 insertions(+), 0 deletions(-)

[...]

    scripts/checkpatch.pl should warn about using = in the *if* statement...

+		return (enum arm_pmu_type) pdev_id->driver_data;
+	}

    scripts/checkpatch.pl should warn about unneeded {} here.
    Did you run your patch thru it?

WBR, Sergei

Hi,

-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov at mvista.com]
Sent: 13 June 2011 13:33
To: Mark Rutland
Cc: Rob Herring; Jamie Iles; Will Deacon; linux-arm-
kernel at lists.infradead.org
Subject: Re: [PATCH 4/4] ARM: pmu: add platform_device_id table support

Hello.

On 13-06-2011 13:35, Mark Rutland wrote:

quoted

This patch adds support for platform_device_id tables, allowing new
PMU types to be registered with the correct type, without requiring
new platform_driver shims to provide the type.

quoted

Macros matching functionality of the of_device_id table macros are
provided for convenience.

quoted

Cc: Jamie Iles<redacted>
Cc: Will Deacon<redacted>
---
  arch/arm/kernel/pmu.c |   19 +++++++++++++++++++
  1 files changed, 19 insertions(+), 0 deletions(-)

quoted

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index d34cf88..23c3af2 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c

[...]

quoted

@@ -73,6 +86,11 @@ enum arm_pmu_type armpmu_device_type(struct

platform_device *pdev)

quoted

  		return (enum arm_pmu_type) of_id->data;
  	}

+	/* Provided by platform_device_id table */
+	if ((pdev_id = platform_get_device_id(pdev))) {

    scripts/checkpatch.pl should warn about using = in the *if*
statement...

quoted

+		return (enum arm_pmu_type) pdev_id->driver_data;
+	}

    scripts/checkpatch.pl should warn about unneeded {} here.
    Did you run your patch thru it?

WBR, Sergei

Thanks, will fix in v2 along with any other issues.

Mark.

Mark,

On 06/13/2011 04:35 AM, Mark Rutland wrote:

This is based on an earlier patch from Rob Herring [off-list ref]

quoted

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.

This modification sets the PMU hardware type based on data embedded in the
binding, allowing easy addition of new PMU types in future.

Support for new PMU types not provided by devicetree can be added later using
platform_device_id tables in a similar fashion.

Cc: Jamie Iles <redacted>
Cc: Rob Herring <redacted>
Cc: Will Deacon <redacted>
---
 
 Rob: would you be happy to merge this into your tree? or would you
 rather I sent it through Russell?

Looks good to me. I'm happy for you to take it.

Rob

-----Original Message-----
From: Rob Herring [mailto:robherring2 at gmail.com]
Sent: 13 June 2011 14:40
To: Mark Rutland
Cc: linux-arm-kernel at lists.infradead.org; Jamie Iles; Will Deacon
Subject: Re: [PATCH 3/4] ARM: pmu: add OF probing support

Mark,

On 06/13/2011 04:35 AM, Mark Rutland wrote:

quoted

This is based on an earlier patch from Rob Herring

[off-list ref]

quoted

quoted

Add OF match table to enable OF style driver binding. The dts entry

is like

quoted

quoted

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

quoted

quoted

based on the OF compatible string.

This modification sets the PMU hardware type based on data embedded

in the

quoted

binding, allowing easy addition of new PMU types in future.

Support for new PMU types not provided by devicetree can be added

later using

quoted

platform_device_id tables in a similar fashion.

Cc: Jamie Iles <redacted>
Cc: Rob Herring <redacted>
Cc: Will Deacon <redacted>
---

 Rob: would you be happy to merge this into your tree? or would you
 rather I sent it through Russell?

Looks good to me. I'm happy for you to take it.

Rob

Thanks. Mind if I add your Ack?

Mark.

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

On 06/13/2011 08:48 AM, Mark Rutland wrote:

quoted

-----Original Message-----
From: Rob Herring [mailto:robherring2 at gmail.com]
Sent: 13 June 2011 14:40
To: Mark Rutland
Cc: linux-arm-kernel at lists.infradead.org; Jamie Iles; Will Deacon
Subject: Re: [PATCH 3/4] ARM: pmu: add OF probing support

Mark,

On 06/13/2011 04:35 AM, Mark Rutland wrote:

quoted

This is based on an earlier patch from Rob Herring

[off-list ref]

quoted

quoted

Add OF match table to enable OF style driver binding. The dts entry

is like

quoted

quoted

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

quoted

quoted

based on the OF compatible string.

This modification sets the PMU hardware type based on data embedded

in the

quoted

binding, allowing easy addition of new PMU types in future.

Support for new PMU types not provided by devicetree can be added

later using

quoted

platform_device_id tables in a similar fashion.

Cc: Jamie Iles <redacted>
Cc: Rob Herring <redacted>
Cc: Will Deacon <redacted>
---

 Rob: would you be happy to merge this into your tree? or would you
 rather I sent it through Russell?

Looks good to me. I'm happy for you to take it.

Rob

Thanks. Mind if I add your Ack?

Acked-by: Rob Herring <redacted>

On Mon, Jun 13, 2011 at 10:35:57AM +0100, Mark Rutland wrote:

I guess we could put "arm-pmu" in here but I'm not sure it's worth a 
respin.

The series looks nice though, feel free to add my Acked-by for them.

Acked-by: Jamie Iles <redacted>

Jamie

On Wed, Jun 08, 2011 at 11:40:01AM -0500, Rob Herring wrote:

Mark,

On 06/08/2011 10:54 AM, Mark Rutland wrote:

quoted

Hi,

quoted

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

I don't think this is the best way to handle the type when we've got an FDT
description:

* release_pmu hasn't been updated to match the type logic here, so it might do
  anything when handed a platform_device initialised by FDT code.

* the warning message for an invalid registration still uses pdev->id rather
  than type. This can't currently be reached when the PMU was handed to us via
  FDT, but it may confuse refactoring later on.

* If we want to add a new PMU type, we'll have to add more logic to
  pmu_device_probe. Given that work is going on to add support for system PMUs,
  this doesn't seem particularly brilliant.

quoted

+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,
 };

This all seems fine for handling CPU PMUs.

I think that a better strategy would be to separate the type logic from the
registration. I have a patch for this:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052455.html

With it, you won't need to change pmu_device_probe, and adding FDT support
should just be a matter of adding the of_match_table.

Okay. I'll rebase mine on top of your changes.

The DT binding looks good to me though.

g.