Skip to contentSkip to conversation

Add CCI PMU support

PATCH New Punit Agrawal 5 participants 15 messages Aug 22, 2013 – Sep 5, 2013 Device tree bindings

lore
Share & tools
lore
#0 lore raw

v3 cover Add CCI PMU support

Hi,

Here's the third version of patches to support the CCI PMU in the devicetree and the driver. The patch was split up to better reflect the split in maintainership between the bindings and the code.

I've addressed all comments raised so far. So if things look fine, I would like to get Acks from the devicetree maintainers.

Thanks,
Punit

Changes since v2:
(posted at http://article.gmane.org/gmane.linux.ports.arm.kernel/261189)
* Improved the description of value type properties for all CCI nodes.
* Corrected example in bindings to use valid compatible string.
* Tightened the description of CCI PMU interrupts bindings.

Changes since v1:
(posted at http://article.gmane.org/gmane.linux.ports.arm.kernel/259542)
* 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.

Punit Agrawal (3):
ARM: dts: Improve CCI bindings documentation.
ARM: dts: Document the CCI PMU DT bindings
drivers: CCI: add ARM CCI PMU support

 Documentation/devicetree/bindings/arm/cci.txt |   58 ++-
 drivers/bus/arm-cci.c                         |  628 +++++++++++++++++++++++++
 2 files changed, 683 insertions(+), 3 deletions(-)
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH v3 1/3 ARM: dts: Improve CCI bindings documentation.

Improve the description of the value types used in the CCI binding.

Cc devicetree at vger.kernel.org
Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Rob Herring <[email protected]>
---
 Documentation/devicetree/bindings/arm/cci.txt |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/arm/cci.txt +9 -3
index 92d36e2..024c03d 100644
@@ -36,14 +36,18 @@ specific to ARM.
 
 	- reg
 		Usage: required
-		Value type: <prop-encoded-array>
+		Value type: Integer cells. A register entry, expressed as a pair
+			    of cells, containing base and size.
 		Definition: A standard property. Specifies base physical
 			    address of CCI control registers common to all
 			    interfaces.
 
 	- ranges:
 		Usage: required
-		Value type: <prop-encoded-array>
+		Value type: Integer cells. An array of range entries, expressed
+			    as a tuple of cells, containing child address,
+			    parent address and the size of the region in the
+			    child address space.
 		Definition: A standard property. Follow rules in the ePAPR for
 			    hierarchical bus addressing. CCI interfaces
 			    addresses refer to the parent node addressing
@@ -74,7 +78,9 @@ specific to ARM.
 
 		- reg:
 			Usage: required
-			Value type: <prop-encoded-array>
+			Value type: Integer cells. A register entry, expressed
+				    as a pair of cells, containing base and
+				    size.
 			Definition: the base address and size of the
 				    corresponding interface programming
 				    registers.
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

In reply to 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.

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

Cc devicetree at vger.kernel.org
Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Rob Herring <[email protected]>
---
 Documentation/devicetree/bindings/arm/cci.txt |   46 +++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
Documentation/devicetree/bindings/arm/cci.txt +46 -0
index 024c03d..551119e 100644
@@ -85,6 +85,42 @@ 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: Integer cells. A register entry, expressed
+				    as a pair of cells, containing base and
+				    size.
+			Definition: the base address and size of the
+				    corresponding interface programming
+				    registers.
+
+		- interrupts:
+			Usage: required
+			Value type: Integer cells. Array of interrupt specifier
+				    entries, as defined in
+				    ../interrupt-controller/interrupts.txt.
+			Definition: 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. The number of interrupts must be
+				    equal to the number of counters.
+
 * CCI interconnect bus masters
 
 	Description: masters in the device tree connected to a CCI port
@@ -169,6 +205,16 @@ Example:
 			interface-type = "ace";
 			reg = <0x5000 0x1000>;
 		};
+
+		pmu at 9000 {
+			 compatible = "arm,cci-400-pmu";
+			 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
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

Quoted 7 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.

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

This also seems fine to me.

#5 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

Hi Stephen,

Quoted 6 lines · Stephen Warren
Quoted 5 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.

Quoted 6 lines
Quoted 2 lines

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

This also seems fine to me.

Thanks for the Ack on both the patches. I'll get these queued for either 3.12 or if it is too late for that then 3.13.

Cheers,
Punit

#6 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

In reply to Punit Agrawal
Quoted 73 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.

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

Cc devicetree at vger.kernel.org
Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Rob Herring <[email protected]>
---
 Documentation/devicetree/bindings/arm/cci.txt |   46 +++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
Documentation/devicetree/bindings/arm/cci.txt +39 -0
index 024c03d..551119e 100644
@@ -85,6 +85,42 @@ 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: Integer cells. A register entry, expressed
+				    as a pair of cells, containing base and
+				    size.
+			Definition: the base address and size of the
+				    corresponding interface programming
+				    registers.
+
+		- interrupts:
+			Usage: required
+			Value type: Integer cells. Array of interrupt specifier
+				    entries, as defined in
+				    ../interrupt-controller/interrupts.txt.
+			Definition: 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. The number of interrupts must be
+				    equal to the number of counters.
+
 * CCI interconnect bus masters
 
 	Description: masters in the device tree connected to a CCI port
@@ -169,6 +205,16 @@ Example:
 			interface-type = "ace";
 			reg = <0x5000 0x1000>;
 		};
+
+		pmu at 9000 {
+			 compatible = "arm,cci-400-pmu";

Documentation/devicetree/bindings/arm/cci.txt Example: · line 220

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

It's not shown in the diff above, but the documentation gives the following for the cci node:

		ranges = <0x0 0x0 0x2c090000 0x6000>;

which doesn't include the PMU nodes, so should that 0x6000 also be changed to 0x10000 (the full range of addresses in the TRM).

Signature · 1 line
Tixy
#7 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

Hi Jon,

Quoted 89 lines · Jon Medhurst (Tixy)
Quoted 73 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.

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

Cc devicetree at vger.kernel.org
Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Rob Herring <[email protected]>
---
  Documentation/devicetree/bindings/arm/cci.txt |   46 +++++++++++++++++++++++++
  1 file changed, 46 insertions(+)
Documentation/devicetree/bindings/arm/cci.txt +39 -0
index 024c03d..551119e 100644
@@ -85,6 +85,42 @@ 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: Integer cells. A register entry, expressed
+				    as a pair of cells, containing base and
+				    size.
+			Definition: the base address and size of the
+				    corresponding interface programming
+				    registers.
+
+		- interrupts:
+			Usage: required
+			Value type: Integer cells. Array of interrupt specifier
+				    entries, as defined in
+				    ../interrupt-controller/interrupts.txt.
+			Definition: 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. The number of interrupts must be
+				    equal to the number of counters.
+
  * CCI interconnect bus masters
 
  	Description: masters in the device tree connected to a CCI port
@@ -169,6 +205,16 @@ Example:
  			interface-type = "ace";
  			reg = <0x5000 0x1000>;
  		};
+
+		pmu at 9000 {
+			 compatible = "arm,cci-400-pmu";

Documentation/devicetree/bindings/arm/cci.txt Example: · line 220

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

It's not shown in the diff above, but the documentation gives the following for the cci node:

Quoted 5 lines
		ranges = <0x0 0x0 0x2c090000 0x6000>;

which doesn't include the PMU nodes, so should that 0x6000 also be changed to 0x10000 (the full range of addresses in the TRM).

You're right. Now that I am adding support for the PMU, this range should be extended to include the rest of the CCI addresses. I'll update it in the next version.

Cheers,
Punit

#8 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

In reply to Punit Agrawal
Quoted 15 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.

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

Cc devicetree at vger.kernel.org
Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Rob Herring <[email protected]>
Quoted 3 lines

---
Documentation/devicetree/bindings/arm/cci.txt | 46 +++++++++++++++++++++++++

1 file changed, 46 insertions(+)

- 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
#9 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

(adding Will, as these patches will be going via his tree.)

Hi Will,

Quoted 22 lines · Kumar Gala
Quoted 15 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.

Introduce a binding for the CCI PMU. The PMU node will be a sub-node of the CCI node.

Cc devicetree at vger.kernel.org
Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Rob Herring <[email protected]>
Quoted 3 lines

---
Documentation/devicetree/bindings/arm/cci.txt | 46 +++++++++++++++++++++++++

1 file changed, 46 insertions(+)
Quoted 2 lines

Can you pick Kumar's ack for the second patch before you send a pull request for the CCI PMU patches? Let me know if you prefer for me to post patches with the Acks applied.

Cheers,
Punit

Quoted 2 lines

- k

#10 lore raw

PATCH v3 2/3 ARM: dts: Document the CCI PMU DT bindings

Quoted 7 lines · Punit Agrawal
Quoted 2 lines · Kumar Gala

Can you pick Kumar's ack for the second patch before you send a pull request for the CCI PMU patches? Let me know if you prefer for me to post patches with the Acks applied.

That's fine, I've added the ack.

Will

#11 lore raw

PATCH v3 3/3 drivers: CCI: add ARM CCI PMU support

In reply to Punit Agrawal

Extend the existing CCI driver to support the PMU by registering a perf backend for it.

Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Dave Martin <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/bus/arm-cci.c |  628 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 628 insertions(+)
drivers/bus/arm-cci.c +628 -0
index 7332889..ddc36f6 100644
@@ -18,11 +18,21 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 
 #include <asm/cacheflush.h>
+#include <asm/irq_regs.h>
+#include <asm/pmu.h>
 #include <asm/smp_plat.h>
 
+#define DRIVER_NAME		"CCI-400"
+#define DRIVER_NAME_PMU		DRIVER_NAME " PMU"
+#define PMU_NAME		"CCI_400"
+
 #define CCI_PORT_CTRL		0x0
 #define CCI_CTRL_STATUS		0xc
 
@@ -54,6 +64,587 @@ static unsigned int nb_cci_ports;
 static void __iomem *cci_ctrl_base;
 static unsigned long cci_ctrl_phys;
 
+#ifdef CONFIG_HW_PERF_EVENTS
+
+#define CCI_PMCR		0x0100
+#define CCI_PID2		0x0fe8
+
+#define CCI_PMCR_CEN		0x00000001
+#define CCI_PMCR_NCNT_MASK	0x0000f800
+#define CCI_PMCR_NCNT_SHIFT	11
+
+#define CCI_PID2_REV_MASK	0xf0
+#define CCI_PID2_REV_SHIFT	4
+
+/* Port ids */
+#define CCI_PORT_S0	0
+#define CCI_PORT_S1	1
+#define CCI_PORT_S2	2
+#define CCI_PORT_S3	3
+#define CCI_PORT_S4	4
+#define CCI_PORT_M0	5
+#define CCI_PORT_M1	6
+#define CCI_PORT_M2	7
+
+#define CCI_REV_R0		0
+#define CCI_REV_R1		1
+#define CCI_REV_R0_P4		4
+#define CCI_REV_R1_P2		6
+
+#define CCI_PMU_EVT_SEL		0x000
+#define CCI_PMU_CNTR		0x004
+#define CCI_PMU_CNTR_CTRL	0x008
+#define CCI_PMU_OVRFLW		0x00c
+
+#define CCI_PMU_OVRFLW_FLAG	1
+
+#define CCI_PMU_CNTR_BASE(idx)	((idx) * SZ_4K)
+
+/*
+ * Instead of an event id to monitor CCI cycles, a dedicated counter is
+ * provided. Use 0xff to represent CCI cycles and hope that no future revisions
+ * make use of this event in hardware.
+ */
+enum cci400_perf_events {
+	CCI_PMU_CYCLES = 0xff
+};
+
+#define CCI_PMU_EVENT_MASK		0xff
+#define CCI_PMU_EVENT_SOURCE(event)	((event >> 5) & 0x7)
+#define CCI_PMU_EVENT_CODE(event)	(event & 0x1f)
+
+#define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
+
+#define CCI_PMU_CYCLE_CNTR_IDX		0
+#define CCI_PMU_CNTR0_IDX		1
+#define CCI_PMU_CNTR_LAST(cci_pmu)	(CCI_PMU_CYCLE_CNTR_IDX + cci_pmu->num_events - 1)
+
+/*
+ * CCI PMU event id is an 8-bit value made of two parts - bits 7:5 for one of 8
+ * ports and bits 4:0 are event codes. There are different event codes
+ * associated with each port type.
+ *
+ * Additionally, the range of events associated with the port types changed
+ * between Rev0 and Rev1.
+ *
+ * The constants below define the range of valid codes for each port type for
+ * the different revisions and are used to validate the event to be monitored.
+ */
+
+#define CCI_REV_R0_SLAVE_PORT_MIN_EV	0x00
+#define CCI_REV_R0_SLAVE_PORT_MAX_EV	0x13
+#define CCI_REV_R0_MASTER_PORT_MIN_EV	0x14
+#define CCI_REV_R0_MASTER_PORT_MAX_EV	0x1a
+
+#define CCI_REV_R1_SLAVE_PORT_MIN_EV	0x00
+#define CCI_REV_R1_SLAVE_PORT_MAX_EV	0x14
+#define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
+#define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
+
+struct pmu_port_event_ranges {
+	u8 slave_min;
+	u8 slave_max;
+	u8 master_min;
+	u8 master_max;
+};
+
+static struct pmu_port_event_ranges port_event_range[] = {
+	[CCI_REV_R0] = {
+		.slave_min = CCI_REV_R0_SLAVE_PORT_MIN_EV,
+		.slave_max = CCI_REV_R0_SLAVE_PORT_MAX_EV,
+		.master_min = CCI_REV_R0_MASTER_PORT_MIN_EV,
+		.master_max = CCI_REV_R0_MASTER_PORT_MAX_EV,
+	},
+	[CCI_REV_R1] = {
+		.slave_min = CCI_REV_R1_SLAVE_PORT_MIN_EV,
+		.slave_max = CCI_REV_R1_SLAVE_PORT_MAX_EV,
+		.master_min = CCI_REV_R1_MASTER_PORT_MIN_EV,
+		.master_max = CCI_REV_R1_MASTER_PORT_MAX_EV,
+	},
+};
+
+struct cci_pmu_drv_data {
+	void __iomem *base;
+	struct arm_pmu *cci_pmu;
+	int nr_irqs;
+	int irqs[CCI_PMU_MAX_HW_EVENTS];
+	unsigned long active_irqs;
+	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
+	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
+	struct pmu_port_event_ranges *port_ranges;
+	struct pmu_hw_events hw_events;
+};
+static struct cci_pmu_drv_data *pmu;
+
+static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++)
+		if (irq == irqs[i])
+			return true;
+
+	return false;
+}
+
+static int probe_cci_revision(void)
+{
+	int rev;
+	rev = readl_relaxed(cci_ctrl_base + CCI_PID2) & CCI_PID2_REV_MASK;
+	rev >>= CCI_PID2_REV_SHIFT;
+
+	if (rev <= CCI_REV_R0_P4)
+		return CCI_REV_R0;
+	else if (rev <= CCI_REV_R1_P2)
+		return CCI_REV_R1;
+
+	return -ENOENT;
+}
+
+static struct pmu_port_event_ranges *port_range_by_rev(void)
+{
+	int rev = probe_cci_revision();
+
+	if (rev < 0)
+		return NULL;
+
+	return &port_event_range[rev];
+}
+
+static int pmu_is_valid_slave_event(u8 ev_code)
+{
+	return pmu->port_ranges->slave_min <= ev_code &&
+		ev_code <= pmu->port_ranges->slave_max;
+}
+
+static int pmu_is_valid_master_event(u8 ev_code)
+{
+	return pmu->port_ranges->master_min <= ev_code &&
+		ev_code <= pmu->port_ranges->master_max;
+}
+
+static int pmu_validate_hw_event(u8 hw_event)
+{
+	u8 ev_source = CCI_PMU_EVENT_SOURCE(hw_event);
+	u8 ev_code = CCI_PMU_EVENT_CODE(hw_event);
+
+	switch (ev_source) {
+	case CCI_PORT_S0:
+	case CCI_PORT_S1:
+	case CCI_PORT_S2:
+	case CCI_PORT_S3:
+	case CCI_PORT_S4:
+		/* Slave Interface */
+		if (pmu_is_valid_slave_event(ev_code))
+			return hw_event;
+		break;
+	case CCI_PORT_M0:
+	case CCI_PORT_M1:
+	case CCI_PORT_M2:
+		/* Master Interface */
+		if (pmu_is_valid_master_event(ev_code))
+			return hw_event;
+		break;
+	}
+
+	return -ENOENT;
+}
+
+static int pmu_is_valid_counter(struct arm_pmu *cci_pmu, int idx)
+{
+	return CCI_PMU_CYCLE_CNTR_IDX <= idx &&
+		idx <= CCI_PMU_CNTR_LAST(cci_pmu);
+}
+
+static u32 pmu_read_register(int idx, unsigned int offset)
+{
+	return readl_relaxed(pmu->base + CCI_PMU_CNTR_BASE(idx) + offset);
+}
+
+static void pmu_write_register(u32 value, int idx, unsigned int offset)
+{
+	return writel_relaxed(value, pmu->base + CCI_PMU_CNTR_BASE(idx) + offset);
+}
+
+static void pmu_disable_counter(int idx)
+{
+	pmu_write_register(0, idx, CCI_PMU_CNTR_CTRL);
+}
+
+static void pmu_enable_counter(int idx)
+{
+	pmu_write_register(1, idx, CCI_PMU_CNTR_CTRL);
+}
+
+static void pmu_set_event(int idx, unsigned long event)
+{
+	event &= CCI_PMU_EVENT_MASK;
+	pmu_write_register(event, idx, CCI_PMU_EVT_SEL);
+}
+
+static u32 pmu_get_max_counters(void)
+{
+	u32 n_cnts = (readl_relaxed(cci_ctrl_base + CCI_PMCR) &
+		      CCI_PMCR_NCNT_MASK) >> CCI_PMCR_NCNT_SHIFT;
+
+	/* add 1 for cycle counter */
+	return n_cnts + 1;
+}
+
+static struct pmu_hw_events *pmu_get_hw_events(void)
+{
+	return &pmu->hw_events;
+}
+
+static int pmu_get_event_idx(struct pmu_hw_events *hw, struct perf_event *event)
+{
+	struct arm_pmu *cci_pmu = to_arm_pmu(event->pmu);
+	struct hw_perf_event *hw_event = &event->hw;
+	unsigned long cci_event = hw_event->config_base & CCI_PMU_EVENT_MASK;
+	int idx;
+
+	if (cci_event == CCI_PMU_CYCLES) {
+		if (test_and_set_bit(CCI_PMU_CYCLE_CNTR_IDX, hw->used_mask))
+			return -EAGAIN;
+
+		return CCI_PMU_CYCLE_CNTR_IDX;
+	}
+
+	for (idx = CCI_PMU_CNTR0_IDX; idx <= CCI_PMU_CNTR_LAST(cci_pmu); ++idx)
+		if (!test_and_set_bit(idx, hw->used_mask))
+			return idx;
+
+	/* No counters available */
+	return -EAGAIN;
+}
+
+static int pmu_map_event(struct perf_event *event)
+{
+	int mapping;
+	u8 config = event->attr.config & CCI_PMU_EVENT_MASK;
+
+	if (event->attr.type < PERF_TYPE_MAX)
+		return -ENOENT;
+
+	if (config == CCI_PMU_CYCLES)
+		mapping = config;
+	else
+		mapping = pmu_validate_hw_event(config);
+
+	return mapping;
+}
+
+static int pmu_request_irq(struct arm_pmu *cci_pmu, irq_handler_t handler)
+{
+	int i;
+	struct platform_device *pmu_device = cci_pmu->plat_device;
+
+	if (unlikely(!pmu_device))
+		return -ENODEV;
+
+	if (pmu->nr_irqs < 1) {
+		dev_err(&pmu_device->dev, "no irqs for CCI PMUs defined\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * Register all available CCI PMU interrupts. In the interrupt handler
+	 * we iterate over the counters checking for interrupt source (the
+	 * overflowing counter) and clear it.
+	 *
+	 * This should allow handling of non-unique interrupt for the counters.
+	 */
+	for (i = 0; i < pmu->nr_irqs; i++) {
+		int err = request_irq(pmu->irqs[i], handler, IRQF_SHARED,
+				"arm-cci-pmu", cci_pmu);
+		if (err) {
+			dev_err(&pmu_device->dev, "unable to request IRQ%d for ARM CCI PMU counters\n",
+				pmu->irqs[i]);
+			return err;
+		}
+
+		set_bit(i, &pmu->active_irqs);
+	}
+
+	return 0;
+}
+
+static irqreturn_t pmu_handle_irq(int irq_num, void *dev)
+{
+	unsigned long flags;
+	struct arm_pmu *cci_pmu = (struct arm_pmu *)dev;
+	struct pmu_hw_events *events = cci_pmu->get_hw_events();
+	struct perf_sample_data data;
+	struct pt_regs *regs;
+	int idx, handled = IRQ_NONE;
+
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+	regs = get_irq_regs();
+	/*
+	 * Iterate over counters and update the corresponding perf events.
+	 * This should work regardless of whether we have per-counter overflow
+	 * interrupt or a combined overflow interrupt.
+	 */
+	for (idx = CCI_PMU_CYCLE_CNTR_IDX; idx <= CCI_PMU_CNTR_LAST(cci_pmu); idx++) {
+		struct perf_event *event = events->events[idx];
+		struct hw_perf_event *hw_counter;
+
+		if (!event)
+			continue;
+
+		hw_counter = &event->hw;
+
+		/* Did this counter overflow? */
+		if (!pmu_read_register(idx, CCI_PMU_OVRFLW) & CCI_PMU_OVRFLW_FLAG)
+			continue;
+
+		pmu_write_register(CCI_PMU_OVRFLW_FLAG, idx, CCI_PMU_OVRFLW);
+
+		handled = IRQ_HANDLED;
+
+		armpmu_event_update(event);
+		perf_sample_data_init(&data, 0, hw_counter->last_period);
+		if (!armpmu_event_set_period(event))
+			continue;
+
+		if (perf_event_overflow(event, &data, regs))
+			cci_pmu->disable(event);
+	}
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+
+	return IRQ_RETVAL(handled);
+}
+
+static void pmu_free_irq(struct arm_pmu *cci_pmu)
+{
+	int i;
+
+	for (i = 0; i < pmu->nr_irqs; i++) {
+		if (!test_and_clear_bit(i, &pmu->active_irqs))
+			continue;
+
+		free_irq(pmu->irqs[i], cci_pmu);
+	}
+}
+
+static void pmu_enable_event(struct perf_event *event)
+{
+	unsigned long flags;
+	struct arm_pmu *cci_pmu = to_arm_pmu(event->pmu);
+	struct pmu_hw_events *events = cci_pmu->get_hw_events();
+	struct hw_perf_event *hw_counter = &event->hw;
+	int idx = hw_counter->idx;
+
+	if (unlikely(!pmu_is_valid_counter(cci_pmu, idx))) {
+		dev_err(&cci_pmu->plat_device->dev, "Invalid CCI PMU counter %d\n", idx);
+		return;
+	}
+
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+
+	/* Configure the event to count, unless you are counting cycles */
+	if (idx != CCI_PMU_CYCLE_CNTR_IDX)
+		pmu_set_event(idx, hw_counter->config_base);
+
+	pmu_enable_counter(idx);
+
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
+static void pmu_disable_event(struct perf_event *event)
+{
+	struct arm_pmu *cci_pmu = to_arm_pmu(event->pmu);
+	struct hw_perf_event *hw_counter = &event->hw;
+	int idx = hw_counter->idx;
+
+	if (unlikely(!pmu_is_valid_counter(cci_pmu, idx))) {
+		dev_err(&cci_pmu->plat_device->dev, "Invalid CCI PMU counter %d\n", idx);
+		return;
+	}
+
+	pmu_disable_counter(idx);
+}
+
+static void pmu_start(struct arm_pmu *cci_pmu)
+{
+	u32 val;
+	unsigned long flags;
+	struct pmu_hw_events *events = cci_pmu->get_hw_events();
+
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+
+	/* Enable all the PMU counters. */
+	val = readl_relaxed(cci_ctrl_base + CCI_PMCR) | CCI_PMCR_CEN;
+	writel(val, cci_ctrl_base + CCI_PMCR);
+
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
+static void pmu_stop(struct arm_pmu *cci_pmu)
+{
+	u32 val;
+	unsigned long flags;
+	struct pmu_hw_events *events = cci_pmu->get_hw_events();
+
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+
+	/* Disable all the PMU counters. */
+	val = readl_relaxed(cci_ctrl_base + CCI_PMCR) & ~CCI_PMCR_CEN;
+	writel(val, cci_ctrl_base + CCI_PMCR);
+
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
+static u32 pmu_read_counter(struct perf_event *event)
+{
+	struct arm_pmu *cci_pmu = to_arm_pmu(event->pmu);
+	struct hw_perf_event *hw_counter = &event->hw;
+	int idx = hw_counter->idx;
+	u32 value;
+
+	if (unlikely(!pmu_is_valid_counter(cci_pmu, idx))) {
+		dev_err(&cci_pmu->plat_device->dev, "Invalid CCI PMU counter %d\n", idx);
+		return 0;
+	}
+	value = pmu_read_register(idx, CCI_PMU_CNTR);
+
+	return value;
+}
+
+static void pmu_write_counter(struct perf_event *event, u32 value)
+{
+	struct arm_pmu *cci_pmu = to_arm_pmu(event->pmu);
+	struct hw_perf_event *hw_counter = &event->hw;
+	int idx = hw_counter->idx;
+
+	if (unlikely(!pmu_is_valid_counter(cci_pmu, idx)))
+		dev_err(&cci_pmu->plat_device->dev, "Invalid CCI PMU counter %d\n", idx);
+	else
+		pmu_write_register(value, idx, CCI_PMU_CNTR);
+}
+
+static int cci_pmu_init(struct arm_pmu *cci_pmu, struct platform_device *pdev)
+{
+	*cci_pmu = (struct arm_pmu){
+		.name             = PMU_NAME,
+		.max_period       = (1LLU << 32) - 1,
+		.get_hw_events    = pmu_get_hw_events,
+		.get_event_idx    = pmu_get_event_idx,
+		.map_event        = pmu_map_event,
+		.request_irq      = pmu_request_irq,
+		.handle_irq       = pmu_handle_irq,
+		.free_irq         = pmu_free_irq,
+		.enable           = pmu_enable_event,
+		.disable          = pmu_disable_event,
+		.start            = pmu_start,
+		.stop             = pmu_stop,
+		.read_counter     = pmu_read_counter,
+		.write_counter    = pmu_write_counter,
+	};
+
+	cci_pmu->plat_device = pdev;
+	cci_pmu->num_events = pmu_get_max_counters();
+
+	return armpmu_register(cci_pmu, -1);
+}
+
+static const struct of_device_id arm_cci_pmu_matches[] = {
+	{
+		.compatible = "arm,cci-400-pmu",
+	},
+	{},
+};
+
+static int cci_pmu_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	int i, ret, irq;
+
+	pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
+	if (!pmu)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_warn(&pdev->dev, "Failed to get mem resource\n");
+		ret = -EINVAL;
+		goto memalloc_err;
+	};
+
+	pmu->base = devm_ioremap_resource(&pdev->dev, res);
+	if (!pmu->base) {
+		dev_warn(&pdev->dev, "Failed to ioremap\n");
+		ret = -ENOMEM;
+		goto memalloc_err;
+	}
+
+	/*
+	 * CCI PMU has 5 overflow signals - one per counter; but some may be tied
+	 * together to a common interrupt.
+	 */
+	pmu->nr_irqs = 0;
+	for (i = 0; i < CCI_PMU_MAX_HW_EVENTS; i++) {
+		irq = platform_get_irq(pdev, i);
+		if (irq < 0)
+			break;
+
+		if (is_duplicate_irq(irq, pmu->irqs, pmu->nr_irqs))
+			continue;
+
+		pmu->irqs[pmu->nr_irqs++] = irq;
+	}
+
+	/*
+	 * Ensure that the device tree has as many interrupts as the number
+	 * of counters.
+	 */
+	if (i < CCI_PMU_MAX_HW_EVENTS) {
+		dev_warn(&pdev->dev, "In-correct number of interrupts: %d, should be %d\n",
+			i, CCI_PMU_MAX_HW_EVENTS);
+		ret = -EINVAL;
+		goto memalloc_err;
+	}
+
+	pmu->port_ranges = port_range_by_rev();
+	if (!pmu->port_ranges) {
+		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
+		ret = -EINVAL;
+		goto memalloc_err;
+	}
+
+	pmu->cci_pmu = devm_kzalloc(&pdev->dev, sizeof(*(pmu->cci_pmu)), GFP_KERNEL);
+	if (!pmu->cci_pmu) {
+		ret = -ENOMEM;
+		goto memalloc_err;
+	}
+
+	pmu->hw_events.events = pmu->events;
+	pmu->hw_events.used_mask = pmu->used_mask;
+	raw_spin_lock_init(&pmu->hw_events.pmu_lock);
+
+	ret = cci_pmu_init(pmu->cci_pmu, pdev);
+	if (ret)
+		goto pmuinit_err;
+
+	return 0;
+
+pmuinit_err:
+	kfree(pmu->cci_pmu);
+memalloc_err:
+	kfree(pmu);
+	return ret;
+}
+
+static int cci_platform_probe(struct platform_device *pdev)
+{
+	if (!cci_probed())
+		return -ENODEV;
+
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+#endif /* CONFIG_HW_PERF_EVENTS */
+
 struct cpu_port {
 	u64 mpidr;
 	u32 port;
@@ -516,6 +1107,42 @@ static int __init cci_init(void)
 	return cci_init_status;
 }
 
+#ifdef CONFIG_HW_PERF_EVENTS
+static struct platform_driver cci_pmu_driver = {
+	.driver = {
+		   .name = DRIVER_NAME_PMU,
+		   .of_match_table = arm_cci_pmu_matches,
+		  },
+	.probe = cci_pmu_probe,
+};
+
+static struct platform_driver cci_platform_driver = {
+	.driver = {
+		   .name = DRIVER_NAME,
+		   .of_match_table = arm_cci_matches,
+		  },
+	.probe = cci_platform_probe,
+};
+
+static int __init cci_platform_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&cci_pmu_driver);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&cci_platform_driver);
+}
+
+#else
+
+static int __init cci_platform_init(void)
+{
+	return 0;
+}
+
+#endif
 /*
  * To sort out early init calls ordering a helper function is provided to
  * check if the CCI driver has beed initialized. Function check if the driver
@@ -529,5 +1156,6 @@ bool __init cci_probed(void)
 EXPORT_SYMBOL_GPL(cci_probed);
 
 early_initcall(cci_init);
+core_initcall(cci_platform_init);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("ARM CCI support");
Signature · 1 line
1.7.10.4
#12 lore raw

PATCH v3 3/3 drivers: CCI: add ARM CCI PMU support

Hi Punit,

Quoted 12 lines · Punit Agrawal

Extend the existing CCI driver to support the PMU by registering a perf backend for it.

Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Dave Martin <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/bus/arm-cci.c |  628 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 628 insertions(+)

[...]

Review

Quoted 7 lines
(quoted diff) +7 -0
+static int cci_platform_probe(struct platform_device *pdev)
+{
+       if (!cci_probed())
+               return -ENODEV;
+
+       return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}

cci_probed is marked as __init, so you have a section mismatch here:

arch/arm/kernel/return_address.c:63:2: warning: #warning "TODO: return_address should use unwind tables" [-Wcpp]
WARNING: drivers/bus/built-in.o(.text+0xb48): Section mismatch in reference from the function cci_platform_probe() to the function .init.text:cci_probed() The function cci_platform_probe() references
the function __init cci_probed().
This is often because cci_platform_probe lacks a __init
annotation or the annotation of cci_probed is wrong.

Unfortunately, resolving this requires removing a bunch of annotations (see patch below), so you might want to reconsider how the probe checking is handled (by moving the cci_init_status check out of cci_init).

Will

--->8

drivers/bus/arm-cci.c +4 -4
index ddc36f6..4816759 100644
@@ -711,7 +711,7 @@ int cci_ace_get_port(struct device_node *dn)
 }
 EXPORT_SYMBOL_GPL(cci_ace_get_port);
 
-static void __init cci_ace_init_ports(void)
+static void cci_ace_init_ports(void)
 {
        int port, ac, cpu;
        u64 hwid;
@@ -991,7 +991,7 @@ static const struct of_device_id arm_cci_ctrl_if_matches[] = {
        {},
 };
 
-static int __init cci_probe(void)
+static int cci_probe(void)
 {
        struct cci_nb_ports const *cci_config;
        int ret, i, nb_ace = 0, nb_ace_lite = 0;
@@ -1095,7 +1095,7 @@ memalloc_err:
 static int cci_init_status = -EAGAIN;
 static DEFINE_MUTEX(cci_probing);
 
-static int __init cci_init(void)
+static int cci_init(void)
 {
        if (cci_init_status != -EAGAIN)
                return cci_init_status;
@@ -1149,7 +1149,7 @@ static int __init cci_platform_init(void)
  * has been initialized, if not it calls the init function that probes
  * the driver and updates the return value.
  */
-bool __init cci_probed(void)
+bool cci_probed(void)
 {
        return cci_init() == 0;
 }
#13 lore raw

PATCH v3 3/3 drivers: CCI: add ARM CCI PMU support

Quoted 28 lines · Will Deacon

Hi Punit,

Quoted 12 lines · Punit Agrawal

Extend the existing CCI driver to support the PMU by registering a perf backend for it.

Cc Lorenzo Pieralisi <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Dave Martin <[email protected]>
Cc Will Deacon <[email protected]>
---
  drivers/bus/arm-cci.c |  628 +++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 628 insertions(+)

[...]

Review

Quoted 7 lines
(quoted diff) +7 -0
+static int cci_platform_probe(struct platform_device *pdev)
+{
+       if (!cci_probed())
+               return -ENODEV;
+
+       return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}

cci_probed is marked as __init, so you have a section mismatch here:

Quoted 7 lines

arch/arm/kernel/return_address.c:63:2: warning: #warning "TODO: return_address should use unwind tables" [-Wcpp]
WARNING: drivers/bus/built-in.o(.text+0xb48): Section mismatch in reference from the function cci_platform_probe() to the function .init.text:cci_probed() The function cci_platform_probe() references
the function __init cci_probed().
This is often because cci_platform_probe lacks a __init
annotation or the annotation of cci_probed is wrong.

Good catch. Missed this one.

Quoted 4 lines

Unfortunately, resolving this requires removing a bunch of annotations (see patch below), so you might want to reconsider how the probe checking is handled (by moving the cci_init_status check out of cci_init).

In cci_platform_probe, cci_probed is called to check whether the CCI has been initialised, and also to initialise it if required.

Even if the check for cci_init_status is moved out, I'll still need to call cci_init which will again cause a section mis-match.

I think removing the annotations is the way to go. Is it OK to fold your patch into the CCI PMU patch, or you prefer to keep it separate?

Cheers,
Punit

Quoted 47 lines

Will

--->8

drivers/bus/arm-cci.c +4 -4
index ddc36f6..4816759 100644
@@ -711,7 +711,7 @@ int cci_ace_get_port(struct device_node *dn)
  }
  EXPORT_SYMBOL_GPL(cci_ace_get_port);
 
-static void __init cci_ace_init_ports(void)
+static void cci_ace_init_ports(void)
  {
         int port, ac, cpu;
         u64 hwid;
@@ -991,7 +991,7 @@ static const struct of_device_id arm_cci_ctrl_if_matches[] = {
         {},
  };
 
-static int __init cci_probe(void)
+static int cci_probe(void)
  {
         struct cci_nb_ports const *cci_config;
         int ret, i, nb_ace = 0, nb_ace_lite = 0;
@@ -1095,7 +1095,7 @@ memalloc_err:
  static int cci_init_status = -EAGAIN;
  static DEFINE_MUTEX(cci_probing);
 
-static int __init cci_init(void)
+static int cci_init(void)
  {
         if (cci_init_status != -EAGAIN)
                 return cci_init_status;
@@ -1149,7 +1149,7 @@ static int __init cci_platform_init(void)
   * has been initialized, if not it calls the init function that probes
   * the driver and updates the return value.
   */
-bool __init cci_probed(void)
+bool cci_probed(void)
  {
         return cci_init() == 0;
  }
#14 lore raw

PATCH v3 3/3 drivers: CCI: add ARM CCI PMU support

Quoted 18 lines · Punit Agrawal
Quoted 9 lines · Will Deacon

cci_probed is marked as __init, so you have a section mismatch here:

arch/arm/kernel/return_address.c:63:2: warning: #warning "TODO: return_address should use unwind tables" [-Wcpp]
WARNING: drivers/bus/built-in.o(.text+0xb48): Section mismatch in reference from the function cci_platform_probe() to the function .init.text:cci_probed() The function cci_platform_probe() references
the function __init cci_probed().
This is often because cci_platform_probe lacks a __init
annotation or the annotation of cci_probed is wrong.

Good catch. Missed this one.

Quoted 4 lines

Unfortunately, resolving this requires removing a bunch of annotations (see patch below), so you might want to reconsider how the probe checking is handled (by moving the cci_init_status check out of cci_init).

Quoted 5 lines

In cci_platform_probe, cci_probed is called to check whether the CCI has been initialised, and also to initialise it if required.

Even if the check for cci_init_status is moved out, I'll still need to call cci_init which will again cause a section mis-match.

Ok.

Quoted 2 lines

I think removing the annotations is the way to go. Is it OK to fold your patch into the CCI PMU patch, or you prefer to keep it separate?

I can just merge it in when I take your patches into my tree (which I'll do now...).

Cheers,

Will