RFC: ux500: add PMU resources

The ux500 SoCs have PMUs, both DB8500 and DB5500. However on the DB8500 the individual per-core IRQs are not routed: instead they are OR:ed into one single IRQ.

Cc Will Deacon <[email protected]>

Signed-off-by Linus Walleij <[email protected]>

---
Hi Will especially, I remember discussing this issue with you back in Florida. IIRC you had some idea on how to go about with this, like we add some tweak for this board where we don't set affinity and let the IRQ be declared chained and then each CPU it's not coming from has to NACK it, do you think it is feasible and could you sort of point me into the right files to poke at?

 arch/arm/mach-ux500/cpu-db5500.c |   26 ++++++++++++++++++++++++--
 arch/arm/mach-ux500/cpu-db8500.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

arch/arm/mach-ux500/cpu-db5500.c +23 -1

index af04e08..3912354 100644
@@ -11,6 +11,7 @@
 #include <linux/irq.h>
 
 #include <asm/mach/map.h>
+#include <asm/pmu.h>
 
 #include <plat/gpio.h>
 

@@ -43,6 +44,26 @@ static struct map_desc u5500_io_desc[] __initdata = {
 	__IO_DEV_DESC(U5500_PRCMU_BASE, SZ_4K),
 };
 
+static struct resource db5500_pmu_resources[] = {
+	[0] = {
+		.start		= IRQ_DB5500_PMU0,
+		.end		= IRQ_DB5500_PMU0,
+		.flags		= IORESOURCE_IRQ,
+	},
+	[1] = {
+		.start		= IRQ_DB5500_PMU1,
+		.end		= IRQ_DB5500_PMU1,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device db5500_pmu_device = {
+	.name			= "arm-pmu",
+	.id			= ARM_PMU_DEVICE_CPU,
+	.num_resources		= ARRAY_SIZE(db5500_pmu_resources),
+	.resource		= db5500_pmu_resources,
+};
+
 static struct resource mbox0_resources[] = {
 	{
 		.name = "mbox_peer",

@@ -127,7 +148,8 @@ static struct platform_device mbox2_device = {
 	.num_resources = ARRAY_SIZE(mbox2_resources),
 };
 
-static struct platform_device *u5500_platform_devs[] __initdata = {
+static struct platform_device *db5500_platform_devs[] __initdata = {
+	&db5500_pmu_device,
 	&mbox0_device,
 	&mbox1_device,
 	&mbox2_device,

3.) Rework the GIC code so that an IRQ can target multiple CPUs and remove
the distributor-level masking. I think this was originally done so that we can service different IRQs simultaneously, but with the deprecation of IRQF_DISABLED I'm not sure if this is still an issue. If not, then we can change to masking at the CPU interfaces which will make supporting your combined IRQ much easier.

Quoted 6 lines

3.) Rework the GIC code so that an IRQ can target multiple CPUs and remove
the distributor-level masking. I think this was originally done so that we can service different IRQs simultaneously, but with the deprecation of IRQF_DISABLED I'm not sure if this is still an issue. If not, then we can change to masking at the CPU interfaces which will make supporting your combined IRQ much easier.

If an interrupt is routed to multiple cores simultaneously, then you end up with a number of problems:

1. Each time an interrupt occurs, you wake up all CPUs.
2. One CPU wins the race and starts to handle the interrupt. The others are left spinning on a lock waiting. Eventually the lock is dropped and they too enter the handler.
3. Another race ensues on the drivers own spinlock. The winning CPU possibly holds this lock for the duration of its handling. Meanwhile the other CPUs are left spinning waiting for the lock to be dropped.
4. When the winning CPU drops the lock, it returns IRQ_HANDLED. The other CPUs find that there is no IRQ pending from the device, and return IRQ_NONE.

(4) may result in the spurious/unhandled IRQ code eventually disabling the interrupt.

Ah yes, I have some hazy memories of this conversation! I think you have three options:

Here's the implementation of a different approach which bounces the IRQ to the other CPU by setting the affinity when the current CPU would return IRQ_NONE.

arch/arm/kernel/perf_event.c +0 -0

index 5efa264..d07990e 100644
@@ -377,9 +377,27 @@ validate_group(struct perf_event *event)
 	return 0;

Quoted 4 lines

(quoted diff) +4 -0

+static irqreturn_t armpmu_bounce_interrupt(int irq, void *dev)
+{
+	irqreturn_t ret = armpmu->handle_irq(irq, dev);
+

Review

Quoted 4 lines

(quoted diff) +4 -0

+	if (ret == IRQ_NONE) {
+		int other = !smp_processor_id();
+		irq_set_affinity(irq, cpumask_of(other));
+	}

Will this work for edge-triggered IRQs? I don't think it will and I'd rather this code lived in some SoC-specific place than in the general ARM code. Maybe you could pass an IRQ callback in the platform data?

2.) Use IPI to cross-call the IRQ handler when the interrupted CPU doesn't ? ?have an interrupt asserted on its PMU. You'll need to sort out problems ? ?with re-enabling interrupts for the cross-call and also nesting of the ? ?PMU interrupt (because it could fire as a result of another PMU overflowing ? ?before you've completed the cross-call).

Quoted 5 lines · Will Deacon

2.) Use IPI to cross-call the IRQ handler when the interrupted CPU doesn't
have an interrupt asserted on its PMU. You'll need to sort out problems with re-enabling interrupts for the cross-call and also nesting of the PMU interrupt (because it could fire as a result of another PMU overflowing before you've completed the cross-call).

Here's the IPI version as well, for comparison with the other approach. The IRQ handling code will mask the interrupt before handling it, so it can't nest, can it?

8<----------
From 84e5936f9285fb03845ac04801430762c1c9b60d Mon Sep 17 00:00:00 2001
From: Rabin Vincent <[email protected]>
Date: Fri, 4 Feb 2011 11:59:26 +0530
Subject: [PATCH] ARM: perf_event: support dual-core with single PMU IRQ

DB8500 (dual-core) has the PMU interrupts of both cores ORed into one. Support this by keeping the interrupt directed at Core0 and IPIing Core1 when Core0 receives the interrupt and finds that its counter has not overflowed.

Signed-off-by Rabin Vincent <[email protected]>

---
 arch/arm/kernel/perf_event.c |   48 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 47 insertions(+), 1 deletions(-)

arch/arm/kernel/perf_event.c +47 -1

index 5efa264..a97e50f 100644
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
 #include <linux/uaccess.h>
+#include <linux/smp.h>
 
 #include <asm/cputype.h>
 #include <asm/irq.h>

@@ -377,9 +378,40 @@ validate_group(struct perf_event *event)
 	return 0;
 }
 
+static irqreturn_t armpmu_core2_irqret;
+
+/* Called via IPI on the second core */
+static void armpmu_kick(void *data)
+{
+	int irq = (int) data;
+
+	armpmu_core2_irqret = armpmu->handle_irq(irq, NULL);
+	smp_wmb();
+}
+
+static irqreturn_t armpmu_single_interrupt(int irq, void *dev)
+{
+	irqreturn_t irqret = armpmu->handle_irq(irq, dev);
+	int err;
+
+	if (irqret != IRQ_NONE)
+		return irqret;
+
+	local_irq_enable();
+	err = smp_call_function_single(1, armpmu_kick, (void *) irq, true);
+	local_irq_disable();
+
+	if (err)
+		return irqret;
+
+	smp_rmb();
+	return armpmu_core2_irqret;
+}
+
 static int
 armpmu_reserve_hardware(void)
 {
+	irq_handler_t irq_handler = armpmu->handle_irq;
 	int i, err = -ENODEV, irq;
 
 	pmu_device = reserve_pmu(ARM_PMU_DEVICE_CPU);

@@ -395,12 +427,26 @@ armpmu_reserve_hardware(void)
 		return -ENODEV;
 	}
 
+	/*
+	 * Some SoCs have the PMU IRQ lines of two cores wired together into a
+	 * single interrupt.  Support these by poking the second core with an
+	 * IPI when its counters overflow.
+	 */
+	if (pmu_device->num_resources < num_online_cpus()) {
+		if (num_online_cpus() > 2) {
+			pr_err(">2 cpus not supported for single-irq workaround");
+			return -ENODEV;
+		}
+
+		irq_handler = armpmu_single_interrupt;
+	}
+
 	for (i = 0; i < pmu_device->num_resources; ++i) {
 		irq = platform_get_irq(pmu_device, i);
 		if (irq < 0)
 			continue;
 
-		err = request_irq(irq, armpmu->handle_irq,
+		err = request_irq(irq, irq_handler,
 				  IRQF_DISABLED | IRQF_NOBALANCING,
 				  "armpmu", NULL);
 		if (err) {

Linus and I have already added our SoBs.

Signed-off-by Lee Jones<[email protected]>

Signed-off-by Linus Walleij<[email protected]>

Quoted 5 lines · Lee Jones

Linus and I have already added our SoBs.

Signed-off-by Lee Jones<[email protected]>

Signed-off-by Linus Walleij<[email protected]>

SoB is for the delivery path, I assume you mean s/Signed-off-by/Acked-by/g...

Yours,
Linus Walleij

Here's the IPI version as well, for comparison with the other approach. The IRQ handling code will mask the interrupt before handling it, so it can't nest, can it?

arch/arm/kernel/perf_event.c +1 -0

index 5efa264..a97e50f 100644
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
 #include <linux/uaccess.h>
+#include <linux/smp.h>