Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 27 Feb 2023 22:12:22 GMT
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 1e48d9d336c0 - main - pps: Simplify the nsec calculation in pps_event()
Message-ID:  <202302272212.31RMCMBg028856@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=1e48d9d336c042a44edc0f66d35b72655f68f258

commit 1e48d9d336c042a44edc0f66d35b72655f68f258
Author:     Sebastian Huber <sebastian.huber@embedded-brains.de>
AuthorDate: 2023-02-27 21:49:09 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-02-27 22:10:55 +0000

    pps: Simplify the nsec calculation in pps_event()
    
    Let A be the current calculation of the frequency accumulator (pps_fcount)
    update in pps_event()
    
      scale = (uint64_t)1 << 63;
      scale /= captc->tc_frequency;
      scale *= 2;
      bt.sec = 0;
      bt.frac = 0;
      bintime_addx(&bt, scale * tcount);
      bintime2timespec(&bt, &ts);
      hardpps(tsp, ts.tv_nsec + 1000000000 * ts.tv_sec);
    
    and hardpps(..., delta_nsec):
    
      u_nsec = delta_nsec;
      if (u_nsec > (NANOSECOND >> 1))
              u_nsec -= NANOSECOND;
      else if (u_nsec < -(NANOSECOND >> 1))
              u_nsec += NANOSECOND;
      pps_fcount += u_nsec;
    
    This change introduces a new calculation which is slightly simpler and more
    straight forward.  Name it B.
    
    Consider the following sample values with a tcount of 2000000100 and a
    tc_frequency of 2000000000 (2GHz).
    
    For A, the scale is 9223372036.  Then scale * tcount is 18446744994337203600
    which is larger than UINT64_MAX (= 18446744073709551615).  The result is
    920627651984 == 18446744994337203600 % UINT64_MAX.  Since all operands are
    unsigned the result is well defined through modulo arithmetic.  The result of
    bintime2timespec(&bt, &ts) is 49.  This is equal to the correct result
    1000000049 % NANOSECOND.
    
    In hardpps(), both conditional statements are not executed and pps_fcount is
    incremented by 49.
    
    For the new calculation B, we have 1000000000 * tcount is 2000000100000000000
    which is less than UINT64_MAX. This yields after the division with tc_frequency
    the correct result of 1000000050 for delta_nsec.
    
    In hardpps(), the first conditional statement is executed and pps_fcount is
    incremented by 50.
    
    This shows that both methods yield roughly the same results.  However, method B
    is easier to understand and requires fewer conditional statements.
    
    Reviewed by: imp
    Pull Request: https://github.com/freebsd/freebsd-src/pull/604
---
 sys/kern/kern_ntptime.c | 12 +++---------
 sys/kern/kern_tc.c      | 16 ++++++----------
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/sys/kern/kern_ntptime.c b/sys/kern/kern_ntptime.c
index b632cecdfadb..815a26182096 100644
--- a/sys/kern/kern_ntptime.c
+++ b/sys/kern/kern_ntptime.c
@@ -771,16 +771,10 @@ hardpps(struct timespec *tsp, long delta_nsec)
 	pps_tf[0].tv_nsec = u_nsec;
 
 	/*
-	 * Compute the difference between the current and previous
-	 * counter values. If the difference exceeds 0.5 s, assume it
-	 * has wrapped around, so correct 1.0 s.
+	 * Update the frequency accumulator using the difference between the
+	 * current and previous PPS event measured directly by the timecounter.
 	 */
-	u_nsec = delta_nsec;
-	if (u_nsec > (NANOSECOND >> 1))
-		u_nsec -= NANOSECOND;
-	else if (u_nsec < -(NANOSECOND >> 1))
-		u_nsec += NANOSECOND;
-	pps_fcount += u_nsec;
+	pps_fcount += delta_nsec - NANOSECOND;
 	if (v_nsec > MAXFREQ || v_nsec < -MAXFREQ)
 		goto out;
 	time_status &= ~STA_PPSJITTER;
diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c
index be564e4347f8..0dc233896baa 100644
--- a/sys/kern/kern_tc.c
+++ b/sys/kern/kern_tc.c
@@ -1781,7 +1781,7 @@ pps_event(struct pps_state *pps, int event)
 	struct timecounter *captc;
 	uint64_t capth_scale;
 	struct bintime bt;
-	struct timespec ts, *tsp, *osp;
+	struct timespec *tsp, *osp;
 	u_int tcount, *pcount;
 	int foff;
 	pps_seq_t *pseq;
@@ -1881,7 +1881,7 @@ pps_event(struct pps_state *pps, int event)
 
 #ifdef PPS_SYNC
 	if (fhard) {
-		uint64_t scale;
+		uint64_t delta_nsec;
 
 		/*
 		 * Feed the NTP PLL/FLL.
@@ -1891,14 +1891,10 @@ pps_event(struct pps_state *pps, int event)
 		tcount = pps->capcount - pps->ppscount[2];
 		pps->ppscount[2] = pps->capcount;
 		tcount &= captc->tc_counter_mask;
-		scale = (uint64_t)1 << 63;
-		scale /= captc->tc_frequency;
-		scale *= 2;
-		bt.sec = 0;
-		bt.frac = 0;
-		bintime_addx(&bt, scale * tcount);
-		bintime2timespec(&bt, &ts);
-		hardpps(tsp, ts.tv_nsec + 1000000000 * ts.tv_sec);
+		delta_nsec = 1000000000;
+		delta_nsec *= tcount;
+		delta_nsec /= captc->tc_frequency;
+		hardpps(tsp, (long)delta_nsec);
 	}
 #endif
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202302272212.31RMCMBg028856>