From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:05
The current GIC per-cpu interrupt (aka PPIs) suffers from a number of
problems:
- It uses a completely separate scheme to handle the interrupts,
mostly because the PPI concept doesn't really match the kernel view
of an interrupt.
- Some low-level code gets duplicated, as usual...
- At least one platform (msm) has started implementing its own
alternative scheme.
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.
This patch set is based on 2.6.39-rc6, and depends on Will Deacon's
GIC fasteoi patches. Tested on VExpress, PB-11MP, Pandaboard and
SMDK-S5PV310.
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:06
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_to_vppi() is used to convert the PPI number
to the per-processor IRQ.
Signed-off-by: Marc Zyngier <redacted>
Reviewed-by: Will Deacon <redacted>
---
arch/arm/common/Kconfig | 5 +
arch/arm/common/gic.c | 133 ++++++++++++++++++++++-
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, 165 insertions(+), 13 deletions(-)
@@ -42,6 +42,12 @@ struct gic_chip_data {unsignedintirq_offset;void__iomem*dist_base;void__iomem*cpu_base;+#ifdef CONFIG_ARM_GIC_VPPI+/* These fields must be 0 on secondary GICs */+intppi_base;+intvppi_base;+u16nrppis;+#endif};/*
@@ -262,12 +268,88 @@ 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_VPPI+unsignedintgic_ppi_to_vppi(unsignedintirq)+{+structgic_chip_data*chip_data=irq_get_chip_data(irq);+unsignedintvppi_irq;+unsignedintppi;++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;++returnvppi_irq;+}++staticvoidgic_handle_ppi(unsignedintirq,structirq_desc*desc)+{+unsignedintvppi_irq;++vppi_irq=gic_ppi_to_vppi(irq);+generic_handle_irq(vppi_irq);+}++staticstructirq_data*gic_vppi_to_ppi(structirq_data*d)+{+structgic_chip_data*chip_data=irq_data_get_irq_chip_data(d);+unsignedintppi_irq;++ppi_irq=d->irq-chip_data->vppi_base-chip_data->nrppis*smp_processor_id();+ppi_irq+=chip_data->ppi_base;++returnirq_get_irq_data(ppi_irq);+}++staticvoidgic_ppi_eoi_irq(structirq_data*d)+{+gic_eoi_irq(gic_vppi_to_ppi(d));+}++staticvoidgic_ppi_mask_irq(structirq_data*d)+{+gic_mask_irq(gic_vppi_to_ppi(d));+}++staticvoidgic_ppi_unmask_irq(structirq_data*d)+{+gic_unmask_irq(gic_vppi_to_ppi(d));+}++staticintgic_ppi_set_type(structirq_data*d,unsignedinttype)+{+returngic_set_type(gic_vppi_to_ppi(d),type);+}++staticintgic_ppi_set_wake(structirq_data*d,unsignedinton)+{+returngic_set_wake(gic_vppi_to_ppi(d),on);+}++staticint__initgic_irq_is_ppi(structgic_chip_data*gic,unsignedintirq)+{+return(irq>=(gic->irq_offset+16)&&irq<=(gic->irq_offset+31));+}++staticstructirq_chipgic_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+staticvoid__initgic_dist_init(structgic_chip_data*gic,unsignedintirq_start){-unsignedintgic_irqs,irq_limit,i;+unsignedintgic_irqs,irq_limit,i,nrvppis=0;void__iomem*base=gic->dist_base;u32cpumask=1<<smp_processor_id();+u32dist_ctr,nrcpus;cpumask|=cpumask<<8;cpumask|=cpumask<<16;
@@ -278,11 +360,32 @@ static void __init gic_dist_init(struct gic_chip_data *gic,*Findouthowmanyinterruptsaresupported.*TheGIConlysupportsupto1020interruptsources.*/-gic_irqs=readl(base+GIC_DIST_CTR)&0x1f;-gic_irqs=(gic_irqs+1)*32;+dist_ctr=readl(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_VPPI+/*+*NobodywouldbeinsaneenoughtousePPIsonasecondary+*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);+/**Setallglobalinterruptstobeleveltriggered,activelow.*/
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:07
When CONFIG_ARM_GIC_VPPI 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_VPPI
is not defined.
Signed-off-by: Marc Zyngier <redacted>
Reviewed-by: Will Deacon <redacted>
---
arch/arm/include/asm/localtimer.h | 6 ++++++
arch/arm/kernel/smp.c | 12 ++++++++++++
arch/arm/kernel/smp_twd.c | 20 +++++++++++++++++++-
3 files changed, 37 insertions(+), 1 deletions(-)
@@ -15,10 +15,11 @@#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 */
@@ -43,6 +44,10 @@ static void twd_set_mode(enum clock_event_mode mode,ctrl=TWD_TIMER_CONTROL_IT_ENABLE|TWD_TIMER_CONTROL_ONESHOT;break;caseCLOCK_EVT_MODE_UNUSED:+#ifdef CONFIG_ARM_GIC_VPPI+free_irq(clk->irq,clk);+/* fall through */+#endifcaseCLOCK_EVT_MODE_SHUTDOWN:default:ctrl=0;
@@ -137,8 +144,19 @@ 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_VPPI+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;+}+#else/* Make sure our local interrupt controller has this enabled */gic_enable_ppi(clk->irq);+#endifclockevents_register_device(clk);}
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:08
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>
Cc: Santosh Shilimkar <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
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(-)
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:09
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 <redacted>
---
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(-)
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:10
Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.
Cc: Paul Mundt <redacted>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Marc Zyngier <redacted>
---
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(-)
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:11
Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.
Cc: Srinidhi Kasagar <redacted>
Cc: Linus Walleij <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
arch/arm/mach-ux500/Kconfig | 1 +
arch/arm/mach-ux500/localtimer.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:12
Use the normal interrupt scheme for the local timers by using
a remapped PPI interrupt.
Cc: Colin Cross <redacted>
Cc: Erik Gilling <redacted>
Cc: Olof Johansson <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
arch/arm/mach-tegra/Kconfig | 1 +
arch/arm/mach-tegra/localtimer.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:13
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 <redacted>
Cc: Daniel Walker <redacted>
Cc: Bryan Huntsman <redacted>
Cc: Stephen Boyd <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
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 | 62 ++++++++----------
4 files changed, 30 insertions(+), 118 deletions(-)
@@ -253,10 +235,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);+irq=gic_ppi_to_vppi(clock->irq);+res=request_irq(irq,percpu_timer_handler,+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);}
@@ -266,6 +251,7 @@ static void __init msm_timer_init(void)int__cpuinitlocal_timer_setup(structclock_event_device*evt){structmsm_clock*clock=&msm_clocks[MSM_GLOBAL_TIMER];+intres;/* Use existing clock_event for cpu 0 */if(!smp_processor_id())
@@ -273,12 +259,13 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)writel(DGT_CLK_CTL_DIV_4,MSM_TMR_BASE+DGT_CLK_CTL);-if(!local_clock_event){+if(!local_timer_inited){writel(0,clock->regbase+TIMER_ENABLE);writel(0,clock->regbase+TIMER_CLEAR);writel(~0,clock->regbase+TIMER_MATCH_VAL);+local_timer_inited=1;}-evt->irq=clock->irq.irq;+evt->irq=gic_ppi_to_vppi(clock->irq);evt->name="local_timer";evt->features=CLOCK_EVT_FEAT_ONESHOT;evt->rating=clock->clockevent.rating;
@@ -290,9 +277,14 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)clockevent_delta2ns(0xf0000000>>clock->shift,evt);evt->min_delta_ns=clockevent_delta2ns(4,evt);-local_clock_event=evt;--gic_enable_ppi(clock->irq.irq);+res=request_irq(evt->irq,percpu_timer_handler,+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);+returnres;+}clockevents_register_device(evt);return0;
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:14
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 <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
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(-)
@@ -153,10 +150,6 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)clk->name,clk->irq,smp_processor_id(),err);return;}-#else-/* Make sure our local interrupt controller has this enabled */-gic_enable_ppi(clk->irq);-#endifclockevents_register_device(clk);}
From: Marc Zyngier <hidden> Date: 2011-05-06 10:33:16
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 <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
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(-)
MSM needs this second patch again. I suppose it's not titled correctly?
I'd rather not have MSM use this function at all though. Perhaps its
better to just hide this in smp_twd.c along with local_timer_ack()?
What is going on with mach-exynos4/mct.c? Their local_timer_ack() is a
return 0 and they do a setup_irq() in their local_timer_setup() path.
That all seems very broken and I would be surprised if hotplug and local
timer stats worked on that system.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
I would prefer to keep the whole interrupt function because 1) MSM
doesn't have a local_timer_ack() to implement and 2) I want to put code
in here to stop the timer so that the timer doesn't wrap and cause
another interrupt (yes the patches haven't been sent yet).
quoted hunk
+static int local_timer_inited;
static cycle_t msm_read_timer_count(struct clocksource *cs)
{
How about assigning evt->irq to gic_ppi_to_vppi() and then use that
instead of a local variable? That would fix the free_irq() bug up above.
Also, percpu_timer_handler() is only defined in arm/kernel/smp.c which
isn't going to be compiled in on non-SMP targets. This timer driver is
common to all MSMs so it needs to work with SMP and non-SMP.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
The current GIC per-cpu interrupt (aka PPIs) suffers from a number of
problems:
- It uses a completely separate scheme to handle the interrupts,
mostly because the PPI concept doesn't really match the kernel view
of an interrupt.
- Some low-level code gets duplicated, as usual...
- At least one platform (msm) has started implementing its own
alternative scheme.
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.
This patch set is based on 2.6.39-rc6, and depends on Will Deacon's
GIC fasteoi patches. Tested on VExpress, PB-11MP, Pandaboard and
SMDK-S5PV310.
Looks like, this series breaks system wide supsend. Please
check.
Regards
Santosh
-------
# echo mem > /sys/power/state
[ 37.503112] PM: Syncing filesystems ... done.
[ 37.552032] Freezing user space processes ... (elapsed 0.01 seconds)
done.
[ 37.577545] Freezing remaining freezable tasks ... (elapsed 0.02
seconds) done.
[ 37.616210] PM: suspend of devices complete after 5.187 msecs
[ 37.623657] PM: late suspend of devices complete after 1.403 msecs
[ 37.630187] Disabling non-boot CPUs ...
[ 37.731964] CPU1: shutdown
[ 38.285888] Enabling non-boot CPUs ...
[ 38.291137] CPU1: Booted secondary processor
[ 38.291168] CPU1: Unknown IPI message 0x1
[ 38.291168] local_timer: can't register interrupt 413 on cpu 1 (-16)
[ 38.365112] Unable to handle kernel NULL pointer dereference at
virtual address 0000004c
[ 38.388946] pgd = c0004000
[ 38.391784] [0000004c] *pgd=00000000
[ 38.395538] Internal error: Oops: 805 [#1] SMP
[ 38.400207] last sysfs file: /sys/devices/virtual/vc/vcsa63/dev
[ 38.406433] Modules linked in:
[ 38.409637] CPU: 1 Not tainted (2.6.39-rc5-00099-g9e06a0a #6)
[ 38.416046] PC is at clockevents_program_event+0x60/0xd4
[ 38.421630] LR is at tick_dev_program_event+0x38/0x140
[ 38.427032] pc : [<c00bc270>] lr : [<c00bd730>] psr: 60000193
[ 38.427032] sp : ef885f78 ip : 00000008 fp : 00000000
[ 38.439086] r10: 00000000 r9 : 00000008 r8 : cebb6148
[ 38.444580] r7 : 00000000 r6 : 0076bbcd r5 : 00000000 r4 : 00000000
[ 38.451446] r3 : 00000008 r2 : cebb6148 r1 : 00000000 r0 : cebb6148
[ 38.458282] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
Segment kernel
[ 38.466064] Control: 10c53c7d Table: 8000404a DAC: 00000017
[ 38.472076] Process swapper (pid: 0, stack limit = 0xef8842f8)
[ 38.478210] Stack: (0xef885f78 to 0xef886000)
[ 38.482788] 5f60:
00000000 00000000
[ 38.491394] 5f80: 00000001 c00bd730 ce44a57b 00000008 ce44a57b
00000008 cebb6148 00000008
[ 38.499969] 5fa0: ce442e45 00000000 411fc092 c00bd8ac 00000000
00000008 00000000 c1164ef0
[ 38.508575] 5fc0: 00000008 c00be464 00773594 00000000 ce442e45
00000008 ef884000 0000001f
[ 38.517150] 5fe0: 10c03c7d c05f8630 00000000 c005b9a4 00000001
c006bd8c 5e3adbb2 b8403c00
[ 38.525756] [<c00bc270>] (clockevents_program_event+0x60/0xd4) from
[<c00bd730>] (tick_dev_program_event+0x38/0x140)
[ 38.536804] [<c00bd730>] (tick_dev_program_event+0x38/0x140) from
[<c00bd8ac>] (tick_program_event+0x3c/0x48)
[ 38.547241] [<c00bd8ac>] (tick_program_event+0x3c/0x48) from
[<c00be464>] (tick_nohz_restart_sched_tick+0x170/0x1b8)
[ 38.558319] [<c00be464>] (tick_nohz_restart_sched_tick+0x170/0x1b8)
from [<c005b9a4>] (cpu_idle+0xdc/0xf8)
[ 38.568481] [<c005b9a4>] (cpu_idle+0xdc/0xf8) from [<c006bd8c>]
(platform_cpu_die+0x3c/0x50)
[ 38.577331] Code: e0c17007 e3560001 e2d71000 ba000018 (e584304c)
[ 38.583740] ---[ end trace fd37aa01e7ce7dcd ]---
[ 38.588592] Kernel panic - not syncing: Attempted to kill the idle task!
[ 38.595642] CPU0: stopping
[ 38.598510] [<c0060cdc>] (unwind_backtrace+0x0/0xe4) from
[<c00502a4>] (do_IPI+0xb4/0x12c)
[ 38.607208] [<c00502a4>] (do_IPI+0xb4/0x12c) from [<c03f6c9c>]
(__irq_svc+0x3c/0x100)
[ 38.615447] Exception stack(0xc058bf88 to 0xc058bfd0)
[ 38.620727] bf80: c005b948 00000000 c058bfc0
00000000 c058a000 c0034c0c
[ 38.629333] bfa0: c0034c08 c0590ddc 80000000 411fc092 00000000
00000000 00000000 c058bfd0
[ 38.637908] bfc0: c005b948 c005b94c 60000013 ffffffff
[ 38.643218] [<c03f6c9c>] (__irq_svc+0x3c/0x100) from [<c005b94c>]
(cpu_idle+0x84/0xf8)
[ 38.651550] [<c005b94c>] (cpu_idle+0x84/0xf8) from [<c000893c>]
(start_kernel+0x298/0x2f0)
[ 38.660247] [<c000893c>] (start_kernel+0x298/0x2f0) from [<8000803c>]
(0x8000803c)
Regards
Santosh
From: Marc Zyngier <hidden> Date: 2011-05-14 16:12:30
On Fri, 13 May 2011 22:36:04 +0530
Santosh Shilimkar [off-list ref] wrote:
Hi Santosh,
Looks like, this series breaks system wide supsend. Please
check.
Regards
Santosh
-------
# echo mem > /sys/power/state
[ 37.503112] PM: Syncing filesystems ... done.
[ 37.552032] Freezing user space processes ... (elapsed 0.01 seconds)
done.
[ 37.577545] Freezing remaining freezable tasks ... (elapsed 0.02
seconds) done.
[ 37.616210] PM: suspend of devices complete after 5.187 msecs
[ 37.623657] PM: late suspend of devices complete after 1.403 msecs
[ 37.630187] Disabling non-boot CPUs ...
[ 37.731964] CPU1: shutdown
[ 38.285888] Enabling non-boot CPUs ...
[ 38.291137] CPU1: Booted secondary processor
[ 38.291168] CPU1: Unknown IPI message 0x1
[ 38.291168] local_timer: can't register interrupt 413 on cpu 1 (-16)
Blah... We obviously don't stop the timer on secondary CPUs, and then
try to request the interrupt again. I'll have a look on Monday.
Thanks for pointing this out.
M.
--
I'm the slime oozin' out from your TV set...
From: Marc Zyngier <hidden> Date: 2011-05-17 14:21:31
Hi Santosh,
On Fri, 2011-05-13 at 22:36 +0530, Santosh Shilimkar wrote:
Marc,
On 5/6/2011 4:03 PM, Marc Zyngier wrote:
quoted
The current GIC per-cpu interrupt (aka PPIs) suffers from a number of
problems:
- It uses a completely separate scheme to handle the interrupts,
mostly because the PPI concept doesn't really match the kernel view
of an interrupt.
- Some low-level code gets duplicated, as usual...
- At least one platform (msm) has started implementing its own
alternative scheme.
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.
This patch set is based on 2.6.39-rc6, and depends on Will Deacon's
GIC fasteoi patches. Tested on VExpress, PB-11MP, Pandaboard and
SMDK-S5PV310.
Looks like, this series breaks system wide supsend. Please
check.
Are you sure you're testing with v2? v1 is known to be broken with
CPU_HOTPLUG, but v2 should deal with it. Here is a suspend/resume cycle
on my Panda:
root at florentine-pogen:~# echo mem>/sys/power/state
[ 136.215667] PM: Syncing filesystems ... done.
[ 136.220977] PM: Preparing system for mem sleep
[ 136.226989] Freezing user space processes ... (elapsed 0.02 seconds) done.
[ 136.257232] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
[ 136.280670] PM: Entering mem sleep
[ 136.332916] usb 1-1.1: unlink qh8-0001/dfb97460 start 2 [1/0 us]
[ 136.339447] usb 1-1.1: usb suspend
[ 136.363861] hub 1-1:1.0: hub_suspend
[ 136.367614] usb 1-1: unlink qh256-0001/dfa846e0 start 1 [1/0 us]
[ 136.373992] usb 1-1: usb suspend
[ 136.395111] hub 1-0:1.0: hub_suspend
[ 136.398864] usb usb1: bus suspend
[ 136.402343] ehci-omap ehci-omap.0: suspend root hub
[ 136.408477] PM: suspend of devices complete after 123.748 msecs
[ 136.414703] PM: suspend devices took 0.132 seconds
[ 136.420135] PM: late suspend of devices complete after 0.427 msecs
[ 136.426605] Disabling non-boot CPUs ...
[ 136.528137] CPU1: shutdown
[ 138.320465] Enabling non-boot CPUs ...
[ 138.324737] CPU1: Booted secondary processor
[ 138.324737] CPU1: Unknown IPI message 0x1
[ 138.332183] Switched to NOHz mode on CPU #1
[ 138.527862] CPU1 is up
[ 138.543518] PM: early resume of devices complete after 0.152 msecs
[ 138.793670] usb usb1: usb resume
[ 138.797058] ehci-omap ehci-omap.0: resume root hub
[ 138.848022] hub 1-0:1.0: hub_resume
[ 138.851684] hub 1-0:1.0: port 1: status 0507 change 0000
[ 138.857299] usb 1-1: usb resume
[ 138.894897] ehci-omap ehci-omap.0: GetStatus port:1 status 001005 0 ACK POWER sig=se0 PE CONNECT
[ 138.926147] usb 1-1: finish resume
[ 138.933868] hub 1-1:1.0: hub_resume
[ 138.937591] hub 1-1:1.0: port 1: status 0507 change 0000
[ 138.957489] ehci-omap ehci-omap.0: reused qh dfa846e0 schedule
[ 138.963592] usb 1-1: link qh256-0001/dfa846e0 start 1 [1/0 us]
[ 138.972991] usb 1-1.1: usb resume
[ 139.037353] usb 1-1.1: finish resume
[ 139.041259] ehci-omap ehci-omap.0: reused qh dfb97460 schedule
[ 139.047363] usb 1-1.1: link qh8-0001/dfb97460 start 2 [1/0 us]
[ 139.053649] PM: resume of devices complete after 503.509 msecs
[ 139.059936] PM: resume devices took 0.515 seconds
[ 139.064880] PM: Finishing wakeup.
[ 139.068359] Restarting tasks ...
[ 139.071685] hub 1-0:1.0: state 7 ports 3 chg 0000 evt 0000
[ 139.072631] done.
[ 139.079681] hub 1-1:1.0: state 7 ports 5 chg 0000 evt 0002
root at florentine-pogen:~#
Seems to be just fine here...
Cheers,
M.
--
Reality is an implementation detail.
--
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Hi Santosh,
On Fri, 2011-05-13 at 22:36 +0530, Santosh Shilimkar wrote:
quoted
Marc,
On 5/6/2011 4:03 PM, Marc Zyngier wrote:
quoted
The current GIC per-cpu interrupt (aka PPIs) suffers from a number of
problems:
- It uses a completely separate scheme to handle the interrupts,
mostly because the PPI concept doesn't really match the kernel view
of an interrupt.
- Some low-level code gets duplicated, as usual...
- At least one platform (msm) has started implementing its own
alternative scheme.
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.
This patch set is based on 2.6.39-rc6, and depends on Will Deacon's
GIC fasteoi patches. Tested on VExpress, PB-11MP, Pandaboard and
SMDK-S5PV310.
Looks like, this series breaks system wide supsend. Please
check.
Are you sure you're testing with v2? v1 is known to be broken with
CPU_HOTPLUG, but v2 should deal with it. Here is a suspend/resume cycle
on my Panda:
May be I tried V1 then. Will try your v2 tomorrow and let you know
MSM needs this second patch again. I suppose it's not titled correctly?
I'd rather not have MSM use this function at all though. Perhaps its
better to just hide this in smp_twd.c along with local_timer_ack()?
I'd like to keep the handler in a separate file, as I have an additional
patch set for the A15 timers that are using the same handler.
What is going on with mach-exynos4/mct.c? Their local_timer_ack() is a
return 0 and they do a setup_irq() in their local_timer_setup() path.
That all seems very broken and I would be surprised if hotplug and local
timer stats worked on that system.
MCT is an odd case. They're using SPI, not PPI, and local_timer_ack() is
only present to please the linker (never called).
Could probably have them to converge as well.
M.
--
Reality is an implementation detail.
--
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Marc,
On 5/17/2011 8:02 PM, Santosh Shilimkar wrote:
On 5/17/2011 7:51 PM, Marc Zyngier wrote:
quoted
Hi Santosh,
[...]
quoted
quoted
Looks like, this series breaks system wide supsend. Please
check.
Are you sure you're testing with v2? v1 is known to be broken with
CPU_HOTPLUG, but v2 should deal with it. Here is a suspend/resume cycle
on my Panda:
May be I tried V1 then. Will try your v2 tomorrow and let you know
From: Marc Zyngier <hidden> Date: 2011-05-18 14:07:28
On Wed, 2011-05-18 at 19:34 +0530, Santosh Shilimkar wrote:
Marc,
On 5/17/2011 8:02 PM, Santosh Shilimkar wrote:
quoted
On 5/17/2011 7:51 PM, Marc Zyngier wrote:
quoted
Hi Santosh,
[...]
quoted
quoted
quoted
Looks like, this series breaks system wide supsend. Please
check.
Are you sure you're testing with v2? v1 is known to be broken with
CPU_HOTPLUG, but v2 should deal with it. Here is a suspend/resume cycle
on my Panda:
May be I tried V1 then. Will try your v2 tomorrow and let you know
V2 seems to work with suspend.
Thanks a lot for testing!
M.
--
Reality is an implementation detail.
I would prefer to keep the whole interrupt function because 1) MSM
doesn't have a local_timer_ack() to implement and 2) I want to put code
in here to stop the timer so that the timer doesn't wrap and cause
another interrupt (yes the patches haven't been sent yet).
I was thinking of reusing the local_timer_ack() for that, possibly
passing some useful parameters (evt, cpu...). I'd really like the
event_handler() call to become common code, and move everything else to
the local_timer_ack() method (with a possible empty default implemented
as a weak symbol).
quoted
+static int local_timer_inited;
static cycle_t msm_read_timer_count(struct clocksource *cs)
{
How about assigning evt->irq to gic_ppi_to_vppi() and then use that
instead of a local variable? That would fix the free_irq() bug up above.
Indeed. Fixed.
Also, percpu_timer_handler() is only defined in arm/kernel/smp.c which
isn't going to be compiled in on non-SMP targets. This timer driver is
common to all MSMs so it needs to work with SMP and non-SMP.
My plan is to move most (if not all) of the timer stuff out of smp.c, as
A15 has the same requirements as MSM (local timers are used even on
non-smp configurations). I'll introduce that earlier than expected,
then.
Expect a v3 shortly...
Thanks for reviewing,
M.
--
Reality is an implementation detail.
We just lost this important line. This prevents spurious interrupts from
crashing the system.
Is this something you actually see on a real system, or just a guard in
case something goes horribly wrong?
I believe a bootloader left a pending interrupt at some point and thus
when we request the interrupt before registering the clockevent the
interrupt handler will be called and evt->event_handler == NULL. Perhaps
we could register the clockevent before registering the interrupt
handler? I'm not sure that works. Otherwise we need to clear the
interrupt in the GIC or something. Any suggestions?
If it's any consolation, x86 seems to do the same thing presumably for
the same reason.
I would prefer to keep the whole interrupt function because 1) MSM
doesn't have a local_timer_ack() to implement and 2) I want to put code
in here to stop the timer so that the timer doesn't wrap and cause
another interrupt (yes the patches haven't been sent yet).
I was thinking of reusing the local_timer_ack() for that, possibly
passing some useful parameters (evt, cpu...). I'd really like the
event_handler() call to become common code, and move everything else to
the local_timer_ack() method (with a possible empty default implemented
as a weak symbol).
Ok. So you're saying there is one interrupt handler that will call down
to the hardware specific handler via local_timer_ack()? That sounds like
one step backwards when you consider we want to compile many machines
into one kernel.
A generic interrupt handler for simple timers where there is nothing to
do besides call the event handler is probably good consolidation. But if
the hardware requires something else, it doesn't seem so bad to write
your own.
What's the use of local_timer_ack() in the scheme of this patch series
again? I was really hoping that function would go away.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
We just lost this important line. This prevents spurious interrupts from
crashing the system.
Is this something you actually see on a real system, or just a guard in
case something goes horribly wrong?
I believe a bootloader left a pending interrupt at some point and thus
when we request the interrupt before registering the clockevent the
interrupt handler will be called and evt->event_handler == NULL. Perhaps
we could register the clockevent before registering the interrupt
handler? I'm not sure that works. Otherwise we need to clear the
interrupt in the GIC or something. Any suggestions?
The generic code could install a dummy event_handler before calling into
the platform code. That way, no need to test for this on the hot path.
If it's any consolation, x86 seems to do the same thing presumably for
the same reason.
I would prefer to keep the whole interrupt function because 1) MSM
doesn't have a local_timer_ack() to implement and 2) I want to put code
in here to stop the timer so that the timer doesn't wrap and cause
another interrupt (yes the patches haven't been sent yet).
I was thinking of reusing the local_timer_ack() for that, possibly
passing some useful parameters (evt, cpu...). I'd really like the
event_handler() call to become common code, and move everything else to
the local_timer_ack() method (with a possible empty default implemented
as a weak symbol).
Ok. So you're saying there is one interrupt handler that will call down
to the hardware specific handler via local_timer_ack()? That sounds like
one step backwards when you consider we want to compile many machines
into one kernel.
That would only be an interim hack. I proposed a solution for that a
while ago, as part of my A15 timer series:
http://www.spinics.net/lists/arm-kernel/msg118579.html
Basically, you register a set of function pointers with the core timer
code, local_timer_ack() being one of them. If you do not provide one,
even better.
That gives you a way to register your timer at runtime (I have the same
binary kernel running on A5 using TWD and A15 using the architected
timers).
A generic interrupt handler for simple timers where there is nothing to
do besides call the event handler is probably good consolidation. But if
the hardware requires something else, it doesn't seem so bad to write
your own.
What's the use of local_timer_ack() in the scheme of this patch series
again? I was really hoping that function would go away.
Maybe this is a bit out of the scope of this patch series, actually.
I'll drop this change, and will create another series only impacting
local_ack()/interrupt handler.
Cheers,
M.
--
Reality is an implementation detail.