[PATCHv3,4/4] arm: Add generic timer broadcast support

Commit Message

Mark Rutland Jan. 14, 2013, 5:05 p.m. UTC

Implement timer_broadcast for the arm architecture, allowing for the use
of clock_event_device_drivers decoupled from the timer tick broadcast
mechanism.

Signed-off-by: Mark Rutland <[e-mail]>
Reviewed-by: Santosh Shilimkar <[e-mail]>
Tested-by: Santosh Shilimkar <[e-mail]>
Reviewed-by: Stephen Boyd <[e-mail]>
---
 arch/arm/Kconfig      |    1 +
 arch/arm/kernel/smp.c |    3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Stephen Warren Feb. 6, 2013, 8:51 p.m. UTC | #1

On 01/14/2013 10:05 AM, Mark Rutland wrote:
> Implement timer_broadcast for the arm architecture, allowing for the use
> of clock_event_device_drivers decoupled from the timer tick broadcast
> mechanism.

Mark, this patch is now in next-20130206 and causes a crash during boot
on Tegra. The reason appears to be because of:

> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c

> @@ -524,7 +524,6 @@ static void __cpuinit percpu_timer_setup(void)
>  	struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
>  
>  	evt->cpumask = cpumask_of(cpu);
> -	evt->broadcast = smp_timer_broadcast;

After that change, evt->broadcast is never assigned, and hence is NULL.
Yet elsewhere in kernel/time/tick-broadcast.c it's used unconditionally:

static void tick_do_broadcast(struct cpumask *mask)
...
	if (!cpumask_empty(mask)) {
...
		td = &per_cpu(tick_cpu_device, cpumask_first(mask));
		td->evtdev->broadcast(mask);

Now perhaps the Tegra timer driver simply isn't being set up correctly,
so the bug is there... But the only other place I can find where
->broadcast is assigned is in tick_device_uses_broadcast() which only
does it for "non-functional" timers, which doesn't apply to Tegra's timer.

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 67874b8..65ae737 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -4,6 +4,7 @@  config ARM
 	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAVE_CUSTOM_GPIO_H
+	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select BUILDTIME_EXTABLE_SORT if MMU
 	select CPU_PM if (SUSPEND || CPU_IDLE)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 9308519..b7e3b50 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -476,7 +476,7 @@  u64 smp_irq_stat_cpu(unsigned int cpu)
 static DEFINE_PER_CPU(struct clock_event_device, percpu_clockevent);
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
-static void smp_timer_broadcast(const struct cpumask *mask)
+void tick_broadcast(const struct cpumask *mask)
 {
 	smp_cross_call(mask, IPI_TIMER);
 }
@@ -524,7 +524,6 @@  static void __cpuinit percpu_timer_setup(void)
 	struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
 
 	evt->cpumask = cpumask_of(cpu);
-	evt->broadcast = smp_timer_broadcast;
 
 	if (!lt_ops || lt_ops->setup(evt))
 		broadcast_timer_setup(evt);