[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Mark Rutland]

Hi,

This is version 2 of the patches originally posted here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052865.html

Changes since v1 include:

 * Added Acks from Jamie Iles & Rob Herring.
 * Add "arm-pmu" binding to the platform_device_id table, as driver name
   matching is not used when platform_driver::id_table exists.
 * Fixed name clash in PLAT_MATCH_PMU macro.
 * Removed unnecessary casts for platform_device_id::driver_data.
 * Aligned trailing backslashes on multi-line *_MATCH_PMU macros.
 * Removed spaces after casts.

The patches use {of,platform}_device_id tables to provide the PMU type, which
can be used similarly (the macros make entries look identical apart from the
{plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
should allow for easier addition/modification of PMU driver bindings.

BSPs need no longer care about the Type of PMUs they register; as long as a
binding exists the type will be assigned correctly.

Rob, Jamie: would you mind if patches 3 & 4 were squashed together?
Patch 4 depends heavily on patch 3, and there doesn't seem to be a clean way of
fixing that.

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                         |   87 ++++++++++++++++++++-----
 4 files changed, 95 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/pmu.txt

[PATCHv2 1/4] ARM: pmu: refactor reservation

[Mark Rutland]

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 <jamie@jamieiles.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Rob Herring <rob.herring@calxeda.com>
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

[PATCHv2 2/4] ARM: pmu: reject duplicate PMU registrations

[Mark Rutland]

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 <jamie@jamieiles.com>
Acked-By: Will Deacon <will.deacon@arm.com>
---
 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

[PATCHv2 3/4] ARM: pmu: add OF probing support

[Mark Rutland]

This is based on an earlier patch 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.

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.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Jamie Iles <jamie@jamieiles.com>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 Documentation/devicetree/bindings/arm/pmu.txt |   22 ++++++++++++++++
 arch/arm/kernel/pmu.c                         |   34 ++++++++++++++++++++++++-
 2 files changed, 55 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..2ce00be 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,45 @@ 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[] = {
+	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

[PATCHv2 4/4] ARM: pmu: add platform_device_id table support

[Mark Rutland]

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. An single entry for
existing devices is provided.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Jamie Iles <jamie@jamieiles.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/pmu.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
index 2ce00be..2b70709 100644
--- a/arch/arm/kernel/pmu.c
+++ b/arch/arm/kernel/pmu.c
@@ -61,9 +61,22 @@ static struct of_device_id armpmu_of_device_ids[] = {
 	{},
 };
 
+#define PLAT_MATCH_PMU(_name, _type) {	\
+	.name		= _name,	\
+	.driver_data	= _type,	\
+}
+
+#define PLAT_MATCH_CPU(_name)	PLAT_MATCH_PMU(_name, ARM_PMU_DEVICE_CPU)
+
+static struct platform_device_id armpmu_plat_device_ids[] = {
+	PLAT_MATCH_CPU("arm-pmu"),
+	{},
+};
+
 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) {
@@ -72,8 +85,10 @@ enum arm_pmu_type armpmu_device_type(struct platform_device *pdev)
 		return (enum arm_pmu_type)of_id->data;
 	}
 
-	/* Provided by a 'legacy' platform_device */
-	return ARM_PMU_DEVICE_CPU;
+	/* Provided by platform_device_id table */
+	pdev_id = platform_get_device_id(pdev);
+	BUG_ON(!pdev_id);
+	return pdev_id->driver_data;
 }
 
 static int __devinit armpmu_device_probe(struct platform_device *pdev)
@@ -87,6 +102,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

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Jamie Iles]

On Wed, Jun 15, 2011 at 03:40:00PM +0100, Mark Rutland wrote:
> Hi,
> 
> This is version 2 of the patches originally posted here:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052865.html
> 
> Changes since v1 include:
> 
>  * Added Acks from Jamie Iles & Rob Herring.
>  * Add "arm-pmu" binding to the platform_device_id table, as driver name
>    matching is not used when platform_driver::id_table exists.
>  * Fixed name clash in PLAT_MATCH_PMU macro.
>  * Removed unnecessary casts for platform_device_id::driver_data.
>  * Aligned trailing backslashes on multi-line *_MATCH_PMU macros.
>  * Removed spaces after casts.
> 
> The patches use {of,platform}_device_id tables to provide the PMU type, which
> can be used similarly (the macros make entries look identical apart from the
> {plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
> should allow for easier addition/modification of PMU driver bindings.
> 
> BSPs need no longer care about the Type of PMUs they register; as long as a
> binding exists the type will be assigned correctly.
> 
> Rob, Jamie: would you mind if patches 3 & 4 were squashed together?
> Patch 4 depends heavily on patch 3, and there doesn't seem to be a clean way of
> fixing that.

Sounds good to me.  No other comments on the rest of it, all looks 
great!

Jamie

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Rob Herring]

Mark,

On 06/15/2011 09:40 AM, Mark Rutland wrote:
> Hi,
> 
> This is version 2 of the patches originally posted here:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052865.html
> 
> Changes since v1 include:
> 
>  * Added Acks from Jamie Iles & Rob Herring.
>  * Add "arm-pmu" binding to the platform_device_id table, as driver name
>    matching is not used when platform_driver::id_table exists.
>  * Fixed name clash in PLAT_MATCH_PMU macro.
>  * Removed unnecessary casts for platform_device_id::driver_data.
>  * Aligned trailing backslashes on multi-line *_MATCH_PMU macros.
>  * Removed spaces after casts.
> 
> The patches use {of,platform}_device_id tables to provide the PMU type, which
> can be used similarly (the macros make entries look identical apart from the
> {plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
> should allow for easier addition/modification of PMU driver bindings.
> 
> BSPs need no longer care about the Type of PMUs they register; as long as a
> binding exists the type will be assigned correctly.
> 
> Rob, Jamie: would you mind if patches 3 & 4 were squashed together?
> Patch 4 depends heavily on patch 3, and there doesn't seem to be a clean way of
> fixing that.
> 
Interdependency is not really a good reason to combine patches, so I
would leave them as is. Functionality wise, they are doing different things.

Rob

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Russell King - ARM Linux]

On Wed, Jun 15, 2011 at 03:40:00PM +0100, Mark Rutland wrote:
> can be used similarly (the macros make entries look identical apart from the
> {plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
> should allow for easier addition/modification of PMU driver bindings.
> 
> BSPs need no longer care about the Type of PMUs they register; as long as a
> binding exists the type will be assigned correctly.

Could someone please look at adding an entry to the MAINTAINERS file
for the ARM PMU stuff, so we know who should be looking after this stuff?

Thanks.

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Will Deacon]

Russell,

On Wed, Jun 29, 2011 at 10:26:22AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 15, 2011 at 03:40:00PM +0100, Mark Rutland wrote:
> > can be used similarly (the macros make entries look identical apart from the
> > {plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
> > should allow for easier addition/modification of PMU driver bindings.
> > 
> > BSPs need no longer care about the Type of PMUs they register; as long as a
> > binding exists the type will be assigned correctly.
> 
> Could someone please look at adding an entry to the MAINTAINERS file
> for the ARM PMU stuff, so we know who should be looking after this stuff?

I'm happy to maintain this stuff if nobody has any objections.

Will

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Jamie Iles]

On Wed, Jun 29, 2011 at 10:36:53AM +0100, Will Deacon wrote:
> Russell,
> 
> On Wed, Jun 29, 2011 at 10:26:22AM +0100, Russell King - ARM Linux wrote:
> > On Wed, Jun 15, 2011 at 03:40:00PM +0100, Mark Rutland wrote:
> > > can be used similarly (the macros make entries look identical apart from the
> > > {plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
> > > should allow for easier addition/modification of PMU driver bindings.
> > > 
> > > BSPs need no longer care about the Type of PMUs they register; as long as a
> > > binding exists the type will be assigned correctly.
> > 
> > Could someone please look at adding an entry to the MAINTAINERS file
> > for the ARM PMU stuff, so we know who should be looking after this stuff?
> 
> I'm happy to maintain this stuff if nobody has any objections.

No objections from me, I think that sounds like a very good idea!

Jamie

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Jean Pihet]

On Wed, Jun 29, 2011 at 12:42 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> On Wed, Jun 29, 2011 at 10:36:53AM +0100, Will Deacon wrote:
Hi Will,

>> Russell,
>>
>> On Wed, Jun 29, 2011 at 10:26:22AM +0100, Russell King - ARM Linux wrote:
>> > On Wed, Jun 15, 2011 at 03:40:00PM +0100, Mark Rutland wrote:
>> > > can be used similarly (the macros make entries look identical apart from the
>> > > {plat,of} prefix). This allows us to stop (ab)using platform_device::id, and
>> > > should allow for easier addition/modification of PMU driver bindings.
>> > >
>> > > BSPs need no longer care about the Type of PMUs they register; as long as a
>> > > binding exists the type will be assigned correctly.
>> >
>> > Could someone please look at adding an entry to the MAINTAINERS file
>> > for the ARM PMU stuff, so we know who should be looking after this stuff?
>>
>> I'm happy to maintain this stuff if nobody has any objections.
>
> No objections from me, I think that sounds like a very good idea!
No objections! I would be glad to lend a hand if neeeded!

Regards,
Jean
>
> Jamie
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCHv2 0/4] ARM: pmu: improve PMU type identification

[Linus Walleij]

2011/6/29 Will Deacon <will.deacon@arm.com>:
> On Wed, Jun 29, 2011 at 10:26:22AM +0100, Russell King - ARM Linux wrote:
>>
>> Could someone please look at adding an entry to the MAINTAINERS file
>> for the ARM PMU stuff, so we know who should be looking after this stuff?
>
> I'm happy to maintain this stuff if nobody has any objections.

Send a patch,
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,
Linus Walleij