drivers: CCI: add ARM CCI PMU support
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
--- Documentation/devicetree/bindings/arm/cci.txt | 38 ++ drivers/bus/arm-cci.c | 642 +++++++++++++++++++++++++ 2 files changed, 680 insertions(+)
Documentation/devicetree/bindings/arm/cci.txt +38 -0
@@ -79,6 +79,34 @@ specific to ARM. corresponding interface programming registers. + - CCI PMU node + + Node name must be "pmu". + Parent node must be CCI interconnect node. + + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1" + + - reg: + Usage: required + Value type: <prop-encoded-array> + Definition: the base address and size of the + corresponding interface programming + registers. + + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of unique PMU + interrupts + * CCI interconnect bus masters Description: masters in the device tree connected to a CCI port
@@ -163,6 +191,16 @@ Example: interface-type = "ace"; reg = <0x5000 0x1000>; }; + + pmu at 9000 { + compatible = "arm,cci-400-pmu,rev0"; + reg = <0x9000 0x5000>; + interrupts = <0 101 4>, + <0 102 4>, + <0 103 4>, + <0 104 4>, + <0 105 4>; + }; }; This CCI node corresponds to a CCI component whose control registers sits
drivers/bus/arm-cci.c +642 -0
670 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Hi Will,
Quoted 16 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>
Quoted 5 lines
--- Documentation/devicetree/bindings/arm/cci.txt | 38 ++ drivers/bus/arm-cci.c | 642 +++++++++++++++++++++++++ 2 files changed, 680 insertions(+)
It's been 2 rc's since this patch was posted. If there are no objections would you be ok to take this patch via your tree?
Thanks,
Punit
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 23 lines · Punit Agrawal
Quoted 16 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>Quoted 5 lines
--- Documentation/devicetree/bindings/arm/cci.txt | 38 ++ drivers/bus/arm-cci.c | 642 +++++++++++++++++++++++++ 2 files changed, 680 insertions(+)
Quoted 2 lines
It's been 2 rc's since this patch was posted. If there are no objections would you be ok to take this patch via your tree?
Sure. I'm currently travelling at the moment, so I'll try and get this done as soon as I'm back.
Will
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 24 lines · Will Deacon
Quoted 23 lines · Punit Agrawal
Quoted 21 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>--- Documentation/devicetree/bindings/arm/cci.txt | 38 ++ drivers/bus/arm-cci.c | 642 +++++++++++++++++++++++++ 2 files changed, 680 insertions(+)
Quoted 5 lines
Quoted 2 lines
It's been 2 rc's since this patch was posted. If there are no objections would you be ok to take this patch via your tree?
Sure. I'm currently travelling at the moment, so I'll try and get this done as soon as I'm back.
So, I just tried to test this patch but I'm getting issues when enabling CCI with mainline:
ARM Versatile Express Boot Monitor
Version: V5.1.5
Build Date: Jul 24 2012
Daughterboard Site 1: V2P-CA15 Cortex A15
Daughterboard Site 2: V2P-CA15_A7 Cortex A15
Running boot script from flash - BOOTSCRIPT
Loaded kernel - linuxtc2
Booting kernel @ 0x80008000
Command line 'ip=dhcp root=/dev/nfs nfsroot=10.1.203.36:/exports/linaro-13.06,tcp rw console=ttyAMA0,38400 mem=2G loglevel=9 user_debug=31 earlyprintk debug' ATAGs @ 0x80000100
Fatal Error: Unhandled Exception - Undefined
We're exploding *really* early, and I have no idea why. If I remove CCI from the equation (either in the kernel or the device-tree), then I can boot as per usual.
Any ideas? Do I need to enable something in the TC2 board firmware?
Will
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
[replying to self]
Quoted 16 lines · Will Deacon
So, I just tried to test this patch but I'm getting issues when enabling CCI with mainline:
ARM Versatile Express Boot Monitor
Version: V5.1.5
Build Date: Jul 24 2012
Daughterboard Site 1: V2P-CA15 Cortex A15
Daughterboard Site 2: V2P-CA15_A7 Cortex A15
Running boot script from flash - BOOTSCRIPT
Loaded kernel - linuxtc2
Booting kernel @0x80008000
Command line 'ip=dhcp root=/dev/nfs nfsroot=10.1.203.36:/exports/linaro-13.06,tcp rw console=ttyAMA0,38400 mem=2G loglevel=9 user_debug=31 earlyprintk debug' ATAGs @0x80000100
Quoted 6 lines
Fatal Error: Unhandled Exception - Undefined
We're exploding *really* early, and I have no idea why. If I remove CCI from the equation (either in the kernel or the device-tree), then I can boot as per usual.
Turns out my .dtb has gone over some limit and appended dtb corrupts the kernel image. Removing some random nodes got things working again.
Will
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Hi Will,
Quoted 22 lines · Will Deacon
[replying to self]
Quoted 19 lines · Will Deacon
So, I just tried to test this patch but I'm getting issues when enabling CCI with mainline:
ARM Versatile Express Boot Monitor
Version: V5.1.5
Build Date: Jul 24 2012
Daughterboard Site 1: V2P-CA15 Cortex A15
Daughterboard Site 2: V2P-CA15_A7 Cortex A15
Running boot script from flash - BOOTSCRIPT
Loaded kernel - linuxtc2
Booting kernel @0x80008000
Command line 'ip=dhcp root=/dev/nfs nfsroot=10.1.203.36:/exports/linaro-13.06,tcp rw console=ttyAMA0,38400 mem=2G loglevel=9 user_debug=31 earlyprintk debug' ATAGs @0x80000100Fatal Error: Unhandled Exception - Undefined
Quoted 7 lines
Quoted 3 lines
We're exploding *really* early, and I have no idea why. If I remove CCI from the equation (either in the kernel or the device-tree), then I can boot as per usual.
Turns out my .dtb has gone over some limit and appended dtb corrupts the kernel image. Removing some random nodes got things working again.
Just saw your mail. I haven't run into this one before. But looks like you've figured out what the problem was.
Let me know if you hit any issues with testing / using the CCI PMU.
Thanks for picking this up.
Punit
Quoted 3 lines
Will
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Punit AgrawalQuoted 33 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +4
-0@@ -79,6 +79,34 @@ specific to ARM. corresponding interface programming registers. + - CCI PMU node + + Node name must be "pmu". + Parent node must be CCI interconnect node.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 95
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +10
-0+ + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1"
Do you really mean only one? Seems like ""arm,cci-400-pmu,rev0", "arm,cci-400-pmu" would be valid.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 108
Quoted 13 lines
Documentation/devicetree/bindings/arm/cci.txt +13
-0+ + - reg: + Usage: required + Value type: <prop-encoded-array> + Definition: the base address and size of the + corresponding interface programming + registers. + + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of unique PMU + interrupts
What is the list of interrupts related to, should there be an associated interrupts-names
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ * CCI interconnect bus masters Description: masters in the device tree connected to a CCI port@@ -163,6 +191,16 @@ Example: interface-type = "ace"; reg = <0x5000 0x1000>; }; + + pmu at 9000 {
Documentation/devicetree/bindings/arm/cci.txt Example: · line 205
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +8
-0+ compatible = "arm,cci-400-pmu,rev0"; + reg = <0x9000 0x5000>; + interrupts = <0 101 4>, + <0 102 4>, + <0 103 4>, + <0 104 4>, + <0 105 4>; + }; };
[ snip ]
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 28 lines · Kumar Gala
Quoted 25 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +0
-0
[snip]
Quoted 8 lines
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 108
Quoted 6 lines
Documentation/devicetree/bindings/arm/cci.txt +6
-0+ + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of unique PMU + interruptsWhat is the list of interrupts related to, should there be an associated interrupts-names
No, interrupt-names is optional, but you are correct that what function each interrupt is for must be defined.
Rob
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Hi Rob,
Quoted 41 lines · Rob Herring
Quoted 28 lines · Kumar Gala
Quoted 25 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +0
-0[snip]
Quoted 8 lines
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 108
Quoted 6 lines
Documentation/devicetree/bindings/arm/cci.txt +6
-0+ + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of unique PMU + interruptsWhat is the list of interrupts related to, should there be an associated interrupts-names
Quoted 3 lines
No, interrupt-names is optional, but you are correct that what function each interrupt is for must be defined.
I'll update the bindings documentation to describe the function of interrupts.
Thanks for the comments.
Punit
Quoted 6 lines
Rob
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Kumar GalaHi Kumar,
Thanks for a review of the bindings.
Quoted 32 lines · Kumar Gala
Quoted 29 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +0
-0@@ -79,6 +79,34 @@ specific to ARM. corresponding interface programming registers.
Quoted 17 lines
Quoted 4 lines
Documentation/devicetree/bindings/arm/cci.txt +4
-0+ - CCI PMU node + + Node name must be "pmu". + Parent node must be CCI interconnect node.Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 95
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +10
-0+ + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1"Do you really mean only one? Seems like ""arm,cci-400-pmu,rev0", "arm,cci-400-pmu" would be valid.
Hmm... yes both would be valid. But...
The event numbering scheme changed between Rev 0 and Rev 1 of the CCI. If the revision is specified then it is used to get the event ranges to validate the events. If not, i.e., "arm,cci-400-pmu" is used, then the driver tries to find the the revision by reading the peripheral id registers.
I was trying to make the bindings robust in the face of change in behaviour between different revisons of the IP.
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/arm/cci.txt +7
-0+ + - reg: + Usage: required + Value type: <prop-encoded-array> + Definition: the base address and size of the + corresponding interface programming + registers.
Quoted 9 lines
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 108
Quoted 6 lines
Documentation/devicetree/bindings/arm/cci.txt +6
-0+ + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of unique PMU + interruptsWhat is the list of interrupts related to, should there be an associated interrupts-names
For the CCI PMU, each interrupt signal corresponds to the overflow of a performance counter.
Here again, I was trying to be robust in the face of differing hardware configurations - specially the scenario where multiple interrupt lines from the CCI PMU are tied together to generate a single interrupt to the CPU.
Cheers,
Punit
Quoted 32 lines
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ * CCI interconnect bus masters Description: masters in the device tree connected to a CCI port@@ -163,6 +191,16 @@ Example: interface-type = "ace"; reg = <0x5000 0x1000>; }; + + pmu at 9000 {Documentation/devicetree/bindings/arm/cci.txt Example: · line 205
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +8
-0+ compatible = "arm,cci-400-pmu,rev0"; + reg = <0x9000 0x5000>; + interrupts = <0 101 4>, + <0 102 4>, + <0 103 4>, + <0 104 4>, + <0 105 4>; + }; };[ snip ]
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 55 lines · Punit Agrawal
Hi Kumar,
Thanks for a review of the bindings.
Quoted 32 lines · Kumar Gala
Quoted 29 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +0
-0@@ -79,6 +79,34 @@ specific to ARM. corresponding interface programming registers.Quoted 17 lines
Quoted 4 lines
Documentation/devicetree/bindings/arm/cci.txt +4
-0+ - CCI PMU node + + Node name must be "pmu". + Parent node must be CCI interconnect node.Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 95
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +10
-0+ + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1"Do you really mean only one? Seems like ""arm,cci-400-pmu,rev0", "arm,cci-400-pmu" would be valid.
Quoted 5 lines
Hmm... yes both would be valid. But...
The event numbering scheme changed between Rev 0 and Rev 1 of the CCI. If the revision is specified then it is used to get the event ranges to validate the events. If not, i.e., "arm,cci-400-pmu" is used, then the driver tries to find the the revision by reading the peripheral id registers.
I was trying to make the bindings robust in the face of change in behaviour between different revisons of the IP.
If there is a periph id register why bother with the device tree having different version info in it?
- k
Signature · 2 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 58 lines · Kumar Gala
Quoted 55 lines · Punit Agrawal
Hi Kumar,
Thanks for a review of the bindings.
Quoted 49 lines · Kumar Gala
Quoted 33 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +4
-0@@ -79,6 +79,34 @@ specific to ARM. corresponding interface programming registers. + - CCI PMU node + + Node name must be "pmu". + Parent node must be CCI interconnect node.Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 95
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +10
-0+ + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1"Do you really mean only one? Seems like ""arm,cci-400-pmu,rev0", "arm,cci-400-pmu" would be valid.
Quoted 8 lines
Quoted 5 lines
Hmm... yes both would be valid. But...
The event numbering scheme changed between Rev 0 and Rev 1 of the CCI. If the revision is specified then it is used to get the event ranges to validate the events. If not, i.e., "arm,cci-400-pmu" is used, then the driver tries to find the the revision by reading the peripheral id registers.
I was trying to make the bindings robust in the face of change in behaviour between different revisons of the IP.
If there is a periph id register why bother with the device tree having different version info in it?
The different version strings are useful when the identification registers are either incorrect or broken.
But I am not aware of any such platforms currently out there. I can remove the additional compatible strings and rely on the peripheral id register solely. Do you prefer that?
Cheers,
Punit
Quoted 2 lines
- k
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 63 lines · Punit Agrawal
Quoted 62 lines · Kumar Gala
Quoted 59 lines · Punit Agrawal
Hi Kumar,
Thanks for a review of the bindings.
Quoted 32 lines · Kumar Gala
Quoted 29 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>Reviewed-by Will Deacon <[email protected]>---
Documentation/devicetree/bindings/arm/cci.txt| 38 ++drivers/bus/arm-cci.c| 642 +++++++++++++++++++++++++2 files changed, 680 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +0
-0@@ -79,6 +79,34 @@ specific to ARM. corresponding interface programming registers.Quoted 17 lines
Quoted 4 lines
Documentation/devicetree/bindings/arm/cci.txt +4
-0+ - CCI PMU node + + Node name must be "pmu". + Parent node must be CCI interconnect node.Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 95
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +10
-0+ + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1"Do you really mean only one? Seems like ""arm,cci-400-pmu,rev0", "arm,cci-400-pmu" would be valid.
Hmm... yes both would be valid. But...
The event numbering scheme changed between Rev 0 and Rev 1 of the CCI. If the revision is specified then it is used to get the event ranges to validate the events. If not, i.e., "arm,cci-400-pmu" is used, then the driver tries to find the the revision by reading the peripheral id registers.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
I was trying to make the bindings robust in the face of change in behaviour between different revisons of the IP.
If there is a periph id register why bother with the device tree having different version info in it?
The different version strings are useful when the identification registers are either incorrect or broken.
But I am not aware of any such platforms currently out there. I can remove the additional compatible strings and rely on the peripheral id register solely. Do you prefer that?
Yes please make this change. While I agree the compat field is useful when ID registers are broken, if they are known to be correct I would recommend utilizing them until the situation arises that they arent.
Signature · 2 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Punit AgrawalQuoted 6 lines · Punit Agrawal
Quoted 2 lines
What is the list of interrupts related to, should there be an associated interrupts-names
For the CCI PMU, each interrupt signal corresponds to the overflow of a performance counter.
Here again, I was trying to be robust in the face of differing hardware configurations - specially the scenario where multiple interrupt lines from the CCI PMU are tied together to generate a single interrupt to the CPU.
Even in the case of multiple interrupt lines tied together, why wouldn't you still specify each interrupt specifier and they all just be the same and thus the interrupt appears to be shared ?
- k
Signature · 2 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Hi Kumar,
Quoted 12 lines · Kumar Gala
Quoted 6 lines · Punit Agrawal
Quoted 2 lines
What is the list of interrupts related to, should there be an associated interrupts-names
For the CCI PMU, each interrupt signal corresponds to the overflow of a performance counter.
Here again, I was trying to be robust in the face of differing hardware configurations - specially the scenario where multiple interrupt lines from the CCI PMU are tied together to generate a single interrupt to the CPU.
Even in the case of multiple interrupt lines tied together, why wouldn't you still specify each interrupt specifier and they all just be the same and thus the interrupt appears to be shared ?
I am not quite sure what you are asking here. Are you suggesting that even if the interrupts are muxed, they should be specified multiple times?
Below is an update I've added to the documentation to better describe the interrupts. Does this help?
@@ -104,8 +103,19 @@ specific to ARM.
- interrupts:
Usage: required
Value type: <prop-encoded-array>
- Definition: comma-separated list of unique PMU
- interrupts
+ Definition: comma-separated list of counter overflow
+ interrupts.
+
+ The CCI PMU has an interrupt signal for each + counters. Typically, the number of + interrupts will be equal to the number of + counters. + + On some platforms, the individual interrupt + signals may be combined in some fashion + before being routed to the interrupt + controller resulting in less number of + interrupts than counters.
* CCI interconnect bus masters
Quoted 2 lines
- k
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 9 lines · Punit Agrawal
Hi Kumar,
Quoted 6 lines · Kumar Gala
Quoted 3 lines · Punit Agrawal
Quoted 2 lines
What is the list of interrupts related to, should there be an associated interrupts-names
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
For the CCI PMU, each interrupt signal corresponds to the overflow of a performance counter.
Here again, I was trying to be robust in the face of differing hardware configurations - specially the scenario where multiple interrupt lines from the CCI PMU are tied together to generate a single interrupt to the CPU.
Even in the case of multiple interrupt lines tied together, why wouldn't you still specify each interrupt specifier and they all just be the same and thus the interrupt appears to be shared ?
I am not quite sure what you are asking here. Are you suggesting that even if the interrupts are muxed, they should be specified multiple times?
I'm saying the # of interrupts should equal the # of counters w/regards to the dts and binding.
Quoted 11 lines
Below is an update I've added to the documentation to better describe the interrupts. Does this help?
(quoted diff) +3
-2@@ -104,8 +103,19 @@ specific to ARM. - interrupts: Usage: required Value type: <prop-encoded-array> - Definition: comma-separated list of unique PMU - interrupts + Definition: comma-separated list of counter overflow + interrupts. +
Review specific to ARM. · line 118
Quoted 10 lines
(quoted diff) +10
-0+ The CCI PMU has an interrupt signal for each + counters. Typically, the number of + interrupts will be equal to the number of + counters. + + On some platforms, the individual interrupt + signals may be combined in some fashion + before being routed to the interrupt + controller resulting in less number of + interrupts than counters.
I'd drop the last paragraph. Think about a case in which the SoC ORs all the interrupts for each counter together, in the .dts it should still look something like (my example assumes 4 counters):
interrupts = < 0 100 4 >, < 0 100 4 >, < 0 100 4 >, < 0 100 4 >;
or lets say the SoC has 2 interrupts with counters 1 & 2 on first interrupt and 3 & 4 on second
interrupts = < 0 100 4 >, < 0 100 4 >, < 0 101 4 >, < 0 101 4 >;
or a crazy SoC (counters 1 & 3 on first interrupt, 2 & 4 on second):
interrupts = < 0 100 4 >, < 0 101 4 >, < 0 100 4 >, < 0 101 4 >;
make sense?
- k
Signature · 2 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 51 lines · Kumar Gala
Quoted 9 lines · Punit Agrawal
Hi Kumar,
Quoted 6 lines · Kumar Gala
Quoted 3 lines · Punit Agrawal
Quoted 2 lines
What is the list of interrupts related to, should there be an associated interrupts-names
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
For the CCI PMU, each interrupt signal corresponds to the overflow of a performance counter.
Here again, I was trying to be robust in the face of differing hardware configurations - specially the scenario where multiple interrupt lines from the CCI PMU are tied together to generate a single interrupt to the CPU.
Even in the case of multiple interrupt lines tied together, why wouldn't you still specify each interrupt specifier and they all just be the same and thus the interrupt appears to be shared ?
I am not quite sure what you are asking here. Are you suggesting that even if the interrupts are muxed, they should be specified multiple times?
I'm saying the # of interrupts should equal the # of counters w/regards to the dts and binding.
Quoted 11 lines
Below is an update I've added to the documentation to better describe the interrupts. Does this help?
(quoted diff) +3
-2@@ -104,8 +103,19 @@ specific to ARM. - interrupts: Usage: required Value type: <prop-encoded-array> - Definition: comma-separated list of unique PMU - interrupts + Definition: comma-separated list of counter overflow + interrupts. +Review specific to ARM. · line 118
Quoted 10 lines
(quoted diff) +10
-0+ The CCI PMU has an interrupt signal for each + counters. Typically, the number of + interrupts will be equal to the number of + counters. + + On some platforms, the individual interrupt + signals may be combined in some fashion + before being routed to the interrupt + controller resulting in less number of + interrupts than counters.I'd drop the last paragraph. Think about a case in which the SoC ORs all the interrupts for each counter together, in the .dts it should still look something like (my example assumes 4 counters):
interrupts = < 0 100 4 >, < 0 100 4 >, < 0 100 4 >, < 0 100 4 >;
or lets say the SoC has 2 interrupts with counters 1 & 2 on first interrupt and 3 & 4 on second
Quoted 8 lines
interrupts = < 0 100 4 >, < 0 100 4 >, < 0 101 4 >, < 0 101 4 >;
or a crazy SoC (counters 1 & 3 on first interrupt, 2 & 4 on second):
interrupts = < 0 100 4 >, < 0 101 4 >, < 0 100 4 >, < 0 101 4 >;
make sense?
Got it. I was trying to prevent the need to specify duplicate interrupts but I'll update the patch with this and the other review comments and post a new version.
Thanks once again for the review.
Cheers,
Punit
Quoted 2 lines
- k
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Punit AgrawalQuoted 10 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 84
Quoted 4 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ - CCI PMU node + + Node name must be "pmu".
I don't think the binding should require the node to have a particular name; node names shouldn't be interpret/used/relied-upon by drivers.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 95
Quoted 11 lines
Documentation/devicetree/bindings/arm/cci.txt +11
-0+ Parent node must be CCI interconnect node. + + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be set to one of + "arm,cci-400-pmu" + "arm,cci-400-pmu,rev0" + "arm,cci-400-pmu,rev1"
What is the first entry in this list for; why wouldn't you always use one of the two versioned compatible values?
The use of , before revN is a little unusual; I would have expected arm,cci-400-pmu-rev0, but this isn't a big deal.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 108
Quoted 5 lines
Documentation/devicetree/bindings/arm/cci.txt +5
-0+ - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of unique PMU + interrupts
Is there more than one interrupt? The text seems to imply that. If so, what are they, and which order must they appear?
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 10 lines · Stephen Warren
Quoted 8 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Quoted 8 lines
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 84
Quoted 5 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ - CCI PMU node + + Node name must be "pmu".I don't think the binding should require the node to have a particular name; node names shouldn't be interpret/used/relied-upon by drivers.
While I agree with that, we should be aiming for some convention and consistency with node names.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 15 lines · Kumar Gala
Quoted 12 lines · Stephen Warren
Quoted 10 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Documentation/devicetree/bindings/arm/cci.txt +0
-0
Quoted 8 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 84
Quoted 3 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ - CCI PMU node + + Node name must be "pmu".I don't think the binding should require the node to have a particular name; node names shouldn't be interpret/used/relied-upon by drivers.
While I agree with that, we should be aiming for some convention and consistency with node names.
Sure. Should there be a Documentation/devictree/bindings/node-names that lists common node names for people to use? Either way though, I still think this is an aspect of authoring the *.dts file, not an aspect of the DT binding? After all, what if there were more than one CCI so they needed to be named pmu at 0, pmu at 1, etc.?
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 23 lines · Stephen Warren
Quoted 22 lines · Kumar Gala
Quoted 18 lines · Stephen Warren
Quoted 10 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 84
Quoted 4 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ - CCI PMU node + + Node name must be "pmu".I don't think the binding should require the node to have a particular name; node names shouldn't be interpret/used/relied-upon by drivers.
Quoted 7 lines
Quoted 1 line
While I agree with that, we should be aiming for some convention and consistency with node names.
Sure. Should there be a
Documentation/devictree/bindings/node-namesthat lists common node names for people to use? Either way though, I still think this is an aspect of authoring the *.dts file, not an aspect of the DT binding? After all, what if there were more than one CCI so they needed to be named pmu at 0, pmu at 1, etc.?
Agreed, I was thinking a bindings/node-names would be a good idea.
I'm guessing 99% of people copy either from the example in the binding of an existing .dts file. So while I agree the binding shouldn't require a node name be a specific thing as part of the spec, we as reviewers should try to ensure consistency in examples or .dts files.
- k
Signature · 2 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 34 lines · Kumar Gala
Quoted 23 lines · Stephen Warren
Quoted 22 lines · Kumar Gala
Quoted 18 lines · Stephen Warren
Quoted 10 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 84
Quoted 4 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0+ - CCI PMU node + + Node name must be "pmu".I don't think the binding should require the node to have a particular name; node names shouldn't be interpret/used/relied-upon by drivers.
Quoted 7 lines
Quoted 1 line
While I agree with that, we should be aiming for some convention and consistency with node names.
Sure. Should there be a
Documentation/devictree/bindings/node-namesthat lists common node names for people to use? Either way though, I still think this is an aspect of authoring the *.dts file, not an aspect of the DT binding? After all, what if there were more than one CCI so they needed to be named pmu at 0, pmu at 1, etc.?
Quoted 4 lines
Agreed, I was thinking a bindings/node-names would be a good idea.
I'm guessing 99% of people copy either from the example in the binding of an existing .dts file. So while I agree the binding shouldn't require a node name be a specific thing as part of the spec, we as reviewers should try to ensure consistency in examples or .dts files.
Based on the comments so far, I will change the bindings documentation submitted with this patch to remove the requirement for a particular node name for CCI PMU.
As it is, this is not required by the driver but was only done for consistency.
Cheers,
Punit
Quoted 2 lines
- k
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Punit AgrawalThe CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
---
Hi,
This is the second version of the CCI PMU driver. The changes incorporate review comments on device tree bindings and corresponding update to the driver.
Cheers,
Punit
Will, I've dropped your reviewed-by tag due to the changes. Let me know if you are ok with the changes and I'll add the tag.
Changes since v1:
* Dropped requirement for node name.
* Dropped compatible strings for different revisions - rely on peripheral id register to provide the revision.
* Change interrupt bindings to require overflow interrupt per counter.
Documentation/devicetree/bindings/arm/cci.txt | 44 ++ drivers/bus/arm-cci.c | 628 +++++++++++++++++++++++++ 2 files changed, 672 insertions(+)
Documentation/devicetree/bindings/arm/cci.txt +44 -0
@@ -79,6 +79,40 @@ specific to ARM. corresponding interface programming registers. + - CCI PMU node + + Parent node must be CCI interconnect node. + + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be "arm,cci-400-pmu" + + - reg: + Usage: required + Value type: <prop-encoded-array> + Definition: the base address and size of the + corresponding interface programming + registers. + + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of counter overflow + interrupts, one per counter. The interrupts + must be specified starting with the cycle + counter overflow interrupt, followed by + counter0 overflow interrupt, counter1 + overflow interrupt,..., counterN overflow + interrupt. + + The CCI PMU has an interrupt signal for each + counter. Typically, the number of + interrupts will be equal to the number of + counters. + * CCI interconnect bus masters Description: masters in the device tree connected to a CCI port
@@ -163,6 +197,16 @@ Example: interface-type = "ace"; reg = <0x5000 0x1000>; }; + + pmu at 9000 { + compatible = "arm,cci-400-pmu,rev0"; + reg = <0x9000 0x5000>; + interrupts = <0 101 4>, + <0 102 4>, + <0 103 4>, + <0 104 4>, + <0 105 4>; + }; }; This CCI node corresponds to a CCI component whose control registers sits
drivers/bus/arm-cci.c +628 -0
656 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 6 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
I think this binding addresses my comments, thanks. Just one comment below:
Documentation/devicetree/bindings/arm/cci.txt specific to ARM.
Quoted 9 lines
Documentation/devicetree/bindings/arm/cci.txt +6
-0+ - reg: + Usage: required + Value type: <prop-encoded-array> + - interrupts: + Usage: required + Value type: <prop-encoded-array>
That makes it sound like the layout/content of those two properties is the same. That's not true; one is an array of (base, size) cells, and the other is of (phandle, args*) cells. The difference between the data being phandles-vs-integers seems important.
Perhaps says:
Value type: Integer cells. Array of register entries, each expressed as a pair of cells, containing base and size.
Value type: Integer cells. Array of interrupt specifier entries, as defined in ../interrupt-controller/interupts.txt.
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 103
Quoted 1 line
Documentation/devicetree/bindings/arm/cci.txt +1
-0+ Definition: comma-separated list of counter overflow
Oh, and lists of cells aren't necessarily comma-separated; comma is used between <> but not inside <>, and there's no requirement that each individual interrupt specifier be in its own <>, vs. just aggregating all of them into a single <>.
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
Hi Stephen,
Thanks for the helpful comments.
Quoted 25 lines · Stephen Warren
Quoted 6 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
I think this binding addresses my comments, thanks. Just one comment below:
Documentation/devicetree/bindings/arm/cci.txt specific to ARM.
Quoted 9 lines
Documentation/devicetree/bindings/arm/cci.txt +6
-0+ - reg: + Usage: required + Value type: <prop-encoded-array> + - interrupts: + Usage: required + Value type: <prop-encoded-array>That makes it sound like the layout/content of those two properties is the same. That's not true; one is an array of (base, size) cells, and the other is of (phandle, args*) cells. The difference between the data being phandles-vs-integers seems important.
Quoted 8 lines
Perhaps says:
Value type: Integer cells. Array of register entries, each expressed as a pair of cells, containing base and size.
Value type: Integer cells. Array of interrupt specifier entries, as defined in
../interrupt-controller/interupts.txt.
This is indeed better. I've updated the documentation for "interrupts" but am not sure about changing the "reg" property. The description used here is similar to other "reg" property description in the same file used for other CCI sub-nodes. Do you think this is sufficiently important clarification to change the other instances as well?
Quoted 7 lines
Documentation/devicetree/bindings/arm/cci.txt specific to ARM. · line 103
Quoted 1 line
Documentation/devicetree/bindings/arm/cci.txt +1
-0+ Definition: comma-separated list of counter overflowOh, and lists of cells aren't necessarily comma-separated; comma is used between <> but not inside <>, and there's no requirement that each individual interrupt specifier be in its own <>, vs. just aggregating all of them into a single <>.
I wasn't aware of this. I've now updated the text to remove "comma-separated". Thanks.
I'll post an updated version (including some of the improvements suggested here and in the other email by Kumar) soon. Please let me know if you'd prefer to change the description of all instances of "reg" in the file or keep this one as is.
Cheers,
Punit
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
Quoted 41 lines · Punit Agrawal
Hi Stephen,
Thanks for the helpful comments.
Quoted 27 lines · Stephen Warren
Quoted 6 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
I think this binding addresses my comments, thanks. Just one comment below:
Documentation/devicetree/bindings/arm/cci.txt specific to ARM.
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +6
-0+ - reg: + Usage: required + Value type: <prop-encoded-array> + - interrupts: + Usage: required + Value type: <prop-encoded-array>That makes it sound like the layout/content of those two properties is the same. That's not true; one is an array of (base, size) cells, and the other is of (phandle, args*) cells. The difference between the data being phandles-vs-integers seems important.
Quoted 8 lines
Perhaps says:
Value type: Integer cells. Array of register entries, each expressed as a pair of cells, containing base and size.
Value type: Integer cells. Array of interrupt specifier entries, as defined in
../interrupt-controller/interupts.txt.
Quoted 5 lines
This is indeed better. I've updated the documentation for "interrupts" but am not sure about changing the "reg" property. The description used here is similar to other "reg" property description in the same file used for other CCI sub-nodes. Do you think this is sufficiently important clarification to change the other instances as well?
Well, you may as well make all the descriptions consistent since you've explicitly attempted to describe the format of each property rather than just using the rather inexact text that's usually used.
Hopefully when we have a DT schema and bindings can inherit from each-other, only one single binding schema will actually have to specify the format of reg, and others will simply define how many entries to the property requires.
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Punit AgrawalQuoted 86 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Kumar Gala <[email protected]>Cc Stephen Warren <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>---
Hi,
This is the second version of the CCI PMU driver. The changes incorporate review comments on device tree bindings and corresponding update to the driver.
Cheers,
PunitWill, I've dropped your reviewed-by tag due to the changes. Let me know if you are ok with the changes and I'll add the tag.
Changes since v1:
* Dropped requirement for node name.
* Dropped compatible strings for different revisions - rely on peripheral id register to provide the revision.
* Change interrupt bindings to require overflow interrupt per counter.
Documentation/devicetree/bindings/arm/cci.txt| 44 ++drivers/bus/arm-cci.c| 628 +++++++++++++++++++++++++2 files changed, 672 insertions(+)Documentation/devicetree/bindings/arm/cci.txt +34
-0@@ -79,6 +79,40 @@ specific to ARM. corresponding interface programming registers. + - CCI PMU node + + Parent node must be CCI interconnect node. + + A CCI pmu node must contain the following properties: + + - compatible + Usage: required + Value type: <string> + Definition: must be "arm,cci-400-pmu" + + - reg: + Usage: required + Value type: <prop-encoded-array> + Definition: the base address and size of the + corresponding interface programming + registers. + + - interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: comma-separated list of counter overflow + interrupts, one per counter. The interrupts + must be specified starting with the cycle + counter overflow interrupt, followed by + counter0 overflow interrupt, counter1 + overflow interrupt,..., counterN overflow + interrupt. + + The CCI PMU has an interrupt signal for each + counter. Typically, the number of + interrupts will be equal to the number of + counters. + * CCI interconnect bus masters Description: masters in the device tree connected to a CCI port
Documentation/devicetree/bindings/arm/cci.txt Example: · line 202
Quoted 6 lines
Documentation/devicetree/bindings/arm/cci.txt +3
-0@@ -163,6 +197,16 @@ Example: interface-type = "ace"; reg = <0x5000 0x1000>; }; + + pmu at 9000 { + compatible = "arm,cci-400-pmu,rev0";
drop the ',rev0'
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +7
-0+ reg = <0x9000 0x5000>; + interrupts = <0 101 4>, + <0 102 4>, + <0 103 4>, + <0 104 4>, + <0 105 4>; + }; };This CCI node corresponds to a CCI component whose control registers sits
[snip]
- k
Signature · 2 lines
Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
Hi Kumar,
Quoted 3 lines · Kumar Gala
On Aug 16, 2013, at 12:19 PM, Punit Agrawal wrote:
[...]
Quoted 12 lines
Documentation/devicetree/bindings/arm/cci.txt Example: · line 202
Quoted 9 lines
Description: masters in the device tree connected to a CCI port
Documentation/devicetree/bindings/arm/cci.txt +3
-0@@ -163,6 +197,16 @@ Example: interface-type = "ace"; reg = <0x5000 0x1000>; }; + + pmu at 9000 { + compatible = "arm,cci-400-pmu,rev0";drop the ',rev0'
Missed this instance in the update. Now taken care of.
Will post the next (and hopefully last) version addressing Stephen's comments as well.
Can I use either your Acked-by or Reviewd-by with these changes in place?
Cheers,
Punit
Quoted 15 lines
Quoted 10 lines
Documentation/devicetree/bindings/arm/cci.txt +7
-0+ reg = <0x9000 0x5000>; + interrupts = <0 101 4>, + <0 102 4>, + <0 103 4>, + <0 104 4>, + <0 105 4>; + }; };This CCI node corresponds to a CCI component whose control registers sits
[snip]
- k
PATCH v2 1/1 drivers: CCI: add ARM CCI PMU support
In reply to Punit AgrawalQuoted 21 lines · Punit Agrawal
The CCI PMU can profile bus transactions at the master and slave interfaces of the CCI. The PMU can be used to observe an aggregated view of the bus traffic between the various components connected to the CCI.
Extend the existing CCI driver to support the PMU by registering a perf backend for it.
Document the device tree binding to describe the CCI PMU.
Cc Lorenzo Pieralisi <[email protected]>Cc Nicolas Pitre <[email protected]>Cc Dave Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Kumar Gala <[email protected]>Cc Stephen Warren <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Punit Agrawal <[email protected]>---
Hi,
Quoted 8 lines
This is the second version of the CCI PMU driver. The changes incorporate review comments on device tree bindings and corresponding update to the driver.
Cheers,
PunitWill, I've dropped your reviewed-by tag due to the changes. Let me know if you are ok with the changes and I'll add the tag.
I'm not the right person to review the DT binding, but the actual driver code is fine. You can either:
1. Add my reviewed-by once you have an Ack from a DT maintainer
or
2. Split the patch in two (binding + driver) and add my reviewed-by to the driver part
Either way, once you have the DT part sorted out, please send a fresh version with the relevant tags and stick me on CC.
Cheers,
Will