Re: [PATCH] clocksource: shift helper

[Roman Zippel <zippel@linux-m68k.org>]

Hi,

On Thursday 1. May 2008, Daniel Walker wrote:

> This is a little helper I pulled from the mips tree. I've seen a couple
> of bogus shift values, and this helper calculates the shift. This
> should move the shift selection away from the user, and systematically
> pick the value base on the hz.
>
> I also modified the acpi_pm timer to use this new interface. The original
> shift was 22 , and after this patch it's increased to 23. Which should make
> the clocksource slightly more accurate, but shouldn't have much of a
> visible effect.

What is this calculation based on?

Unless I miss something even 22 is far more than enough. acpi_pm has a 
resolution of 279ns, thus the clock can't be more accurate than half of this 
on average and increasing the shift doesn't change much directly about it.
What the shift value does influence is the size of the adjustment step the 
clock code can do during an update, so with shift=22 and HZ=100 the possible 
adjustment step would be 0.008ns (freq/2^shift/ntp_hz). The problem is that 
this value is also used for how soon the clock is being adjusted, this means 
the clock code is constantly busy adjusting for a very small error.
So from a clock adjustment perspective a shift value close to log2
(freq/res/10^9/ntp_hz) (with res=1/freq and being limited to 1ns<=res<=1us) 
would provide sufficient accuracy, while keeping the need for adjustment low, 
this means with the above example a shift value of much more than 8 doesn't 
improve accuracy significantly.

That said, I agree that a dynamic calculation of the shift value is a good 
thing, but I don't think that the method in the patch is a good one.

bye, Roman