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

[Russell King - ARM Linux]

On Mon, Jan 24, 2011 at 11:39:13PM -0800, Colin Cross wrote:
> On Mon, Jan 24, 2011 at 3:11 AM, Russell King - ARM Linux
> <linux at 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.

The next question is can we teach the generic time infrastructure about
this so we don't have to modify every clock event driver for it?  We
really need to get away from having this kind of knowledge buried down
in the lowest levels of every driver.