Re: [patch 3/3] xtensa: ccount clocksource

[Daniel Walker <dwalker@fifo99.com>]

On Tue, 2009-03-03 at 20:54 +0100, Johannes Weiner wrote:
> On Tue, Mar 03, 2009 at 09:55:45AM -0800, Daniel Walker wrote:
> > On Tue, 2009-03-03 at 16:30 +0100, Johannes Weiner wrote:
> > > @ -29,6 +30,19 @@ unsigned long ccount_per_jiffy;              /* per 
> > >  unsigned long nsec_per_ccount;         /* nsec per ccount increment
> > > */
> > >  #endif
> > >  
> > > +static cycle_t ccount_read(void)
> > > +{
> > > +       return (cycle_t)get_ccount();
> > > +}
> > > +
> > > +static struct clocksource ccount_clocksource = {
> > > +       .name = "ccount",
> > > +       .rating = 200,
> > > +       .read = ccount_read,
> > > +       .mask = CLOCKSOURCE_MASK(32),
> > > +       .mult = NSEC_PER_CCOUNT,
> > > +};
> > 
> > You don't want to use the shift field?
> 
> Thanks for pointing it out.
> 
> To make sure I understood this:
> 
> If shift is 0, then a walltime adjustment would be done in 1/2^0
> steps, meaning an adjustment of counting one nanosecond more or less
> per ccount.

The shift and mult are just used to convert your cycle counters current
count into nanoseconds .. I'm not sure how much a zero shift would
degrade the conversion to nanoseconds for your cycles counter tho. So it
could be along the lines of what your suggesting above.

> To give this a finer granularity and smooth out adjustments, the shift
> should be a trade-off between too much adjustment and no adjustment
> progress in a sane amount of time (and, of course, to stay within
> bounds of the used type).
> 
> Does that make sense?

I don't really look at it in terms of walltime adjustments. The actual
frequency of kernel time adjustments isn't defined in the clocksource
structure AFAIK. From my experience you just want the clocksource to
produce the most accurate nanosecond value your hardware can provide ,
which would mean setting the shift as high as is safe for your hardware.

I added John Stultz to the CC so he could comment further if he cares
too.

> I found a patch of yours that introduced clocksource_hz2shift() but it
> seems it hasn't been merged (yet).  Is it yet to get integrated?

I've been meaning to update it, but haven't gotten around to it. If you
know the input values you can run that function once just to produce a
shift which you define statically in the clocksource structure. Then use
that shift value with the other helper clocksource_hz2mult() to produce
a mult value.

Daniel