netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 00/11] Time Counter fixes and improvements
@ 2014-12-21 18:46 Richard Cochran
  2014-12-21 18:46 ` [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file Richard Cochran
                   ` (12 more replies)
  0 siblings, 13 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:46 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

Several PTP Hardware Clock (PHC) drivers implement the clock in
software using the timecounter/cyclecounter code. This series adds one
simple improvement and one more subtle fix to the shared timecounter
facility. Credit for this series goes to Janusz Użycki, who pointed
the issues out to me off list.

Patch #1 simply move the timecounter code into its own file. When
working on this series, it was really annoying to see half the kernel
recompile after every tweak to the timecounter stuff. There is no
reason to keep this together with the clocksource code.

Patch #2 implements an improved adjtime() method, and patches 3-10
convert all of the drivers over to the new method.

Patch #11 fixes a subtle but important issue with the timecounter WRT
frequency adjustment. As it stands now, a timecounter based PHC will
exhibit a variable frequency resolution (and variable time error)
depending on how often the clock is read.

In timecounter_read_delta(), the expression

   (delta * cc->mult) >> cc->shift;

can lose resolution from the adjusted value of 'mult'. If the value
of 'delta' is too small, then small changes in 'mult' have no effect.
However, if the delta value is large enough, then small changes in
'mult' will have an effect.

Reading the clock too often means smaller 'delta' values which in turn
will spoil the fine adjustments made to 'mult'. Up until now, this
effect did not show up in my testing. The following example explains
why.

The CPTS has an input clock of 250 MHz, and the clock source uses
mult=0x80000000 and shift=29, making the ticks to nanoseconds
conversion like this:

   ticks * 2^31
   ------------
       2^29

Imagine what happens if the clock is read every 10 milliseconds. Ten
milliseconds are about 2500000 ticks, which corresponds to about 21
bits. The product in the numerator has then 52 bits. After the shift
operation, 23 bits are preserved. This results in a frequency
adjustment resolution of about 0.1 ppm (not _too_ bad.)

A frequency resolution of 1 ppm requires 20 bits.
A frequency resolution of 1 ppb requires 30 bits.

For the 250 MHz CPTS clock, reading every 4 seconds yields a 1 ppb
resolution (which is the finest that our API allows).

However, the error can be much higher if the clock is read too often
or if time stamps occur close in time to read operations. In general
it is really not acceptable to allow the rate of clock readings to
influence the clock accuracy.

Thanks,
Richard

Richard Cochran (11):
  time: move the timecounter/cyclecounter code into its own file.
  timecounter: provide a helper function to shift the time.
  net: xgbe: convert to timecounter adjtime.
  net: bnx2x: convert to timecounter adjtime.
  net: fec: convert to timecounter adjtime.
  net: e1000e: convert to timecounter adjtime.
  net: igb: convert to timecounter adjtime.
  net: ixgbe: convert to timecounter adjtime.
  net: mlx4: convert to timecounter adjtime.
  net: cpts: convert to timecounter adjtime.
  timecounter: keep track of accumulated fractional nanoseconds

 drivers/net/ethernet/amd/xgbe/xgbe-ptp.c         |    8 +-
 drivers/net/ethernet/amd/xgbe/xgbe.h             |    2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h      |    2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    6 +-
 drivers/net/ethernet/freescale/fec.h             |    1 +
 drivers/net/ethernet/freescale/fec_ptp.c         |   16 +--
 drivers/net/ethernet/intel/e1000e/e1000.h        |    2 +-
 drivers/net/ethernet/intel/e1000e/ptp.c          |    5 +-
 drivers/net/ethernet/intel/igb/igb.h             |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c         |    7 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h         |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c     |   11 +-
 drivers/net/ethernet/mellanox/mlx4/en_clock.c    |    9 +-
 drivers/net/ethernet/ti/cpts.c                   |    5 +-
 drivers/net/ethernet/ti/cpts.h                   |    1 +
 include/clocksource/arm_arch_timer.h             |    2 +-
 include/linux/clocksource.h                      |  102 ----------------
 include/linux/mlx4/device.h                      |    2 +-
 include/linux/timecounter.h                      |  136 ++++++++++++++++++++++
 include/linux/types.h                            |    3 +
 kernel/time/Makefile                             |    2 +-
 kernel/time/clocksource.c                        |   76 ------------
 kernel/time/timecounter.c                        |  112 ++++++++++++++++++
 sound/pci/hda/hda_priv.h                         |    2 +-
 virt/kvm/arm/arch_timer.c                        |    3 +-
 25 files changed, 274 insertions(+), 245 deletions(-)
 create mode 100644 include/linux/timecounter.h
 create mode 100644 kernel/time/timecounter.c

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
@ 2014-12-21 18:46 ` Richard Cochran
  2014-12-23 21:03   ` Jeff Kirsher
  2014-12-21 18:46 ` [PATCH net-next 02/11] timecounter: provide a helper function to shift the time Richard Cochran
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:46 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

The timecounter code has almost nothing to do with the clocksource
code. Let it live in its own file. This will help isolate the
timecounter users from the clocksource users in the source tree.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe.h        |    2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h |    2 +-
 drivers/net/ethernet/freescale/fec.h        |    1 +
 drivers/net/ethernet/intel/e1000e/e1000.h   |    2 +-
 drivers/net/ethernet/intel/igb/igb.h        |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h    |    2 +-
 drivers/net/ethernet/ti/cpts.h              |    1 +
 include/clocksource/arm_arch_timer.h        |    2 +-
 include/linux/clocksource.h                 |  102 ----------------------
 include/linux/mlx4/device.h                 |    2 +-
 include/linux/timecounter.h                 |  122 +++++++++++++++++++++++++++
 include/linux/types.h                       |    3 +
 kernel/time/Makefile                        |    2 +-
 kernel/time/clocksource.c                   |   76 -----------------
 kernel/time/timecounter.c                   |   95 +++++++++++++++++++++
 sound/pci/hda/hda_priv.h                    |    2 +-
 16 files changed, 231 insertions(+), 187 deletions(-)
 create mode 100644 include/linux/timecounter.h
 create mode 100644 kernel/time/timecounter.c

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index f9ec762..2af6aff 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -124,7 +124,7 @@
 #include <linux/if_vlan.h>
 #include <linux/bitops.h>
 #include <linux/ptp_clock_kernel.h>
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 #include <linux/net_tstamp.h>
 #include <net/dcbnl.h>
 
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index c3a6072..792ba72 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -22,7 +22,7 @@
 
 #include <linux/ptp_clock_kernel.h>
 #include <linux/net_tstamp.h>
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 
 /* compilation time flags */
 
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 469691a..df8bbdd 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -16,6 +16,7 @@
 #include <linux/clocksource.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
+#include <linux/timecounter.h>
 
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
     defined(CONFIG_M520x) || defined(CONFIG_M532x) || \
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 7785240..9416e5a 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -34,7 +34,7 @@
 #include <linux/pci-aspm.h>
 #include <linux/crc32.h>
 #include <linux/if_vlan.h>
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/ptp_classify.h>
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 82d891e..ee22da3 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -29,7 +29,7 @@
 #include "e1000_mac.h"
 #include "e1000_82575.h"
 
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/bitops.h>
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index b6137be..38fc64c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -38,7 +38,7 @@
 #include <linux/if_vlan.h>
 #include <linux/jiffies.h>
 
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
 
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index 1a581ef..69a46b9 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -27,6 +27,7 @@
 #include <linux/list.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/skbuff.h>
+#include <linux/timecounter.h>
 
 struct cpsw_cpts {
 	u32 idver;                /* Identification and version */
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 6d26b40..9916d0e 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -16,7 +16,7 @@
 #ifndef __CLKSOURCE_ARM_ARCH_TIMER_H
 #define __CLKSOURCE_ARM_ARCH_TIMER_H
 
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 #include <linux/types.h>
 
 #define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index abcafaa..9c78d15 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -18,8 +18,6 @@
 #include <asm/div64.h>
 #include <asm/io.h>
 
-/* clocksource cycle base type */
-typedef u64 cycle_t;
 struct clocksource;
 struct module;
 
@@ -28,106 +26,6 @@ struct module;
 #endif
 
 /**
- * struct cyclecounter - hardware abstraction for a free running counter
- *	Provides completely state-free accessors to the underlying hardware.
- *	Depending on which hardware it reads, the cycle counter may wrap
- *	around quickly. Locking rules (if necessary) have to be defined
- *	by the implementor and user of specific instances of this API.
- *
- * @read:		returns the current cycle value
- * @mask:		bitmask for two's complement
- *			subtraction of non 64 bit counters,
- *			see CLOCKSOURCE_MASK() helper macro
- * @mult:		cycle to nanosecond multiplier
- * @shift:		cycle to nanosecond divisor (power of two)
- */
-struct cyclecounter {
-	cycle_t (*read)(const struct cyclecounter *cc);
-	cycle_t mask;
-	u32 mult;
-	u32 shift;
-};
-
-/**
- * struct timecounter - layer above a %struct cyclecounter which counts nanoseconds
- *	Contains the state needed by timecounter_read() to detect
- *	cycle counter wrap around. Initialize with
- *	timecounter_init(). Also used to convert cycle counts into the
- *	corresponding nanosecond counts with timecounter_cyc2time(). Users
- *	of this code are responsible for initializing the underlying
- *	cycle counter hardware, locking issues and reading the time
- *	more often than the cycle counter wraps around. The nanosecond
- *	counter will only wrap around after ~585 years.
- *
- * @cc:			the cycle counter used by this instance
- * @cycle_last:		most recent cycle counter value seen by
- *			timecounter_read()
- * @nsec:		continuously increasing count
- */
-struct timecounter {
-	const struct cyclecounter *cc;
-	cycle_t cycle_last;
-	u64 nsec;
-};
-
-/**
- * cyclecounter_cyc2ns - converts cycle counter cycles to nanoseconds
- * @cc:		Pointer to cycle counter.
- * @cycles:	Cycles
- *
- * XXX - This could use some mult_lxl_ll() asm optimization. Same code
- * as in cyc2ns, but with unsigned result.
- */
-static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
-				      cycle_t cycles)
-{
-	u64 ret = (u64)cycles;
-	ret = (ret * cc->mult) >> cc->shift;
-	return ret;
-}
-
-/**
- * timecounter_init - initialize a time counter
- * @tc:			Pointer to time counter which is to be initialized/reset
- * @cc:			A cycle counter, ready to be used.
- * @start_tstamp:	Arbitrary initial time stamp.
- *
- * After this call the current cycle register (roughly) corresponds to
- * the initial time stamp. Every call to timecounter_read() increments
- * the time stamp counter by the number of elapsed nanoseconds.
- */
-extern void timecounter_init(struct timecounter *tc,
-			     const struct cyclecounter *cc,
-			     u64 start_tstamp);
-
-/**
- * timecounter_read - return nanoseconds elapsed since timecounter_init()
- *                    plus the initial time stamp
- * @tc:          Pointer to time counter.
- *
- * In other words, keeps track of time since the same epoch as
- * the function which generated the initial time stamp.
- */
-extern u64 timecounter_read(struct timecounter *tc);
-
-/**
- * timecounter_cyc2time - convert a cycle counter to same
- *                        time base as values returned by
- *                        timecounter_read()
- * @tc:		Pointer to time counter.
- * @cycle_tstamp:	a value returned by tc->cc->read()
- *
- * Cycle counts that are converted correctly as long as they
- * fall into the interval [-1/2 max cycle count, +1/2 max cycle count],
- * with "max cycle count" == cs->mask+1.
- *
- * This allows conversion of cycle counter values which were generated
- * in the past.
- */
-extern u64 timecounter_cyc2time(struct timecounter *tc,
-				cycle_t cycle_tstamp);
-
-/**
  * struct clocksource - hardware abstraction for a free running counter
  *	Provides mostly state-free accessors to the underlying hardware.
  *	This is the structure used for system time.
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 25c791e..f1e41b3 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -42,7 +42,7 @@
 
 #include <linux/atomic.h>
 
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 
 #define MAX_MSIX_P_PORT		17
 #define MAX_MSIX		64
diff --git a/include/linux/timecounter.h b/include/linux/timecounter.h
new file mode 100644
index 0000000..146f07a
--- /dev/null
+++ b/include/linux/timecounter.h
@@ -0,0 +1,122 @@
+/*
+ * linux/include/linux/timecounter.h
+ *
+ * based on code that migrated away from
+ * linux/include/linux/clocksource.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef _LINUX_TIMECOUNTER_H
+#define _LINUX_TIMECOUNTER_H
+
+#include <linux/types.h>
+
+/**
+ * struct cyclecounter - hardware abstraction for a free running counter
+ *	Provides completely state-free accessors to the underlying hardware.
+ *	Depending on which hardware it reads, the cycle counter may wrap
+ *	around quickly. Locking rules (if necessary) have to be defined
+ *	by the implementor and user of specific instances of this API.
+ *
+ * @read:		returns the current cycle value
+ * @mask:		bitmask for two's complement
+ *			subtraction of non 64 bit counters,
+ *			see CLOCKSOURCE_MASK() helper macro
+ * @mult:		cycle to nanosecond multiplier
+ * @shift:		cycle to nanosecond divisor (power of two)
+ */
+struct cyclecounter {
+	cycle_t (*read)(const struct cyclecounter *cc);
+	cycle_t mask;
+	u32 mult;
+	u32 shift;
+};
+
+/**
+ * struct timecounter - layer above a %struct cyclecounter which counts nanoseconds
+ *	Contains the state needed by timecounter_read() to detect
+ *	cycle counter wrap around. Initialize with
+ *	timecounter_init(). Also used to convert cycle counts into the
+ *	corresponding nanosecond counts with timecounter_cyc2time(). Users
+ *	of this code are responsible for initializing the underlying
+ *	cycle counter hardware, locking issues and reading the time
+ *	more often than the cycle counter wraps around. The nanosecond
+ *	counter will only wrap around after ~585 years.
+ *
+ * @cc:			the cycle counter used by this instance
+ * @cycle_last:		most recent cycle counter value seen by
+ *			timecounter_read()
+ * @nsec:		continuously increasing count
+ */
+struct timecounter {
+	const struct cyclecounter *cc;
+	cycle_t cycle_last;
+	u64 nsec;
+};
+
+/**
+ * cyclecounter_cyc2ns - converts cycle counter cycles to nanoseconds
+ * @cc:		Pointer to cycle counter.
+ * @cycles:	Cycles
+ *
+ * XXX - This could use some mult_lxl_ll() asm optimization. Same code
+ * as in cyc2ns, but with unsigned result.
+ */
+static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
+				      cycle_t cycles)
+{
+	u64 ret = (u64)cycles;
+	ret = (ret * cc->mult) >> cc->shift;
+	return ret;
+}
+
+/**
+ * timecounter_init - initialize a time counter
+ * @tc:			Pointer to time counter which is to be initialized/reset
+ * @cc:			A cycle counter, ready to be used.
+ * @start_tstamp:	Arbitrary initial time stamp.
+ *
+ * After this call the current cycle register (roughly) corresponds to
+ * the initial time stamp. Every call to timecounter_read() increments
+ * the time stamp counter by the number of elapsed nanoseconds.
+ */
+extern void timecounter_init(struct timecounter *tc,
+			     const struct cyclecounter *cc,
+			     u64 start_tstamp);
+
+/**
+ * timecounter_read - return nanoseconds elapsed since timecounter_init()
+ *                    plus the initial time stamp
+ * @tc:          Pointer to time counter.
+ *
+ * In other words, keeps track of time since the same epoch as
+ * the function which generated the initial time stamp.
+ */
+extern u64 timecounter_read(struct timecounter *tc);
+
+/**
+ * timecounter_cyc2time - convert a cycle counter to same
+ *                        time base as values returned by
+ *                        timecounter_read()
+ * @tc:		Pointer to time counter.
+ * @cycle_tstamp:	a value returned by tc->cc->read()
+ *
+ * Cycle counts that are converted correctly as long as they
+ * fall into the interval [-1/2 max cycle count, +1/2 max cycle count],
+ * with "max cycle count" == cs->mask+1.
+ *
+ * This allows conversion of cycle counter values which were generated
+ * in the past.
+ */
+extern u64 timecounter_cyc2time(struct timecounter *tc,
+				cycle_t cycle_tstamp);
+
+#endif
diff --git a/include/linux/types.h b/include/linux/types.h
index a0bb704..6232382 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -213,5 +213,8 @@ struct callback_head {
 };
 #define rcu_head callback_head
 
+/* clocksource cycle base type */
+typedef u64 cycle_t;
+
 #endif /*  __ASSEMBLY__ */
 #endif /* _LINUX_TYPES_H */
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index f622cf2..c09c078 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,6 +1,6 @@
 obj-y += time.o timer.o hrtimer.o itimer.o posix-timers.o posix-cpu-timers.o
 obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o
-obj-y += timeconv.o posix-clock.o alarmtimer.o
+obj-y += timeconv.o timecounter.o posix-clock.o alarmtimer.o
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)		+= clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= tick-common.o
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index b79f39b..4892352 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -34,82 +34,6 @@
 #include "tick-internal.h"
 #include "timekeeping_internal.h"
 
-void timecounter_init(struct timecounter *tc,
-		      const struct cyclecounter *cc,
-		      u64 start_tstamp)
-{
-	tc->cc = cc;
-	tc->cycle_last = cc->read(cc);
-	tc->nsec = start_tstamp;
-}
-EXPORT_SYMBOL_GPL(timecounter_init);
-
-/**
- * timecounter_read_delta - get nanoseconds since last call of this function
- * @tc:         Pointer to time counter
- *
- * When the underlying cycle counter runs over, this will be handled
- * correctly as long as it does not run over more than once between
- * calls.
- *
- * The first call to this function for a new time counter initializes
- * the time tracking and returns an undefined result.
- */
-static u64 timecounter_read_delta(struct timecounter *tc)
-{
-	cycle_t cycle_now, cycle_delta;
-	u64 ns_offset;
-
-	/* read cycle counter: */
-	cycle_now = tc->cc->read(tc->cc);
-
-	/* calculate the delta since the last timecounter_read_delta(): */
-	cycle_delta = (cycle_now - tc->cycle_last) & tc->cc->mask;
-
-	/* convert to nanoseconds: */
-	ns_offset = cyclecounter_cyc2ns(tc->cc, cycle_delta);
-
-	/* update time stamp of timecounter_read_delta() call: */
-	tc->cycle_last = cycle_now;
-
-	return ns_offset;
-}
-
-u64 timecounter_read(struct timecounter *tc)
-{
-	u64 nsec;
-
-	/* increment time by nanoseconds since last call */
-	nsec = timecounter_read_delta(tc);
-	nsec += tc->nsec;
-	tc->nsec = nsec;
-
-	return nsec;
-}
-EXPORT_SYMBOL_GPL(timecounter_read);
-
-u64 timecounter_cyc2time(struct timecounter *tc,
-			 cycle_t cycle_tstamp)
-{
-	u64 cycle_delta = (cycle_tstamp - tc->cycle_last) & tc->cc->mask;
-	u64 nsec;
-
-	/*
-	 * Instead of always treating cycle_tstamp as more recent
-	 * than tc->cycle_last, detect when it is too far in the
-	 * future and treat it as old time stamp instead.
-	 */
-	if (cycle_delta > tc->cc->mask / 2) {
-		cycle_delta = (tc->cycle_last - cycle_tstamp) & tc->cc->mask;
-		nsec = tc->nsec - cyclecounter_cyc2ns(tc->cc, cycle_delta);
-	} else {
-		nsec = cyclecounter_cyc2ns(tc->cc, cycle_delta) + tc->nsec;
-	}
-
-	return nsec;
-}
-EXPORT_SYMBOL_GPL(timecounter_cyc2time);
-
 /**
  * clocks_calc_mult_shift - calculate mult/shift factors for scaled math of clocks
  * @mult:	pointer to mult variable
diff --git a/kernel/time/timecounter.c b/kernel/time/timecounter.c
new file mode 100644
index 0000000..59a1ec3
--- /dev/null
+++ b/kernel/time/timecounter.c
@@ -0,0 +1,95 @@
+/*
+ * linux/kernel/time/timecounter.c
+ *
+ * based on code that migrated away from
+ * linux/kernel/time/clocksource.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/export.h>
+#include <linux/timecounter.h>
+
+void timecounter_init(struct timecounter *tc,
+		      const struct cyclecounter *cc,
+		      u64 start_tstamp)
+{
+	tc->cc = cc;
+	tc->cycle_last = cc->read(cc);
+	tc->nsec = start_tstamp;
+}
+EXPORT_SYMBOL_GPL(timecounter_init);
+
+/**
+ * timecounter_read_delta - get nanoseconds since last call of this function
+ * @tc:         Pointer to time counter
+ *
+ * When the underlying cycle counter runs over, this will be handled
+ * correctly as long as it does not run over more than once between
+ * calls.
+ *
+ * The first call to this function for a new time counter initializes
+ * the time tracking and returns an undefined result.
+ */
+static u64 timecounter_read_delta(struct timecounter *tc)
+{
+	cycle_t cycle_now, cycle_delta;
+	u64 ns_offset;
+
+	/* read cycle counter: */
+	cycle_now = tc->cc->read(tc->cc);
+
+	/* calculate the delta since the last timecounter_read_delta(): */
+	cycle_delta = (cycle_now - tc->cycle_last) & tc->cc->mask;
+
+	/* convert to nanoseconds: */
+	ns_offset = cyclecounter_cyc2ns(tc->cc, cycle_delta);
+
+	/* update time stamp of timecounter_read_delta() call: */
+	tc->cycle_last = cycle_now;
+
+	return ns_offset;
+}
+
+u64 timecounter_read(struct timecounter *tc)
+{
+	u64 nsec;
+
+	/* increment time by nanoseconds since last call */
+	nsec = timecounter_read_delta(tc);
+	nsec += tc->nsec;
+	tc->nsec = nsec;
+
+	return nsec;
+}
+EXPORT_SYMBOL_GPL(timecounter_read);
+
+u64 timecounter_cyc2time(struct timecounter *tc,
+			 cycle_t cycle_tstamp)
+{
+	u64 cycle_delta = (cycle_tstamp - tc->cycle_last) & tc->cc->mask;
+	u64 nsec;
+
+	/*
+	 * Instead of always treating cycle_tstamp as more recent
+	 * than tc->cycle_last, detect when it is too far in the
+	 * future and treat it as old time stamp instead.
+	 */
+	if (cycle_delta > tc->cc->mask / 2) {
+		cycle_delta = (tc->cycle_last - cycle_tstamp) & tc->cc->mask;
+		nsec = tc->nsec - cyclecounter_cyc2ns(tc->cc, cycle_delta);
+	} else {
+		nsec = cyclecounter_cyc2ns(tc->cc, cycle_delta) + tc->nsec;
+	}
+
+	return nsec;
+}
+EXPORT_SYMBOL_GPL(timecounter_cyc2time);
diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
index aa484fd..3fc7cfe 100644
--- a/sound/pci/hda/hda_priv.h
+++ b/sound/pci/hda/hda_priv.h
@@ -15,7 +15,7 @@
 #ifndef __SOUND_HDA_PRIV_H
 #define __SOUND_HDA_PRIV_H
 
-#include <linux/clocksource.h>
+#include <linux/timecounter.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 02/11] timecounter: provide a helper function to shift the time.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
  2014-12-21 18:46 ` [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file Richard Cochran
@ 2014-12-21 18:46 ` Richard Cochran
  2014-12-21 18:46 ` [PATCH net-next 03/11] net: xgbe: convert to timecounter adjtime Richard Cochran
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:46 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

Some PTP Hardware Clock drivers use a struct timecounter to represent
their clock. To adjust the time by a given offset, these drivers all
perform a two step read/write of their timecounter. However, it is
better and simpler just to adjust the offset in one step. This patch
introduces a little routine to help drivers implement the adjtime
method.

Suggested-by: Janusz Użycki <j.uzycki@elproma.com.pl>
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 include/linux/timecounter.h |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/timecounter.h b/include/linux/timecounter.h
index 146f07a..af3dfa4 100644
--- a/include/linux/timecounter.h
+++ b/include/linux/timecounter.h
@@ -79,6 +79,15 @@ static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
 }
 
 /**
+ * timecounter_adjtime - Shifts the time of the clock.
+ * @delta:	Desired change in nanoseconds.
+ */
+static inline void timecounter_adjtime(struct timecounter *tc, s64 delta)
+{
+	tc->nsec += delta;
+}
+
+/**
  * timecounter_init - initialize a time counter
  * @tc:			Pointer to time counter which is to be initialized/reset
  * @cc:			A cycle counter, ready to be used.
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 03/11] net: xgbe: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
  2014-12-21 18:46 ` [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file Richard Cochran
  2014-12-21 18:46 ` [PATCH net-next 02/11] timecounter: provide a helper function to shift the time Richard Cochran
@ 2014-12-21 18:46 ` Richard Cochran
  2014-12-21 18:46 ` [PATCH net-next 04/11] net: bnx2x: " Richard Cochran
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:46 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Compile tested only.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-ptp.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ptp.c b/drivers/net/ethernet/amd/xgbe/xgbe-ptp.c
index a1bf9d1c..f5acf4c 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ptp.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ptp.c
@@ -171,15 +171,9 @@ static int xgbe_adjtime(struct ptp_clock_info *info, s64 delta)
 						   struct xgbe_prv_data,
 						   ptp_clock_info);
 	unsigned long flags;
-	u64 nsec;
 
 	spin_lock_irqsave(&pdata->tstamp_lock, flags);
-
-	nsec = timecounter_read(&pdata->tstamp_tc);
-
-	nsec += delta;
-	timecounter_init(&pdata->tstamp_tc, &pdata->tstamp_cc, nsec);
-
+	timecounter_adjtime(&pdata->tstamp_tc, delta);
 	spin_unlock_irqrestore(&pdata->tstamp_lock, flags);
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 04/11] net: bnx2x: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (2 preceding siblings ...)
  2014-12-21 18:46 ` [PATCH net-next 03/11] net: xgbe: convert to timecounter adjtime Richard Cochran
@ 2014-12-21 18:46 ` Richard Cochran
  2014-12-21 19:01   ` Richard Cochran
  2014-12-21 18:47 ` [PATCH net-next 05/11] net: fec: " Richard Cochran
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:46 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Compile tested only.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 691f0bf..7b2cc40 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -13265,14 +13265,10 @@ static int bnx2x_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 static int bnx2x_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info);
-	u64 now;
 
 	DP(BNX2X_MSG_PTP, "PTP adjtime called, delta = %llx\n", delta);
 
-	now = timecounter_read(&bp->timecounter);
-	now += delta;
-	/* Re-init the timecounter */
-	timecounter_init(&bp->timecounter, &bp->cyclecounter, now);
+	timecounter_adjtime(&bp->timecounter, delta);
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 05/11] net: fec: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (3 preceding siblings ...)
  2014-12-21 18:46 ` [PATCH net-next 04/11] net: bnx2x: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-21 18:47 ` [PATCH net-next 06/11] net: e1000e: " Richard Cochran
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Compile tested only.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/freescale/fec_ptp.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 992c8c3..1f9cf23 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -374,23 +374,9 @@ static int fec_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	struct fec_enet_private *fep =
 	    container_of(ptp, struct fec_enet_private, ptp_caps);
 	unsigned long flags;
-	u64 now;
-	u32 counter;
 
 	spin_lock_irqsave(&fep->tmreg_lock, flags);
-
-	now = timecounter_read(&fep->tc);
-	now += delta;
-
-	/* Get the timer value based on adjusted timestamp.
-	 * Update the counter with the masked value.
-	 */
-	counter = now & fep->cc.mask;
-	writel(counter, fep->hwp + FEC_ATIME);
-
-	/* reset the timecounter */
-	timecounter_init(&fep->tc, &fep->cc, now);
-
+	timecounter_adjtime(&fep->tc, delta);
 	spin_unlock_irqrestore(&fep->tmreg_lock, flags);
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 06/11] net: e1000e: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (4 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 05/11] net: fec: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-23 21:04   ` Jeff Kirsher
  2014-12-21 18:47 ` [PATCH net-next 07/11] net: igb: " Richard Cochran
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/ptp.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index fb1a914..978ef9c 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -90,12 +90,9 @@ static int e1000e_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
 						     ptp_clock_info);
 	unsigned long flags;
-	s64 now;
 
 	spin_lock_irqsave(&adapter->systim_lock, flags);
-	now = timecounter_read(&adapter->tc);
-	now += delta;
-	timecounter_init(&adapter->tc, &adapter->cc, now);
+	timecounter_adjtime(&adapter->tc, delta);
 	spin_unlock_irqrestore(&adapter->systim_lock, flags);
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 07/11] net: igb: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (5 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 06/11] net: e1000e: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-23 21:04   ` Jeff Kirsher
  2014-12-21 18:47 ` [PATCH net-next 08/11] net: ixgbe: " Richard Cochran
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 794c139..1d27f2d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -256,14 +256,9 @@ static int igb_ptp_adjtime_82576(struct ptp_clock_info *ptp, s64 delta)
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
 	unsigned long flags;
-	s64 now;
 
 	spin_lock_irqsave(&igb->tmreg_lock, flags);
-
-	now = timecounter_read(&igb->tc);
-	now += delta;
-	timecounter_init(&igb->tc, &igb->cc, now);
-
+	timecounter_adjtime(&igb->tc, delta);
 	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 08/11] net: ixgbe: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (6 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 07/11] net: igb: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-23 21:07   ` Jeff Kirsher
  2014-12-21 18:47 ` [PATCH net-next 09/11] net: mlx4: " Richard Cochran
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Compile tested only.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 5fd4b52..47c29ea 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -261,18 +261,9 @@ static int ixgbe_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	struct ixgbe_adapter *adapter =
 		container_of(ptp, struct ixgbe_adapter, ptp_caps);
 	unsigned long flags;
-	u64 now;
 
 	spin_lock_irqsave(&adapter->tmreg_lock, flags);
-
-	now = timecounter_read(&adapter->tc);
-	now += delta;
-
-	/* reset the timecounter */
-	timecounter_init(&adapter->tc,
-			 &adapter->cc,
-			 now);
-
+	timecounter_adjtime(&adapter->tc, delta);
 	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
 
 	ixgbe_ptp_setup_sdp(adapter);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 09/11] net: mlx4: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (7 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 08/11] net: ixgbe: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-21 18:47 ` [PATCH net-next 10/11] net: cpts: " Richard Cochran
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Compile tested only.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index 9990144..df35d0e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
@@ -147,12 +147,9 @@ static int mlx4_en_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	struct mlx4_en_dev *mdev = container_of(ptp, struct mlx4_en_dev,
 						ptp_clock_info);
 	unsigned long flags;
-	s64 now;
 
 	write_lock_irqsave(&mdev->clock_lock, flags);
-	now = timecounter_read(&mdev->clock);
-	now += delta;
-	timecounter_init(&mdev->clock, &mdev->cycles, now);
+	timecounter_adjtime(&mdev->clock, delta);
 	write_unlock_irqrestore(&mdev->clock_lock, flags);
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 10/11] net: cpts: convert to timecounter adjtime.
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (8 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 09/11] net: mlx4: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-21 18:47 ` [PATCH net-next 11/11] timecounter: keep track of accumulated fractional nanoseconds Richard Cochran
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

This patch changes the driver to use the new and improved method
for adjusting the offset of a timecounter.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/ti/cpts.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 4a4388b..fbe42cb 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -157,14 +157,11 @@ static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 
 static int cpts_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
-	s64 now;
 	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
 
 	spin_lock_irqsave(&cpts->lock, flags);
-	now = timecounter_read(&cpts->tc);
-	now += delta;
-	timecounter_init(&cpts->tc, &cpts->cc, now);
+	timecounter_adjtime(&cpts->tc, delta);
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 11/11] timecounter: keep track of accumulated fractional nanoseconds
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (9 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 10/11] net: cpts: " Richard Cochran
@ 2014-12-21 18:47 ` Richard Cochran
  2014-12-30 23:32 ` [PATCH net-next 00/11] Time Counter fixes and improvements David Miller
  2015-01-02 18:37 ` John Stultz
  12 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 18:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

The current timecounter implementation will drop a variable amount
of resolution, depending on the magnitude of the time delta. In
other words, reading the clock too often or too close to a time
stamp conversion will introduce errors into the time values. This
patch fixes the issue by introducing a fractional nanosecond field
that accumulates the low order bits.

Reported-by: Janusz Użycki <j.uzycki@elproma.com.pl>
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    4 ++--
 include/linux/timecounter.h                   |   19 +++++++++------
 kernel/time/timecounter.c                     |   31 +++++++++++++++++++------
 virt/kvm/arm/arch_timer.c                     |    3 ++-
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index df35d0e..e9cce4f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
@@ -240,7 +240,7 @@ void mlx4_en_init_timestamp(struct mlx4_en_dev *mdev)
 {
 	struct mlx4_dev *dev = mdev->dev;
 	unsigned long flags;
-	u64 ns;
+	u64 ns, zero = 0;
 
 	rwlock_init(&mdev->clock_lock);
 
@@ -265,7 +265,7 @@ void mlx4_en_init_timestamp(struct mlx4_en_dev *mdev)
 	/* Calculate period in seconds to call the overflow watchdog - to make
 	 * sure counter is checked at least once every wrap around.
 	 */
-	ns = cyclecounter_cyc2ns(&mdev->cycles, mdev->cycles.mask);
+	ns = cyclecounter_cyc2ns(&mdev->cycles, mdev->cycles.mask, zero, &zero);
 	do_div(ns, NSEC_PER_SEC / 2 / HZ);
 	mdev->overflow_period = ns;
 
diff --git a/include/linux/timecounter.h b/include/linux/timecounter.h
index af3dfa4..74f4549 100644
--- a/include/linux/timecounter.h
+++ b/include/linux/timecounter.h
@@ -55,27 +55,32 @@ struct cyclecounter {
  * @cycle_last:		most recent cycle counter value seen by
  *			timecounter_read()
  * @nsec:		continuously increasing count
+ * @mask:		bit mask for maintaining the 'frac' field
+ * @frac:		accumulated fractional nanoseconds
  */
 struct timecounter {
 	const struct cyclecounter *cc;
 	cycle_t cycle_last;
 	u64 nsec;
+	u64 mask;
+	u64 frac;
 };
 
 /**
  * cyclecounter_cyc2ns - converts cycle counter cycles to nanoseconds
  * @cc:		Pointer to cycle counter.
  * @cycles:	Cycles
- *
- * XXX - This could use some mult_lxl_ll() asm optimization. Same code
- * as in cyc2ns, but with unsigned result.
+ * @mask:	bit mask for maintaining the 'frac' field
+ * @frac:	pointer to storage for the fractional nanoseconds.
  */
 static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
-				      cycle_t cycles)
+				      cycle_t cycles, u64 mask, u64 *frac)
 {
-	u64 ret = (u64)cycles;
-	ret = (ret * cc->mult) >> cc->shift;
-	return ret;
+	u64 ns = (u64) cycles;
+
+	ns = (ns * cc->mult) + *frac;
+	*frac = ns & mask;
+	return ns >> cc->shift;
 }
 
 /**
diff --git a/kernel/time/timecounter.c b/kernel/time/timecounter.c
index 59a1ec3..4687b31 100644
--- a/kernel/time/timecounter.c
+++ b/kernel/time/timecounter.c
@@ -25,6 +25,8 @@ void timecounter_init(struct timecounter *tc,
 	tc->cc = cc;
 	tc->cycle_last = cc->read(cc);
 	tc->nsec = start_tstamp;
+	tc->mask = (1ULL << cc->shift) - 1;
+	tc->frac = 0;
 }
 EXPORT_SYMBOL_GPL(timecounter_init);
 
@@ -51,7 +53,8 @@ static u64 timecounter_read_delta(struct timecounter *tc)
 	cycle_delta = (cycle_now - tc->cycle_last) & tc->cc->mask;
 
 	/* convert to nanoseconds: */
-	ns_offset = cyclecounter_cyc2ns(tc->cc, cycle_delta);
+	ns_offset = cyclecounter_cyc2ns(tc->cc, cycle_delta,
+					tc->mask, &tc->frac);
 
 	/* update time stamp of timecounter_read_delta() call: */
 	tc->cycle_last = cycle_now;
@@ -72,22 +75,36 @@ u64 timecounter_read(struct timecounter *tc)
 }
 EXPORT_SYMBOL_GPL(timecounter_read);
 
+/*
+ * This is like cyclecounter_cyc2ns(), but it is used for computing a
+ * time previous to the time stored in the cycle counter.
+ */
+static u64 cc_cyc2ns_backwards(const struct cyclecounter *cc,
+			       cycle_t cycles, u64 mask, u64 frac)
+{
+	u64 ns = (u64) cycles;
+
+	ns = ((ns * cc->mult) - frac) >> cc->shift;
+
+	return ns;
+}
+
 u64 timecounter_cyc2time(struct timecounter *tc,
 			 cycle_t cycle_tstamp)
 {
-	u64 cycle_delta = (cycle_tstamp - tc->cycle_last) & tc->cc->mask;
-	u64 nsec;
+	u64 delta = (cycle_tstamp - tc->cycle_last) & tc->cc->mask;
+	u64 nsec = tc->nsec, frac = tc->frac;
 
 	/*
 	 * Instead of always treating cycle_tstamp as more recent
 	 * than tc->cycle_last, detect when it is too far in the
 	 * future and treat it as old time stamp instead.
 	 */
-	if (cycle_delta > tc->cc->mask / 2) {
-		cycle_delta = (tc->cycle_last - cycle_tstamp) & tc->cc->mask;
-		nsec = tc->nsec - cyclecounter_cyc2ns(tc->cc, cycle_delta);
+	if (delta > tc->cc->mask / 2) {
+		delta = (tc->cycle_last - cycle_tstamp) & tc->cc->mask;
+		nsec -= cc_cyc2ns_backwards(tc->cc, delta, tc->mask, frac);
 	} else {
-		nsec = cyclecounter_cyc2ns(tc->cc, cycle_delta) + tc->nsec;
+		nsec += cyclecounter_cyc2ns(tc->cc, delta, tc->mask, &frac);
 	}
 
 	return nsec;
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 22fa819..75d9564 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -150,7 +150,8 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
 		return;
 	}
 
-	ns = cyclecounter_cyc2ns(timecounter->cc, cval - now);
+	ns = cyclecounter_cyc2ns(timecounter->cc, cval - now, timecounter->mask,
+				 &timecounter->frac);
 	timer_arm(timer, ns);
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 04/11] net: bnx2x: convert to timecounter adjtime.
  2014-12-21 18:46 ` [PATCH net-next 04/11] net: bnx2x: " Richard Cochran
@ 2014-12-21 19:01   ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-21 19:01 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

On Sun, Dec 21, 2014 at 07:46:59PM +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.

BTW, I noticed that this driver does not serialize access to the
timecounter in any way, but probably it should. Ariel?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file.
  2014-12-21 18:46 ` [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file Richard Cochran
@ 2014-12-23 21:03   ` Jeff Kirsher
  0 siblings, 0 replies; 22+ messages in thread
From: Jeff Kirsher @ 2014-12-23 21:03 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

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

On Sun, 2014-12-21 at 19:46 +0100, Richard Cochran wrote:
> The timecounter code has almost nothing to do with the clocksource
> code. Let it live in its own file. This will help isolate the
> timecounter users from the clocksource users in the source tree.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
For the Intel driver changes...

> ---
>  drivers/net/ethernet/amd/xgbe/xgbe.h        |    2 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x.h |    2 +-
>  drivers/net/ethernet/freescale/fec.h        |    1 +
>  drivers/net/ethernet/intel/e1000e/e1000.h   |    2 +-
>  drivers/net/ethernet/intel/igb/igb.h        |    2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h    |    2 +-
>  drivers/net/ethernet/ti/cpts.h              |    1 +
>  include/clocksource/arm_arch_timer.h        |    2 +-
>  include/linux/clocksource.h                 |  102
> ----------------------
>  include/linux/mlx4/device.h                 |    2 +-
>  include/linux/timecounter.h                 |  122
> +++++++++++++++++++++++++++
>  include/linux/types.h                       |    3 +
>  kernel/time/Makefile                        |    2 +-
>  kernel/time/clocksource.c                   |   76 -----------------
>  kernel/time/timecounter.c                   |   95
> +++++++++++++++++++++
>  sound/pci/hda/hda_priv.h                    |    2 +-
>  16 files changed, 231 insertions(+), 187 deletions(-)
>  create mode 100644 include/linux/timecounter.h
>  create mode 100644 kernel/time/timecounter.c



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 06/11] net: e1000e: convert to timecounter adjtime.
  2014-12-21 18:47 ` [PATCH net-next 06/11] net: e1000e: " Richard Cochran
@ 2014-12-23 21:04   ` Jeff Kirsher
  0 siblings, 0 replies; 22+ messages in thread
From: Jeff Kirsher @ 2014-12-23 21:04 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

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

On Sun, 2014-12-21 at 19:47 +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/e1000e/ptp.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 07/11] net: igb: convert to timecounter adjtime.
  2014-12-21 18:47 ` [PATCH net-next 07/11] net: igb: " Richard Cochran
@ 2014-12-23 21:04   ` Jeff Kirsher
  0 siblings, 0 replies; 22+ messages in thread
From: Jeff Kirsher @ 2014-12-23 21:04 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

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

On Sun, 2014-12-21 at 19:47 +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 08/11] net: ixgbe: convert to timecounter adjtime.
  2014-12-21 18:47 ` [PATCH net-next 08/11] net: ixgbe: " Richard Cochran
@ 2014-12-23 21:07   ` Jeff Kirsher
  2014-12-23 21:42     ` Richard Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Jeff Kirsher @ 2014-12-23 21:07 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

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

On Sun, 2014-12-21 at 19:47 +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.
> 
> Compile tested only.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Just for sanity sake, I will have Phillip test the changes, but there is
no need to hold this up from being committed in the mean time.

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 08/11] net: ixgbe: convert to timecounter adjtime.
  2014-12-23 21:07   ` Jeff Kirsher
@ 2014-12-23 21:42     ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-23 21:42 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

On Tue, Dec 23, 2014 at 01:07:19PM -0800, Jeff Kirsher wrote:
> Just for sanity sake, I will have Phillip test the changes, but there is
> no need to hold this up from being committed in the mean time.

FYI, here is a test that may show the improved behavior when comparing
before/after.

1. Start ptp4l, let it run long enough to find the frequency offset.

2. Stop ptp4l with ^C.

3. Wait a bit, to let the clock drift away.

4. Restart ptp4l, it will now start with the frequency offset learned
   in step 1.

5. Notice the the offsets in states s0 -> s1 -> s2.
   There will likely be less offset error with the patches applied.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 00/11] Time Counter fixes and improvements
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (10 preceding siblings ...)
  2014-12-21 18:47 ` [PATCH net-next 11/11] timecounter: keep track of accumulated fractional nanoseconds Richard Cochran
@ 2014-12-30 23:32 ` David Miller
  2014-12-31 17:42   ` Richard Cochran
  2015-01-02 18:37 ` John Stultz
  12 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2014-12-30 23:32 UTC (permalink / raw)
  To: richardcochran
  Cc: netdev, linux-kernel, amirv, ariel.elior, carolyn.wyborny,
	Frank.Li, jeffrey.t.kirsher, john.stultz, matthew.vick, mlichvar,
	mugunthanvnm, ogerlitz, tglx, thomas.lendacky

From: Richard Cochran <richardcochran@gmail.com>
Date: Sun, 21 Dec 2014 19:46:55 +0100

> Several PTP Hardware Clock (PHC) drivers implement the clock in
> software using the timecounter/cyclecounter code. This series adds one
> simple improvement and one more subtle fix to the shared timecounter
> facility. Credit for this series goes to Janusz Użycki, who pointed
> the issues out to me off list.

Series applied, thanks Richard.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 00/11] Time Counter fixes and improvements
  2014-12-30 23:32 ` [PATCH net-next 00/11] Time Counter fixes and improvements David Miller
@ 2014-12-31 17:42   ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2014-12-31 17:42 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-kernel, amirv, ariel.elior, carolyn.wyborny,
	Frank.Li, jeffrey.t.kirsher, john.stultz, matthew.vick, mlichvar,
	mugunthanvnm, ogerlitz, tglx, thomas.lendacky

On Tue, Dec 30, 2014 at 06:32:01PM -0500, David Miller wrote:
> 
> Series applied, thanks Richard.

I got an automated email from

  kbuild test robot <fengguang.wu@intel.com>

showing new warnings and errors introduced by this series. I'll follow
up with fixes soon.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 00/11] Time Counter fixes and improvements
  2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
                   ` (11 preceding siblings ...)
  2014-12-30 23:32 ` [PATCH net-next 00/11] Time Counter fixes and improvements David Miller
@ 2015-01-02 18:37 ` John Stultz
  2015-01-02 18:49   ` Richard Cochran
  12 siblings, 1 reply; 22+ messages in thread
From: John Stultz @ 2015-01-02 18:37 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, lkml, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

On Sun, Dec 21, 2014 at 10:46 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> Several PTP Hardware Clock (PHC) drivers implement the clock in
> software using the timecounter/cyclecounter code. This series adds one
> simple improvement and one more subtle fix to the shared timecounter
> facility. Credit for this series goes to Janusz Użycki, who pointed
> the issues out to me off list.
>
> Patch #1 simply move the timecounter code into its own file. When
> working on this series, it was really annoying to see half the kernel
> recompile after every tweak to the timecounter stuff. There is no
> reason to keep this together with the clocksource code.

I did have some faint hope we could merge the cyclecounter and
clocksource struct at some point, and while we've gotten closer with
much of the time-specific values being kept in the timekeeper, the
full cleanup hasn't happened in a few years here, so its clearly not
something I've prioritized.

So no objection in concept.


> Patch #2 implements an improved adjtime() method, and patches 3-10
> convert all of the drivers over to the new method.
>
> Patch #11 fixes a subtle but important issue with the timecounter WRT
> frequency adjustment. As it stands now, a timecounter based PHC will
> exhibit a variable frequency resolution (and variable time error)
> depending on how often the clock is read.
>
> In timecounter_read_delta(), the expression
>
>    (delta * cc->mult) >> cc->shift;
>
> can lose resolution from the adjusted value of 'mult'. If the value
> of 'delta' is too small, then small changes in 'mult' have no effect.
> However, if the delta value is large enough, then small changes in
> 'mult' will have an effect.
>
> Reading the clock too often means smaller 'delta' values which in turn
> will spoil the fine adjustments made to 'mult'. Up until now, this
> effect did not show up in my testing. The following example explains
> why.
>
> The CPTS has an input clock of 250 MHz, and the clock source uses
> mult=0x80000000 and shift=29, making the ticks to nanoseconds
> conversion like this:
>
>    ticks * 2^31
>    ------------
>        2^29
>
> Imagine what happens if the clock is read every 10 milliseconds. Ten
> milliseconds are about 2500000 ticks, which corresponds to about 21
> bits. The product in the numerator has then 52 bits. After the shift
> operation, 23 bits are preserved. This results in a frequency
> adjustment resolution of about 0.1 ppm (not _too_ bad.)
>
> A frequency resolution of 1 ppm requires 20 bits.
> A frequency resolution of 1 ppb requires 30 bits.
>
> For the 250 MHz CPTS clock, reading every 4 seconds yields a 1 ppb
> resolution (which is the finest that our API allows).
>
> However, the error can be much higher if the clock is read too often
> or if time stamps occur close in time to read operations. In general
> it is really not acceptable to allow the rate of clock readings to
> influence the clock accuracy.

Though, this does start to sound like issues the timekeeping code had
to resolve, so while its probably time to let some flowers bloom and
see what happens, we should be somewhat watchful for too much logic
duplication.

The patch set itself seems fairly straight forward (at least to my
back from vacation brain), so..
Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 00/11] Time Counter fixes and improvements
  2015-01-02 18:37 ` John Stultz
@ 2015-01-02 18:49   ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2015-01-02 18:49 UTC (permalink / raw)
  To: John Stultz
  Cc: netdev, lkml, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, Jeff Kirsher, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky

On Fri, Jan 02, 2015 at 10:37:09AM -0800, John Stultz wrote:
> Though, this does start to sound like issues the timekeeping code had
> to resolve, so while its probably time to let some flowers bloom and
> see what happens, we should be somewhat watchful for too much logic
> duplication.

You took the words right out of my mouth.

My feeling is that this timecounter code might become more
complicated, and if the core time keeping becomes more
straightforward, then maybe one day the two will join.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-01-02 18:49 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-21 18:46 [PATCH net-next 00/11] Time Counter fixes and improvements Richard Cochran
2014-12-21 18:46 ` [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file Richard Cochran
2014-12-23 21:03   ` Jeff Kirsher
2014-12-21 18:46 ` [PATCH net-next 02/11] timecounter: provide a helper function to shift the time Richard Cochran
2014-12-21 18:46 ` [PATCH net-next 03/11] net: xgbe: convert to timecounter adjtime Richard Cochran
2014-12-21 18:46 ` [PATCH net-next 04/11] net: bnx2x: " Richard Cochran
2014-12-21 19:01   ` Richard Cochran
2014-12-21 18:47 ` [PATCH net-next 05/11] net: fec: " Richard Cochran
2014-12-21 18:47 ` [PATCH net-next 06/11] net: e1000e: " Richard Cochran
2014-12-23 21:04   ` Jeff Kirsher
2014-12-21 18:47 ` [PATCH net-next 07/11] net: igb: " Richard Cochran
2014-12-23 21:04   ` Jeff Kirsher
2014-12-21 18:47 ` [PATCH net-next 08/11] net: ixgbe: " Richard Cochran
2014-12-23 21:07   ` Jeff Kirsher
2014-12-23 21:42     ` Richard Cochran
2014-12-21 18:47 ` [PATCH net-next 09/11] net: mlx4: " Richard Cochran
2014-12-21 18:47 ` [PATCH net-next 10/11] net: cpts: " Richard Cochran
2014-12-21 18:47 ` [PATCH net-next 11/11] timecounter: keep track of accumulated fractional nanoseconds Richard Cochran
2014-12-30 23:32 ` [PATCH net-next 00/11] Time Counter fixes and improvements David Miller
2014-12-31 17:42   ` Richard Cochran
2015-01-02 18:37 ` John Stultz
2015-01-02 18:49   ` Richard Cochran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).