Consolidating GIC per-cpu interrupts
Subsystems (8)
v8 cover Consolidating GIC per-cpu interrupts
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 ofgit://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
PATCH v8 1/14 ARM: gic: add per-cpu interrupt mapping
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.
--- 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(-)
arch/arm/common/Kconfig +5 -0
@@ -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
arch/arm/common/gic.c +133 -4
172 lines not on this page · raw message
arch/arm/include/asm/entry-macro-multi.S +1 -1
@@ -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)
arch/arm/include/asm/hardware/entry-macro-gic.S +12 -7
@@ -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
arch/arm/include/asm/hardware/gic.h +11 -0
@@ -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
arch/arm/kernel/irq.c +7 -1
@@ -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
Signature · 1 line
1.7.0.4
PATCH v8 2/14 ARM: smp: add interrupt handler for local timers
In reply to Marc ZyngierProvide a "normal" interrupt handler for local timers. It is expected that most timer implementations will use this handler unless they have more specific requirements.
--- arch/arm/include/asm/localtimer.h | 6 ++++++ arch/arm/kernel/smp.c | 12 ++++++++++++ 2 files changed, 18 insertions(+), 0 deletions(-)
arch/arm/include/asm/localtimer.h +6 -0
@@ -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
arch/arm/kernel/smp.c +12 -0
@@ -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;
Signature · 1 line
1.7.0.4
PATCH v8 3/14 ARM: smp_twd: add support for remapped PPI interrupts
In reply to Marc ZyngierWhen 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.
--- arch/arm/kernel/smp_twd.c | 28 +++++++++++++++++++++++++++- 1 files changed, 27 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +27 -1
@@ -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); }
Signature · 1 line
1.7.0.4
PATCH v8 3/14 ARM: smp_twd: add support for remapped PPI interrupts
arch/arm/kernel/smp_twd.c static void twd_set_mode(enum clock_event_mode mode, · line 51
Quoted 8 lines · Marc Zyngier
arch/arm/kernel/smp_twd.c +4
-0@@ -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.
PATCH v8 3/14 ARM: smp_twd: add support for remapped PPI interrupts
Quoted 30 lines · On Fri, 8 Jul 2011 20:27:34 +0100 Russell King - ARM Linux…
arch/arm/kernel/smp_twd.c static void twd_set_mode(enum clock_event_mode mode, · line 51
Quoted 8 lines · Marc Zyngier
arch/arm/kernel/smp_twd.c +4
-0@@ -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 voidpercpu_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); }
Quoted 4 lines
Maybe that needs a
local_timer_stop()->twd_timer_stop() call rather than trying to be clever by doing this via theset_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.
Signature · 1 line
I'm the slime oozin' out from your TV set...
PATCH v8 4/14 ARM: omap4: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
Tested on a Pandaboard.
--- 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(-)
arch/arm/mach-omap2/Kconfig +1 -0
@@ -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
arch/arm/mach-omap2/include/mach/entry-macro.S +1 -13
@@ -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 */
arch/arm/mach-omap2/timer-mpu.c +2 -1
@@ -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 ; }
Signature · 1 line
1.7.0.4
Tony Lindgren
maintainerPATCH v8 4/14 ARM: omap4: use remapped PPI interrupts for local timer
Quoted 9 lines · Marc Zyngier
Use the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
Tested on a Pandaboard.
Cc Tony Lindgren <[email protected]>Reviewed-by Santosh Shilimkar <[email protected]>Tested-by Santosh Shilimkar <[email protected]>Signed-off-by Marc Zyngier <[email protected]>
PATCH v8 5/14 ARM: versatile: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
Tested on VExpress and PB-11MP.
--- 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(-)
arch/arm/mach-realview/Kconfig +2 -0
@@ -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
arch/arm/mach-vexpress/Kconfig +1 -0
@@ -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
arch/arm/plat-versatile/localtimer.c +2 -1
@@ -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 ; }
Signature · 1 line
1.7.0.4
PATCH v8 6/14 ARM: shmobile: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
--- 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(-)
arch/arm/mach-shmobile/Kconfig +1 -0
@@ -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"
arch/arm/mach-shmobile/entry-intc.S +0 -3
@@ -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
arch/arm/mach-shmobile/include/mach/entry-macro.S +0 -3
@@ -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
arch/arm/mach-shmobile/localtimer.c +2 -1
@@ -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 ; }
Signature · 1 line
1.7.0.4
PATCH v8 7/14 ARM: ux500: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
--- arch/arm/mach-ux500/Kconfig | 1 + arch/arm/mach-ux500/localtimer.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletions(-)
arch/arm/mach-ux500/Kconfig +1 -0
@@ -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
arch/arm/mach-ux500/localtimer.c +2 -1
@@ -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 ; }
Signature · 1 line
1.7.0.4
PATCH v8 8/14 ARM: tegra: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
Tested on a Harmony Tegra-2 board.
--- arch/arm/mach-tegra/Kconfig | 1 + arch/arm/mach-tegra/localtimer.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletions(-)
arch/arm/mach-tegra/Kconfig +1 -0
@@ -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
arch/arm/mach-tegra/localtimer.c +2 -1
@@ -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 ; }
Signature · 1 line
1.7.0.4
PATCH v8 9/14 ARM: msm: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse 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.
--- 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(-)
arch/arm/mach-msm/Kconfig +2 -0
@@ -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
arch/arm/mach-msm/board-msm8x60.c +0 -11
@@ -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 )
arch/arm/mach-msm/include/mach/entry-macro-qgic.S +1 -72
82 lines not on this page · raw message
arch/arm/mach-msm/timer.c +29 -29
119 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v8 9/14 ARM: msm: use remapped PPI interrupts for local timer
Quoted 6 lines · Marc Zyngier
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.
Quoted 7 lines
Fixes and ideas courtesy of Stephen Boyd.
Cc David Brown <[email protected]>Cc Daniel Walker <[email protected]>Cc Bryan Huntsman <[email protected]>Reviewed-and-Tested-by Stephen Boyd <[email protected]>Signed-off-by Marc Zyngier <[email protected]>
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH v8 10/14 ARM: exynos4: use remapped PPI interrupts for local timer
In reply to Marc ZyngierUse the normal interrupt scheme for the local timers by using a remapped PPI interrupt.
Tested on a SMDK-S5PV310 board.
--- 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(-)
arch/arm/mach-exynos4/include/mach/entry-macro.S +1 -11
@@ -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
arch/arm/mach-exynos4/localtimer.c +2 -1
@@ -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 ; }
arch/arm/plat-s5p/Kconfig +1 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH v8 11/14 ARM: gic: remove previous local timer interrupt handling
In reply to Marc ZyngierNow that all users of local timers are using CONFIG_ARM_GIC_PPI_MAP, the now unused local timer infrastructure can be safely removed.
--- 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(-)
arch/arm/common/gic.c +0 -10
@@ -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) {
arch/arm/include/asm/entry-macro-multi.S +0 -7
@@ -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
arch/arm/include/asm/hardirq.h +0 -3
@@ -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
arch/arm/include/asm/hardware/entry-macro-gic.S +0 -13
@@ -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
arch/arm/include/asm/hardware/gic.h +0 -1
@@ -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
arch/arm/include/asm/localtimer.h +0 -5
@@ -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);
arch/arm/include/asm/smp.h +0 -5
@@ -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 */
arch/arm/kernel/irq.c +0 -3
@@ -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 ; }
arch/arm/kernel/smp.c +0 -29
@@ -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
arch/arm/kernel/smp_twd.c +0 -7
@@ -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); }
Signature · 1 line
1.7.0.4
PATCH v8 12/14 ARM: gic: add compute_irqnr macro for exynos4
In reply to Marc Zyngierexynos4 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.
--- 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(-)
arch/arm/include/asm/hardware/entry-macro-gic.S +3 -0
@@ -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
arch/arm/mach-exynos4/include/mach/entry-macro.S +5 -55
71 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v8 13/14 ARM: SMP: automatically select ARM_GIC_PPI_MAP
In reply to Marc ZyngierAll SMP platforms are selecting ARM_GIC_PPI_MAP.
Instead, make SMP select ARM_GIC_PPI_MAP and remove select statements from the platform code.
--- 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(-)
arch/arm/Kconfig +1 -0
@@ -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
arch/arm/mach-msm/Kconfig +0 -2
@@ -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
arch/arm/mach-omap2/Kconfig +0 -1
@@ -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
arch/arm/mach-realview/Kconfig +0 -2
@@ -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
arch/arm/mach-shmobile/Kconfig +0 -1
@@ -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"
arch/arm/mach-tegra/Kconfig +0 -1
@@ -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
arch/arm/mach-ux500/Kconfig +0 -1
@@ -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
arch/arm/mach-vexpress/Kconfig +0 -1
@@ -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
arch/arm/plat-s5p/Kconfig +0 -1
@@ -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
Signature · 1 line
1.7.0.4
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
In reply to Marc ZyngierIt 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.
--- arch/arm/common/gic.c | 9 +++++++-- arch/arm/include/asm/hardware/gic.h | 7 +++---- 2 files changed, 10 insertions(+), 6 deletions(-)
arch/arm/common/gic.c +7 -2
@@ -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;
arch/arm/include/asm/hardware/gic.h +3 -4
@@ -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
Signature · 1 line
1.7.0.4
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 3 lines · Marc Zyngier
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().
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 7 lines · On Fri, 8 Jul 2011 20:57:08 +0100 Russell King - ARM Linux…
Quoted 3 lines · Marc Zyngier
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.
Quoted 5 lines
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.
Quoted 12 lines
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 akzalloc()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 thesecondary_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.
Quoted 4 lines
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, asfree_irq()can't be used - it'll try tokfree()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.
Quoted 7 lines
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.
Signature · 1 line
I'm the slime oozin' out from your TV set...
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 8 lines · Marc Zyngier
Quoted 5 lines · On Fri, 8 Jul 2011 20:57:08 +0100 Russell King - ARM Linux…
Quoted 3 lines · Marc Zyngier
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).
Quoted 5 lines
Quoted 2 lines
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_datastructure.
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.
Quoted 9 lines
Quoted 5 lines
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->IRQmapping, *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.
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 4 lines · Russell King - ARM Linux
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:
Quoted 8 lines
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_OFFSTACKnot 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, ¤t->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, ¤t->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(-)
arch/arm/common/gic.c +67 -3
@@ -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
arch/arm/include/asm/entry-macro-multi.S +1 -4
@@ -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:
arch/arm/include/asm/hardware/entry-macro-gic.S +10 -7
@@ -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
arch/arm/include/asm/hardware/gic.h +4 -1
@@ -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
arch/arm/include/asm/localtimer.h +2 -12
@@ -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
arch/arm/include/asm/smp_twd.h +1 -1
@@ -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
arch/arm/kernel/smp.c +1 -17
@@ -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
arch/arm/kernel/smp_twd.c +24 -17
65 lines not on this page · raw message
arch/arm/mach-exynos4/include/mach/entry-macro.S +4 -4
@@ -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
arch/arm/mach-msm/include/mach/entry-macro-qgic.S +7 -7
@@ -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
arch/arm/mach-msm/timer.c +17 -3
@@ -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
arch/arm/mach-omap2/include/mach/entry-macro.S +4 -5
@@ -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 */
arch/arm/mach-shmobile/entry-intc.S +1 -1
@@ -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
arch/arm/mach-shmobile/include/mach/entry-macro.S +1 -1
@@ -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
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 65 lines · Russell King - ARM Linux
Quoted 4 lines · Russell King - ARM Linux
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:
Quoted 8 lines
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_OFFSTACKnot being set.)So, we can either do something like this:
int gic_request_ppi(unsigned int ppi,
irq_handler_thandler,
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, ¤t->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, ¤t->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);
Quoted 6 lines
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).
Quoted 4 lines
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.
Quoted 1 line
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 8 lines · Marc Zyngier
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.
Quoted 10 lines
Quoted 4 lines
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.
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · Marc Zyngier
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).
Quoted 8 lines
Who says
gic_request_ppi()will exist on systems without GIC? Or evengic_ppi_map()?Let me make the point plain: no driver must EVER use
gic_ppi_map(). No driver must EVER callrequest_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.
Quoted 14 lines
Quoted 10 lines
Quoted 4 lines
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.
Quoted 6 lines
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.
Quoted 5 lines
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.
Quoted 8 lines
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.
Quoted 7 lines
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 7 lines · Marc Zyngier
And that's exactly what it does:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.htmlWhat 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.
PATCH v8 14/14 ARM: gic: add gic_ppi_map_on_cpu()
Quoted 13 lines · Russell King - ARM Linux
Quoted 7 lines · Marc Zyngier
And that's exactly what it does:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.htmlWhat 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.
Quoted 7 lines
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
v8 cover Consolidating GIC per-cpu interrupts
In reply to Marc ZyngierQuoted 24 lines · Marc Zyngier
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.
Quoted 7 lines
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.
Signature · 2 lines
Regards, Shawn
v8 cover Consolidating GIC per-cpu interrupts
Quoted 32 lines · Shawn Guo
Quoted 24 lines · Marc Zyngier
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.Quoted 7 lines
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.
Quoted 3 lines
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
v8 cover Consolidating GIC per-cpu interrupts
In reply to Marc ZyngierQuoted 108 lines · Marc Zyngier
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 ofgit://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.gitFrom v7:
- Rebased on v3.0-rc5
- Renamegic_ppi_to_vppi()togic_ppi_map()
- RenameCONFIG_ARM_GIC_VPPItoCONFIG_ARM_GIC_PPI_MAP
- Addgic_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:
- Fixarch/arm/common/Kconfigindentation
- Fix MSM warning when building UP.From v3:
- Fix yet anotherCPU_HOTPLUGproblem: instead of callingfree_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 arequest_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 ofpercpu_timer_handlerto a separate patchFrom v1:
- Fix crash while hotpluging a CPU
- Usehandle_percpu_irq()instead ofhandle_fasteoi_irq()
- MSM fixes courtesy of Stephen Boyd
- MSM switched topercpu_timer_handler()
- Remove local timer interrupt accounting
- Restructure patches #1 and #2Marc 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: addcompute_irqnrmacro for exynos4
ARM: SMP: automatically selectARM_GIC_PPI_MAP
ARM: gic: addgic_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 +--
Quoted 8 lines
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
v8 cover Consolidating GIC per-cpu interrupts
Quoted 118 lines · Marc Zyngier
Quoted 108 lines · Marc Zyngier
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 ofgit://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.gitFrom v7:
- Rebased on v3.0-rc5
- Renamegic_ppi_to_vppi()togic_ppi_map()
- RenameCONFIG_ARM_GIC_VPPItoCONFIG_ARM_GIC_PPI_MAP
- Addgic_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:
- Fixarch/arm/common/Kconfigindentation
- Fix MSM warning when building UP.From v3:
- Fix yet anotherCPU_HOTPLUGproblem: instead of callingfree_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 arequest_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 ofpercpu_timer_handlerto a separate patchFrom v1:
- Fix crash while hotpluging a CPU
- Usehandle_percpu_irq()instead ofhandle_fasteoi_irq()
- MSM fixes courtesy of Stephen Boyd
- MSM switched topercpu_timer_handler()
- Remove local timer interrupt accounting
- Restructure patches #1 and #2Marc 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: addcompute_irqnrmacro for exynos4
ARM: SMP: automatically selectARM_GIC_PPI_MAP
ARM: gic: addgic_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 +--Quoted 8 lines
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(-)
Quoted 4 lines
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.
v8 cover Consolidating GIC per-cpu interrupts
Quoted 121 lines · Russell King - ARM Linux
Quoted 120 lines · Marc Zyngier
Quoted 108 lines · Marc Zyngier
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 ofgit://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.gitFrom v7:
- Rebased on v3.0-rc5
- Renamegic_ppi_to_vppi()togic_ppi_map()
- RenameCONFIG_ARM_GIC_VPPItoCONFIG_ARM_GIC_PPI_MAP
- Addgic_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:
- Fixarch/arm/common/Kconfigindentation
- Fix MSM warning when building UP.From v3:
- Fix yet anotherCPU_HOTPLUGproblem: instead of callingfree_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 arequest_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 ofpercpu_timer_handlerto a separate patchFrom v1:
- Fix crash while hotpluging a CPU
- Usehandle_percpu_irq()instead ofhandle_fasteoi_irq()
- MSM fixes courtesy of Stephen Boyd
- MSM switched topercpu_timer_handler()
- Remove local timer interrupt accounting
- Restructure patches #1 and #2Marc 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: addcompute_irqnrmacro for exynos4
ARM: SMP: automatically selectARM_GIC_PPI_MAP
ARM: gic: addgic_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 +--Quoted 8 lines
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,
Quoted 7 lines
Quoted 2 lines
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).
Quoted 3 lines
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.
Signature · 1 line
Jazz is not dead. It just smells funny...
v8 cover Consolidating GIC per-cpu interrupts
In reply to Russell King - ARM LinuxQuoted 3 lines · Russell King - ARM Linux
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