Wall clock accuracy

Wall clock accuracy

[Rune Torgersen]

[-- Attachment #1: Type: text/plain, Size: 5176 bytes --]

Hi

I have discovered that the accuracy of the wallclock (xtime) on ppc is
not very good.
I am running a custom telco board based on a 8266, and the main busclock
is derived off of a T1 reference clock.
I was noticing a huge number of logentries fron OpenNTPD about
adjustiing the clock, so I started to check.
The drift of the walltime was a little over 7 seconds in 15 hours (7
seconds slow) (equals about 130us per s)

When I checked around, I found that xtime is updated in kernel/time.c
and is updated by time_nsec nanoseconds per timer interrupt. 
This value is hardcoded (through some obscure macros) to be 999848
ns/timertick. Tis is 152ns slow per timertick. This works out to be
152us per second, almos the value I found.

I have tried a few workarounds, and made one that modifies time_nsec
before update_wall_time_one_tick() is called.
With this fix (which is 8260 specific at the time) I have less than 1 ms
drift after 5 days.

The fix works by calculating how many ns and fractions of ns there is in
one timer-tick (one jiffie).
Then, at each timer interrupt, I accumulate the ns fractions, and add
whole ns to the timer when needed.

This is tested on 8266 under 80.00MHz and 82.575360MHz main busclock. It
also works correct if HZ=1024 instead of 1000

It is not meant as a patch to be added to the kernel right now, but more
of a possible solution to a problem that it doesn't seem anybody has
noticed before.

Index: arch/ppc/kernel/time.c
===================================================================
--- arch/ppc/kernel/time.c	(revision 20)
+++ arch/ppc/kernel/time.c	(working copy)
@@ -119,6 +119,11 @@
 EXPORT_SYMBOL(profile_pc);
 #endif
 
+
+#ifdef CONFIG_8260
+extern unsigned long get_ns_delta(void); /*
arch/ppc/syslib/m8260_setup.c */
+extern unsigned long tick_nsec;          /* kernel/time.c */
+#endif
 /*
  * timer_interrupt - gets called when the decrementer overflows,
  * with interrupts disabled.
@@ -148,6 +153,10 @@
 		/* We are in an interrupt, no need to save/restore flags
*/
 		write_seqlock(&xtime_lock);
 		tb_last_stamp = jiffy_stamp;
+
+#ifdef CONFIG_8260		
+		tick_nsec = get_ns_delta();
+#endif		
 		do_timer(regs);
 
 		/*
@@ -179,7 +188,7 @@
 		write_sequnlock(&xtime_lock);
 	}
 	if ( !disarm_decr[smp_processor_id()] )
-		set_dec(next_dec);
+		set_dec(next_dec-1);
 	last_jiffy_stamp(cpu) = jiffy_stamp;
 
 	if (ppc_md.heartbeat && !ppc_md.heartbeat_count--)
Index: arch/ppc/syslib/m8260_setup.c
===================================================================
--- arch/ppc/syslib/m8260_setup.c	(revision 20)
+++ arch/ppc/syslib/m8260_setup.c	(working copy)
@@ -31,6 +31,8 @@
 
 #include "cpm2_pic.h"
 
+#include <linux/module.h>    
+
 unsigned char __res[sizeof(bd_t)];
 
 extern void cpm2_reset(void);
@@ -82,16 +84,63 @@
 /* The decrementer counts at the system (internal) clock frequency
  * divided by four.
  */
+unsigned long tb_time_nsec;
+unsigned long tb_frac_nsec;
+unsigned long tb_frac_limit;     
+ 
+extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
+ 
 static void __init
 m8260_calibrate_decr(void)
+{    
+    bd_t *binfo = (bd_t *)__res;
+    int freq, divisor, tick_freq;
+    uint64_t tmp;
+    printk("Calibrating Decrementer\n");
+
+    freq = binfo->bi_busfreq;
+    divisor = 4;
+    
+    /* get rounded off decrementer frequency */
+    tick_freq = (freq + 3) / 4;
+    
+    /* get number of timebase ticks per jiffy (timerint) */
+    tb_ticks_per_jiffy = (tick_freq + (HZ/2)) / HZ;
+    tb_to_us = mulhwu_scale_factor(tick_freq, 1000000);
+
+    /* calculate timer interrupt interval in nano seconds */
+    tmp = 1000000000ULL * (u64)tb_ticks_per_jiffy;
+    __div64_32(&tmp, tick_freq);
+    tb_time_nsec = tmp;
+    
+    /* calculate fractions of nanosecons per timer interrupt */
+    tb_frac_limit = 100000UL;
+    tmp = 1000000000ULL * (u64)tb_ticks_per_jiffy * tb_frac_limit;
+    __div64_32(&tmp, tick_freq);
+    tb_frac_nsec = (u32)(tmp - (u64)tb_time_nsec * (u64)tb_frac_limit);
+    
+    printk("busfreq = %u\n", freq);
+    printk("tb_ticks_per_jiffy = %u\ntb_to_us = %u\n",
tb_ticks_per_jiffy, tb_to_us);
+    printk("tick_freq = %u\ntime_nsec = %u\ntb_frac_nsec = %u\n",
tick_freq, tb_time_nsec, tb_frac_nsec);
+}
+
+unsigned long get_ns_delta(void)
 {
-	bd_t *binfo = (bd_t *)__res;
-	int freq, divisor;
+    static uint32_t frac = 0;
 
-	freq = binfo->bi_busfreq;
-        divisor = 4;
-        tb_ticks_per_jiffy = freq / HZ / divisor;
-	tb_to_us = mulhwu_scale_factor(freq / divisor, 1000000);
+    unsigned long delta_ns;
+
+    delta_ns = tb_time_nsec;
+
+    /* adjust for inaccuracy in calculation */
+    frac += tb_frac_nsec;
+    if (frac >= tb_frac_limit)
+    {
+        delta_ns += frac / tb_frac_limit;
+        frac %= tb_frac_limit;
+    }
+    
+    return delta_ns;
 }
 
 /* The 8260 has an internal 1-second timer update register that

Rune Torgersen
System Developer
Innovative Systems LLC
1000 Innovative Drive
Mitchell, SD 57301
Ph: 605-995-6120
www.innovsys.com

[-- Attachment #2: WallClock-fix --]
[-- Type: application/octet-stream, Size: 3451 bytes --]

Index: arch/ppc/kernel/time.c
===================================================================
--- arch/ppc/kernel/time.c	(revision 20)
+++ arch/ppc/kernel/time.c	(working copy)
@@ -119,6 +119,11 @@
 EXPORT_SYMBOL(profile_pc);
 #endif
 
+
+#ifdef CONFIG_8260
+extern unsigned long get_ns_delta(void); /* arch/ppc/syslib/m8260_setup.c */
+extern unsigned long tick_nsec;          /* kernel/time.c */
+#endif
 /*
  * timer_interrupt - gets called when the decrementer overflows,
  * with interrupts disabled.
@@ -148,6 +153,10 @@
 		/* We are in an interrupt, no need to save/restore flags */
 		write_seqlock(&xtime_lock);
 		tb_last_stamp = jiffy_stamp;
+
+#ifdef CONFIG_8260		
+		tick_nsec = get_ns_delta();
+#endif		
 		do_timer(regs);
 
 		/*
@@ -179,7 +188,7 @@
 		write_sequnlock(&xtime_lock);
 	}
 	if ( !disarm_decr[smp_processor_id()] )
-		set_dec(next_dec);
+		set_dec(next_dec-1);
 	last_jiffy_stamp(cpu) = jiffy_stamp;
 
 	if (ppc_md.heartbeat && !ppc_md.heartbeat_count--)
Index: arch/ppc/syslib/m8260_setup.c
===================================================================
--- arch/ppc/syslib/m8260_setup.c	(revision 20)
+++ arch/ppc/syslib/m8260_setup.c	(working copy)
@@ -31,6 +31,8 @@
 
 #include "cpm2_pic.h"
 
+#include <linux/module.h>    
+
 unsigned char __res[sizeof(bd_t)];
 
 extern void cpm2_reset(void);
@@ -82,16 +84,63 @@
 /* The decrementer counts at the system (internal) clock frequency
  * divided by four.
  */
+unsigned long tb_time_nsec;
+unsigned long tb_frac_nsec;
+unsigned long tb_frac_limit;     
+ 
+extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
+ 
 static void __init
 m8260_calibrate_decr(void)
+{    
+    bd_t *binfo = (bd_t *)__res;
+    int freq, divisor, tick_freq;
+    uint64_t tmp;
+    printk("Calibrating Decrementer\n");
+
+    freq = binfo->bi_busfreq;
+    divisor = 4;
+    
+    /* get rounded off decrementer frequency */
+    tick_freq = (freq + 3) / 4;
+    
+    /* get number of timebase ticks per jiffy (timerint) */
+    tb_ticks_per_jiffy = (tick_freq + (HZ/2)) / HZ;
+    tb_to_us = mulhwu_scale_factor(tick_freq, 1000000);
+
+    /* calculate timer interrupt interval in nano seconds */
+    tmp = 1000000000ULL * (u64)tb_ticks_per_jiffy;
+    __div64_32(&tmp, tick_freq);
+    tb_time_nsec = tmp;
+    
+    /* calculate fractions of nanosecons per timer interrupt */
+    tb_frac_limit = 100000UL;
+    tmp = 1000000000ULL * (u64)tb_ticks_per_jiffy * tb_frac_limit;
+    __div64_32(&tmp, tick_freq);
+    tb_frac_nsec = (u32)(tmp - (u64)tb_time_nsec * (u64)tb_frac_limit);
+    
+    printk("busfreq = %u\n", freq);
+    printk("tb_ticks_per_jiffy = %u\ntb_to_us = %u\n", tb_ticks_per_jiffy, tb_to_us);
+    printk("tick_freq = %u\ntime_nsec = %u\ntb_frac_nsec = %u\n", tick_freq, tb_time_nsec, tb_frac_nsec);
+}
+
+unsigned long get_ns_delta(void)
 {
-	bd_t *binfo = (bd_t *)__res;
-	int freq, divisor;
+    static uint32_t frac = 0;
 
-	freq = binfo->bi_busfreq;
-        divisor = 4;
-        tb_ticks_per_jiffy = freq / HZ / divisor;
-	tb_to_us = mulhwu_scale_factor(freq / divisor, 1000000);
+    unsigned long delta_ns;
+
+    delta_ns = tb_time_nsec;
+
+    /* adjust for inaccuracy in calculation */
+    frac += tb_frac_nsec;
+    if (frac >= tb_frac_limit)
+    {
+        delta_ns += frac / tb_frac_limit;
+        frac %= tb_frac_limit;
+    }
+    
+    return delta_ns;
 }
 
 /* The 8260 has an internal 1-second timer update register that

Re: Wall clock accuracy

[Eugene Surovegin]

On Tue, Aug 09, 2005 at 10:23:19AM -0500, Rune Torgersen wrote:
> Hi
> 
> I have discovered that the accuracy of the wallclock (xtime) on ppc is
> not very good.
> I am running a custom telco board based on a 8266, and the main busclock
> is derived off of a T1 reference clock.
> I was noticing a huge number of logentries fron OpenNTPD about
> adjustiing the clock, so I started to check.
> The drift of the walltime was a little over 7 seconds in 15 hours (7
> seconds slow) (equals about 130us per s)

Hmm, if I'm correct this clock drift (130ppm) should be handled easily 
by NTPD without stepping clock but with slewing. This is why NTPD 
exists in the first place, so I don't see any reason to change 
the kernel.

It's not small drift (I usually have clock accuracy within +-50ppm), 
but still is much less than maximum 1024ppm NTPD can deal with.

Maybe it's an OpenNTPD problem? I use original NTPD (ntp.org) which 
handles such drifts quite well.

-- 
Eugene

RE: Wall clock accuracy

[Rune Torgersen]

> -----Original Message-----
> From: Eugene Surovegin [mailto:ebs@ebshome.net]=20
> Sent: Tuesday, August 09, 2005 11:41
> Hmm, if I'm correct this clock drift (130ppm) should be=20
> handled easily=20
> by NTPD without stepping clock but with slewing. This is why NTPD=20
> exists in the first place, so I don't see any reason to change=20
> the kernel.

NTPD probably handles this correct, but I would like the time to be
correct anyways. In our case we might not always have access to a ntpd
server, and our input clock is very accurate to begin with.

Re: Wall clock accuracy

[Eugene Surovegin]

On Tue, Aug 09, 2005 at 11:57:04AM -0500, Rune Torgersen wrote:
> > -----Original Message-----
> > From: Eugene Surovegin [mailto:ebs@ebshome.net] 
> > Sent: Tuesday, August 09, 2005 11:41
> > Hmm, if I'm correct this clock drift (130ppm) should be 
> > handled easily 
> > by NTPD without stepping clock but with slewing. This is why NTPD 
> > exists in the first place, so I don't see any reason to change 
> > the kernel.
> 
> NTPD probably handles this correct, but I would like the time to be
> correct anyways. In our case we might not always have access to a ntpd
> server, and our input clock is very accurate to begin with.

Well, NTPD doesn't mean you need to have network connectivity, IIRC if 
you have exact frequency source, you can add it to NTPD and it'll use 
it to correct drift.

-- 
Eugene

Re: Wall clock accuracy

[Eugene Surovegin]

On Tue, Aug 09, 2005 at 09:59:57AM -0700, Eugene Surovegin wrote:
> On Tue, Aug 09, 2005 at 11:57:04AM -0500, Rune Torgersen wrote:
> > > -----Original Message-----
> > > From: Eugene Surovegin [mailto:ebs@ebshome.net] 
> > > Sent: Tuesday, August 09, 2005 11:41
> > > Hmm, if I'm correct this clock drift (130ppm) should be 
> > > handled easily 
> > > by NTPD without stepping clock but with slewing. This is why NTPD 
> > > exists in the first place, so I don't see any reason to change 
> > > the kernel.
> > 
> > NTPD probably handles this correct, but I would like the time to be
> > correct anyways. In our case we might not always have access to a ntpd
> > server, and our input clock is very accurate to begin with.
> 
> Well, NTPD doesn't mean you need to have network connectivity, IIRC if 
> you have exact frequency source, you can add it to NTPD and it'll use 
> it to correct drift.

To rephrase this - Linux kernel already has infrastructure for 
precision time keeping, just use it (even without NTPD) and don't add 
any CPU-specific hacks :).

-- 
Eugene

RE: Wall clock accuracy

[Rune Torgersen]

Sorry for double post...
=20

> -----Original Message-----
> From: Eugene Surovegin [mailto:ebs@ebshome.net]=20
> Sent: Tuesday, August 09, 2005 11:41
> It's not small drift (I usually have clock accuracy within +-50ppm),=20
> but still is much less than maximum 1024ppm NTPD can deal with.

My bigger problem with the walltime is that time_nsec should have been
1000000, not 999848 to begin with. If it had been, I would probably
never even have noticed it.

The thing is that that value is set based on a value I cannot find any
reference as how was chosen. Looks to be a leftover when porting PPC
fron i386 once upon a time.

time_nsec basically gets is value (via some macros) from
CLOCK_TICK_RATE, which is defined in asm-ppc/timex.h to be 1193180

In my opinion, time_nsec should have been calculated based on the actual
clock input rate to begin with (like in calibrate_decrementer()).

Re: Wall clock accuracy

[Eugene Surovegin]

On Tue, Aug 09, 2005 at 12:04:10PM -0500, Rune Torgersen wrote:
> 
> My bigger problem with the walltime is that time_nsec should have been
> 1000000, not 999848 to begin with. If it had been, I would probably
> never even have noticed it.
> 
> The thing is that that value is set based on a value I cannot find any
> reference as how was chosen. Looks to be a leftover when porting PPC
> fron i386 once upon a time.
> 
> time_nsec basically gets is value (via some macros) from
> CLOCK_TICK_RATE, which is defined in asm-ppc/timex.h to be 1193180
> 
> In my opinion, time_nsec should have been calculated based on the actual
> clock input rate to begin with (like in calibrate_decrementer()).

Fair enough, why then instead of fixing the root cause you are making 
ugly workarounds :) ?

-- 
Eugene

RE: Wall clock accuracy

[Rune Torgersen]

=20

> -----Original Message-----
> From: Eugene Surovegin [mailto:ebs@ebshome.net]=20
> Sent: Tuesday, August 09, 2005 12:21

> Fair enough, why then instead of fixing the root cause you are making=20
> ugly workarounds :) ?

Probably because I did not want to fiddle with the code that did the
original calculation, and I wanted to see if I could get it totally
accurate (for the case where there is not a whole number of ns in a
jiffy).

A proper fix would be for platfrom code to have a way to set time_nsec
instead of having it set by a constant.

I did say in my original email that thencluded code was not a proper
patch but more something to show on how a fix could look like, and to
start a discussion about it.

Re: Wall clock accuracy

[Eugene Surovegin]

On Tue, Aug 09, 2005 at 01:21:09PM -0500, Rune Torgersen wrote:
> 
> A proper fix would be for platfrom code to have a way to set time_nsec
> instead of having it set by a constant.

Or configure CLOCK_TICK_RATE in board/CPU specific fashion, e.g. like 
ARM does.

-- 
Eugene

RE: Wall clock accuracy

[Rune Torgersen]

=20

> -----Original Message-----
> From: Eugene Surovegin [mailto:ebs@ebshome.net]=20
> Sent: Tuesday, August 09, 2005 13:35
> To: Rune Torgersen
> Cc: linuxppc-embedded@ozlabs.org
> Subject: Re: Wall clock accuracy
>=20
> On Tue, Aug 09, 2005 at 01:21:09PM -0500, Rune Torgersen wrote:
> >=20
> > A proper fix would be for platfrom code to have a way to=20
> set time_nsec
> > instead of having it set by a constant.
>=20
> Or configure CLOCK_TICK_RATE in board/CPU specific fashion, e.g. like=20
> ARM does.

Then it would have to be a constant, and it would be much better to just
use the clockrate given to the kernel than have to hardcode it into the
kernel.

Re: Wall clock accuracy

[Eugene Surovegin]

On Tue, Aug 09, 2005 at 01:52:33PM -0500, Rune Torgersen wrote:
>  
> 
> > -----Original Message-----
> > From: Eugene Surovegin [mailto:ebs@ebshome.net] 
> > Sent: Tuesday, August 09, 2005 13:35
> > To: Rune Torgersen
> > Cc: linuxppc-embedded@ozlabs.org
> > Subject: Re: Wall clock accuracy
> > 
> > On Tue, Aug 09, 2005 at 01:21:09PM -0500, Rune Torgersen wrote:
> > > 
> > > A proper fix would be for platfrom code to have a way to 
> > set time_nsec
> > > instead of having it set by a constant.
> > 
> > Or configure CLOCK_TICK_RATE in board/CPU specific fashion, e.g. like 
> > ARM does.
> 
> Then it would have to be a constant, 

Why?

> and it would be much better to just
> use the clockrate given to the kernel than have to hardcode it into the
> kernel.

Nothing prevents you from defining this macro as a function call for 
example. If this is needed for your CPU/board port. Probably common
case will be a constant, though.

-- 
Eugene

RE: Wall clock accuracy

[Rune Torgersen]

> -----Original Message-----
> From: Eugene Surovegin [mailto:ebs@ebshome.net]=20
> Sent: Tuesday, August 09, 2005 13:57
> > and it would be much better to just
> > use the clockrate given to the kernel than have to hardcode=20
> it into the
> > kernel.
>=20
> Nothing prevents you from defining this macro as a function call for=20
> example. If this is needed for your CPU/board port. Probably common
> case will be a constant, though.

Problem is that time_nsec is not directly set to CLOCK_TICK_RATE but
goes through some calculations I'd rather not have to lots of times in
runtime (maybe I'm not seeing how it could be done)

Anyways, the #define chain leading to time_nsec is:

unsigned long tick_nsec =3D TICK_NSEC;		/* ACTHZ period (nsec)
*/	kernel/time.c:562

#define CLOCK_TICK_RATE	1193180 /* Underlying HZ */
asm-ppc/timex.h:14

Include/linux/jiffies.h:45 and out
/* LATCH is used in the interval timer and ftape setup. */
#define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */

/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, the we
can
 * improve accuracy by shifting LSH bits, hence calculating:
 *     (NOM << LSH) / DEN
 * This however means trouble for large NOM, because (NOM << LSH) may no
 * longer fit in 32 bits. The following way of calculating this gives us
 * some slack, under the following conditions:
 *   - (NOM / DEN) fits in (32 - LSH) bits.
 *   - (NOM % DEN) fits in (32 - LSH) bits.
 */
#define SH_DIV(NOM,DEN,LSH) (   ((NOM / DEN) << LSH)
\
                             + (((NOM % DEN) << LSH) + DEN / 2) / DEN)

/* HZ is the requested value. ACTHZ is actual HZ ("<< 8" is for
accuracy) */
#define ACTHZ (SH_DIV (CLOCK_TICK_RATE, LATCH, 8))

/* TICK_NSEC is the time between ticks in nsec assuming real ACTHZ */
#define TICK_NSEC (SH_DIV (1000000UL * 1000, ACTHZ, 8))


I cannot se a clean easy way of changing time_nsec or TIME_NSEC runtime
without messing with code that probably should not be messed with.
Of course I am probably wrong.