Re: [PATCH 3/5] ARM: twd: Add context save restore support

Colin Cross
2011-01-25T07:39:15Z

On Mon, Jan 24, 2011 at 3:11 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 24, 2011 at 11:06:09AM +0000, Russell King - ARM Linux wrote:
>> On Mon, Jan 24, 2011 at 02:21:17PM +0530, Santosh Shilimkar wrote:
>> > In CPU low power state, local timer looses its register context. This
>> > patch adds context save restore hooks which can be used by platforms
>> > appropriately.
>>
>> I thought the whole point of CLOCK_EVT_FEAT_C3STOP was that the generic
>> timer stuff wouldn't rely on it being kept alive?
>>
>> Hmm, it looks like we bypass the clockevents code by only setting the
>> TWD load value at initialization time, not when we switch to periodic
>> mode.  We really ought to rewrite it whenever we switch back to periodic
>> mode.
>>
>> I suspect fixing that means you won't need this save/restore support.
>
> Untested, but should do what's required.
>
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index fd91566..60636f4 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -36,6 +36,7 @@ static void twd_set_mode(enum clock_event_mode mode,
>                /* timer load already set up */
>                ctrl = TWD_TIMER_CONTROL_ENABLE | TWD_TIMER_CONTROL_IT_ENABLE
>                        | TWD_TIMER_CONTROL_PERIODIC;
> +               __raw_writel(twd_timer_rate / HZ, twd_base + TWD_TIMER_LOAD);
>                break;
>        case CLOCK_EVT_MODE_ONESHOT:
>                /* period set, and timer enabled in 'next_event' hook */
> @@ -81,7 +82,7 @@ int twd_timer_ack(void)
>
>  static void __cpuinit twd_calibrate_rate(void)
>  {
> -       unsigned long load, count;
> +       unsigned long count;
>        u64 waitjiffies;
>
>        /*
> @@ -116,10 +117,6 @@ static void __cpuinit twd_calibrate_rate(void)
>                printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
>                        (twd_timer_rate / 1000000) % 100);
>        }
> -
> -       load = twd_timer_rate / HZ;
> -
> -       __raw_writel(load, twd_base + TWD_TIMER_LOAD);
>  }
>
>  /*

This doesn't work for oneshot timers if the TWD_TIMER_CONTROL register
gets reset by cpu idle between twd_set_mode and twd_set_next_event.
Shadowing ctrl in a percpu variable and rewriting it during every call
to twd_set_next_event does work, but that's not much different, and a
little less efficient, than just saving and restoring the control and
load registers in idle. It does have the advantage that platforms
don't need any extra calls.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html