ARM: pmu: improve PMU type identification
Mark Rutland
maintainerv2 cover ARM: pmu: improve PMU type identification
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
Mark Rutland
maintainerPATCH v2 1/4 ARM: pmu: refactor reservation
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
--- 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(-)
arch/arm/include/asm/pmu.h +1 -1
@@ -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.
arch/arm/kernel/perf_event.c +2 -2
@@ -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 ; }
arch/arm/kernel/pmu.c +19 -14
@@ -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);
Signature · 1 line
1.7.0.4
Mark Rutland
maintainerPATCH v2 2/4 ARM: pmu: reject duplicate PMU registrations
In reply to Mark RutlandCurrently, 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.
--- arch/arm/kernel/pmu.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
arch/arm/kernel/pmu.c +6 -6
@@ -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 ; }
Signature · 1 line
1.7.0.4
Mark Rutland
maintainerPATCH v2 3/4 ARM: pmu: add OF probing support
In reply to Mark RutlandThis is based on an earlier patch from Rob Herring <[email protected]>
Quoted 10 lines
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->idas 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.
--- 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
Documentation/devicetree/bindings/arm/pmu.txt new file +22 -0
@@ -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>; +}; +
arch/arm/kernel/pmu.c +33 -1
@@ -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, };
Signature · 1 line
1.7.0.4
Mark Rutland
maintainerPATCH v2 4/4 ARM: pmu: add platform_device_id table support
In reply to Mark RutlandThis 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.
--- arch/arm/kernel/pmu.c | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-)
arch/arm/kernel/pmu.c +18 -2
@@ -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 )
Signature · 1 line
1.7.0.4
v2 cover ARM: pmu: improve PMU type identification
In reply to Mark RutlandQuoted 20 lines · Mark Rutland
Hi,
This is version 2 of the patches originally posted here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052865.htmlChanges since v1 include:
* Added Acks from Jamie Iles & Rob Herring.
* Add "arm-pmu" binding to theplatform_device_idtable, as driver name matching is not used whenplatform_driver::id_tableexists.
* Fixed name clash inPLAT_MATCH_PMUmacro.
* Removed unnecessary casts forplatform_device_id::driver_data.
* Aligned trailing backslashes on multi-line*_MATCH_PMUmacros.
* 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.
Quoted 6 lines
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
v2 cover ARM: pmu: improve PMU type identification
In reply to Mark RutlandMark,
Quoted 20 lines · Mark Rutland
Hi,
This is version 2 of the patches originally posted here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052865.htmlChanges since v1 include:
* Added Acks from Jamie Iles & Rob Herring.
* Add "arm-pmu" binding to theplatform_device_idtable, as driver name matching is not used whenplatform_driver::id_tableexists.
* Fixed name clash inPLAT_MATCH_PMUmacro.
* Removed unnecessary casts forplatform_device_id::driver_data.
* Aligned trailing backslashes on multi-line*_MATCH_PMUmacros.
* 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.
Quoted 7 lines
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
v2 cover ARM: pmu: improve PMU type identification
In reply to Mark RutlandQuoted 6 lines · Mark Rutland
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.
v2 cover ARM: pmu: improve PMU type identification
Russell,
Quoted 10 lines · Russell King - ARM Linux
Quoted 6 lines · Mark Rutland
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
MAINTAINERSfile 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
v2 cover ARM: pmu: improve PMU type identification
Quoted 8 lines · Will Deacon
Russell,
Quoted 5 lines · Russell King - ARM Linux
Quoted 4 lines · Mark Rutland
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.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
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
MAINTAINERSfile 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
v2 cover ARM: pmu: improve PMU type identification
Quoted 1 line · Jamie Iles
On Wed, Jun 29, 2011 at 10:36:53AM +0100, Will Deacon wrote:
Hi Will,
Quoted 11 lines
Quoted 11 lines
Russell,
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Mark Rutland
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.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Could someone please look at adding an entry to the
MAINTAINERSfile 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
Quoted 8 lines
Jamie
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
v2 cover ARM: pmu: improve PMU type identification
In reply to Will Deacon2011/6/29 Will Deacon <[email protected]>:
Quoted 6 lines · Russell King - ARM Linux
Quoted 3 lines
Could someone please look at adding an entry to the
MAINTAINERSfile 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,
Thanks,
Linus Walleij