Re: [PATCHv3 4/4] arm: Add generic timer broadcast support
From: Santosh Shilimkar
Date: Thu Feb 07 2013 - 06:39:31 EST
Mark,
On Thursday 07 February 2013 03:34 PM, Mark Rutland wrote:
Hi Stephen,
Sorry about this; I'm to blame for the bug.
On Wed, Feb 06, 2013 at 08:51:43PM +0000, Stephen Warren wrote:
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.
The intent of 12ad100046: "clockevents: Add generic timer broadcast function"
was to setup the broadcast function both for non-functional/dummy timers and
those that stop in low-power states (CLOCK_EVT_FEAT_C3STOP). I missed the
CLOCK_EVT_FEAT_C3STOP case.
I am not sure this exactly the case. Because in my testing, the C3STOP
path was exercised already. And if the C3STOP is used then notifiers
calls are expected for switching of clock-events to broadcast mode.
And dummy broad-cast hook should come into picture only if the per CPU
local timer clock-event are not registered.
I just tried "next-20130207" on OMAP and I see the broad-cast mechanism
works and didn't observe any crash.
------------------------------
Tick Device: mode: 1
Broadcast device
Clock Event Device: gp_timer
max_delta_ns: 131071523464981
min_delta_ns: 91552
mult: 70369
shift: 31
mode: 3
next_event: 89984375000 nsecs
set_next_event: omap2_gp_timer_set_next_event
set_mode: omap2_gp_timer_set_mode
event_handler: tick_handle_oneshot_broadcast
retries: 0
tick_broadcast_mask: 00000003
tick_broadcast_oneshot_mask: 00000000
Tick Device: mode: 1
Per CPU device: 0
Clock Event Device: local_timer
max_delta_ns: 10737418240
min_delta_ns: 1000
mult: 858993459
shift: 31
mode: 3
next_event: 125250000000 nsecs
set_next_event: twd_set_next_event
set_mode: twd_set_mode
event_handler: hrtimer_interrupt
retries: 346
Tick Device: mode: 1
Per CPU device: 1
Clock Event Device: local_timer
max_delta_ns: 10737418240
min_delta_ns: 1000
mult: 858993459
shift: 31
mode: 3
next_event: 89921875000 nsecs
set_next_event: twd_set_next_event
set_mode: twd_set_mode
event_handler: hrtimer_interrupt
retries: 258
#
------------------------------
I believe the patch below will fix this for Tegra and any other platforms where
broadcast is required in low power states.
Am not sure you really need that patch unless and until am missing a
scenario in my test.
Stephan,
Do we have crash log seen on Tegra ? Which tegra CPUIDLE driver
is being used when crash is seen ?
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/