linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/4] Consolidating GIC per-cpu interrupts
@ 2011-08-09  9:56 Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 1/4] ARM: gic: consolidate PPI handling Marc Zyngier
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Marc Zyngier @ 2011-08-09  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

The current GIC per-cpu interrupts (aka PPIs) suffer from a number of
problems:

- They use a completely separate scheme to handle the interrupts,
  mostly because the PPI concept doesn't really match the kernel view
  of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level
  assembly code.
- The local timer code can only be used by devices generating PPIs,
  and not SPIs.
- At least one platform (msm) has started implementing its own
  alternative scheme.
- Some low-level code gets duplicated, as usual...

As the previous solution which tried to map PPIs to normal interrupts
has been proved to be buggy, I've opted to a much simpler scheme
(based on Russell's input).

The proposed solution is to handle the interrupt using the same path
as SPIs, with a common handler for all PPIs. Each PPI can be requested
using gic_request_ppi(), similar to request_irq(). The local timer
code is updated to reflect these changes.

Patches against v3.1-rc1 + Will Deacon's TWD patch ("ARM: twd:
register clockevents device before enabling PPI"). Tested on PB11MP,
VE, OMAP4 (Panda) and Tegra (Harmony). As this patch series is quite
different from the previous one, I've dropped all previous acks from
platform maintainers.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v11 1/4] ARM: gic: consolidate PPI handling
  2011-08-09  9:56 [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Marc Zyngier
@ 2011-08-09  9:56 ` Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 2/4] ARM: gic: Add PPI registration interface Marc Zyngier
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2011-08-09  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

PPI handling is a bit of an odd beast. It uses its own low level
handling code and is hardwired to the local timers (hence lacking
a registration interface).

Instead, switch the low handling to the normal SPI handling code.

This also allows the removal of some duplicated code.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: David Brown <davidb@codeaurora.org>
Cc: Bryan Huntsman <bryanh@codeaurora.org>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/common/gic.c                             |   69 +++++++++++++++++++-
 arch/arm/include/asm/entry-macro-multi.S          |    7 --
 arch/arm/include/asm/hardirq.h                    |    3 -
 arch/arm/include/asm/hardware/entry-macro-gic.S   |   19 +-----
 arch/arm/include/asm/localtimer.h                 |    6 +-
 arch/arm/include/asm/smp.h                        |    5 --
 arch/arm/kernel/irq.c                             |    3 -
 arch/arm/kernel/smp.c                             |   27 ++------
 arch/arm/mach-exynos4/include/mach/entry-macro.S  |    6 +--
 arch/arm/mach-msm/board-msm8x60.c                 |   11 ---
 arch/arm/mach-msm/include/mach/entry-macro-qgic.S |   73 +--------------------
 arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +----
 arch/arm/mach-shmobile/entry-intc.S               |    3 -
 arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
 14 files changed, 82 insertions(+), 167 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 3227ca9..8adc002 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,10 +28,14 @@
 #include <linux/smp.h>
 #include <linux/cpumask.h>
 #include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/percpu.h>
+#include <linux/slab.h>
 
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 #include <asm/hardware/gic.h>
+#include <asm/localtimer.h>
 
 static DEFINE_SPINLOCK(irq_controller_lock);
 
@@ -255,12 +259,39 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
+#ifdef CONFIG_LOCAL_TIMERS
+#define gic_ppi_handler		percpu_timer_handler
+#else
+static irqreturn_t gic_ppi_handler(int irq, void *dev_id)
+{
+	return IRQ_NONE;
+}
+#endif
+
+#define PPI_IRQACT(nr)						\
+	{							\
+		.handler	= gic_ppi_handler,		\
+		.flags		= IRQF_PERCPU | IRQF_TIMER,	\
+		.irq		= nr,				\
+		.name		= "PPI-" # nr,			\
+	}
+
+static struct irqaction ppi_irqaction_template[16] __initdata = {
+	PPI_IRQACT(0),  PPI_IRQACT(1),  PPI_IRQACT(2),  PPI_IRQACT(3),
+	PPI_IRQACT(4),  PPI_IRQACT(5),  PPI_IRQACT(6),  PPI_IRQACT(7),
+	PPI_IRQACT(8),  PPI_IRQACT(9),  PPI_IRQACT(10), PPI_IRQACT(11),
+	PPI_IRQACT(12), PPI_IRQACT(13), PPI_IRQACT(14), PPI_IRQACT(15),
+};
+
+static struct irqaction *ppi_irqaction;
+
 static void __init gic_dist_init(struct gic_chip_data *gic,
 	unsigned int irq_start)
 {
 	unsigned int gic_irqs, irq_limit, i;
 	void __iomem *base = gic->dist_base;
 	u32 cpumask = 1 << smp_processor_id();
+	u32 nrppis = 0, ppi_base = 0;
 
 	cpumask |= cpumask << 8;
 	cpumask |= cpumask << 16;
@@ -277,6 +308,28 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 		gic_irqs = 1020;
 
 	/*
+	 * Nobody would be insane enough to use PPIs on a secondary
+	 * GIC, right?
+	 */
+	if (gic == &gic_data[0]) {
+		nrppis = 16 - (irq_start & 15);
+		ppi_base = gic->irq_offset + 32 - nrppis;
+
+		ppi_irqaction = kmemdup(&ppi_irqaction_template[16 - nrppis],
+					sizeof(*ppi_irqaction) * nrppis,
+					GFP_KERNEL);
+
+		if (nrppis && !ppi_irqaction) {
+			pr_err("GIC: Can't allocate PPI memory");
+			nrppis = 0;
+			ppi_base = 0;
+		}
+	}
+
+	pr_info("Configuring GIC with %d sources (%d PPIs)\n",
+		gic_irqs, (gic == &gic_data[0]) ? nrppis : 0);
+
+	/*
 	 * Set all global interrupts to be level triggered, active low.
 	 */
 	for (i = 32; i < gic_irqs; i += 16)
@@ -311,7 +364,21 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	/*
 	 * Setup the Linux IRQ subsystem.
 	 */
-	for (i = irq_start; i < irq_limit; i++) {
+	for (i = 0; i < nrppis; i++) {
+		int ppi = i + ppi_base;
+		int err;
+
+		irq_set_chip_and_handler(ppi, &gic_chip, handle_percpu_irq);
+		irq_set_chip_data(ppi, gic);
+		irq_set_status_flags(ppi, IRQ_NOAUTOEN | IRQ_NOPROBE);
+
+		err = setup_irq(ppi, &ppi_irqaction[i]);
+		if (err)
+			pr_err("GIC: can't setup IRQ%d PPI%d (%d)\n",
+			       i, ppi, err);
+	}
+
+	for (i = irq_start + nrppis; i < irq_limit; i++) {
 		irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
 		irq_set_chip_data(i, gic);
 		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
diff --git a/arch/arm/include/asm/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index 2f1e209..88d6181 100644
--- a/arch/arm/include/asm/entry-macro-multi.S
+++ b/arch/arm/include/asm/entry-macro-multi.S
@@ -25,13 +25,6 @@
 	movne	r1, sp
 	adrne	lr, BSYM(1b)
 	bne	do_IPI
-
-#ifdef CONFIG_LOCAL_TIMERS
-	test_for_ltirq r0, r2, r6, lr
-	movne	r0, sp
-	adrne	lr, BSYM(1b)
-	bne	do_local_timer
-#endif
 #endif
 9997:
 	.endm
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index 89ad180..ddf07a9 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -9,9 +9,6 @@
 
 typedef struct {
 	unsigned int __softirq_pending;
-#ifdef CONFIG_LOCAL_TIMERS
-	unsigned int local_timer_irqs;
-#endif
 #ifdef CONFIG_SMP
 	unsigned int ipi_irqs[NR_IPI];
 #endif
diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index c115b82..74ebc80 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -22,15 +22,11 @@
  * interrupt controller spec.  To wit:
  *
  * Interrupts 0-15 are IPI
- * 16-28 are reserved
- * 29-31 are local.  We allow 30 to be used for the watchdog.
+ * 16-31 are local.  We allow 30 to be used for the watchdog.
  * 32-1020 are global
  * 1021-1022 are reserved
  * 1023 is "spurious" (no interrupt)
  *
- * For now, we ignore all local interrupts so only return an interrupt if it's
- * between 30 and 1020.  The test_for_ipi routine below will pick up on IPIs.
- *
  * A simple read from the controller will tell us the number of the highest
  * priority enabled interrupt.  We then just need to check whether it is in the
  * valid range for an IRQ (30-1020 inclusive).
@@ -43,7 +39,7 @@
 
 	ldr	\tmp, =1021
 	bic     \irqnr, \irqstat, #0x1c00
-	cmp     \irqnr, #29
+	cmp     \irqnr, #15
 	cmpcc	\irqnr, \irqnr
 	cmpne	\irqnr, \tmp
 	cmpcs	\irqnr, \irqnr
@@ -62,14 +58,3 @@
 	strcc	\irqstat, [\base, #GIC_CPU_EOI]
 	cmpcs	\irqnr, \irqnr
 	.endm
-
-/* As above, this assumes that irqstat and base are preserved.. */
-
-	.macro test_for_ltirq, irqnr, irqstat, base, tmp
-	bic	\irqnr, \irqstat, #0x1c00
-	mov 	\tmp, #0
-	cmp	\irqnr, #29
-	moveq	\tmp, #1
-	streq	\irqstat, [\base, #GIC_CPU_EOI]
-	cmp	\tmp, #0
-	.endm
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 080d74f..e3663f7 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -10,6 +10,8 @@
 #ifndef __ASM_ARM_LOCALTIMER_H
 #define __ASM_ARM_LOCALTIMER_H
 
+#include <linux/interrupt.h>
+
 struct clock_event_device;
 
 /*
@@ -18,9 +20,9 @@ struct clock_event_device;
 void percpu_timer_setup(void);
 
 /*
- * Called from assembly, this is the local timer IRQ handler
+ * Per-cpu timer IRQ handler
  */
-asmlinkage void do_local_timer(struct pt_regs *);
+irqreturn_t percpu_timer_handler(int irq, void *dev_id);
 
 
 #ifdef CONFIG_LOCAL_TIMERS
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index e42d96a..73ec155 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -88,9 +88,4 @@ extern void platform_cpu_enable(unsigned int cpu);
 extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 
-/*
- * show local interrupt info
- */
-extern void show_local_irqs(struct seq_file *, int);
-
 #endif /* ifndef __ASM_ARM_SMP_H */
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index de3dcab..8f24dc9 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -59,9 +59,6 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 #ifdef CONFIG_SMP
 	show_ipi_list(p, prec);
 #endif
-#ifdef CONFIG_LOCAL_TIMERS
-	show_local_irqs(p, prec);
-#endif
 	seq_printf(p, "%*s: %10lu\n", prec, "Err", irq_err_count);
 	return 0;
 }
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 8e60a4f..bf11ffe 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -453,10 +453,6 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
 	for (i = 0; i < NR_IPI; i++)
 		sum += __get_irq_stat(cpu, ipi_irqs[i]);
 
-#ifdef CONFIG_LOCAL_TIMERS
-	sum += __get_irq_stat(cpu, local_timer_irqs);
-#endif
-
 	return sum;
 }
 
@@ -474,29 +470,16 @@ static void ipi_timer(void)
 }
 
 #ifdef CONFIG_LOCAL_TIMERS
-asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs)
+irqreturn_t percpu_timer_handler(int irq, void *dev_id)
 {
-	struct pt_regs *old_regs = set_irq_regs(regs);
-	int cpu = smp_processor_id();
+	struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
 
 	if (local_timer_ack()) {
-		__inc_irq_stat(cpu, local_timer_irqs);
-		ipi_timer();
+		evt->event_handler(evt);
+		return IRQ_HANDLED;
 	}
 
-	set_irq_regs(old_regs);
-}
-
-void show_local_irqs(struct seq_file *p, int prec)
-{
-	unsigned int cpu;
-
-	seq_printf(p, "%*s: ", prec, "LOC");
-
-	for_each_present_cpu(cpu)
-		seq_printf(p, "%10u ", __get_irq_stat(cpu, local_timer_irqs));
-
-	seq_printf(p, " Local timer interrupts\n");
+	return IRQ_NONE;
 }
 #endif
 
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d7a1e28..807d05d 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -55,7 +55,7 @@
 
 		bic     \irqnr, \irqstat, #0x1c00
 
-		cmp     \irqnr, #29
+		cmp     \irqnr, #15
 		cmpcc	\irqnr, \irqnr
 		cmpne	\irqnr, \tmp
 		cmpcs	\irqnr, \irqnr
@@ -77,7 +77,3 @@
 		cmpcs	\irqnr, \irqnr
 		.endm
 
-		/* As above, this assumes that irqstat and base are preserved.. */
-
-		.macro test_for_ltirq, irqnr, irqstat, base, tmp
-		.endm
diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
index 1163b6f..d70a2f6 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -36,8 +36,6 @@ static void __init msm8x60_map_io(void)
 
 static void __init msm8x60_init_irq(void)
 {
-	unsigned int i;
-
 	gic_init(0, GIC_PPI_START, MSM_QGIC_DIST_BASE,
 		 (void *)MSM_QGIC_CPU_BASE);
 
@@ -49,15 +47,6 @@ static void __init msm8x60_init_irq(void)
 	 */
 	if (!machine_is_msm8x60_sim())
 		writel(0x0000FFFF, MSM_QGIC_DIST_BASE + GIC_DIST_ENABLE_SET);
-
-	/* FIXME: Not installing AVS_SVICINT and AVS_SVICINTSWDONE yet
-	 * as they are configured as level, which does not play nice with
-	 * handle_percpu_irq.
-	 */
-	for (i = GIC_PPI_START; i < GIC_SPI_START; i++) {
-		if (i != AVS_SVICINT && i != AVS_SVICINTSWDONE)
-			irq_set_handler(i, handle_percpu_irq);
-	}
 }
 
 static void __init msm8x60_init(void)
diff --git a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
index 1246715..717076f 100644
--- a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
+++ b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
@@ -8,81 +8,10 @@
  * warranty of any kind, whether express or implied.
  */
 
-#include <mach/hardware.h>
-#include <asm/hardware/gic.h>
+#include <asm/hardware/entry-macro-gic.S>
 
 	.macro	disable_fiq
 	.endm
 
-	.macro  get_irqnr_preamble, base, tmp
-	ldr	\base, =gic_cpu_base_addr
-	ldr	\base, [\base]
-	.endm
-
 	.macro  arch_ret_to_user, tmp1, tmp2
 	.endm
-
-	/*
-	 * The interrupt numbering scheme is defined in the
-	 * interrupt controller spec.  To wit:
-	 *
-	 * Migrated the code from ARM MP port to be more consistent
-	 * with interrupt processing , the following still holds true
-	 * however, all interrupts are treated the same regardless of
-	 * if they are local IPI or PPI
-	 *
-	 * Interrupts 0-15 are IPI
-	 * 16-31 are PPI
-	 *   (16-18 are the timers)
-	 * 32-1020 are global
-	 * 1021-1022 are reserved
-	 * 1023 is "spurious" (no interrupt)
-	 *
-	 * A simple read from the controller will tell us the number of the
-	 * highest priority enabled interrupt.  We then just need to check
-	 * whether it is in the valid range for an IRQ (0-1020 inclusive).
-	 *
-	 * Base ARM code assumes that the local (private) peripheral interrupts
-	 * are not valid, we treat them differently, in that the privates are
-	 * handled like normal shared interrupts with the exception that only
-	 * one processor can register the interrupt and the handler must be
-	 * the same for all processors.
-	 */
-
-	.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
-
-	ldr  \irqstat, [\base, #GIC_CPU_INTACK] /* bits 12-10 =srcCPU,
-						   9-0 =int # */
-
-	bic     \irqnr, \irqstat, #0x1c00	@mask src
-	cmp     \irqnr, #15
-	ldr		\tmp, =1021
-	cmpcc	\irqnr, \irqnr
-	cmpne	\irqnr, \tmp
-	cmpcs	\irqnr, \irqnr
-
-	.endm
-
-	/* We assume that irqstat (the raw value of the IRQ acknowledge
-	 * register) is preserved from the macro above.
-	 * If there is an IPI, we immediately signal end of interrupt on the
-	 * controller, since this requires the original irqstat value which
-	 * we won't easily be able to recreate later.
-	 */
-	.macro test_for_ipi, irqnr, irqstat, base, tmp
-    bic \irqnr, \irqstat, #0x1c00
-    cmp \irqnr, #16
-    strcc   \irqstat, [\base, #GIC_CPU_EOI]
-    cmpcs   \irqnr, \irqnr
-	.endm
-
-	/* As above, this assumes that irqstat and base are preserved.. */
-
-	.macro test_for_ltirq, irqnr, irqstat, base, tmp
-    bic \irqnr, \irqstat, #0x1c00
-    mov     \tmp, #0
-    cmp \irqnr, #16
-    moveq   \tmp, #1
-    streq   \irqstat, [\base, #GIC_CPU_EOI]
-    cmp \tmp, #0
-	.endm
diff --git a/arch/arm/mach-omap2/include/mach/entry-macro.S b/arch/arm/mach-omap2/include/mach/entry-macro.S
index ceb8b7e..feb90a1 100644
--- a/arch/arm/mach-omap2/include/mach/entry-macro.S
+++ b/arch/arm/mach-omap2/include/mach/entry-macro.S
@@ -78,7 +78,7 @@
 4401:		ldr     \irqstat, [\base, #GIC_CPU_INTACK]
 		ldr     \tmp, =1021
 		bic     \irqnr, \irqstat, #0x1c00
-		cmp     \irqnr, #29
+		cmp     \irqnr, #15
 		cmpcc   \irqnr, \irqnr
 		cmpne   \irqnr, \tmp
 		cmpcs   \irqnr, \irqnr
@@ -101,18 +101,6 @@
 		it	cs
 		cmpcs	\irqnr, \irqnr
 		.endm
-
-		/* As above, this assumes that irqstat and base are preserved */
-
-		.macro test_for_ltirq, irqnr, irqstat, base, tmp
-		bic	\irqnr, \irqstat, #0x1c00
-		mov 	\tmp, #0
-		cmp	\irqnr, #29
-		itt	eq
-		moveq	\tmp, #1
-		streq	\irqstat, [\base, #GIC_CPU_EOI]
-		cmp	\tmp, #0
-		.endm
 #endif	/* CONFIG_SMP */
 
 #else	/* MULTI_OMAP2 */
diff --git a/arch/arm/mach-shmobile/entry-intc.S b/arch/arm/mach-shmobile/entry-intc.S
index cac0a7a..1a1c00c 100644
--- a/arch/arm/mach-shmobile/entry-intc.S
+++ b/arch/arm/mach-shmobile/entry-intc.S
@@ -51,7 +51,4 @@
 	.macro  test_for_ipi, irqnr, irqstat, base, tmp
 	.endm
 
-	.macro  test_for_ltirq, irqnr, irqstat, base, tmp
-	.endm
-
 	arch_irq_handler shmobile_handle_irq_intc
diff --git a/arch/arm/mach-shmobile/include/mach/entry-macro.S b/arch/arm/mach-shmobile/include/mach/entry-macro.S
index d791f10..8d4a416 100644
--- a/arch/arm/mach-shmobile/include/mach/entry-macro.S
+++ b/arch/arm/mach-shmobile/include/mach/entry-macro.S
@@ -27,8 +27,5 @@
 	.macro  test_for_ipi, irqnr, irqstat, base, tmp
 	.endm
 
-	.macro  test_for_ltirq, irqnr, irqstat, base, tmp
-	.endm
-
 	.macro  arch_ret_to_user, tmp1, tmp2
 	.endm
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v11 2/4] ARM: gic: Add PPI registration interface
  2011-08-09  9:56 [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 1/4] ARM: gic: consolidate PPI handling Marc Zyngier
@ 2011-08-09  9:56 ` Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 3/4] ARM: local timers: drop local_timer_ack() Marc Zyngier
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2011-08-09  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

This patch makes it possible to request (and free) a PPI.
It thus makes it useable for more than just the local
timers.

Update TWD and MSM timers to use that functionnality.

Based on an earlier patch by Russell King.

Cc: David Brown <davidb@codeaurora.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/common/gic.c               |   94 ++++++++++++++++++++++++++++------
 arch/arm/include/asm/hardware/gic.h |    5 ++-
 arch/arm/include/asm/localtimer.h   |   11 ++++-
 arch/arm/include/asm/smp_twd.h      |    1 +
 arch/arm/kernel/smp.c               |    4 +-
 arch/arm/kernel/smp_twd.c           |   15 +++++-
 arch/arm/mach-msm/timer.c           |   56 +++++++++++----------
 7 files changed, 137 insertions(+), 49 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 8adc002..9022836 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -35,7 +35,6 @@
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 #include <asm/hardware/gic.h>
-#include <asm/localtimer.h>
 
 static DEFINE_SPINLOCK(irq_controller_lock);
 
@@ -259,14 +258,25 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
-#ifdef CONFIG_LOCAL_TIMERS
-#define gic_ppi_handler		percpu_timer_handler
-#else
+struct gic_action {
+	irq_handler_t handler;
+	void *data;
+};
+
+static DEFINE_PER_CPU(struct gic_action *, gic_ppi_action);
+static unsigned int gic_nr_ppis, gic_ppi_base;
+
 static irqreturn_t gic_ppi_handler(int irq, void *dev_id)
 {
+	unsigned int ppi = irq - gic_ppi_base;
+	struct gic_action *act;
+
+	act = &__get_cpu_var(gic_ppi_action)[ppi];
+	if (likely(act->handler))
+		return act->handler(irq, act->data);
+
 	return IRQ_NONE;
 }
-#endif
 
 #define PPI_IRQACT(nr)						\
 	{							\
@@ -291,6 +301,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	unsigned int gic_irqs, irq_limit, i;
 	void __iomem *base = gic->dist_base;
 	u32 cpumask = 1 << smp_processor_id();
+	u32 dist_ctr, nrcpus;
 	u32 nrppis = 0, ppi_base = 0;
 
 	cpumask |= cpumask << 8;
@@ -302,11 +313,14 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	 * Find out how many interrupts are supported.
 	 * The GIC only supports up to 1020 interrupt sources.
 	 */
-	gic_irqs = readl_relaxed(base + GIC_DIST_CTR) & 0x1f;
-	gic_irqs = (gic_irqs + 1) * 32;
+	dist_ctr = readl_relaxed(base + GIC_DIST_CTR);
+	gic_irqs = ((dist_ctr & 0x1f) + 1) * 32;
 	if (gic_irqs > 1020)
 		gic_irqs = 1020;
 
+	/* Find out how many CPUs are supported (8 max). */
+	nrcpus = ((dist_ctr >> 5) & 7) + 1;
+
 	/*
 	 * Nobody would be insane enough to use PPIs on a secondary
 	 * GIC, right?
@@ -324,6 +338,19 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 			nrppis = 0;
 			ppi_base = 0;
 		}
+
+		for (i = 0; i < nrcpus; i++) {
+			struct gic_action **ppiacts;
+
+			ppiacts = &per_cpu(gic_ppi_action, i);
+			*ppiacts = kzalloc(sizeof(*gic_ppi_action) * nrppis,
+					   GFP_KERNEL);
+			if (!*ppiacts)
+				pr_err("GIC: Can't allocate PPI CPU%d memory", i);
+		}
+
+		gic_nr_ppis = nrppis;
+		gic_ppi_base = ppi_base;
 	}
 
 	pr_info("Configuring GIC with %d sources (%d PPIs)\n",
@@ -387,6 +414,49 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
 
+int gic_request_ppi(unsigned int irq, irq_handler_t handler, void *data)
+{
+	struct gic_action *act;
+	unsigned long flags;
+	unsigned int ppi = irq - gic_ppi_base;
+	int ret = -EBUSY;
+
+	if (ppi >= gic_nr_ppis)
+		return -EINVAL;
+
+	local_irq_save(flags);
+	act = &__get_cpu_var(gic_ppi_action)[ppi];
+	if (!act->handler) {
+		act->handler = handler;
+		act->data = data;
+
+		gic_unmask_irq(irq_get_irq_data(irq));
+		ret = 0;
+	}
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+void gic_free_ppi(unsigned int irq, void *data)
+{
+	struct gic_action *act;
+	unsigned long flags;
+	unsigned int ppi = irq - gic_ppi_base;
+
+	if (ppi >= gic_nr_ppis)
+		return;
+
+	local_irq_save(flags);
+	act = &__get_cpu_var(gic_ppi_action)[ppi];
+	if (act->data == data) {
+		gic_mask_irq(irq_get_irq_data(irq));
+		act->handler = NULL;
+		act->data = NULL;
+	}
+	local_irq_restore(flags);
+}
+
 static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
 {
 	void __iomem *dist_base = gic->dist_base;
@@ -436,16 +506,6 @@ void __cpuinit gic_secondary_init(unsigned int gic_nr)
 	gic_cpu_init(&gic_data[gic_nr]);
 }
 
-void __cpuinit gic_enable_ppi(unsigned int irq)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	irq_set_status_flags(irq, IRQ_NOPROBE);
-	gic_unmask_irq(irq_get_irq_data(irq));
-	local_irq_restore(flags);
-}
-
 #ifdef CONFIG_SMP
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..b37780f 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -33,6 +33,8 @@
 #define GIC_DIST_SOFTINT		0xf00
 
 #ifndef __ASSEMBLY__
+#include <linux/interrupt.h>
+
 extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
@@ -40,7 +42,8 @@ void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
 void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-void gic_enable_ppi(unsigned int);
+int gic_request_ppi(unsigned int irq, irq_handler_t handler, void *data);
+void gic_free_ppi(unsigned int irq, void *data);
 
 struct gic_chip_data {
 	unsigned int irq_offset;
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index e3663f7..136a31b 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -24,7 +24,6 @@ void percpu_timer_setup(void);
  */
 irqreturn_t percpu_timer_handler(int irq, void *dev_id);
 
-
 #ifdef CONFIG_LOCAL_TIMERS
 
 #ifdef CONFIG_HAVE_ARM_TWD
@@ -32,6 +31,7 @@ irqreturn_t percpu_timer_handler(int irq, void *dev_id);
 #include "smp_twd.h"
 
 #define local_timer_ack()	twd_timer_ack()
+#define local_timer_stop(c)	twd_timer_stop((c))
 
 #else
 
@@ -41,6 +41,11 @@ irqreturn_t percpu_timer_handler(int irq, void *dev_id);
  */
 int local_timer_ack(void);
 
+/*
+ * Stop the local timer
+ */
+void local_timer_stop(struct clock_event_device *);
+
 #endif
 
 /*
@@ -54,6 +59,10 @@ static inline int local_timer_setup(struct clock_event_device *evt)
 {
 	return -ENXIO;
 }
+
+static inline void local_timer_stop(struct clock_event_device *evt)
+{
+}
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index fed9981..6923037 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -24,5 +24,6 @@ extern void __iomem *twd_base;
 
 int twd_timer_ack(void);
 void twd_timer_setup(struct clock_event_device *);
+void twd_timer_stop(struct clock_event_device *);
 
 #endif
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index bf11ffe..93b7c9a 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -472,7 +472,7 @@ static void ipi_timer(void)
 #ifdef CONFIG_LOCAL_TIMERS
 irqreturn_t percpu_timer_handler(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
+	struct clock_event_device *evt = dev_id;
 
 	if (local_timer_ack()) {
 		evt->event_handler(evt);
@@ -533,7 +533,7 @@ static void percpu_timer_stop(void)
 	unsigned int cpu = smp_processor_id();
 	struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
 
-	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+	local_timer_stop(evt);
 }
 #endif
 
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 01c1862..1bf0631 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -19,6 +19,7 @@
 #include <linux/io.h>
 
 #include <asm/smp_twd.h>
+#include <asm/localtimer.h>
 #include <asm/hardware/gic.h>
 
 /* set up by the platform code */
@@ -80,6 +81,12 @@ int twd_timer_ack(void)
 	return 0;
 }
 
+void twd_timer_stop(struct clock_event_device *clk)
+{
+	twd_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+	gic_free_ppi(clk->irq, clk);
+}
+
 static void __cpuinit twd_calibrate_rate(void)
 {
 	unsigned long count;
@@ -124,6 +131,8 @@ static void __cpuinit twd_calibrate_rate(void)
  */
 void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 {
+	int err;
+
 	twd_calibrate_rate();
 
 	clk->name = "local_timer";
@@ -139,6 +148,8 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 
 	clockevents_register_device(clk);
 
-	/* Make sure our local interrupt controller has this enabled */
-	gic_enable_ppi(clk->irq);
+	err = gic_request_ppi(clk->irq, percpu_timer_handler, clk);
+	if (err)
+		pr_err("%s: can't register interrupt %d on cpu %d (%d)\n",
+		       clk->name, clk->irq, smp_processor_id(), err);
 }
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 63621f1..1bcdf66 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -71,7 +71,7 @@ enum timer_location {
 struct msm_clock {
 	struct clock_event_device   clockevent;
 	struct clocksource          clocksource;
-	struct irqaction            irq;
+	unsigned int		    irq;
 	void __iomem                *regbase;
 	uint32_t                    freq;
 	uint32_t                    shift;
@@ -87,13 +87,10 @@ enum {
 
 
 static struct msm_clock msm_clocks[];
-static struct clock_event_device *local_clock_event;
 
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
-	if (smp_processor_id() != 0)
-		evt = local_clock_event;
 	if (evt->event_handler == NULL)
 		return IRQ_HANDLED;
 	evt->event_handler(evt);
@@ -171,13 +168,7 @@ static struct msm_clock msm_clocks[] = {
 			.mask           = CLOCKSOURCE_MASK(32),
 			.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 		},
-		.irq = {
-			.name    = "gp_timer",
-			.flags   = IRQF_DISABLED | IRQF_TIMER | IRQF_TRIGGER_RISING,
-			.handler = msm_timer_interrupt,
-			.dev_id  = &msm_clocks[0].clockevent,
-			.irq     = INT_GP_TIMER_EXP
-		},
+		.irq = INT_GP_TIMER_EXP,
 		.freq = GPT_HZ,
 	},
 	[MSM_CLOCK_DGT] = {
@@ -196,13 +187,7 @@ static struct msm_clock msm_clocks[] = {
 			.mask           = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
 			.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 		},
-		.irq = {
-			.name    = "dg_timer",
-			.flags   = IRQF_DISABLED | IRQF_TIMER | IRQF_TRIGGER_RISING,
-			.handler = msm_timer_interrupt,
-			.dev_id  = &msm_clocks[1].clockevent,
-			.irq     = INT_DEBUG_TIMER_EXP
-		},
+		.irq = INT_DEBUG_TIMER_EXP,
 		.freq = DGT_HZ >> MSM_DGT_SHIFT,
 		.shift = MSM_DGT_SHIFT,
 	}
@@ -261,10 +246,17 @@ static void __init msm_timer_init(void)
 			printk(KERN_ERR "msm_timer_init: clocksource_register "
 			       "failed for %s\n", cs->name);
 
-		res = setup_irq(clock->irq.irq, &clock->irq);
+		ce->irq = clock->irq;
+		if (cpu_is_msm8x60() || cpu_is_msm8960())
+			res = gic_request_ppi(ce->irq, msm_timer_interrupt, ce);
+		else
+			res = request_irq(ce->irq, msm_timer_interrupt,
+					  IRQF_TIMER | IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
+					  ce->name, ce);
+
 		if (res)
-			printk(KERN_ERR "msm_timer_init: setup_irq "
-			       "failed for %s\n", cs->name);
+			pr_err("msm_timer_init: request_irq failed for %s\n",
+			       ce->name);
 
 		clockevents_register_device(ce);
 	}
@@ -273,7 +265,9 @@ static void __init msm_timer_init(void)
 #ifdef CONFIG_SMP
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
+	static bool local_timer_inited;
 	struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER];
+	int res;
 
 	/* Use existing clock_event for cpu 0 */
 	if (!smp_processor_id())
@@ -281,12 +275,13 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 
 	writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
 
-	if (!local_clock_event) {
+	if (!local_timer_inited) {
 		writel(0, clock->regbase  + TIMER_ENABLE);
 		writel(0, clock->regbase + TIMER_CLEAR);
 		writel(~0, clock->regbase + TIMER_MATCH_VAL);
+		local_timer_inited = true;
 	}
-	evt->irq = clock->irq.irq;
+	evt->irq = clock->irq;
 	evt->name = "local_timer";
 	evt->features = CLOCK_EVT_FEAT_ONESHOT;
 	evt->rating = clock->clockevent.rating;
@@ -298,14 +293,23 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 		clockevent_delta2ns(0xf0000000 >> clock->shift, evt);
 	evt->min_delta_ns = clockevent_delta2ns(4, evt);
 
-	local_clock_event = evt;
-
-	gic_enable_ppi(clock->irq.irq);
+	res = gic_request_ppi(evt->irq, msm_timer_interrupt, evt);
+	if (res) {
+		pr_err("local_timer_setup: request_irq failed for %s\n",
+			       clock->clockevent.name);
+		return res;
+	}
 
 	clockevents_register_device(evt);
 	return 0;
 }
 
+void local_timer_stop(struct clock_event_device *evt)
+{
+	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+	gic_free_ppi(evt->irq, evt);
+}
+
 inline int local_timer_ack(void)
 {
 	return 1;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v11 3/4] ARM: local timers: drop local_timer_ack()
  2011-08-09  9:56 [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 1/4] ARM: gic: consolidate PPI handling Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 2/4] ARM: gic: Add PPI registration interface Marc Zyngier
@ 2011-08-09  9:56 ` Marc Zyngier
  2011-08-09  9:56 ` [PATCH v11 4/4] ARM: gic: add compute_irqnr macro for exynos4 Marc Zyngier
  2011-08-15 12:13 ` [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Russell King - ARM Linux
  4 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2011-08-09  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

Only TWD uses local_timer_ack() to perform something useful.
Make it the real (private) interrupt handler, remove the
common interrupt handler as well as the useless stubs from
MCT and MSM.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: David Brown <davidb@codeaurora.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/localtimer.h |   12 ------------
 arch/arm/include/asm/smp_twd.h    |    1 -
 arch/arm/kernel/smp.c             |   14 --------------
 arch/arm/kernel/smp_twd.c         |   17 +++++++----------
 arch/arm/mach-exynos4/mct.c       |    6 ------
 arch/arm/mach-msm/timer.c         |    6 ------
 6 files changed, 7 insertions(+), 49 deletions(-)

diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 136a31b..f5e1cec 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -19,29 +19,17 @@ struct clock_event_device;
  */
 void percpu_timer_setup(void);
 
-/*
- * Per-cpu timer IRQ handler
- */
-irqreturn_t percpu_timer_handler(int irq, void *dev_id);
-
 #ifdef CONFIG_LOCAL_TIMERS
 
 #ifdef CONFIG_HAVE_ARM_TWD
 
 #include "smp_twd.h"
 
-#define local_timer_ack()	twd_timer_ack()
 #define local_timer_stop(c)	twd_timer_stop((c))
 
 #else
 
 /*
- * Platform provides this to acknowledge a local timer IRQ.
- * Returns true if the local timer IRQ is to be processed.
- */
-int local_timer_ack(void);
-
-/*
  * Stop the local timer
  */
 void local_timer_stop(struct clock_event_device *);
diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index 6923037..ef9ffba 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -22,7 +22,6 @@ struct clock_event_device;
 
 extern void __iomem *twd_base;
 
-int twd_timer_ack(void);
 void twd_timer_setup(struct clock_event_device *);
 void twd_timer_stop(struct clock_event_device *);
 
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 93b7c9a..5d1f2e2 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -469,20 +469,6 @@ static void ipi_timer(void)
 	irq_exit();
 }
 
-#ifdef CONFIG_LOCAL_TIMERS
-irqreturn_t percpu_timer_handler(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = dev_id;
-
-	if (local_timer_ack()) {
-		evt->event_handler(evt);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-#endif
-
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 static void smp_timer_broadcast(const struct cpumask *mask)
 {
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 1bf0631..7156f09 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -65,20 +65,17 @@ static int twd_set_next_event(unsigned long evt,
 	return 0;
 }
 
-/*
- * local_timer_ack: checks for a local timer interrupt.
- *
- * If a local timer interrupt has occurred, acknowledge and return 1.
- * Otherwise, return 0.
- */
-int twd_timer_ack(void)
+static irqreturn_t twd_timer_handler(int irq, void *dev_id)
 {
+	struct clock_event_device *clk = dev_id;
+
 	if (__raw_readl(twd_base + TWD_TIMER_INTSTAT)) {
 		__raw_writel(1, twd_base + TWD_TIMER_INTSTAT);
-		return 1;
+		clk->event_handler(clk);
+		return IRQ_HANDLED;
 	}
 
-	return 0;
+	return IRQ_NONE;
 }
 
 void twd_timer_stop(struct clock_event_device *clk)
@@ -148,7 +145,7 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 
 	clockevents_register_device(clk);
 
-	err = gic_request_ppi(clk->irq, percpu_timer_handler, clk);
+	err = gic_request_ppi(clk->irq, twd_timer_handler, clk);
 	if (err)
 		pr_err("%s: can't register interrupt %d on cpu %d (%d)\n",
 		       clk->name, clk->irq, smp_processor_id(), err);
diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
index 1ae059b..194fc6d 100644
--- a/arch/arm/mach-exynos4/mct.c
+++ b/arch/arm/mach-exynos4/mct.c
@@ -393,12 +393,6 @@ void __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
 	exynos4_mct_tick_init(evt);
 }
-
-int local_timer_ack(void)
-{
-	return 0;
-}
-
 #endif /* CONFIG_LOCAL_TIMERS */
 
 static void __init exynos4_timer_resources(void)
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 1bcdf66..94e6fc5 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -309,12 +309,6 @@ void local_timer_stop(struct clock_event_device *evt)
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	gic_free_ppi(evt->irq, evt);
 }
-
-inline int local_timer_ack(void)
-{
-	return 1;
-}
-
 #endif
 
 struct sys_timer msm_timer = {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v11 4/4] ARM: gic: add compute_irqnr macro for exynos4
  2011-08-09  9:56 [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (2 preceding siblings ...)
  2011-08-09  9:56 ` [PATCH v11 3/4] ARM: local timers: drop local_timer_ack() Marc Zyngier
@ 2011-08-09  9:56 ` Marc Zyngier
  2011-08-15 12:13 ` [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Russell King - ARM Linux
  4 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2011-08-09  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

exynos4 has a full copy of entry-macro-gic.S, just for the sake
of an offset added to the IRQ number read from the GIC.

Add a compute_irqnr macro to entry-macro-gic.S so that any platform
can add it's own hook without having to copy the whole file again.

Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/hardware/entry-macro-gic.S  |    3 +
 arch/arm/mach-exynos4/include/mach/entry-macro.S |   63 ++-------------------
 2 files changed, 9 insertions(+), 57 deletions(-)

diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index 74ebc80..fbb50dc 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -43,6 +43,9 @@
 	cmpcc	\irqnr, \irqnr
 	cmpne	\irqnr, \tmp
 	cmpcs	\irqnr, \irqnr
+#ifdef HAVE_GIC_BASE_OFFSET
+	compute_irqnr	\irqnr, \tmp
+#endif
 	.endm
 
 /* We assume that irqstat (the raw value of the IRQ acknowledge
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index 807d05d..663310b 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -9,71 +9,20 @@
  * warranty of any kind, whether express or implied.
 */
 
+#define HAVE_GIC_BASE_OFFSET	1
+
 #include <mach/hardware.h>
 #include <mach/map.h>
 #include <asm/hardware/gic.h>
+#include <asm/hardware/entry-macro-gic.S>
 
-		.macro	disable_fiq
+		.macro	compute_irqnr, irqnr, tmp
+		addne	\irqnr, #32
 		.endm
 
-		.macro  get_irqnr_preamble, base, tmp
-		ldr	\base, =gic_cpu_base_addr
-		ldr	\base, [\base]
-		mrc     p15, 0, \tmp, c0, c0, 5
-		and     \tmp, \tmp, #3
-		cmp     \tmp, #1
-		addeq   \base, \base, #EXYNOS4_GIC_BANK_OFFSET
+		.macro	disable_fiq
 		.endm
 
 		.macro  arch_ret_to_user, tmp1, tmp2
 		.endm
 
-		/*
-		 * The interrupt numbering scheme is defined in the
-		 * interrupt controller spec.  To wit:
-		 *
-		 * Interrupts 0-15 are IPI
-		 * 16-28 are reserved
-		 * 29-31 are local.  We allow 30 to be used for the watchdog.
-		 * 32-1020 are global
-		 * 1021-1022 are reserved
-		 * 1023 is "spurious" (no interrupt)
-		 *
-		 * For now, we ignore all local interrupts so only return an interrupt if it's
-		 * between 30 and 1020.  The test_for_ipi routine below will pick up on IPIs.
-		 *
-		 * A simple read from the controller will tell us the number of the highest
-                 * priority enabled interrupt.  We then just need to check whether it is in the
-		 * valid range for an IRQ (30-1020 inclusive).
-		 */
-
-		.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
-
-		ldr     \irqstat, [\base, #GIC_CPU_INTACK] /* bits 12-10 = src CPU, 9-0 = int # */
-
-		ldr	\tmp, =1021
-
-		bic     \irqnr, \irqstat, #0x1c00
-
-		cmp     \irqnr, #15
-		cmpcc	\irqnr, \irqnr
-		cmpne	\irqnr, \tmp
-		cmpcs	\irqnr, \irqnr
-		addne	\irqnr, \irqnr, #32
-
-		.endm
-
-		/* We assume that irqstat (the raw value of the IRQ acknowledge
-		 * register) is preserved from the macro above.
-		 * If there is an IPI, we immediately signal end of interrupt on the
-		 * controller, since this requires the original irqstat value which
-		 * we won't easily be able to recreate later.
-		 */
-
-		.macro test_for_ipi, irqnr, irqstat, base, tmp
-		bic	\irqnr, \irqstat, #0x1c00
-		cmp	\irqnr, #16
-		strcc	\irqstat, [\base, #GIC_CPU_EOI]
-		cmpcs	\irqnr, \irqnr
-		.endm
-
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v11 0/4] Consolidating GIC per-cpu interrupts
  2011-08-09  9:56 [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (3 preceding siblings ...)
  2011-08-09  9:56 ` [PATCH v11 4/4] ARM: gic: add compute_irqnr macro for exynos4 Marc Zyngier
@ 2011-08-15 12:13 ` Russell King - ARM Linux
  2011-09-08 13:14   ` Thomas Gleixner
  4 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2011-08-15 12:13 UTC (permalink / raw)
  To: linux-arm-kernel

Thomas,

Could you _please_ look Marcs patch series and give an opinion on it.
I've also attached my patch to this reply too, which is an alternative
approach to Marcs.

On Tue, Aug 09, 2011 at 10:56:38AM +0100, Marc Zyngier wrote:
> The current GIC per-cpu interrupts (aka PPIs) suffer from a number of
> problems:
> 
> - They use a completely separate scheme to handle the interrupts,
>   mostly because the PPI concept doesn't really match the kernel view
>   of an interrupt.
> - PPIs can only be used by the timer code, unless we add more low-level
>   assembly code.
> - The local timer code can only be used by devices generating PPIs,
>   and not SPIs.
> - At least one platform (msm) has started implementing its own
>   alternative scheme.
> - Some low-level code gets duplicated, as usual...
> 
> As the previous solution which tried to map PPIs to normal interrupts
> has been proved to be buggy, I've opted to a much simpler scheme
> (based on Russell's input).
> 
> The proposed solution is to handle the interrupt using the same path
> as SPIs, with a common handler for all PPIs. Each PPI can be requested
> using gic_request_ppi(), similar to request_irq(). The local timer
> code is updated to reflect these changes.
> 
> Patches against v3.1-rc1 + Will Deacon's TWD patch ("ARM: twd:
> register clockevents device before enabling PPI"). Tested on PB11MP,
> VE, OMAP4 (Panda) and Tegra (Harmony). As this patch series is quite
> different from the previous one, I've dropped all previous acks from
> platform maintainers.
> 
> >From v10:
> - Fixed exynos4 compilation
> 
> >From v9:
> - Fixed PPI request in the MSM timer code
> - Fixed UP/non-local-timer build
> - Moved some bits and pieces from patch 1 to patch 2
> 
> Marc Zyngier (4):
>   ARM: gic: consolidate PPI handling
>   ARM: gic: Add PPI registration interface
>   ARM: local timers: drop local_timer_ack()
>   ARM: gic: add compute_irqnr macro for exynos4
> 
>  arch/arm/common/gic.c                             |  153 +++++++++++++++++++--
>  arch/arm/include/asm/entry-macro-multi.S          |    7 -
>  arch/arm/include/asm/hardirq.h                    |    3 -
>  arch/arm/include/asm/hardware/entry-macro-gic.S   |   22 +---
>  arch/arm/include/asm/hardware/gic.h               |    5 +-
>  arch/arm/include/asm/localtimer.h                 |   19 ++--
>  arch/arm/include/asm/smp.h                        |    5 -
>  arch/arm/include/asm/smp_twd.h                    |    2 +-
>  arch/arm/kernel/irq.c                             |    3 -
>  arch/arm/kernel/smp.c                             |   33 +-----
>  arch/arm/kernel/smp_twd.c                         |   30 +++--
>  arch/arm/mach-exynos4/include/mach/entry-macro.S  |   67 +--------
>  arch/arm/mach-exynos4/mct.c                       |    6 -
>  arch/arm/mach-msm/board-msm8x60.c                 |   11 --
>  arch/arm/mach-msm/include/mach/entry-macro-qgic.S |   73 +----------
>  arch/arm/mach-msm/timer.c                         |   56 ++++----
>  arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--
>  arch/arm/mach-shmobile/entry-intc.S               |    3 -
>  arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
>  19 files changed, 214 insertions(+), 301 deletions(-)
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-------------- next part --------------
 arch/arm/common/gic.c                             |   70 ++++++++++++++++++++-
 arch/arm/include/asm/entry-macro-multi.S          |    5 +-
 arch/arm/include/asm/hardware/entry-macro-gic.S   |   17 +++--
 arch/arm/include/asm/hardware/gic.h               |    5 +-
 arch/arm/include/asm/localtimer.h                 |   14 +----
 arch/arm/include/asm/smp_twd.h                    |    2 +-
 arch/arm/kernel/smp.c                             |   18 +-----
 arch/arm/kernel/smp_twd.c                         |   41 +++++++-----
 arch/arm/mach-exynos4/include/mach/entry-macro.S  |    8 +-
 arch/arm/mach-msm/include/mach/entry-macro-qgic.S |   14 ++--
 arch/arm/mach-msm/timer.c                         |   20 +++++-
 arch/arm/mach-omap2/include/mach/entry-macro.S    |    9 +--
 arch/arm/mach-shmobile/entry-intc.S               |    2 +-
 arch/arm/mach-shmobile/include/mach/entry-macro.S |    2 +-
 14 files changed, 144 insertions(+), 83 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4ddd0a6..148367d 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -27,12 +27,16 @@
 #include <linux/list.h>
 #include <linux/smp.h>
 #include <linux/cpumask.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 #include <asm/hardware/gic.h>
 
+#define GIC_FIRST_PPI	16
+#define NR_PPI		16
+
 static DEFINE_SPINLOCK(irq_controller_lock);
 
 /* Address of GIC 0 CPU interface */
@@ -376,14 +380,74 @@ void __cpuinit gic_secondary_init(unsigned int gic_nr)
 	gic_cpu_init(&gic_data[gic_nr]);
 }
 
-void __cpuinit gic_enable_ppi(unsigned int irq)
+struct gic_action {
+	irq_handler_t handler;
+	void *data;
+};
+
+static DEFINE_PER_CPU(struct gic_action[NR_PPI], gic_ppi_action);
+
+asmlinkage void __exception_irq_entry gic_call_ppi(unsigned int irq,
+	struct pt_regs *regs)
+{
+	unsigned int ppi = irq - GIC_FIRST_PPI;
+	struct gic_action *act;
+
+	act = &__get_cpu_var(gic_ppi_action)[ppi];
+	if (act->handler) {
+		struct pt_regs *old_regs = set_irq_regs(regs);
+
+		/* FIXME: need to deal with PPI IRQ stats better.. */
+		__inc_irq_stat(smp_processor_id(), local_timer_irqs);
+
+		irq_enter();
+		act->handler(irq, act->data);
+		irq_exit();
+
+		set_irq_regs(old_regs);
+	}
+}
+
+int gic_request_ppi(unsigned int irq, irq_handler_t handler, void *data)
 {
+	struct gic_action *act;
 	unsigned long flags;
+	unsigned int ppi = irq - GIC_FIRST_PPI;
+	int ret = -EBUSY;
+
+	if (ppi >= NR_PPI)
+		return -EINVAL;
 
 	local_irq_save(flags);
-	irq_set_status_flags(irq, IRQ_NOPROBE);
-	gic_unmask_irq(irq_get_irq_data(irq));
+	act = &__get_cpu_var(gic_ppi_action)[ppi];
+	if (!act->handler) {
+		act->handler = handler;
+		act->data = data;
+
+		irq_set_status_flags(irq, IRQ_NOPROBE);
+		gic_unmask_irq(irq_get_irq_data(irq));
+	}
 	local_irq_restore(flags);
+
+	return ret;
+}
+
+void gic_free_ppi(unsigned int irq, void *data)
+{
+	struct gic_action *act;
+	unsigned long flags;
+	unsigned int ppi = irq - GIC_FIRST_PPI;
+
+	if (ppi < NR_PPI) {
+		local_irq_save(flags);
+		act = &__get_cpu_var(gic_ppi_action)[ppi];
+		if (act->data == data) {
+			gic_mask_irq(irq_get_irq_data(irq));
+			act->handler = NULL;
+			act->data = NULL;
+		}
+		local_irq_restore(flags);
+	}
 }
 
 #ifdef CONFIG_SMP
diff --git a/arch/arm/include/asm/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index 2f1e209..f1ee1e6 100644
--- a/arch/arm/include/asm/entry-macro-multi.S
+++ b/arch/arm/include/asm/entry-macro-multi.S
@@ -27,10 +27,7 @@
 	bne	do_IPI
 
 #ifdef CONFIG_LOCAL_TIMERS
-	test_for_ltirq r0, r2, r6, lr
-	movne	r0, sp
-	adrne	lr, BSYM(1b)
-	bne	do_local_timer
+	test_for_ppi r0, r2, r6, lr, sp, BSYM(1b)
 #endif
 #endif
 9997:
diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index c115b82..a74990f 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -63,13 +63,16 @@
 	cmpcs	\irqnr, \irqnr
 	.endm
 
-/* As above, this assumes that irqstat and base are preserved.. */
+/*
+ * We will have checked for normal IRQs (32+) and IPIs (0-15) so only
+ * PPIs are left here.
+ */
 
-	.macro test_for_ltirq, irqnr, irqstat, base, tmp
+	.macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym
 	bic	\irqnr, \irqstat, #0x1c00
-	mov 	\tmp, #0
-	cmp	\irqnr, #29
-	moveq	\tmp, #1
-	streq	\irqstat, [\base, #GIC_CPU_EOI]
-	cmp	\tmp, #0
+	cmp	\irqnr, #32
+	strcc	\irqstat, [\base, #GIC_CPU_EOI]
+	movcc	r1, \regs
+	adrcc	lr, \sym
+	bcc	gic_call_ppi
 	.endm
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 0691f9d..768521d4 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -33,6 +33,8 @@
 #define GIC_DIST_SOFTINT		0xf00
 
 #ifndef __ASSEMBLY__
+#include <linux/interrupt.h>
+
 extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
@@ -40,7 +42,8 @@ void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
 void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-void gic_enable_ppi(unsigned int);
+int gic_request_ppi(unsigned int, irq_handler_t, void *);
+void gic_free_ppi(unsigned int, void *);
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 080d74f..42a842f 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -17,27 +17,17 @@ struct clock_event_device;
  */
 void percpu_timer_setup(void);
 
-/*
- * Called from assembly, this is the local timer IRQ handler
- */
-asmlinkage void do_local_timer(struct pt_regs *);
-
-
 #ifdef CONFIG_LOCAL_TIMERS
 
 #ifdef CONFIG_HAVE_ARM_TWD
 
 #include "smp_twd.h"
 
-#define local_timer_ack()	twd_timer_ack()
+#define local_timer_stop twd_timer_stop
 
 #else
 
-/*
- * Platform provides this to acknowledge a local timer IRQ.
- * Returns true if the local timer IRQ is to be processed.
- */
-int local_timer_ack(void);
+int local_timer_stop(struct clock_event_device *);
 
 #endif
 
diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index fed9981..ef9ffba9 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -22,7 +22,7 @@ struct clock_event_device;
 
 extern void __iomem *twd_base;
 
-int twd_timer_ack(void);
 void twd_timer_setup(struct clock_event_device *);
+void twd_timer_stop(struct clock_event_device *);
 
 #endif
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 167e3cb..f83ef5e 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -458,19 +458,6 @@ static void ipi_timer(void)
 }
 
 #ifdef CONFIG_LOCAL_TIMERS
-asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs)
-{
-	struct pt_regs *old_regs = set_irq_regs(regs);
-	int cpu = smp_processor_id();
-
-	if (local_timer_ack()) {
-		__inc_irq_stat(cpu, local_timer_irqs);
-		ipi_timer();
-	}
-
-	set_irq_regs(old_regs);
-}
-
 void show_local_irqs(struct seq_file *p, int prec)
 {
 	unsigned int cpu;
@@ -531,10 +518,7 @@ void __cpuinit percpu_timer_setup(void)
  */
 static void percpu_timer_stop(void)
 {
-	unsigned int cpu = smp_processor_id();
-	struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
-
-	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+	local_timer_stop(&per_cpu(percpu_clockevent, smp_processor_id()));
 }
 #endif
 
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 2c277d4..5f1e124 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -26,6 +26,18 @@ void __iomem *twd_base;
 
 static unsigned long twd_timer_rate;
 
+static irqreturn_t twd_irq(int irq, void *data)
+{
+	struct clock_event_device *evt = data;
+
+	if (__raw_readl(twd_base + TWD_TIMER_INTSTAT)) {
+		__raw_writel(1, twd_base + TWD_TIMER_INTSTAT);
+		evt->event_handler(evt);
+	}
+
+	return IRQ_HANDLED;
+}
+
 static void twd_set_mode(enum clock_event_mode mode,
 			struct clock_event_device *clk)
 {
@@ -64,22 +76,6 @@ static int twd_set_next_event(unsigned long evt,
 	return 0;
 }
 
-/*
- * local_timer_ack: checks for a local timer interrupt.
- *
- * If a local timer interrupt has occurred, acknowledge and return 1.
- * Otherwise, return 0.
- */
-int twd_timer_ack(void)
-{
-	if (__raw_readl(twd_base + TWD_TIMER_INTSTAT)) {
-		__raw_writel(1, twd_base + TWD_TIMER_INTSTAT);
-		return 1;
-	}
-
-	return 0;
-}
-
 static void __cpuinit twd_calibrate_rate(void)
 {
 	unsigned long count;
@@ -124,6 +120,8 @@ static void __cpuinit twd_calibrate_rate(void)
  */
 void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 {
+	int ret;
+
 	twd_calibrate_rate();
 
 	clk->name = "local_timer";
@@ -138,7 +136,16 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 	clk->min_delta_ns = clockevent_delta2ns(0xf, clk);
 
 	/* Make sure our local interrupt controller has this enabled */
-	gic_enable_ppi(clk->irq);
+	ret = gic_request_ppi(clk->irq, twd_irq, clk);
+	if (ret)
+		pr_err("CPU%u: unable to request TWD interrupt\n",
+			smp_processor_id());
 
 	clockevents_register_device(clk);
 }
+
+void twd_timer_stop(struct clock_event_device *clk)
+{
+	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+	gic_free_ppi(clk->irq, clk);
+}
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d8f38c2..fdb24bb 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -74,11 +74,11 @@
 
 		/* As above, this assumes that irqstat and base are preserved.. */
 
-		.macro test_for_ltirq, irqnr, irqstat, base, tmp
+		.macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym
 		bic	\irqnr, \irqstat, #0x1c00
-		mov	\tmp, #0
 		cmp	\irqnr, #29
-		moveq	\tmp, #1
 		streq	\irqstat, [\base, #GIC_CPU_EOI]
-		cmp	\tmp, #0
+		moveq	r1, \regs
+		adreq	lr, \sym
+		bcc	gic_call_ppi
 		.endm
diff --git a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
index 1246715..b3ebb06 100644
--- a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
+++ b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
@@ -78,11 +78,11 @@
 
 	/* As above, this assumes that irqstat and base are preserved.. */
 
-	.macro test_for_ltirq, irqnr, irqstat, base, tmp
-    bic \irqnr, \irqstat, #0x1c00
-    mov     \tmp, #0
-    cmp \irqnr, #16
-    moveq   \tmp, #1
-    streq   \irqstat, [\base, #GIC_CPU_EOI]
-    cmp \tmp, #0
+	.macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym
+	bic	\irqnr, \irqstat, #0x1c00
+	cmp	\irqnr, #16
+	streq	\irqstat, [\base, #GIC_CPU_EOI]
+	moveq	r1, \regs
+	adreq	lr, \sym
+	beq	gic_call_ppi
 	.endm
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 63621f1..b553a14 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -271,9 +271,19 @@ static void __init msm_timer_init(void)
 }
 
 #ifdef CONFIG_SMP
+static irqreturn_t local_timer_irq(int irq, void *data)
+{
+	struct clock_event_device *evt = data;
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
 	struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER];
+	int ret;
 
 	/* Use existing clock_event for cpu 0 */
 	if (!smp_processor_id())
@@ -300,15 +310,19 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 
 	local_clock_event = evt;
 
-	gic_enable_ppi(clock->irq.irq);
+	ret = gic_request_ppi(evt->irq, local_timer_irq, evt);
+	if (ret)
+		pr_err("CPU%u: unable to request local timer interrupt\n",
+			smp_processor_id());
 
 	clockevents_register_device(evt);
 	return 0;
 }
 
-inline int local_timer_ack(void)
+void local_timer_stop(struct clock_event_device *evt)
 {
-	return 1;
+	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+	gic_free_ppi(clk->irq, clk);
 }
 
 #endif
diff --git a/arch/arm/mach-omap2/include/mach/entry-macro.S b/arch/arm/mach-omap2/include/mach/entry-macro.S
index ceb8b7e..66329f4 100644
--- a/arch/arm/mach-omap2/include/mach/entry-macro.S
+++ b/arch/arm/mach-omap2/include/mach/entry-macro.S
@@ -104,14 +104,13 @@
 
 		/* As above, this assumes that irqstat and base are preserved */
 
-		.macro test_for_ltirq, irqnr, irqstat, base, tmp
+		.macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym
 		bic	\irqnr, \irqstat, #0x1c00
-		mov 	\tmp, #0
 		cmp	\irqnr, #29
-		itt	eq
-		moveq	\tmp, #1
 		streq	\irqstat, [\base, #GIC_CPU_EOI]
-		cmp	\tmp, #0
+		moveq	r1, \regs
+		adreq	lr, \sym
+		beq	gic_call_ppi
 		.endm
 #endif	/* CONFIG_SMP */
 
diff --git a/arch/arm/mach-shmobile/entry-intc.S b/arch/arm/mach-shmobile/entry-intc.S
index cac0a7a..b4ece8e 100644
--- a/arch/arm/mach-shmobile/entry-intc.S
+++ b/arch/arm/mach-shmobile/entry-intc.S
@@ -51,7 +51,7 @@
 	.macro  test_for_ipi, irqnr, irqstat, base, tmp
 	.endm
 
-	.macro  test_for_ltirq, irqnr, irqstat, base, tmp
+	.macro  test_for_ppi, irqnr, irqstat, base, tmp, regs, sym
 	.endm
 
 	arch_irq_handler shmobile_handle_irq_intc
diff --git a/arch/arm/mach-shmobile/include/mach/entry-macro.S b/arch/arm/mach-shmobile/include/mach/entry-macro.S
index d791f10..e6dcafd 100644
--- a/arch/arm/mach-shmobile/include/mach/entry-macro.S
+++ b/arch/arm/mach-shmobile/include/mach/entry-macro.S
@@ -27,7 +27,7 @@
 	.macro  test_for_ipi, irqnr, irqstat, base, tmp
 	.endm
 
-	.macro  test_for_ltirq, irqnr, irqstat, base, tmp
+	.macro  test_for_ppi, irqnr, irqstat, base, tmp, regs, sym
 	.endm
 
 	.macro  arch_ret_to_user, tmp1, tmp2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v11 0/4] Consolidating GIC per-cpu interrupts
  2011-08-15 12:13 ` [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Russell King - ARM Linux
@ 2011-09-08 13:14   ` Thomas Gleixner
  2011-09-08 16:12     ` Marc Zyngier
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2011-09-08 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

B1;2601;0cRussell,

On Mon, 15 Aug 2011, Russell King - ARM Linux wrote:

> Thomas,
> 
> Could you _please_ look Marcs patch series and give an opinion on it.
> I've also attached my patch to this reply too, which is an alternative
> approach to Marcs.

I don't have fundamental objections to Marcs or your approach. In fact
they are very similar.

The main difference is that Marc sets up regular interrupts with
handle_percpu_irq instead of going through a separate entry point. The
only downside is that it exposes the PPI interrupts to the generic irq
API, so nothing can prevent stupid drivers to call disable/enable_irq
& al. I'm not sure how much of an issue that is in reality. If it
matters we can add a flag to the core code which excludes such
interrupts from being accessed.

Another thing, which sticks out compared to other percpu interrupt
users in arch/* is that you provide the ability to assign different
handlers on different CPUs to a given PPI interrupt number. Most other
percpu implementations setup the interrupt with a unique percpu aware
handler and just enable/disable it per core in the low level
setup/shutdown code. Is running different handlers on different cores
a real requirement or just a nice feature with no usecase?

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v11 0/4] Consolidating GIC per-cpu interrupts
  2011-09-08 13:14   ` Thomas Gleixner
@ 2011-09-08 16:12     ` Marc Zyngier
  2011-09-08 18:05       ` Thomas Gleixner
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2011-09-08 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

Thomas,

On 08/09/11 14:14, Thomas Gleixner wrote:

> Another thing, which sticks out compared to other percpu interrupt
> users in arch/* is that you provide the ability to assign different
> handlers on different CPUs to a given PPI interrupt number. Most other
> percpu implementations setup the interrupt with a unique percpu aware
> handler and just enable/disable it per core in the low level
> setup/shutdown code. Is running different handlers on different cores
> a real requirement or just a nice feature with no usecase?

At the moment, it sort of falls into the second category. MSM has
"asymmetric" timers (each core has its private timer on a different
PPI), but that doesn't mandate having separate handlers per core, unless
someone decides to connect something on another CPU, using the same
PPI... The architecture would probably allow it.

But a clear requirement we have is that the handler has to be called
with a per-cpu dev_id pointer (we use this to obtain the
clock_event_device in the timer handler, for example). Which makes
having something similar to request_irq() quite the natural thing.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v11 0/4] Consolidating GIC per-cpu interrupts
  2011-09-08 16:12     ` Marc Zyngier
@ 2011-09-08 18:05       ` Thomas Gleixner
  2011-09-09  8:47         ` Marc Zyngier
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2011-09-08 18:05 UTC (permalink / raw)
  To: linux-arm-kernel

Marc,

On Thu, 8 Sep 2011, Marc Zyngier wrote:
> Thomas,
> 
> On 08/09/11 14:14, Thomas Gleixner wrote:
> 
> > Another thing, which sticks out compared to other percpu interrupt
> > users in arch/* is that you provide the ability to assign different
> > handlers on different CPUs to a given PPI interrupt number. Most other
> > percpu implementations setup the interrupt with a unique percpu aware
> > handler and just enable/disable it per core in the low level
> > setup/shutdown code. Is running different handlers on different cores
> > a real requirement or just a nice feature with no usecase?
> 
> At the moment, it sort of falls into the second category. MSM has
> "asymmetric" timers (each core has its private timer on a different
> PPI), but that doesn't mandate having separate handlers per core, unless
> someone decides to connect something on another CPU, using the same
> PPI... The architecture would probably allow it.

The question is whether we really want to allow it from the OS
side. That makes irq accounting an utter mess as you end up with
devA,B,C,D on the same interrupt line and each counts on the
corresponding CPU0,1,2,3

> But a clear requirement we have is that the handler has to be called
> with a per-cpu dev_id pointer (we use this to obtain the
> clock_event_device in the timer handler, for example). Which makes
> having something similar to request_irq() quite the natural thing.

That makes a lot of sense, but it requires your extra percpu handler
registration/free interface ....

Looking at the other PERCPU irq users there might be a general
interest for this.

If we can apply the following set of (sane) restricitions to this:

   - interrupt is never shared
   - interrupt is never threaded
   - handler is common for all CPUs

then we could do something like the patch below. Warning, this is
incomplete and requires a bunch of other changes like adding per cpu
aware enable/disable functions and excluding the other interfaces from
fiddling with such an interrupt.

So a request/setup_irq() of such an interrupt would require the
following steps.

  irq_set_percpu_devid(irq);

    This would set: IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOPROBE | IRQ_PER_CPU_DEVID
    and 

  irq_set_handler(irq, handle_irq_per_cpu_devid);

  setup/request_percpu_irq(irq, .....);

    The dev_id pointer for those interrupts would be a percpu pointer
    which holds the real dev_ids, e.g. the percpu clockevents

Due to the restricted nature of those interrupts we probably can
ignore nested disable/enable_percpu_irq() calls and just keep the
*_percpu_irq API to a bare minimum.

Thoughts ?

Thanks,

	tglx

Index: linux-2.6/include/linux/interrupt.h
===================================================================
--- linux-2.6.orig/include/linux/interrupt.h
+++ linux-2.6/include/linux/interrupt.h
@@ -95,6 +97,7 @@ typedef irqreturn_t (*irq_handler_t)(int
  * @flags:	flags (see IRQF_* above)
  * @name:	name of the device
  * @dev_id:	cookie to identify the device
+ * @percpu_dev_id:	cookie to identify the device
  * @next:	pointer to the next irqaction for shared interrupts
  * @irq:	interrupt number
  * @dir:	pointer to the proc/irq/NN/name entry
@@ -104,17 +107,20 @@ typedef irqreturn_t (*irq_handler_t)(int
  * @thread_mask:	bitmask for keeping track of @thread activity
  */
 struct irqaction {
-	irq_handler_t handler;
-	unsigned long flags;
-	void *dev_id;
-	struct irqaction *next;
-	int irq;
-	irq_handler_t thread_fn;
-	struct task_struct *thread;
-	unsigned long thread_flags;
-	unsigned long thread_mask;
-	const char *name;
-	struct proc_dir_entry *dir;
+	irq_handler_t		handler;
+	unsigned long		flags;
+	union {
+		void		*dev_id;
+		void __percpu	*percpu_dev_id;
+	};
+	struct irqaction	*next;
+	int			irq;
+	irq_handler_t		thread_fn;
+	struct task_struct	*thread;
+	unsigned long		thread_flags;
+	unsigned long		thread_mask;
+	const char		*name;
+	struct proc_dir_entry	*dir;
 } ____cacheline_internodealigned_in_smp;
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
Index: linux-2.6/kernel/irq/chip.c
===================================================================
--- linux-2.6.orig/kernel/irq/chip.c
+++ linux-2.6/kernel/irq/chip.c
@@ -544,6 +544,37 @@ handle_percpu_irq(unsigned int irq, stru
 		chip->irq_eoi(&desc->irq_data);
 }
 
+/**
+ * handle_percpu_devid_irq - Per CPU local irq handler with per cpu dev ids
+ * @irq:	the interrupt number
+ * @desc:	the interrupt description structure for this irq
+ *
+ * Per CPU interrupts on SMP machines without locking requirements. Same as
+ * handle_percpu_irq() above but with the following extras:
+ *
+ * action->dev_id is a pointer to percpu variables which contain the
+ * real device id for the cpu on which this handler is called
+ */
+void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct irqaction *action = desc->action;
+	void *dev_id = __this_cpu_ptr(action->percpu_dev_id);
+	irqreturn_t res;
+
+	kstat_incr_irqs_this_cpu(irq, desc);
+
+	if (chip->irq_ack)
+		chip->irq_ack(&desc->irq_data);
+
+	trace_irq_handler_entry(irq, action);
+	res = action->handler(irq, dev_id);
+	trace_irq_handler_exit(irq, action, res);
+
+	if (chip->irq_eoi)
+		chip->irq_eoi(&desc->irq_data);
+}
+
 void
 __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
 		  const char *name)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v11 0/4] Consolidating GIC per-cpu interrupts
  2011-09-08 18:05       ` Thomas Gleixner
@ 2011-09-09  8:47         ` Marc Zyngier
  0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2011-09-09  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

Thomas,

On 08/09/11 19:05, Thomas Gleixner wrote:
> Marc,
> 
> On Thu, 8 Sep 2011, Marc Zyngier wrote:
>> Thomas,
>>
>> On 08/09/11 14:14, Thomas Gleixner wrote:
>>
>>> Another thing, which sticks out compared to other percpu interrupt
>>> users in arch/* is that you provide the ability to assign different
>>> handlers on different CPUs to a given PPI interrupt number. Most other
>>> percpu implementations setup the interrupt with a unique percpu aware
>>> handler and just enable/disable it per core in the low level
>>> setup/shutdown code. Is running different handlers on different cores
>>> a real requirement or just a nice feature with no usecase?
>>
>> At the moment, it sort of falls into the second category. MSM has
>> "asymmetric" timers (each core has its private timer on a different
>> PPI), but that doesn't mandate having separate handlers per core, unless
>> someone decides to connect something on another CPU, using the same
>> PPI... The architecture would probably allow it.
> 
> The question is whether we really want to allow it from the OS
> side. That makes irq accounting an utter mess as you end up with
> devA,B,C,D on the same interrupt line and each counts on the
> corresponding CPU0,1,2,3
> 
>> But a clear requirement we have is that the handler has to be called
>> with a per-cpu dev_id pointer (we use this to obtain the
>> clock_event_device in the timer handler, for example). Which makes
>> having something similar to request_irq() quite the natural thing.
> 
> That makes a lot of sense, but it requires your extra percpu handler
> registration/free interface ....
> 
> Looking at the other PERCPU irq users there might be a general
> interest for this.
> 
> If we can apply the following set of (sane) restricitions to this:
> 
>    - interrupt is never shared
>    - interrupt is never threaded
>    - handler is common for all CPUs
> 
> then we could do something like the patch below. Warning, this is
> incomplete and requires a bunch of other changes like adding per cpu
> aware enable/disable functions and excluding the other interfaces from
> fiddling with such an interrupt.
> 
> So a request/setup_irq() of such an interrupt would require the
> following steps.
> 
>   irq_set_percpu_devid(irq);
> 
>     This would set: IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOPROBE | IRQ_PER_CPU_DEVID
>     and 
> 
>   irq_set_handler(irq, handle_irq_per_cpu_devid);
> 
>   setup/request_percpu_irq(irq, .....);
> 
>     The dev_id pointer for those interrupts would be a percpu pointer
>     which holds the real dev_ids, e.g. the percpu clockevents
> 
> Due to the restricted nature of those interrupts we probably can
> ignore nested disable/enable_percpu_irq() calls and just keep the
> *_percpu_irq API to a bare minimum.
> 
> Thoughts ?

I quite like it. Specially if it can be useful to other architectures.
Let me glue all that together to get a feel of how it would work, and
I'll get back to you.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-09-09  8:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-09  9:56 [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Marc Zyngier
2011-08-09  9:56 ` [PATCH v11 1/4] ARM: gic: consolidate PPI handling Marc Zyngier
2011-08-09  9:56 ` [PATCH v11 2/4] ARM: gic: Add PPI registration interface Marc Zyngier
2011-08-09  9:56 ` [PATCH v11 3/4] ARM: local timers: drop local_timer_ack() Marc Zyngier
2011-08-09  9:56 ` [PATCH v11 4/4] ARM: gic: add compute_irqnr macro for exynos4 Marc Zyngier
2011-08-15 12:13 ` [PATCH v11 0/4] Consolidating GIC per-cpu interrupts Russell King - ARM Linux
2011-09-08 13:14   ` Thomas Gleixner
2011-09-08 16:12     ` Marc Zyngier
2011-09-08 18:05       ` Thomas Gleixner
2011-09-09  8:47         ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).