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

[Thomas Gleixner]

On Tue, 25 Jan 2011, Russell King - ARM Linux wrote:
> 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.

In which way? I mean the generic code issues a call to the set_mode
function when we leave the broadcast mode. So what should the generic
code do more ?

Thanks,

	tglx