All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
@ 2011-07-05  8:49 Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 01/14] ARM: gic: add per-cpu interrupt mapping Marc Zyngier
                   ` (15 more replies)
  0 siblings, 16 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 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...

The proposed solution is to let the GIC code expose the PPIs as
something that the kernel can manage. Instead of having a single
interrupt number shared on all cores, make the interrupt number be
different on each CPU.

This enables the use of the normal kernel API (request_irq() and
friends) and the elimination of some low level code. It also serves as
the basis of the GIC device tree support code. On the other side, it
causes quite a bit of churn in the timer code.

Previous versions of this series have received ACKs from some platform
maintainers, but so far there has been very little comment on the core
code. As I now have quite a large number of patches relying on this
series (local timers as devices, device tree support for GIC and TWD),
I'd be grateful for at least a hint indicating that this is going in
the right direction. Or not.

This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2
(Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git

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

* [PATCH v8 01/14] ARM: gic: add per-cpu interrupt mapping
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 02/14] ARM: smp: add interrupt handler for local timers Marc Zyngier
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

The kernel doesn't handle very well the concept of per-cpu interrupt
as implemented by the ARM architecture (the same interrupt level is
exposed on each core).

To work around the problem, add another irq_chip to handle PPIs and
remap them so that a single interrupt number is only used on a given
CPU (for example, IRQ 29 and 30 get exposed as IRQ 128 and 129 on
core 0, 130 and 131 on core 1...).

A helper function gic_ppi_map() is used to convert the PPI number
to the per-processor IRQ.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Tested-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/common/Kconfig                         |    5 +
 arch/arm/common/gic.c                           |  137 ++++++++++++++++++++++-
 arch/arm/include/asm/entry-macro-multi.S        |    2 +-
 arch/arm/include/asm/hardware/entry-macro-gic.S |   19 ++-
 arch/arm/include/asm/hardware/gic.h             |   11 ++
 arch/arm/kernel/irq.c                           |    8 +-
 6 files changed, 169 insertions(+), 13 deletions(-)

diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 4b71766..daf6272 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -1,6 +1,11 @@
 config ARM_GIC
 	bool
 
+config ARM_GIC_PPI_MAP
+	depends on ARM_GIC
+	select SPARSE_IRQ
+	bool
+
 config ARM_VIC
 	bool
 
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4ddd0a6..e0ad9f0 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -42,6 +42,12 @@ struct gic_chip_data {
 	unsigned int irq_offset;
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+	/* These fields must be 0 on secondary GICs */
+	int	     ppi_base;
+	int	     vppi_base;
+	u16	     nrppis;
+#endif
 };
 
 /*
@@ -262,12 +268,92 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+unsigned int gic_ppi_map(unsigned int irq)
+{
+	struct gic_chip_data *chip_data = irq_get_chip_data(irq);
+	unsigned int vppi_irq;
+	unsigned int ppi;
+
+	WARN_ON(!chip_data->vppi_base);
+
+	ppi = irq - chip_data->ppi_base;
+	vppi_irq = ppi + chip_data->nrppis * smp_processor_id();
+	vppi_irq += chip_data->vppi_base;
+
+	return vppi_irq;
+}
+
+static void gic_handle_ppi(unsigned int irq, struct irq_desc *desc)
+{
+	unsigned int vppi_irq;
+
+	vppi_irq = gic_ppi_map(irq);
+	generic_handle_irq(vppi_irq);
+}
+
+static struct irq_data *gic_vppi_to_ppi(struct irq_data *d)
+{
+	struct gic_chip_data *chip_data = irq_data_get_irq_chip_data(d);
+	unsigned int ppi_irq;
+
+	ppi_irq = d->irq - chip_data->vppi_base - chip_data->nrppis * smp_processor_id();
+	ppi_irq += chip_data->ppi_base;
+
+	return irq_get_irq_data(ppi_irq);
+}
+
+static void gic_ppi_eoi_irq(struct irq_data *d)
+{
+	gic_eoi_irq(gic_vppi_to_ppi(d));
+}
+
+static void gic_ppi_mask_irq(struct irq_data *d)
+{
+	gic_mask_irq(gic_vppi_to_ppi(d));
+}
+
+static void gic_ppi_unmask_irq(struct irq_data *d)
+{
+	gic_unmask_irq(gic_vppi_to_ppi(d));
+}
+
+static int gic_ppi_set_type(struct irq_data *d, unsigned int type)
+{
+	return gic_set_type(gic_vppi_to_ppi(d), type);
+}
+
+#ifdef CONFIG_PM
+static int gic_ppi_set_wake(struct irq_data *d, unsigned int on)
+{
+	return gic_set_wake(gic_vppi_to_ppi(d), on);
+}
+#else
+#define gic_ppi_set_wake	NULL
+#endif
+
+static int __init gic_irq_is_ppi(struct gic_chip_data *gic, unsigned int irq)
+{
+	return (irq >= (gic->irq_offset + 16) && irq <= (gic->irq_offset + 31));
+}
+
+static struct irq_chip gic_ppi_chip = {
+	.name			= "GIC-PPI",
+	.irq_eoi		= gic_ppi_eoi_irq,
+	.irq_mask		= gic_ppi_mask_irq,
+	.irq_unmask		= gic_ppi_unmask_irq,
+	.irq_set_type		= gic_ppi_set_type,
+	.irq_set_wake		= gic_ppi_set_wake,
+};
+#endif
+
 static void __init gic_dist_init(struct gic_chip_data *gic,
 	unsigned int irq_start)
 {
-	unsigned int gic_irqs, irq_limit, i;
+	unsigned int gic_irqs, irq_limit, i, nrvppis = 0;
 	void __iomem *base = gic->dist_base;
 	u32 cpumask = 1 << smp_processor_id();
+	u32 dist_ctr, nrcpus;
 
 	cpumask |= cpumask << 8;
 	cpumask |= cpumask << 16;
@@ -278,11 +364,32 @@ 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;
+
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+	/*
+	 * Nobody would be insane enough to use PPIs on a secondary
+	 * GIC, right?
+	 */
+	if (gic == &gic_data[0]) {
+		gic->nrppis = 16 - (irq_start % 16);
+		gic->ppi_base = gic->irq_offset + 32 - gic->nrppis;
+		nrvppis = gic->nrppis * nrcpus;
+	} else {
+		gic->ppi_base = 0;
+		gic->vppi_base = 0;
+	}
+#endif
+
+	pr_info("Configuring GIC with %d sources (%d additional PPIs)\n",
+		gic_irqs, nrvppis);
+
 	/*
 	 * Set all global interrupts to be level triggered, active low.
 	 */
@@ -319,10 +426,32 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	 * Setup the Linux IRQ subsystem.
 	 */
 	for (i = irq_start; i < irq_limit; i++) {
-		irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+		if (nrvppis && gic_irq_is_ppi(gic, i))
+			irq_set_chip_and_handler(i, &gic_chip, gic_handle_ppi);
+		else
+#endif
+		{
+			irq_set_chip_and_handler(i, &gic_chip,
+						 handle_fasteoi_irq);
+			set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
+		}
+		irq_set_chip_data(i, gic);
+	}
+
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+	if (!nrvppis)
+		goto out;
+	gic->vppi_base = irq_alloc_descs(-1, 0, nrvppis, 0);
+	if (WARN_ON(gic->vppi_base < 0))
+		goto out;
+	for (i = gic->vppi_base; i < (gic->vppi_base + nrvppis); i++) {
+		irq_set_chip_and_handler(i, &gic_ppi_chip, handle_percpu_irq);
 		irq_set_chip_data(i, gic);
 		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
 	}
+out:
+#endif
 
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
diff --git a/arch/arm/include/asm/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index 2da8547..c3f9db4 100644
--- a/arch/arm/include/asm/entry-macro-multi.S
+++ b/arch/arm/include/asm/entry-macro-multi.S
@@ -26,7 +26,7 @@
 	adrne	lr, BSYM(1b)
 	bne	do_IPI
 
-#ifdef CONFIG_LOCAL_TIMERS
+#if defined(CONFIG_LOCAL_TIMERS) && !defined(CONFIG_ARM_GIC_PPI_MAP)
 	test_for_ltirq r0, r6, r5, lr
 	movne	r0, sp
 	adrne	lr, BSYM(1b)
diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index c115b82..cf8895f 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -17,23 +17,26 @@
 	.endm
 #endif
 
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+#define DO_IRQ_BASE	16
+#else
+#define DO_IRQ_BASE	30
+#endif
+
 /*
  * 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.
+ * 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).
+ * valid range for an IRQ (30-1020 inclusive). If CONFIG_ARM_GIC_PPI_MAP is
+ * enabled, local interrupts are handled the same way as global ones.
  */
 
 	.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
@@ -43,7 +46,7 @@
 
 	ldr	\tmp, =1021
 	bic     \irqnr, \irqstat, #0x1c00
-	cmp     \irqnr, #29
+	cmp     \irqnr, #(DO_IRQ_BASE - 1)
 	cmpcc	\irqnr, \irqnr
 	cmpne	\irqnr, \tmp
 	cmpcs	\irqnr, \irqnr
@@ -63,6 +66,7 @@
 	cmpcs	\irqnr, \irqnr
 	.endm
 
+#ifndef CONFIG_ARM_GIC_PPI_MAP
 /* As above, this assumes that irqstat and base are preserved.. */
 
 	.macro test_for_ltirq, irqnr, irqstat, base, tmp
@@ -73,3 +77,4 @@
 	streq	\irqstat, [\base, #GIC_CPU_EOI]
 	cmp	\tmp, #0
 	.endm
+#endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 0691f9d..cdaff9d 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/cpumask.h>
+
 extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
@@ -41,6 +43,15 @@ 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);
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+unsigned int gic_ppi_map(unsigned int irq);
+#else
+static inline unsigned int gic_ppi_map(unsigned int irq)
+{
+	return irq;
+}
+#endif
+
 #endif
 
 #endif
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 83bbad0..a033d75 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -124,8 +124,14 @@ void __init init_IRQ(void)
 #ifdef CONFIG_SPARSE_IRQ
 int __init arch_probe_nr_irqs(void)
 {
+	int initcnt;
+
 	nr_irqs = machine_desc->nr_irqs ? machine_desc->nr_irqs : NR_IRQS;
-	return nr_irqs;
+	initcnt = nr_irqs;
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+	nr_irqs += 16 * 8;	/* 16 PPIs, 8 CPUs */
+#endif
+	return initcnt;
 }
 #endif
 
-- 
1.7.0.4

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

* [PATCH v8 02/14] ARM: smp: add interrupt handler for local timers
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 01/14] ARM: gic: add per-cpu interrupt mapping Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts Marc Zyngier
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Provide a "normal" interrupt handler for local timers.
It is expected that most timer implementations will use this
handler unless they have more specific requirements.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/localtimer.h |    6 ++++++
 arch/arm/kernel/smp.c             |   12 ++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 080d74f..bef44b3 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;
 
 /*
@@ -22,6 +24,10 @@ void percpu_timer_setup(void);
  */
 asmlinkage void do_local_timer(struct pt_regs *);
 
+/*
+ * Per-cpu timer IRQ handler
+ */
+irqreturn_t percpu_timer_handler(int irq, void *dev_id);
 
 #ifdef CONFIG_LOCAL_TIMERS
 
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index e7f92a4..a35b38d 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -464,6 +464,18 @@ asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs)
 	set_irq_regs(old_regs);
 }
 
+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;
+}
+
 void show_local_irqs(struct seq_file *p, int prec)
 {
 	unsigned int cpu;
-- 
1.7.0.4

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

* [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 01/14] ARM: gic: add per-cpu interrupt mapping Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 02/14] ARM: smp: add interrupt handler for local timers Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-08 19:27   ` Russell King - ARM Linux
  2011-07-05  8:49 ` [PATCH v8 04/14] ARM: omap4: use remapped PPI interrupts for local timer Marc Zyngier
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

When CONFIG_ARM_GIC_PPI_MAP is enabled, let smp_twd request
interrupts the normal way (ie using request_irq()).

This involves letting PPIs go via the same code path as SPIs and
having normal interrupt handler for the local timer code.

The previous ad-hoc code is still supported when CONFIG_ARM_GIC_PPI_MAP
is not defined.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/smp_twd.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 60636f4..2a37320 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -15,16 +15,18 @@
 #include <linux/smp.h>
 #include <linux/jiffies.h>
 #include <linux/clockchips.h>
-#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 
 #include <asm/smp_twd.h>
+#include <asm/localtimer.h>
 #include <asm/hardware/gic.h>
 
 /* set up by the platform code */
 void __iomem *twd_base;
 
 static unsigned long twd_timer_rate;
+static DEFINE_PER_CPU(bool, irq_reqd);
 
 static void twd_set_mode(enum clock_event_mode mode,
 			struct clock_event_device *clk)
@@ -43,6 +45,10 @@ static void twd_set_mode(enum clock_event_mode mode,
 		ctrl = TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_ONESHOT;
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+		disable_irq(clk->irq);
+		/* fall through */
+#endif
 	case CLOCK_EVT_MODE_SHUTDOWN:
 	default:
 		ctrl = 0;
@@ -124,6 +130,9 @@ static void __cpuinit twd_calibrate_rate(void)
  */
 void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 {
+	int err;
+	bool *reqd;
+
 	twd_calibrate_rate();
 
 	clk->name = "local_timer";
@@ -137,8 +146,25 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 	clk->max_delta_ns = clockevent_delta2ns(0xffffffff, clk);
 	clk->min_delta_ns = clockevent_delta2ns(0xf, clk);
 
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+	reqd = &__get_cpu_var(irq_reqd);
+	if (!*reqd) {
+		err = request_irq(clk->irq, percpu_timer_handler,
+				  IRQF_PERCPU | IRQF_NOBALANCING | IRQF_TIMER,
+				  clk->name, clk);
+		if (err) {
+			pr_err("%s: can't register interrupt %d on cpu %d (%d)\n",
+			       clk->name, clk->irq, smp_processor_id(), err);
+			return;
+		}
+
+		*reqd = true;
+	} else
+		enable_irq(clk->irq);
+#else
 	/* Make sure our local interrupt controller has this enabled */
 	gic_enable_ppi(clk->irq);
+#endif
 
 	clockevents_register_device(clk);
 }
-- 
1.7.0.4

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

* [PATCH v8 04/14] ARM: omap4: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (2 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-07 14:32   ` Tony Lindgren
  2011-07-05  8:49 ` [PATCH v8 05/14] ARM: versatile: " Marc Zyngier
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

Tested on a Pandaboard.

Cc: Tony Lindgren <tony@atomide.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-omap2/Kconfig                    |    1 +
 arch/arm/mach-omap2/include/mach/entry-macro.S |   14 +-------------
 arch/arm/mach-omap2/timer-mpu.c                |    3 ++-
 3 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 19d5891..833717f 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -44,6 +44,7 @@ config ARCH_OMAP4
 	depends on ARCH_OMAP2PLUS
 	select CPU_V7
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select LOCAL_TIMERS if SMP
 	select PL310_ERRATA_588369
 	select PL310_ERRATA_727915
diff --git a/arch/arm/mach-omap2/include/mach/entry-macro.S b/arch/arm/mach-omap2/include/mach/entry-macro.S
index a48690b..22d86ef 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-omap2/timer-mpu.c b/arch/arm/mach-omap2/timer-mpu.c
index 31c0ac4..61b57fb 100644
--- a/arch/arm/mach-omap2/timer-mpu.c
+++ b/arch/arm/mach-omap2/timer-mpu.c
@@ -22,6 +22,7 @@
 #include <asm/irq.h>
 #include <asm/smp_twd.h>
 #include <asm/localtimer.h>
+#include <asm/hardware/gic.h>
 
 /*
  * Setup the local clock events for a CPU.
@@ -32,7 +33,7 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 	if (omap_rev() == OMAP4430_REV_ES1_0)
 		return -ENXIO;
 
-	evt->irq = OMAP44XX_IRQ_LOCALTIMER;
+	evt->irq = gic_ppi_map(OMAP44XX_IRQ_LOCALTIMER);
 	twd_timer_setup(evt);
 	return 0;
 }
-- 
1.7.0.4

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

* [PATCH v8 05/14] ARM: versatile: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (3 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 04/14] ARM: omap4: use remapped PPI interrupts for local timer Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 06/14] ARM: shmobile: " Marc Zyngier
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

Tested on VExpress and PB-11MP.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-realview/Kconfig       |    2 ++
 arch/arm/mach-vexpress/Kconfig       |    1 +
 arch/arm/plat-versatile/localtimer.c |    3 ++-
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
index b9a9805..b236996 100644
--- a/arch/arm/mach-realview/Kconfig
+++ b/arch/arm/mach-realview/Kconfig
@@ -38,6 +38,7 @@ config MACH_REALVIEW_PB11MP
 	bool "Support RealView(R) Platform Baseboard for ARM11MPCore"
 	select CPU_V6K
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select HAVE_PATA_PLATFORM
 	select ARCH_HAS_BARRIERS if SMP
 	help
@@ -76,6 +77,7 @@ config MACH_REALVIEW_PBA8
 config MACH_REALVIEW_PBX
 	bool "Support RealView(R) Platform Baseboard Explore"
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select HAVE_PATA_PLATFORM
 	select ARCH_SPARSEMEM_ENABLE if CPU_V7 && !REALVIEW_HIGH_PHYS_OFFSET
 	select ZONE_DMA if SPARSEMEM
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 9311484..fb34d1e 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -5,6 +5,7 @@ config ARCH_VEXPRESS_CA9X4
 	bool "Versatile Express Cortex-A9x4 tile"
 	select CPU_V7
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
diff --git a/arch/arm/plat-versatile/localtimer.c b/arch/arm/plat-versatile/localtimer.c
index 0fb3961..0589e24 100644
--- a/arch/arm/plat-versatile/localtimer.c
+++ b/arch/arm/plat-versatile/localtimer.c
@@ -14,6 +14,7 @@
 
 #include <asm/smp_twd.h>
 #include <asm/localtimer.h>
+#include <asm/hardware/gic.h>
 #include <mach/irqs.h>
 
 /*
@@ -21,7 +22,7 @@
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
-	evt->irq = IRQ_LOCALTIMER;
+	evt->irq = gic_ppi_map(IRQ_LOCALTIMER);
 	twd_timer_setup(evt);
 	return 0;
 }
-- 
1.7.0.4

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

* [PATCH v8 06/14] ARM: shmobile: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (4 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 05/14] ARM: versatile: " Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 07/14] ARM: ux500: " Marc Zyngier
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

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/mach-shmobile/Kconfig                    |    1 +
 arch/arm/mach-shmobile/entry-intc.S               |    3 ---
 arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 ---
 arch/arm/mach-shmobile/localtimer.c               |    3 ++-
 4 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 0c8f6cf..ca8e06c 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -26,6 +26,7 @@ config ARCH_SH73A0
 	select SH_CLK_CPG
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 
 comment "SH-Mobile Board Type"
 
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
diff --git a/arch/arm/mach-shmobile/localtimer.c b/arch/arm/mach-shmobile/localtimer.c
index ad9ccc9..b6166d8 100644
--- a/arch/arm/mach-shmobile/localtimer.c
+++ b/arch/arm/mach-shmobile/localtimer.c
@@ -14,13 +14,14 @@
 #include <linux/clockchips.h>
 #include <asm/smp_twd.h>
 #include <asm/localtimer.h>
+#include <asm/hardware/gic.h>
 
 /*
  * Setup the local clock events for a CPU.
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
-	evt->irq = 29;
+	evt->irq = gic_ppi_map(29);
 	twd_timer_setup(evt);
 	return 0;
 }
-- 
1.7.0.4

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

* [PATCH v8 07/14] ARM: ux500: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (5 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 06/14] ARM: shmobile: " Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 08/14] ARM: tegra: " Marc Zyngier
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

Cc: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>
Cc: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-ux500/Kconfig      |    1 +
 arch/arm/mach-ux500/localtimer.c |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig
index f8b9392..a48b83c 100644
--- a/arch/arm/mach-ux500/Kconfig
+++ b/arch/arm/mach-ux500/Kconfig
@@ -4,6 +4,7 @@ config UX500_SOC_COMMON
 	bool
 	default y
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select HAS_MTU
 	select ARM_ERRATA_753970
 
diff --git a/arch/arm/mach-ux500/localtimer.c b/arch/arm/mach-ux500/localtimer.c
index 5ba1133..53e9aba 100644
--- a/arch/arm/mach-ux500/localtimer.c
+++ b/arch/arm/mach-ux500/localtimer.c
@@ -17,13 +17,14 @@
 #include <asm/irq.h>
 #include <asm/smp_twd.h>
 #include <asm/localtimer.h>
+#include <asm/hardware/gic.h>
 
 /*
  * Setup the local clock events for a CPU.
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
-	evt->irq = IRQ_LOCALTIMER;
+	evt->irq = gic_ppi_map(IRQ_LOCALTIMER);
 	twd_timer_setup(evt);
 	return 0;
 }
-- 
1.7.0.4

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

* [PATCH v8 08/14] ARM: tegra: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (6 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 07/14] ARM: ux500: " Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 09/14] ARM: msm: " Marc Zyngier
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

Tested on a Harmony Tegra-2 board.

Cc: Colin Cross <ccross@android.com>
Cc: Erik Gilling <konkers@android.com>
Cc: Olof Johansson <olof@lixom.net>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-tegra/Kconfig      |    1 +
 arch/arm/mach-tegra/localtimer.c |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index 5ec1846..dad8f6a 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -9,6 +9,7 @@ config ARCH_TEGRA_2x_SOC
 	bool "Tegra 2 family"
 	select CPU_V7
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select ARCH_REQUIRE_GPIOLIB
 	select USB_ARCH_HAS_EHCI if USB_SUPPORT
 	select USB_ULPI if USB_SUPPORT
diff --git a/arch/arm/mach-tegra/localtimer.c b/arch/arm/mach-tegra/localtimer.c
index e91d681..d148a25 100644
--- a/arch/arm/mach-tegra/localtimer.c
+++ b/arch/arm/mach-tegra/localtimer.c
@@ -14,13 +14,14 @@
 #include <asm/irq.h>
 #include <asm/smp_twd.h>
 #include <asm/localtimer.h>
+#include <asm/hardware/gic.h>
 
 /*
  * Setup the local clock events for a CPU.
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
-	evt->irq = IRQ_LOCALTIMER;
+	evt->irq = gic_ppi_map(IRQ_LOCALTIMER);
 	twd_timer_setup(evt);
 	return 0;
 }
-- 
1.7.0.4

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

* [PATCH v8 09/14] ARM: msm: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (7 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 08/14] ARM: tegra: " Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05 18:06   ` David Brown
  2011-07-05  8:49 ` [PATCH v8 10/14] ARM: exynos4: " Marc Zyngier
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

MSM already had a very similar scheme, though still mixing both
GIC-specific and generic APIs.

Fixes and ideas courtesy of Stephen Boyd.

Cc: David Brown <davidb@codeaurora.org>
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Bryan Huntsman <bryanh@codeaurora.org>
Reviewed-and-Tested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-msm/Kconfig                         |    2 +
 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                         |   58 ++++++++--------
 4 files changed, 32 insertions(+), 112 deletions(-)

diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index 1516896..5be3ee9 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -42,6 +42,7 @@ config ARCH_MSM8X60
 				  && !MACH_MSM8X60_FFA)
 	select ARCH_MSM_SCORPIONMP
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select CPU_V7
 	select MSM_V2_TLMM
 	select MSM_GPIOMUX
@@ -52,6 +53,7 @@ config ARCH_MSM8960
 	select ARCH_MSM_SCORPIONMP
 	select MACH_MSM8960_SIM if (!MACH_MSM8960_RUMI3)
 	select ARM_GIC
+	select ARM_GIC_PPI_MAP
 	select CPU_V7
 	select MSM_V2_TLMM
 	select MSM_GPIOMUX
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-msm/timer.c b/arch/arm/mach-msm/timer.c
index 63621f1..d8d5296 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);
@@ -148,6 +145,8 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
 		writel(TIMER_ENABLE_EN, clock->regbase + TIMER_ENABLE);
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
+		disable_irq(evt->irq);
+		/* fall through */
 	case CLOCK_EVT_MODE_SHUTDOWN:
 		writel(0, clock->regbase + TIMER_ENABLE);
 		break;
@@ -171,13 +170,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 +189,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 +248,13 @@ 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 = gic_ppi_map(clock->irq);
+		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 +263,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 +273,7 @@ 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) {
-		writel(0, clock->regbase  + TIMER_ENABLE);
-		writel(0, clock->regbase + TIMER_CLEAR);
-		writel(~0, clock->regbase + TIMER_MATCH_VAL);
-	}
-	evt->irq = clock->irq.irq;
+	evt->irq = gic_ppi_map(clock->irq);
 	evt->name = "local_timer";
 	evt->features = CLOCK_EVT_FEAT_ONESHOT;
 	evt->rating = clock->clockevent.rating;
@@ -298,9 +285,22 @@ 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;
+	if (!local_timer_inited) {
+		writel(0, clock->regbase  + TIMER_ENABLE);
+		writel(0, clock->regbase + TIMER_CLEAR);
+		writel(~0, clock->regbase + TIMER_MATCH_VAL);
 
-	gic_enable_ppi(clock->irq.irq);
+		res = request_irq(evt->irq, msm_timer_interrupt,
+				  IRQF_TIMER | IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
+				  clock->clockevent.name, evt);
+		if (res) {
+			pr_err("local_timer_setup: request_irq failed for %s\n",
+			       clock->clockevent.name);
+			return res;
+		}
+		local_timer_inited = true;
+	} else
+		enable_irq(evt->irq);
 
 	clockevents_register_device(evt);
 	return 0;
-- 
1.7.0.4

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

* [PATCH v8 10/14] ARM: exynos4: use remapped PPI interrupts for local timer
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (8 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 09/14] ARM: msm: " Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 11/14] ARM: gic: remove previous local timer interrupt handling Marc Zyngier
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.

Tested on a SMDK-S5PV310 board.

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/mach-exynos4/include/mach/entry-macro.S |   12 +-----------
 arch/arm/mach-exynos4/localtimer.c               |    3 ++-
 arch/arm/plat-s5p/Kconfig                        |    1 +
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d8f38c2..f007168 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -50,7 +50,7 @@
 
 		bic     \irqnr, \irqstat, #0x1c00
 
-		cmp     \irqnr, #29
+		cmp     \irqnr, #15
 		cmpcc	\irqnr, \irqnr
 		cmpne	\irqnr, \tmp
 		cmpcs	\irqnr, \irqnr
@@ -72,13 +72,3 @@
 		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/mach-exynos4/localtimer.c b/arch/arm/mach-exynos4/localtimer.c
index 6bf3d0a..a08cf85 100644
--- a/arch/arm/mach-exynos4/localtimer.c
+++ b/arch/arm/mach-exynos4/localtimer.c
@@ -14,13 +14,14 @@
 
 #include <asm/irq.h>
 #include <asm/localtimer.h>
+#include <asm/hardware/gic.h>
 
 /*
  * Setup the local clock events for a CPU.
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
-	evt->irq = IRQ_LOCALTIMER;
+	evt->irq = gic_ppi_map(IRQ_LOCALTIMER);
 	twd_timer_setup(evt);
 	return 0;
 }
diff --git a/arch/arm/plat-s5p/Kconfig b/arch/arm/plat-s5p/Kconfig
index e98f5c5..40e6e45 100644
--- a/arch/arm/plat-s5p/Kconfig
+++ b/arch/arm/plat-s5p/Kconfig
@@ -11,6 +11,7 @@ config PLAT_S5P
 	default y
 	select ARM_VIC if !ARCH_EXYNOS4
 	select ARM_GIC if ARCH_EXYNOS4
+	select ARM_GIC_PPI_MAP if ARCH_EXYNOS4
 	select NO_IOPORT
 	select ARCH_REQUIRE_GPIOLIB
 	select S3C_GPIO_TRACK
-- 
1.7.0.4

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

* [PATCH v8 11/14] ARM: gic: remove previous local timer interrupt handling
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (9 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 10/14] ARM: exynos4: " Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 12/14] ARM: gic: add compute_irqnr macro for exynos4 Marc Zyngier
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Now that all users of local timers are using CONFIG_ARM_GIC_PPI_MAP,
the now unused local timer infrastructure can be safely removed.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/common/gic.c                           |   10 --------
 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 |   13 ----------
 arch/arm/include/asm/hardware/gic.h             |    1 -
 arch/arm/include/asm/localtimer.h               |    5 ----
 arch/arm/include/asm/smp.h                      |    5 ----
 arch/arm/kernel/irq.c                           |    3 --
 arch/arm/kernel/smp.c                           |   29 -----------------------
 arch/arm/kernel/smp_twd.c                       |    7 -----
 10 files changed, 0 insertions(+), 83 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index e0ad9f0..14ab235 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -505,16 +505,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/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index c3f9db4..7d0eae3 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
-
-#if defined(CONFIG_LOCAL_TIMERS) && !defined(CONFIG_ARM_GIC_PPI_MAP)
-	test_for_ltirq r0, r6, r5, 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 cf8895f..661661b 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -65,16 +65,3 @@
 	strcc	\irqstat, [\base, #GIC_CPU_EOI]
 	cmpcs	\irqnr, \irqnr
 	.endm
-
-#ifndef CONFIG_ARM_GIC_PPI_MAP
-/* 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
-#endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index cdaff9d..bad26c2 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -42,7 +42,6 @@ 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);
 #ifdef CONFIG_ARM_GIC_PPI_MAP
 unsigned int gic_ppi_map(unsigned int irq);
 #else
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index bef44b3..5c8acb4 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -20,11 +20,6 @@ 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 *);
-
-/*
  * Per-cpu timer IRQ handler
  */
 irqreturn_t percpu_timer_handler(int irq, void *dev_id);
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 a033d75..39cc28c 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 a35b38d..0082bfd 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -430,10 +430,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;
 }
 
@@ -451,19 +447,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);
-}
-
 irqreturn_t percpu_timer_handler(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
@@ -475,18 +458,6 @@ irqreturn_t percpu_timer_handler(int irq, void *dev_id)
 
 	return IRQ_NONE;
 }
-
-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");
-}
 #endif
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 2a37320..aa99656 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -45,10 +45,8 @@ static void twd_set_mode(enum clock_event_mode mode,
 		ctrl = TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_ONESHOT;
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
-#ifdef CONFIG_ARM_GIC_PPI_MAP
 		disable_irq(clk->irq);
 		/* fall through */
-#endif
 	case CLOCK_EVT_MODE_SHUTDOWN:
 	default:
 		ctrl = 0;
@@ -146,7 +144,6 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 	clk->max_delta_ns = clockevent_delta2ns(0xffffffff, clk);
 	clk->min_delta_ns = clockevent_delta2ns(0xf, clk);
 
-#ifdef CONFIG_ARM_GIC_PPI_MAP
 	reqd = &__get_cpu_var(irq_reqd);
 	if (!*reqd) {
 		err = request_irq(clk->irq, percpu_timer_handler,
@@ -161,10 +158,6 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 		*reqd = true;
 	} else
 		enable_irq(clk->irq);
-#else
-	/* Make sure our local interrupt controller has this enabled */
-	gic_enable_ppi(clk->irq);
-#endif
 
 	clockevents_register_device(clk);
 }
-- 
1.7.0.4

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

* [PATCH v8 12/14] ARM: gic: add compute_irqnr macro for exynos4
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (10 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 11/14] ARM: gic: remove previous local timer interrupt handling Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 13/14] ARM: SMP: automatically select ARM_GIC_PPI_MAP Marc Zyngier
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 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 |   60 ++--------------------
 2 files changed, 8 insertions(+), 55 deletions(-)

diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index 661661b..469ef8c 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -50,6 +50,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 f007168..7d87d4e 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -9,66 +9,16 @@
  * warranty of any kind, whether express or implied.
 */
 
-#include <mach/hardware.h>
-#include <asm/hardware/gic.h>
+#define HAVE_GIC_BASE_OFFSET	1
+#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]
+		.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] 34+ messages in thread

* [PATCH v8 13/14] ARM: SMP: automatically select ARM_GIC_PPI_MAP
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (11 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 12/14] ARM: gic: add compute_irqnr macro for exynos4 Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-05  8:49 ` [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu() Marc Zyngier
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

All SMP platforms are selecting ARM_GIC_PPI_MAP.
Instead, make SMP select ARM_GIC_PPI_MAP and remove select statements
from the platform code.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/Kconfig               |    1 +
 arch/arm/mach-msm/Kconfig      |    2 --
 arch/arm/mach-omap2/Kconfig    |    1 -
 arch/arm/mach-realview/Kconfig |    2 --
 arch/arm/mach-shmobile/Kconfig |    1 -
 arch/arm/mach-tegra/Kconfig    |    1 -
 arch/arm/mach-ux500/Kconfig    |    1 -
 arch/arm/mach-vexpress/Kconfig |    1 -
 arch/arm/plat-s5p/Kconfig      |    1 -
 9 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9adc278..a06ab26 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1313,6 +1313,7 @@ config SMP
 		 ARCH_EXYNOS4 || ARCH_TEGRA || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || \
 		 ARCH_MSM_SCORPIONMP || ARCH_SHMOBILE
 	select USE_GENERIC_SMP_HELPERS
+	select ARM_GIC_PPI_MAP
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
 	help
 	  This enables support for systems with more than one CPU. If you have
diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index 5be3ee9..1516896 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -42,7 +42,6 @@ config ARCH_MSM8X60
 				  && !MACH_MSM8X60_FFA)
 	select ARCH_MSM_SCORPIONMP
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select CPU_V7
 	select MSM_V2_TLMM
 	select MSM_GPIOMUX
@@ -53,7 +52,6 @@ config ARCH_MSM8960
 	select ARCH_MSM_SCORPIONMP
 	select MACH_MSM8960_SIM if (!MACH_MSM8960_RUMI3)
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select CPU_V7
 	select MSM_V2_TLMM
 	select MSM_GPIOMUX
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 833717f..19d5891 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -44,7 +44,6 @@ config ARCH_OMAP4
 	depends on ARCH_OMAP2PLUS
 	select CPU_V7
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select LOCAL_TIMERS if SMP
 	select PL310_ERRATA_588369
 	select PL310_ERRATA_727915
diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
index b236996..b9a9805 100644
--- a/arch/arm/mach-realview/Kconfig
+++ b/arch/arm/mach-realview/Kconfig
@@ -38,7 +38,6 @@ config MACH_REALVIEW_PB11MP
 	bool "Support RealView(R) Platform Baseboard for ARM11MPCore"
 	select CPU_V6K
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select HAVE_PATA_PLATFORM
 	select ARCH_HAS_BARRIERS if SMP
 	help
@@ -77,7 +76,6 @@ config MACH_REALVIEW_PBA8
 config MACH_REALVIEW_PBX
 	bool "Support RealView(R) Platform Baseboard Explore"
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select HAVE_PATA_PLATFORM
 	select ARCH_SPARSEMEM_ENABLE if CPU_V7 && !REALVIEW_HIGH_PHYS_OFFSET
 	select ZONE_DMA if SPARSEMEM
diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index ca8e06c..0c8f6cf 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -26,7 +26,6 @@ config ARCH_SH73A0
 	select SH_CLK_CPG
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 
 comment "SH-Mobile Board Type"
 
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index dad8f6a..5ec1846 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -9,7 +9,6 @@ config ARCH_TEGRA_2x_SOC
 	bool "Tegra 2 family"
 	select CPU_V7
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select ARCH_REQUIRE_GPIOLIB
 	select USB_ARCH_HAS_EHCI if USB_SUPPORT
 	select USB_ULPI if USB_SUPPORT
diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig
index a48b83c..f8b9392 100644
--- a/arch/arm/mach-ux500/Kconfig
+++ b/arch/arm/mach-ux500/Kconfig
@@ -4,7 +4,6 @@ config UX500_SOC_COMMON
 	bool
 	default y
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select HAS_MTU
 	select ARM_ERRATA_753970
 
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index fb34d1e..9311484 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -5,7 +5,6 @@ config ARCH_VEXPRESS_CA9X4
 	bool "Versatile Express Cortex-A9x4 tile"
 	select CPU_V7
 	select ARM_GIC
-	select ARM_GIC_PPI_MAP
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
diff --git a/arch/arm/plat-s5p/Kconfig b/arch/arm/plat-s5p/Kconfig
index 40e6e45..e98f5c5 100644
--- a/arch/arm/plat-s5p/Kconfig
+++ b/arch/arm/plat-s5p/Kconfig
@@ -11,7 +11,6 @@ config PLAT_S5P
 	default y
 	select ARM_VIC if !ARCH_EXYNOS4
 	select ARM_GIC if ARCH_EXYNOS4
-	select ARM_GIC_PPI_MAP if ARCH_EXYNOS4
 	select NO_IOPORT
 	select ARCH_REQUIRE_GPIOLIB
 	select S3C_GPIO_TRACK
-- 
1.7.0.4

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (12 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 13/14] ARM: SMP: automatically select ARM_GIC_PPI_MAP Marc Zyngier
@ 2011-07-05  8:49 ` Marc Zyngier
  2011-07-08 19:57   ` Russell King - ARM Linux
  2011-07-06  5:46 ` [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Shawn Guo
  2011-07-06 11:02 ` Marc Zyngier
  15 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-05  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

It is sometimes usefull to compute the mapping of a PPI on a CPU
that is not the one handling the interrupt (in the probe function
of a driver, for example).

The new gic_ppi_map_on_cpu() function does just that.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/common/gic.c               |    9 +++++++--
 arch/arm/include/asm/hardware/gic.h |    7 +++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 14ab235..70ef6a8 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -269,7 +269,7 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 }
 
 #ifdef CONFIG_ARM_GIC_PPI_MAP
-unsigned int gic_ppi_map(unsigned int irq)
+unsigned int gic_ppi_map_on_cpu(unsigned int irq, int cpu)
 {
 	struct gic_chip_data *chip_data = irq_get_chip_data(irq);
 	unsigned int vppi_irq;
@@ -278,12 +278,17 @@ unsigned int gic_ppi_map(unsigned int irq)
 	WARN_ON(!chip_data->vppi_base);
 
 	ppi = irq - chip_data->ppi_base;
-	vppi_irq = ppi + chip_data->nrppis * smp_processor_id();
+	vppi_irq = ppi + chip_data->nrppis * cpu;
 	vppi_irq += chip_data->vppi_base;
 
 	return vppi_irq;
 }
 
+unsigned int gic_ppi_map(unsigned int irq)
+{
+	return gic_ppi_map_on_cpu(irq, smp_processor_id());
+}
+
 static void gic_handle_ppi(unsigned int irq, struct irq_desc *desc)
 {
 	unsigned int vppi_irq;
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index bad26c2..52dc4dd 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -44,11 +44,10 @@ void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
 #ifdef CONFIG_ARM_GIC_PPI_MAP
 unsigned int gic_ppi_map(unsigned int irq);
+unsigned int gic_ppi_map_on_cpu(unsigned int irq, int cpu);
 #else
-static inline unsigned int gic_ppi_map(unsigned int irq)
-{
-	return irq;
-}
+#define gic_ppi_map(irq)		(irq)
+#define gic_ppi_map_on_cpu(irq, cpu)	(irq)
 #endif
 
 #endif
-- 
1.7.0.4

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

* [PATCH v8 09/14] ARM: msm: use remapped PPI interrupts for local timer
  2011-07-05  8:49 ` [PATCH v8 09/14] ARM: msm: " Marc Zyngier
@ 2011-07-05 18:06   ` David Brown
  0 siblings, 0 replies; 34+ messages in thread
From: David Brown @ 2011-07-05 18:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 05 2011, Marc Zyngier wrote:

> Use the normal interrupt scheme for the local timers by using
> a remapped PPI interrupt.
>
> MSM already had a very similar scheme, though still mixing both
> GIC-specific and generic APIs.
>
> Fixes and ideas courtesy of Stephen Boyd.
>
> Cc: David Brown <davidb@codeaurora.org>
> Cc: Daniel Walker <dwalker@fifo99.com>
> Cc: Bryan Huntsman <bryanh@codeaurora.org>
> Reviewed-and-Tested-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Tested-By: David Brown <davidb@codeaurora.org>
Acked-by: David Brown <davidb@codeaurora.org>

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (13 preceding siblings ...)
  2011-07-05  8:49 ` [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu() Marc Zyngier
@ 2011-07-06  5:46 ` Shawn Guo
  2011-07-06  9:53   ` Marc Zyngier
  2011-07-06 11:02 ` Marc Zyngier
  15 siblings, 1 reply; 34+ messages in thread
From: Shawn Guo @ 2011-07-06  5:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 05, 2011 at 09:49:01AM +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...
> 
> The proposed solution is to let the GIC code expose the PPIs as
> something that the kernel can manage. Instead of having a single
> interrupt number shared on all cores, make the interrupt number be
> different on each CPU.
> 
> This enables the use of the normal kernel API (request_irq() and
> friends) and the elimination of some low level code. It also serves as
> the basis of the GIC device tree support code. On the other side, it
> causes quite a bit of churn in the timer code.
> 
> Previous versions of this series have received ACKs from some platform
> maintainers, but so far there has been very little comment on the core
> code. As I now have quite a large number of patches relying on this
> series (local timers as devices, device tree support for GIC and TWD),
> I'd be grateful for at least a hint indicating that this is going in
> the right direction. Or not.
> 
I'm about to start mainlining Freescale i.MX6 core code.  It will be
wonderful if these great consolidation works could hit v3.1 merge
window, so that I can base i.MX6 off them in the next cycle.

-- 
Regards,
Shawn

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

* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
  2011-07-06  5:46 ` [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Shawn Guo
@ 2011-07-06  9:53   ` Marc Zyngier
  0 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-06  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 06/07/11 06:46, Shawn Guo wrote:
> On Tue, Jul 05, 2011 at 09:49:01AM +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...
>>
>> The proposed solution is to let the GIC code expose the PPIs as
>> something that the kernel can manage. Instead of having a single
>> interrupt number shared on all cores, make the interrupt number be
>> different on each CPU.
>>
>> This enables the use of the normal kernel API (request_irq() and
>> friends) and the elimination of some low level code. It also serves as
>> the basis of the GIC device tree support code. On the other side, it
>> causes quite a bit of churn in the timer code.
>>
>> Previous versions of this series have received ACKs from some platform
>> maintainers, but so far there has been very little comment on the core
>> code. As I now have quite a large number of patches relying on this
>> series (local timers as devices, device tree support for GIC and TWD),
>> I'd be grateful for at least a hint indicating that this is going in
>> the right direction. Or not.
>>
> I'm about to start mainlining Freescale i.MX6 core code.  It will be
> wonderful if these great consolidation works could hit v3.1 merge
> window, so that I can base i.MX6 off them in the next cycle.

I'd love to see this being merged, but so far there has been no comment
from Russell about this, and there is no way this is going move forward
without his approval.

Cheers,

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

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

* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
  2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
                   ` (14 preceding siblings ...)
  2011-07-06  5:46 ` [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Shawn Guo
@ 2011-07-06 11:02 ` Marc Zyngier
  2011-07-06 11:11   ` Russell King - ARM Linux
  15 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-06 11:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/07/11 09:49, 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...
> 
> The proposed solution is to let the GIC code expose the PPIs as
> something that the kernel can manage. Instead of having a single
> interrupt number shared on all cores, make the interrupt number be
> different on each CPU.
> 
> This enables the use of the normal kernel API (request_irq() and
> friends) and the elimination of some low level code. It also serves as
> the basis of the GIC device tree support code. On the other side, it
> causes quite a bit of churn in the timer code.
> 
> Previous versions of this series have received ACKs from some platform
> maintainers, but so far there has been very little comment on the core
> code. As I now have quite a large number of patches relying on this
> series (local timers as devices, device tree support for GIC and TWD),
> I'd be grateful for at least a hint indicating that this is going in
> the right direction. Or not.
> 
> This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2
> (Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
> 
> From v7:
> - Rebased on v3.0-rc5
> - Rename gic_ppi_to_vppi() to gic_ppi_map()
> - Rename CONFIG_ARM_GIC_VPPI to CONFIG_ARM_GIC_PPI_MAP
> - Add gic_ppi_map_on_cpu()
> 
> From v6:
> - Fix compilation when !CONFIG_PM.
> 
> From v5:
> - Update Exynos4 MCT to use the local timer infrastructure, now that
>   it doesn't depend on PPIs anymore. Untested, as the board I have
>   doesn't implement MCT (EVT0 syndrome).
> 
> From v4:
> - Fix arch/arm/common/Kconfig indentation
> - Fix MSM warning when building UP.
> 
> From v3:
> - Fix yet another CPU_HOTPLUG problem: instead of calling free_irq()
>   on timer halt, just disable the interrupt. On timer restart, detect
>   that the interrupt has been requested already and just enable the
>   interrupt back. This avoid doing a request_irq() with preemption
>   disabled, which triggers an ugly warning. Reported by Stephen Boyd.
> 
> From v2:
> - More MSM fixes
> - Dropped MSM interrupt handler removal, as this requires more discussion,
>   and will be addressed in a separate series.
> - Moved introduction of percpu_timer_handler to a separate patch
> 
> From v1:
> - Fix crash while hotpluging a CPU
> - Use handle_percpu_irq() instead of handle_fasteoi_irq()
> - MSM fixes courtesy of Stephen Boyd
> - MSM switched to percpu_timer_handler()
> - Remove local timer interrupt accounting
> - Restructure patches #1 and #2
> 
> Marc Zyngier (14):
>   ARM: gic: add per-cpu interrupt mapping
>   ARM: smp: add interrupt handler for local timers
>   ARM: smp_twd: add support for remapped PPI interrupts
>   ARM: omap4: use remapped PPI interrupts for local timer
>   ARM: versatile: use remapped PPI interrupts for local timer
>   ARM: shmobile: use remapped PPI interrupts for local timer
>   ARM: ux500: use remapped PPI interrupts for local timer
>   ARM: tegra: use remapped PPI interrupts for local timer
>   ARM: msm: use remapped PPI interrupts for local timer
>   ARM: exynos4: use remapped PPI interrupts for local timer
>   ARM: gic: remove previous local timer interrupt handling
>   ARM: gic: add compute_irqnr macro for exynos4
>   ARM: SMP: automatically select ARM_GIC_PPI_MAP
>   ARM: gic: add gic_ppi_map_on_cpu()
> 
>  arch/arm/Kconfig                                  |    1 +
>  arch/arm/common/Kconfig                           |    5 +
>  arch/arm/common/gic.c                             |  152 +++++++++++++++++++--
>  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   |   31 ++---
>  arch/arm/include/asm/hardware/gic.h               |   11 ++-
>  arch/arm/include/asm/localtimer.h                 |    7 +-
>  arch/arm/include/asm/smp.h                        |    5 -
>  arch/arm/kernel/irq.c                             |   11 +-
>  arch/arm/kernel/smp.c                             |   27 +---
>  arch/arm/kernel/smp_twd.c                         |   25 +++-
>  arch/arm/mach-exynos4/include/mach/entry-macro.S  |   70 +---------
>  arch/arm/mach-exynos4/localtimer.c                |    3 +-
>  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                         |   58 ++++----
>  arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--
>  arch/arm/mach-omap2/timer-mpu.c                   |    3 +-
>  arch/arm/mach-shmobile/entry-intc.S               |    3 -
>  arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
>  arch/arm/mach-shmobile/localtimer.c               |    3 +-
>  arch/arm/mach-tegra/localtimer.c                  |    3 +-
>  arch/arm/mach-ux500/localtimer.c                  |    3 +-
>  arch/arm/plat-versatile/localtimer.c              |    3 +-
>  25 files changed, 253 insertions(+), 282 deletions(-)

Russell,

Unless you have any objection, I'd like to push this to Arnd's arm-soc
tree. Would that be OK with you?

Thanks,

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

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

* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
  2011-07-06 11:02 ` Marc Zyngier
@ 2011-07-06 11:11   ` Russell King - ARM Linux
  2011-07-06 11:18     ` Marc Zyngier
  2011-07-06 12:26     ` Arnd Bergmann
  0 siblings, 2 replies; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-06 11:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 06, 2011 at 12:02:26PM +0100, Marc Zyngier wrote:
> On 05/07/11 09:49, 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...
> > 
> > The proposed solution is to let the GIC code expose the PPIs as
> > something that the kernel can manage. Instead of having a single
> > interrupt number shared on all cores, make the interrupt number be
> > different on each CPU.
> > 
> > This enables the use of the normal kernel API (request_irq() and
> > friends) and the elimination of some low level code. It also serves as
> > the basis of the GIC device tree support code. On the other side, it
> > causes quite a bit of churn in the timer code.
> > 
> > Previous versions of this series have received ACKs from some platform
> > maintainers, but so far there has been very little comment on the core
> > code. As I now have quite a large number of patches relying on this
> > series (local timers as devices, device tree support for GIC and TWD),
> > I'd be grateful for at least a hint indicating that this is going in
> > the right direction. Or not.
> > 
> > This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2
> > (Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
> > git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
> > 
> > From v7:
> > - Rebased on v3.0-rc5
> > - Rename gic_ppi_to_vppi() to gic_ppi_map()
> > - Rename CONFIG_ARM_GIC_VPPI to CONFIG_ARM_GIC_PPI_MAP
> > - Add gic_ppi_map_on_cpu()
> > 
> > From v6:
> > - Fix compilation when !CONFIG_PM.
> > 
> > From v5:
> > - Update Exynos4 MCT to use the local timer infrastructure, now that
> >   it doesn't depend on PPIs anymore. Untested, as the board I have
> >   doesn't implement MCT (EVT0 syndrome).
> > 
> > From v4:
> > - Fix arch/arm/common/Kconfig indentation
> > - Fix MSM warning when building UP.
> > 
> > From v3:
> > - Fix yet another CPU_HOTPLUG problem: instead of calling free_irq()
> >   on timer halt, just disable the interrupt. On timer restart, detect
> >   that the interrupt has been requested already and just enable the
> >   interrupt back. This avoid doing a request_irq() with preemption
> >   disabled, which triggers an ugly warning. Reported by Stephen Boyd.
> > 
> > From v2:
> > - More MSM fixes
> > - Dropped MSM interrupt handler removal, as this requires more discussion,
> >   and will be addressed in a separate series.
> > - Moved introduction of percpu_timer_handler to a separate patch
> > 
> > From v1:
> > - Fix crash while hotpluging a CPU
> > - Use handle_percpu_irq() instead of handle_fasteoi_irq()
> > - MSM fixes courtesy of Stephen Boyd
> > - MSM switched to percpu_timer_handler()
> > - Remove local timer interrupt accounting
> > - Restructure patches #1 and #2
> > 
> > Marc Zyngier (14):
> >   ARM: gic: add per-cpu interrupt mapping
> >   ARM: smp: add interrupt handler for local timers
> >   ARM: smp_twd: add support for remapped PPI interrupts
> >   ARM: omap4: use remapped PPI interrupts for local timer
> >   ARM: versatile: use remapped PPI interrupts for local timer
> >   ARM: shmobile: use remapped PPI interrupts for local timer
> >   ARM: ux500: use remapped PPI interrupts for local timer
> >   ARM: tegra: use remapped PPI interrupts for local timer
> >   ARM: msm: use remapped PPI interrupts for local timer
> >   ARM: exynos4: use remapped PPI interrupts for local timer
> >   ARM: gic: remove previous local timer interrupt handling
> >   ARM: gic: add compute_irqnr macro for exynos4
> >   ARM: SMP: automatically select ARM_GIC_PPI_MAP
> >   ARM: gic: add gic_ppi_map_on_cpu()
> > 
> >  arch/arm/Kconfig                                  |    1 +
> >  arch/arm/common/Kconfig                           |    5 +
> >  arch/arm/common/gic.c                             |  152 +++++++++++++++++++--
> >  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   |   31 ++---
> >  arch/arm/include/asm/hardware/gic.h               |   11 ++-
> >  arch/arm/include/asm/localtimer.h                 |    7 +-
> >  arch/arm/include/asm/smp.h                        |    5 -
> >  arch/arm/kernel/irq.c                             |   11 +-
> >  arch/arm/kernel/smp.c                             |   27 +---
> >  arch/arm/kernel/smp_twd.c                         |   25 +++-
> >  arch/arm/mach-exynos4/include/mach/entry-macro.S  |   70 +---------
> >  arch/arm/mach-exynos4/localtimer.c                |    3 +-
> >  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                         |   58 ++++----
> >  arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--
> >  arch/arm/mach-omap2/timer-mpu.c                   |    3 +-
> >  arch/arm/mach-shmobile/entry-intc.S               |    3 -
> >  arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
> >  arch/arm/mach-shmobile/localtimer.c               |    3 +-
> >  arch/arm/mach-tegra/localtimer.c                  |    3 +-
> >  arch/arm/mach-ux500/localtimer.c                  |    3 +-
> >  arch/arm/plat-versatile/localtimer.c              |    3 +-
> >  25 files changed, 253 insertions(+), 282 deletions(-)
> 
> Russell,
> 
> Unless you have any objection, I'd like to push this to Arnd's arm-soc
> tree. Would that be OK with you?

I've yet to look at any of these patches in any depth - my immediate
concern is gobbling up soo many IRQ numbers for the per-cpu stuff
(which also eats up memory for all the irq structures) when this
isn't really necessary.

Plus is this patch set core stuff or SoC stuff?  It looks more core
to me, so that means it should come through my tree rather than
Arnd's.

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

* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
  2011-07-06 11:11   ` Russell King - ARM Linux
@ 2011-07-06 11:18     ` Marc Zyngier
  2011-07-06 12:26     ` Arnd Bergmann
  1 sibling, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-06 11:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 06/07/11 12:11, Russell King - ARM Linux wrote:
> On Wed, Jul 06, 2011 at 12:02:26PM +0100, Marc Zyngier wrote:
>> On 05/07/11 09:49, 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...
>>>
>>> The proposed solution is to let the GIC code expose the PPIs as
>>> something that the kernel can manage. Instead of having a single
>>> interrupt number shared on all cores, make the interrupt number be
>>> different on each CPU.
>>>
>>> This enables the use of the normal kernel API (request_irq() and
>>> friends) and the elimination of some low level code. It also serves as
>>> the basis of the GIC device tree support code. On the other side, it
>>> causes quite a bit of churn in the timer code.
>>>
>>> Previous versions of this series have received ACKs from some platform
>>> maintainers, but so far there has been very little comment on the core
>>> code. As I now have quite a large number of patches relying on this
>>> series (local timers as devices, device tree support for GIC and TWD),
>>> I'd be grateful for at least a hint indicating that this is going in
>>> the right direction. Or not.
>>>
>>> This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2
>>> (Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
>>>
>>> From v7:
>>> - Rebased on v3.0-rc5
>>> - Rename gic_ppi_to_vppi() to gic_ppi_map()
>>> - Rename CONFIG_ARM_GIC_VPPI to CONFIG_ARM_GIC_PPI_MAP
>>> - Add gic_ppi_map_on_cpu()
>>>
>>> From v6:
>>> - Fix compilation when !CONFIG_PM.
>>>
>>> From v5:
>>> - Update Exynos4 MCT to use the local timer infrastructure, now that
>>>   it doesn't depend on PPIs anymore. Untested, as the board I have
>>>   doesn't implement MCT (EVT0 syndrome).
>>>
>>> From v4:
>>> - Fix arch/arm/common/Kconfig indentation
>>> - Fix MSM warning when building UP.
>>>
>>> From v3:
>>> - Fix yet another CPU_HOTPLUG problem: instead of calling free_irq()
>>>   on timer halt, just disable the interrupt. On timer restart, detect
>>>   that the interrupt has been requested already and just enable the
>>>   interrupt back. This avoid doing a request_irq() with preemption
>>>   disabled, which triggers an ugly warning. Reported by Stephen Boyd.
>>>
>>> From v2:
>>> - More MSM fixes
>>> - Dropped MSM interrupt handler removal, as this requires more discussion,
>>>   and will be addressed in a separate series.
>>> - Moved introduction of percpu_timer_handler to a separate patch
>>>
>>> From v1:
>>> - Fix crash while hotpluging a CPU
>>> - Use handle_percpu_irq() instead of handle_fasteoi_irq()
>>> - MSM fixes courtesy of Stephen Boyd
>>> - MSM switched to percpu_timer_handler()
>>> - Remove local timer interrupt accounting
>>> - Restructure patches #1 and #2
>>>
>>> Marc Zyngier (14):
>>>   ARM: gic: add per-cpu interrupt mapping
>>>   ARM: smp: add interrupt handler for local timers
>>>   ARM: smp_twd: add support for remapped PPI interrupts
>>>   ARM: omap4: use remapped PPI interrupts for local timer
>>>   ARM: versatile: use remapped PPI interrupts for local timer
>>>   ARM: shmobile: use remapped PPI interrupts for local timer
>>>   ARM: ux500: use remapped PPI interrupts for local timer
>>>   ARM: tegra: use remapped PPI interrupts for local timer
>>>   ARM: msm: use remapped PPI interrupts for local timer
>>>   ARM: exynos4: use remapped PPI interrupts for local timer
>>>   ARM: gic: remove previous local timer interrupt handling
>>>   ARM: gic: add compute_irqnr macro for exynos4
>>>   ARM: SMP: automatically select ARM_GIC_PPI_MAP
>>>   ARM: gic: add gic_ppi_map_on_cpu()
>>>
>>>  arch/arm/Kconfig                                  |    1 +
>>>  arch/arm/common/Kconfig                           |    5 +
>>>  arch/arm/common/gic.c                             |  152 +++++++++++++++++++--
>>>  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   |   31 ++---
>>>  arch/arm/include/asm/hardware/gic.h               |   11 ++-
>>>  arch/arm/include/asm/localtimer.h                 |    7 +-
>>>  arch/arm/include/asm/smp.h                        |    5 -
>>>  arch/arm/kernel/irq.c                             |   11 +-
>>>  arch/arm/kernel/smp.c                             |   27 +---
>>>  arch/arm/kernel/smp_twd.c                         |   25 +++-
>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S  |   70 +---------
>>>  arch/arm/mach-exynos4/localtimer.c                |    3 +-
>>>  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                         |   58 ++++----
>>>  arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--
>>>  arch/arm/mach-omap2/timer-mpu.c                   |    3 +-
>>>  arch/arm/mach-shmobile/entry-intc.S               |    3 -
>>>  arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
>>>  arch/arm/mach-shmobile/localtimer.c               |    3 +-
>>>  arch/arm/mach-tegra/localtimer.c                  |    3 +-
>>>  arch/arm/mach-ux500/localtimer.c                  |    3 +-
>>>  arch/arm/plat-versatile/localtimer.c              |    3 +-
>>>  25 files changed, 253 insertions(+), 282 deletions(-)
>>
>> Russell,
>>
>> Unless you have any objection, I'd like to push this to Arnd's arm-soc
>> tree. Would that be OK with you?
> 
> I've yet to look at any of these patches in any depth - my immediate
> concern is gobbling up soo many IRQ numbers for the per-cpu stuff
> (which also eats up memory for all the irq structures) when this
> isn't really necessary.

Note that this mandates SPARSE_IRQ, so the number of allocated irq
structures is actually very limited (most platforms only allocate 3
structures per CPU for PPI #29, 30 and 31).

> Plus is this patch set core stuff or SoC stuff?  It looks more core
> to me, so that means it should come through my tree rather than
> Arnd's.

Fair enough, I have no problem with you merging it once you're happy
with it.

Cheers,

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

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

* [PATCH v8 00/14] Consolidating GIC per-cpu interrupts
  2011-07-06 11:11   ` Russell King - ARM Linux
  2011-07-06 11:18     ` Marc Zyngier
@ 2011-07-06 12:26     ` Arnd Bergmann
  1 sibling, 0 replies; 34+ messages in thread
From: Arnd Bergmann @ 2011-07-06 12:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 06 July 2011, Russell King - ARM Linux wrote:
> Plus is this patch set core stuff or SoC stuff?  It looks more core
> to me, so that means it should come through my tree rather than
> Arnd's.

Yes, I agree.

	Arnd

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

* [PATCH v8 04/14] ARM: omap4: use remapped PPI interrupts for local timer
  2011-07-05  8:49 ` [PATCH v8 04/14] ARM: omap4: use remapped PPI interrupts for local timer Marc Zyngier
@ 2011-07-07 14:32   ` Tony Lindgren
  0 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2011-07-07 14:32 UTC (permalink / raw)
  To: linux-arm-kernel

* Marc Zyngier <marc.zyngier@arm.com> [110705 11:45]:
> Use the normal interrupt scheme for the local timers by using
> a remapped PPI interrupt.
> 
> Tested on a Pandaboard.
> 
> Cc: Tony Lindgren <tony@atomide.com>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Acked-by: Tony Lindgren <tony@atomide.com>

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

* [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts
  2011-07-05  8:49 ` [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts Marc Zyngier
@ 2011-07-08 19:27   ` Russell King - ARM Linux
  2011-07-10 15:13     ` Marc Zyngier
  0 siblings, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-08 19:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 05, 2011 at 09:49:04AM +0100, Marc Zyngier wrote:
> @@ -43,6 +45,10 @@ static void twd_set_mode(enum clock_event_mode mode,
>  		ctrl = TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_ONESHOT;
>  		break;
>  	case CLOCK_EVT_MODE_UNUSED:
> +#ifdef CONFIG_ARM_GIC_PPI_MAP
> +		disable_irq(clk->irq);
> +		/* fall through */
> +#endif

Eww, no.  This is wrong.  You can get an UNUSED event if the TWD clock
event device gets swapped out for some other clock event device (eg,
one with a higher preference.)

This is at best a hack around the problem.  The existing code deals with
hot unplug via this in arch/arm/kernel/smp.c:

/*
 * The generic clock events code purposely does not stop the local timer
 * on CPU_DEAD/CPU_DEAD_FROZEN hotplug events, so we have to do it
 * manually here.
 */
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);
}

Maybe that needs a local_timer_stop() ->twd_timer_stop() call rather
than trying to be clever by doing this via the set_mode() method.  That
would seem to be overall a cleaner solution.

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-05  8:49 ` [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu() Marc Zyngier
@ 2011-07-08 19:57   ` Russell King - ARM Linux
  2011-07-10 15:10     ` Marc Zyngier
  0 siblings, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-08 19:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 05, 2011 at 09:49:15AM +0100, Marc Zyngier wrote:
> It is sometimes usefull to compute the mapping of a PPI on a CPU
> that is not the one handling the interrupt (in the probe function
> of a driver, for example).

This is unsafe.  Firstly, smp_processor_id() in preemptible contexts
is not valid as you can be migrated to another CPU at any moment.

Secondly, if you have a driver probe function, and you want to
register a PPI interrupt for a different CPU, while you can call the
request function, the resulting ->unmask will be called on the
calling CPU (assuming preempt is disabled and we don't schedule).
You might get lucky and it may be called on your intended CPU...

That means the mask registers you access are on the local CPU, not
the target CPU, so you end up with the wrong interrupt enabled.

As request_threaded_irq() contains a kzalloc() which can schedule,
I can't see how you can safely request a particular per-CPU interrupt
and guarantee that you'll enable it on the appropriate core.

Finally, note that using request_irq() from the secondary_startup()
thread is probably a very bad idea - should it require to schedule,
there is nothing for the secondary CPU to switch to - we're the
secondary CPUs idle thread.

I think you'll need to use setup_irq() to register the PPI interrupts,
especially for the TWD.  However, that then gives you a problem when
you come to tear it down on CPU hot unplug, as free_irq() can't be
used - it'll try to kfree() the static irqaction structure.

So I think more thought is required on this entire patch set.

I have thought (before this patch set appeared) about making the PPI
stuff entirely GIC specific, having an array of 16 function pointers
in the GIC code to allow things like TWD to hook into.  No preemption
or sleep problems, no genirq overhead to their handling, etc.  Then
again, I think we do need irq_enter()..irq_exit().

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-08 19:57   ` Russell King - ARM Linux
@ 2011-07-10 15:10     ` Marc Zyngier
  2011-07-10 15:37       ` Russell King - ARM Linux
  0 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-10 15:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 8 Jul 2011 20:57:08 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Tue, Jul 05, 2011 at 09:49:15AM +0100, Marc Zyngier wrote:
> > It is sometimes usefull to compute the mapping of a PPI on a CPU
> > that is not the one handling the interrupt (in the probe function
> > of a driver, for example).
> 
> This is unsafe.  Firstly, smp_processor_id() in preemptible contexts
> is not valid as you can be migrated to another CPU at any moment.

Point taken. I think I can solve this by having a per-cpu variable
instead of hijacking the gic_chip_data structure.

> Secondly, if you have a driver probe function, and you want to
> register a PPI interrupt for a different CPU, while you can call the
> request function, the resulting ->unmask will be called on the
> calling CPU (assuming preempt is disabled and we don't schedule).
> You might get lucky and it may be called on your intended CPU...

I think you're going way beyond than the intended use of this function.
It is only there to be able to compute the PPI->IRQ mapping, *not* to
register the IRQ, which of course only make sense on the target CPU.

> That means the mask registers you access are on the local CPU, not
> the target CPU, so you end up with the wrong interrupt enabled.
> 
> As request_threaded_irq() contains a kzalloc() which can schedule,
> I can't see how you can safely request a particular per-CPU interrupt
> and guarantee that you'll enable it on the appropriate core.
> 
> Finally, note that using request_irq() from the secondary_startup()
> thread is probably a very bad idea - should it require to schedule,
> there is nothing for the secondary CPU to switch to - we're the
> secondary CPUs idle thread.
> 
> I think you'll need to use setup_irq() to register the PPI interrupts,
> especially for the TWD.  However, that then gives you a problem when
> you come to tear it down on CPU hot unplug, as free_irq() can't be
> used - it'll try to kfree() the static irqaction structure.

Actually, you wouldn't need to to do anything on CPU hot unplug (just
as it is now, actually). You could directly do another setup_irq() once
the CPU is hot plugged back. I'll have a look at this.

> So I think more thought is required on this entire patch set.
> 
> I have thought (before this patch set appeared) about making the PPI
> stuff entirely GIC specific, having an array of 16 function pointers
> in the GIC code to allow things like TWD to hook into.  No preemption
> or sleep problems, no genirq overhead to their handling, etc.  Then
> again, I think we do need irq_enter()..irq_exit().

Wouldn't that be another interrupt mechanism? I agree that it would be
simpler, but it would make the PPI handling different from normal
interrupts. genirq already has a percpu flow that is relatively cheap.
And being able to rely on the usual kernel semantics is probably the
best we can do while undergoing drastic changes such as DT.

If in the end we cannot efficiently cast PPI handling into genirq, then
I agree that your proposal is a sensible alternative.

	M.
-- 
I'm the slime oozin' out from your TV set...

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

* [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts
  2011-07-08 19:27   ` Russell King - ARM Linux
@ 2011-07-10 15:13     ` Marc Zyngier
  0 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-10 15:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 8 Jul 2011 20:27:34 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Tue, Jul 05, 2011 at 09:49:04AM +0100, Marc Zyngier wrote:
> > @@ -43,6 +45,10 @@ static void twd_set_mode(enum clock_event_mode mode,
> >  		ctrl = TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_ONESHOT;
> >  		break;
> >  	case CLOCK_EVT_MODE_UNUSED:
> > +#ifdef CONFIG_ARM_GIC_PPI_MAP
> > +		disable_irq(clk->irq);
> > +		/* fall through */
> > +#endif
> 
> Eww, no.  This is wrong.  You can get an UNUSED event if the TWD clock
> event device gets swapped out for some other clock event device (eg,
> one with a higher preference.)
> 
> This is at best a hack around the problem.  The existing code deals with
> hot unplug via this in arch/arm/kernel/smp.c:
> 
> /*
>  * The generic clock events code purposely does not stop the local timer
>  * on CPU_DEAD/CPU_DEAD_FROZEN hotplug events, so we have to do it
>  * manually here.
>  */
> 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);
> }
> 
> Maybe that needs a local_timer_stop() ->twd_timer_stop() call rather
> than trying to be clever by doing this via the set_mode() method.  That
> would seem to be overall a cleaner solution.
> 

I think your suggestion to use request_irq() should solve it nicely.
No need to disable the interrupt, it will be re-enabled on the next
setup_irq() when the CPU comes back up.

	M.
-- 
I'm the slime oozin' out from your TV set...

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-10 15:10     ` Marc Zyngier
@ 2011-07-10 15:37       ` Russell King - ARM Linux
  2011-07-10 18:30         ` Russell King - ARM Linux
  0 siblings, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-10 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Jul 10, 2011 at 04:10:38PM +0100, Marc Zyngier wrote:
> On Fri, 8 Jul 2011 20:57:08 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Tue, Jul 05, 2011 at 09:49:15AM +0100, Marc Zyngier wrote:
> > > It is sometimes usefull to compute the mapping of a PPI on a CPU
> > > that is not the one handling the interrupt (in the probe function
> > > of a driver, for example).
> > 
> > This is unsafe.  Firstly, smp_processor_id() in preemptible contexts
> > is not valid as you can be migrated to another CPU at any moment.
> 
> Point taken. I think I can solve this by having a per-cpu variable
> instead of hijacking the gic_chip_data structure.

I think you miss the point.

	global_irq = gic_ppi_map(ppi_irq);

is broken, no matter what kind of implementation you have behind it.

To illustrate it, lets define gic_ppi_map() as the following:

	(ppi_irq * NR_CPUS + cpu_nr)

And you have this code:

	irq = gic_ppi_map(2);
	ret = request_irq(irq, ...);

Now, lets say that when you call this, you're running on CPU1, you
have 4 CPUs and you want PPI 2.  global_irq will be 2 * 4 + 1 = 9.

You intend to pass this to request_irq() to claim the PPI 2 for the
current CPU.  However, you're preempted by another thread, so you
stop executing on CPU1.

CPU3 is about to idle, and so you're migrated to CPU3 which now starts
executing your code.  You continue into request_irq(), still with
IRQ9.  You request the interrupt, and the unmask function is called.
The GIC now unmasks PPI 2 on CPU 3 - but this is IRQ11.

However, your IRQ handler is registered in IRQ9's slot but IRQ11 is
enabled instead.

Now, lets say you decide to disable preemption over this code section:

	preempt_disable();
	irq = gic_ppi_map(2);
	ret = request_irq(irq, ...);
	preempt_enable();

This is _still_ unsafe.  Instead of the explicit preemption, consider
what happens if the attempt to allocate memory inside request_irq()
blocks:

        action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);

There is _still_ no guarantee that you will continue execution on the
same CPU as the one you were originally executing on.

> > Secondly, if you have a driver probe function, and you want to
> > register a PPI interrupt for a different CPU, while you can call the
> > request function, the resulting ->unmask will be called on the
> > calling CPU (assuming preempt is disabled and we don't schedule).
> > You might get lucky and it may be called on your intended CPU...
> 
> I think you're going way beyond than the intended use of this function.
> It is only there to be able to compute the PPI->IRQ mapping, *not* to
> register the IRQ, which of course only make sense on the target CPU.

No, my point is that this is not a useful function, period.  It should
not even exist, because it is *dangerous* for anyone to use it.  See
the examples above.

However, my point is more than that: the whole idea behind this patch
series is unsafe: it is _unsafe_ to assume that calling request_irq()
on any particular CPU will result in execution being bound solely to
that IRQ.

The only way to ensure that this code works as you've intended is:

1. save the threads affinity
2. set a new affinity for a single CPU for the thread
3. force execution to switch to that CPU
4. translate the IRQ and request it
5. restore the threads affinity

And to ensure that drivers do that safely, that would _have_ to be a
separate function which takes care of all that.  The code would look
something like this:

        cpumask_t cpus_allowed = current->cpus_allowed;
        set_cpus_allowed(current, cpumask_of_cpu(cpu));
        BUG_ON(cpu != smp_processor_id());
	irq = gic_ppi_map(ppi);
	ret = request_irq(irq, ...);
	set_cpus_allowed(current, cpus_allowed);

(But not this: this depends on CPUMASK_OFFSTACK not being set.)

In any case, this is _not_ what we want drivers to get involved with -
we'll get different implementations of this which are likely to be
buggy.  We need an interface which doesn't have this complexity.  So
we need something like this:

int request_ppi_irq(int ppi, irq_handler_t handler, void *dev);

which contains all this complexity.  At that point, we've _already_
moved away from the standard genirq interfaces to a GIC specific PPI
request interface.

So, all in all, I see _zero_ reason to use genirq for this ARM specific
oddity.

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-10 15:37       ` Russell King - ARM Linux
@ 2011-07-10 18:30         ` Russell King - ARM Linux
  2011-07-11  9:52           ` Marc Zyngier
  0 siblings, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-10 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Jul 10, 2011 at 04:37:59PM +0100, Russell King - ARM Linux wrote:
> And to ensure that drivers do that safely, that would _have_ to be a
> separate function which takes care of all that.  The code would look
> something like this:
> 
>         cpumask_t cpus_allowed = current->cpus_allowed;
>         set_cpus_allowed(current, cpumask_of_cpu(cpu));
>         BUG_ON(cpu != smp_processor_id());
> 	irq = gic_ppi_map(ppi);
> 	ret = request_irq(irq, ...);
> 	set_cpus_allowed(current, cpus_allowed);
> 
> (But not this: this depends on CPUMASK_OFFSTACK not being set.)

So, we can either do something like this:

int gic_request_ppi(unsigned int ppi, irq_handler_t handler,
	unsigned long flags, const char *name, void *data)
{
	cpumask_var_t cpus_allowed;
	unsigned int irq, cpu = get_cpu();
	int ret;

	/* Allocate cpus_allowed mask */
	if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL)) {
		ret = -ENOMEM;
		goto err;
	}

	/* Copy current thread affinity */
	cpumask_copy(cups_allowed, &current->cpus_allowed);

	/* Bind to the current CPU */
	set_cpus_allowed_ptr(current, cpumask_of(cpu));
	irq = gic_ppi_map(ppi);
	ret = request_irq(irq, handler, flags, name, data);
	set_cpus_allowed_ptr(current, cpus_allowed);

	free_cpumask_var(cpus_allowed);

err:
	put_cpu();
	return ret;
}

void gic_free_ppi(unsigned int ppi, void *data)
{
	cpumask_var_t cpus_allowed;
	unsigned int irq, cpu = get_cpu();

	/* Allocate cpus_allowed mask */
	if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL))
		goto err; /* BUG */

	/* Copy current thread affinity */
	cpumask_copy(cups_allowed, &current->cpus_allowed);

	/* Bind to the current CPU */
	set_cpus_allowed_ptr(current, cpumask_of(cpu));
	irq = gic_ppi_map(ppi);
	free_irq(irq, data);
	set_cpus_allowed_ptr(current, cpus_allowed);

	free_cpumask_var(cpus_allowed);

err:
	put_cpu();
}

Or the below, which will need platform people to tweak their entry-macro
stuff to allow through IRQs 16-31.

There's also the question about whether we should pass in the desired CPU
number to these too, in case we have a requirement to ensure that we get
the PPI on a specific CPU, or whether we only care about the _current_
CPU we happen to be running on.  That depends on what else PPIs get used
for other than TWD.

 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] 34+ messages in thread

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-10 18:30         ` Russell King - ARM Linux
@ 2011-07-11  9:52           ` Marc Zyngier
  2011-07-11 10:17             ` Russell King - ARM Linux
  0 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-11  9:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/07/11 19:30, Russell King - ARM Linux wrote:
> On Sun, Jul 10, 2011 at 04:37:59PM +0100, Russell King - ARM Linux wrote:
>> And to ensure that drivers do that safely, that would _have_ to be a
>> separate function which takes care of all that.  The code would look
>> something like this:
>>
>>         cpumask_t cpus_allowed = current->cpus_allowed;
>>         set_cpus_allowed(current, cpumask_of_cpu(cpu));
>>         BUG_ON(cpu != smp_processor_id());
>>       irq = gic_ppi_map(ppi);
>>       ret = request_irq(irq, ...);
>>       set_cpus_allowed(current, cpus_allowed);
>>
>> (But not this: this depends on CPUMASK_OFFSTACK not being set.)
> 
> So, we can either do something like this:
> 
> int gic_request_ppi(unsigned int ppi, irq_handler_t handler,
>         unsigned long flags, const char *name, void *data)
> {
>         cpumask_var_t cpus_allowed;
>         unsigned int irq, cpu = get_cpu();
>         int ret;
> 
>         /* Allocate cpus_allowed mask */
>         if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL)) {
>                 ret = -ENOMEM;
>                 goto err;
>         }
> 
>         /* Copy current thread affinity */
>         cpumask_copy(cups_allowed, &current->cpus_allowed);
> 
>         /* Bind to the current CPU */
>         set_cpus_allowed_ptr(current, cpumask_of(cpu));
>         irq = gic_ppi_map(ppi);
>         ret = request_irq(irq, handler, flags, name, data);
>         set_cpus_allowed_ptr(current, cpus_allowed);
> 
>         free_cpumask_var(cpus_allowed);
> 
> err:
>         put_cpu();
>         return ret;
> }
> 
> void gic_free_ppi(unsigned int ppi, void *data)
> {
>         cpumask_var_t cpus_allowed;
>         unsigned int irq, cpu = get_cpu();
> 
>         /* Allocate cpus_allowed mask */
>         if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL))
>                 goto err; /* BUG */
> 
>         /* Copy current thread affinity */
>         cpumask_copy(cups_allowed, &current->cpus_allowed);
> 
>         /* Bind to the current CPU */
>         set_cpus_allowed_ptr(current, cpumask_of(cpu));
>         irq = gic_ppi_map(ppi);
>         free_irq(irq, data);
>         set_cpus_allowed_ptr(current, cpus_allowed);
> 
>         free_cpumask_var(cpus_allowed);
> 
> err:
>         put_cpu();
> }
> 
> Or the below, which will need platform people to tweak their entry-macro
> stuff to allow through IRQs 16-31.

You won't be surprised if I say that I prefer your first version. The
second one, while much simpler, keeps the additional low level entry
point (gic_call_ppi), and has to do its own accounting.

But more than that. MSM timers are implemented using the same code on
both UP and SMP, with or without GIC. which means we have to request the
interrupt using the same API. Your first version would work in that case
(gic_ppi_map() on a non-PPI should return the same number).

> There's also the question about whether we should pass in the desired CPU
> number to these too, in case we have a requirement to ensure that we get
> the PPI on a specific CPU, or whether we only care about the _current_
> CPU we happen to be running on.

As long as we tie mapping and interrupt request together, there is no
reason to offer that functionality. But DT may need to resolve the
mappings independently (while creating the platform devices), and the
driver would then request the mapped PPI. In that case, we need to
ensure we're running on the right CPU.

> That depends on what else PPIs get used for other than TWD.

The MSM code suggests that PPIs are used for more than just timers
(though apparently by out of tree drivers).

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

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-11  9:52           ` Marc Zyngier
@ 2011-07-11 10:17             ` Russell King - ARM Linux
  2011-07-11 11:14               ` Marc Zyngier
  0 siblings, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-11 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 11, 2011 at 10:52:05AM +0100, Marc Zyngier wrote:
> You won't be surprised if I say that I prefer your first version. The
> second one, while much simpler, keeps the additional low level entry
> point (gic_call_ppi), and has to do its own accounting.
> 
> But more than that. MSM timers are implemented using the same code on
> both UP and SMP, with or without GIC. which means we have to request the
> interrupt using the same API. Your first version would work in that case
> (gic_ppi_map() on a non-PPI should return the same number).

Who says gic_request_ppi() will exist on systems without GIC?  Or
even gic_ppi_map()?

Let me make the point plain: no driver must EVER use gic_ppi_map().
No driver must EVER call request_irq() any other genirq function for
a PPI interrupt directly.  They must all be wrapped in suitable
containers to bind the current thread to the current CPU.  Not doing
so will lead to failures due to the wrong CPUs registers being hit.

> > There's also the question about whether we should pass in the desired CPU
> > number to these too, in case we have a requirement to ensure that we get
> > the PPI on a specific CPU, or whether we only care about the _current_
> > CPU we happen to be running on.
> 
> As long as we tie mapping and interrupt request together, there is no
> reason to offer that functionality. But DT may need to resolve the
> mappings independently (while creating the platform devices), and the
> driver would then request the mapped PPI. In that case, we need to
> ensure we're running on the right CPU.

You still don't get the issue.  Really you don't.  And you don't seem
to get DT either.

DT has precisely nothing to do with this, and should never have anything
to do with this kind of mapping.  Mapping a PPI + CPU to a _Linux_ IRQ
number is a _Linux_ specific operation.  It's not something that will
be acceptable to be represented in DT.  What DT describes is the
_hardware_.  So, if DT wants to describe a PPI, then DT should describe
PPI N on CPU N.

However, the basis of my argument is that this has got precisely nothing
to do with the mapping of PPIs to IRQ numbers.  It's about the using
unique IRQ numbers to describe an IRQ which can _only_ be accessed on one
particular CPU.

The PPIs are really not standard interrupts.  Trying to treat them as such
means that all the standard genirq interfaces will need to be _wrapped_ to
ensure that they're bound to the particular CPU that you're interested in.

The reason for that is to ensure that you hit the right hardware registers
for the IRQ number you're passing in.  Using my previous example, it's no
good if you pass in IRQ9 (PPI2 CPU1) but end up hitting IRQ11's (PPI2 CPU3)
registers instead.

Plus, someone will have to audit drivers even more carefully to ensure that
they're not trying to use the standard request_irq() (or any other of the
genirq interfaces) with PPI interrupt numbers.  Who's going to do that?

So, I believe your patches are fundamentally misdesigned on a technical
level, are fragile, and therefore are not suitable for integration into
mainline.

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-11 10:17             ` Russell King - ARM Linux
@ 2011-07-11 11:14               ` Marc Zyngier
  2011-07-11 11:38                 ` Russell King - ARM Linux
  0 siblings, 1 reply; 34+ messages in thread
From: Marc Zyngier @ 2011-07-11 11:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/07/11 11:17, Russell King - ARM Linux wrote:
> On Mon, Jul 11, 2011 at 10:52:05AM +0100, Marc Zyngier wrote:
>> You won't be surprised if I say that I prefer your first version. The
>> second one, while much simpler, keeps the additional low level entry
>> point (gic_call_ppi), and has to do its own accounting.
>>
>> But more than that. MSM timers are implemented using the same code on
>> both UP and SMP, with or without GIC. which means we have to request the
>> interrupt using the same API. Your first version would work in that case
>> (gic_ppi_map() on a non-PPI should return the same number).
> 
> Who says gic_request_ppi() will exist on systems without GIC?  Or
> even gic_ppi_map()?
> 
> Let me make the point plain: no driver must EVER use gic_ppi_map().
> No driver must EVER call request_irq() any other genirq function for
> a PPI interrupt directly.  They must all be wrapped in suitable
> containers to bind the current thread to the current CPU.  Not doing
> so will lead to failures due to the wrong CPUs registers being hit.

I didn't suggest using request_irq() on a PPI. I suggested using
gic_request_ppi() on a normal IRQ number (which is ugly but would work).

If gic_request_ppi() is not available for non GIC setups, then at least
the MSM timer code must be fixed.

>>> There's also the question about whether we should pass in the desired CPU
>>> number to these too, in case we have a requirement to ensure that we get
>>> the PPI on a specific CPU, or whether we only care about the _current_
>>> CPU we happen to be running on.
>>
>> As long as we tie mapping and interrupt request together, there is no
>> reason to offer that functionality. But DT may need to resolve the
>> mappings independently (while creating the platform devices), and the
>> driver would then request the mapped PPI. In that case, we need to
>> ensure we're running on the right CPU.
> 
> You still don't get the issue.  Really you don't.  And you don't seem
> to get DT either.
> 
> DT has precisely nothing to do with this, and should never have anything
> to do with this kind of mapping.  Mapping a PPI + CPU to a _Linux_ IRQ
> number is a _Linux_ specific operation.  It's not something that will
> be acceptable to be represented in DT.  What DT describes is the
> _hardware_.  So, if DT wants to describe a PPI, then DT should describe
> PPI N on CPU N.

And that's exactly what it does:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.html

What I was trying to explain (and obviously failed to) is that the
_Linux_ DT _code_ will try to resolve the PPI number and convert it to a
_Linux_ IRQ number. Unless of course you don't encode it as an interrupt
at all, which seems to be what you're aiming for.

> However, the basis of my argument is that this has got precisely nothing
> to do with the mapping of PPIs to IRQ numbers.  It's about the using
> unique IRQ numbers to describe an IRQ which can _only_ be accessed on one
> particular CPU.
> 
> The PPIs are really not standard interrupts.  Trying to treat them as such
> means that all the standard genirq interfaces will need to be _wrapped_ to
> ensure that they're bound to the particular CPU that you're interested in.
> 
> The reason for that is to ensure that you hit the right hardware registers
> for the IRQ number you're passing in.  Using my previous example, it's no
> good if you pass in IRQ9 (PPI2 CPU1) but end up hitting IRQ11's (PPI2 CPU3)
> registers instead.

I've understood that loud and clear.

> Plus, someone will have to audit drivers even more carefully to ensure that
> they're not trying to use the standard request_irq() (or any other of the
> genirq interfaces) with PPI interrupt numbers.  Who's going to do that?
> 
> So, I believe your patches are fundamentally misdesigned on a technical
> level, are fragile, and therefore are not suitable for integration into
> mainline.


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

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-11 11:14               ` Marc Zyngier
@ 2011-07-11 11:38                 ` Russell King - ARM Linux
  2011-07-11 12:36                   ` Marc Zyngier
  0 siblings, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2011-07-11 11:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 11, 2011 at 12:14:36PM +0100, Marc Zyngier wrote:
> And that's exactly what it does:
> http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.html
> 
> What I was trying to explain (and obviously failed to) is that the
> _Linux_ DT _code_ will try to resolve the PPI number and convert it to a
> _Linux_ IRQ number. Unless of course you don't encode it as an interrupt
> at all, which seems to be what you're aiming for.

I'm not aiming for anything.  I'm trying to get you to fully understand
the issue I've raised with your patches.  So, let's try a new scenario
based on your statement above.

You have a device which happens to use a PPI.  You obtain its IRQ number
from DT, which tells you IRQ 9, because the DT information said PPI 2
CPU 1.  So you pass IRQ 9 into the IRQ request function, but as you're
running on CPU 3, you have no access to the hardware for IRQ 9.

Please describe in detail how, with your patches, PPI 2 CPU 1 gets enabled
rather than PPI 2 CPU 3 when IRQ 9 is requested.

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

* [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu()
  2011-07-11 11:38                 ` Russell King - ARM Linux
@ 2011-07-11 12:36                   ` Marc Zyngier
  0 siblings, 0 replies; 34+ messages in thread
From: Marc Zyngier @ 2011-07-11 12:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/07/11 12:38, Russell King - ARM Linux wrote:
> On Mon, Jul 11, 2011 at 12:14:36PM +0100, Marc Zyngier wrote:
>> And that's exactly what it does:
>> http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.html
>>
>> What I was trying to explain (and obviously failed to) is that the
>> _Linux_ DT _code_ will try to resolve the PPI number and convert it to a
>> _Linux_ IRQ number. Unless of course you don't encode it as an interrupt
>> at all, which seems to be what you're aiming for.
> 
> I'm not aiming for anything.  I'm trying to get you to fully understand
> the issue I've raised with your patches.  So, let's try a new scenario
> based on your statement above.
> 
> You have a device which happens to use a PPI.  You obtain its IRQ number
> from DT, which tells you IRQ 9, because the DT information said PPI 2
> CPU 1.  So you pass IRQ 9 into the IRQ request function, but as you're
> running on CPU 3, you have no access to the hardware for IRQ 9.
> 
> Please describe in detail how, with your patches, PPI 2 CPU 1 gets enabled
> rather than PPI 2 CPU 3 when IRQ 9 is requested.

You simply do not do that. You store the mapping for later use on the
right CPU. My code is buggy as I didn't think of the requesting thread
being preempted, but I never intended to do this sort of cross-CPU request.
-- 
Jazz is not dead. It just smells funny...

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

end of thread, other threads:[~2011-07-11 12:36 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-05  8:49 [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 01/14] ARM: gic: add per-cpu interrupt mapping Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 02/14] ARM: smp: add interrupt handler for local timers Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 03/14] ARM: smp_twd: add support for remapped PPI interrupts Marc Zyngier
2011-07-08 19:27   ` Russell King - ARM Linux
2011-07-10 15:13     ` Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 04/14] ARM: omap4: use remapped PPI interrupts for local timer Marc Zyngier
2011-07-07 14:32   ` Tony Lindgren
2011-07-05  8:49 ` [PATCH v8 05/14] ARM: versatile: " Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 06/14] ARM: shmobile: " Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 07/14] ARM: ux500: " Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 08/14] ARM: tegra: " Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 09/14] ARM: msm: " Marc Zyngier
2011-07-05 18:06   ` David Brown
2011-07-05  8:49 ` [PATCH v8 10/14] ARM: exynos4: " Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 11/14] ARM: gic: remove previous local timer interrupt handling Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 12/14] ARM: gic: add compute_irqnr macro for exynos4 Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 13/14] ARM: SMP: automatically select ARM_GIC_PPI_MAP Marc Zyngier
2011-07-05  8:49 ` [PATCH v8 14/14] ARM: gic: add gic_ppi_map_on_cpu() Marc Zyngier
2011-07-08 19:57   ` Russell King - ARM Linux
2011-07-10 15:10     ` Marc Zyngier
2011-07-10 15:37       ` Russell King - ARM Linux
2011-07-10 18:30         ` Russell King - ARM Linux
2011-07-11  9:52           ` Marc Zyngier
2011-07-11 10:17             ` Russell King - ARM Linux
2011-07-11 11:14               ` Marc Zyngier
2011-07-11 11:38                 ` Russell King - ARM Linux
2011-07-11 12:36                   ` Marc Zyngier
2011-07-06  5:46 ` [PATCH v8 00/14] Consolidating GIC per-cpu interrupts Shawn Guo
2011-07-06  9:53   ` Marc Zyngier
2011-07-06 11:02 ` Marc Zyngier
2011-07-06 11:11   ` Russell King - ARM Linux
2011-07-06 11:18     ` Marc Zyngier
2011-07-06 12:26     ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.