linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms
@ 2015-08-07 23:01 Christopher Hall
  2015-08-07 23:01 ` [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code Christopher Hall
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Christopher Hall @ 2015-08-07 23:01 UTC (permalink / raw)
  To: john.stultz, tglx, richardcochran, mingo, jeffrey.t.kirsher,
	john.ronciak, hpa, x86
  Cc: linux-kernel, netdev, Christopher Hall

6th generation Intel platforms will have an Always Running
Timer (ART) that always runs when the system is powered and
is available to both the CPU and various on-board devices.
Initially, those devices include audio and network.  The
ART will give these devices the capability of precisely
cross timestamping their local device clock with the system
clock.

A system clock value like TSC or ART is not useful
unless translated to system time. The first *two* patches
enable this by changing the timekeeping code to return a
system time given a system clock value and translating ART
to TSC.

The last two patches modify the PTP driver to call a
cross timestamp function in the driver when available and
perform the cross timestamp in the e1000e driver.

Given the precise relationship between the network device
clock and system time enables better synchronization of
events on multiple network connected devices.

Changelog:

* The PTP portion of the patch set was posted 7/8/2015 (v3)
  and rejected because of there wasn't a driver that
  implemented the new API.  Now, the driver patch is added
  and the PTP patch operation is modified to revert to
  previous behavior when cross timestamp can't be
  completed.  This is indicated by the driver returning a
  non-zero value.

* v2 re-submit based on tglx provided correlated clocksource patch.  This has
  been included verbatim as the first patch in the series.  Additions and
  modifications are in the second patch.  The PTP driver patch is unchanged
  and the e1000e driver patch uses the *new* correlated clocksource interface
  but is otherwise (in terms of hardware and PTP driver) unchanged.

* ART is removed as a compile option

* ART is added as an X86_FEATURE

Christopher Hall (4):
  Add generic correlated clocksource code and ART to TSC conversion code
  Add ART initialization code
  Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl
  Added getsynctime64() callback

 Documentation/ptp/testptp.c                 |  6 +-
 arch/x86/include/asm/cpufeature.h           |  3 +-
 arch/x86/include/asm/tsc.h                  |  1 +
 arch/x86/kernel/tsc.c                       | 45 +++++++++++++++
 drivers/net/ethernet/intel/e1000e/defines.h |  7 +++
 drivers/net/ethernet/intel/e1000e/ptp.c     | 88 +++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/e1000e/regs.h    |  4 ++
 drivers/ptp/ptp_chardev.c                   | 29 +++++++---
 include/linux/clocksource.h                 | 32 +++++++++++
 include/linux/ptp_clock_kernel.h            |  7 +++
 include/linux/timekeeping.h                 |  4 ++
 include/uapi/linux/ptp_clock.h              |  4 +-
 kernel/time/timekeeping.c                   | 64 +++++++++++++++++++++
 13 files changed, 282 insertions(+), 12 deletions(-)

-- 
1.9.1


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

* [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code
  2015-08-07 23:01 [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher Hall
@ 2015-08-07 23:01 ` Christopher Hall
  2015-08-07 23:44   ` Andy Lutomirski
  2015-08-07 23:01 ` [PATCH v2 2/4] Add ART initialization code Christopher Hall
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Christopher Hall @ 2015-08-07 23:01 UTC (permalink / raw)
  To: john.stultz, tglx, richardcochran, mingo, jeffrey.t.kirsher,
	john.ronciak, hpa, x86
  Cc: linux-kernel, netdev, Christopher Hall

Original patch description:

Subject: ptp: Get sync timestamps
From: Thomas Gleixner <tglx@linutronix.de>
Date: Wed, 29 Jul 2015 10:52:06 +0200

The ART stuff wants to be splitted out.

======== Changes =======

Add struct correlated_cs (clocksource) with pointer to original clocksource
	and function pointer to convert correlated clocksource to the original

Add struct correlated_ts (timestamp) with function pointer to read correlated
	clocksource, device and system (in terms of correlated clocksource)
	counter values (input) with resulting converted real and monotonic raw
	system times (output)

Add get_correlated_timestamp() function which given specific correlated_cs
	and correlated_ts convert correlated counter value to system time

Add art_to_tsc conversion function translated Always Running Timer (ART) to
	TSC value
---
 arch/x86/kernel/tsc.c       | 31 ++++++++++++++++++++++
 include/linux/clocksource.h | 30 +++++++++++++++++++++
 include/linux/timekeeping.h |  4 +++
 kernel/time/timekeeping.c   | 63 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 7437b41..a90aa6a 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1059,6 +1059,27 @@ int unsynchronized_tsc(void)
 	return 0;
 }
 
+static u32 tsc_numerator;
+static u32 tsc_denominator;
+/*
+ * CHECKME: Do we need the adjust value? It should be 0, but if we run
+ * in a VM this might be a different story.
+ */
+static u64 tsc_adjust;
+
+static u64 art_to_tsc(u64 cycles)
+{
+	u64 tmp, res = tsc_adjust;
+
+	res += (cycles / tsc_denominator) * tsc_numerator;
+	tmp = (cycles % tsc_denominator) * tsc_numerator;
+	res += tmp / tsc_denominator;
+	return res;
+}
+
+struct correlated_cs art_timestamper = {
+	.convert	= art_to_tsc,
+};
 
 static void tsc_refine_calibration_work(struct work_struct *work);
 static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
@@ -1129,6 +1150,16 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 		(unsigned long)tsc_khz / 1000,
 		(unsigned long)tsc_khz % 1000);
 
+	/*
+	 * TODO:
+	 *
+	 * If the system has ART, initialize the art_to_tsc conversion
+	 * and set: art_timestamp.related_cs = &tsc_clocksource.
+	 *
+	 * Before that point a call to get_correlated_timestamp will
+	 * fail the clocksource match check.
+	 */
+
 out:
 	clocksource_register_khz(&clocksource_tsc, tsc_khz);
 }
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 278dd27..2ed3d0c 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -258,4 +258,34 @@ void acpi_generic_timer_init(void);
 static inline void acpi_generic_timer_init(void) { }
 #endif
 
+/**
+ * struct correlated_cs - Descriptor for a clocksource correlated to another clocksource
+ * @related_cs:		Pointer to the related timekeeping clocksource
+ * @convert:		Conversion function to convert a timestamp from
+ *			the correlated clocksource to cycles of the related
+ *			timekeeping clocksource
+ */
+struct correlated_cs {
+	struct clocksource	*related_cs;
+	u64			(*convert)(u64 cycles);
+};
+
+struct correlated_ts;
+
+/**
+ * struct correlated_ts - Descriptor for taking a correlated time stamp
+ * @get_ts:		Function to read out a synced system and device
+ *			timestamp
+ * @system_ts:		The raw system clock timestamp
+ * @device_ts:		The raw device timestamp
+ * @system_real:	@system_ts converted to CLOCK_REALTIME
+ * @system_raw:		@system_ts converted to CLOCK_MONOTONIC_RAW
+ */
+struct correlated_ts {
+	int			(*get_ts)(struct correlated_ts *ts);
+	u64			system_ts;
+	u64			device_ts;
+	u64			system_real;
+	u64			system_raw;
+};
 #endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 6e191e4..a9e1a2d 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -258,6 +258,10 @@ extern void timekeeping_inject_sleeptime64(struct timespec64 *delta);
  */
 extern void getnstime_raw_and_real(struct timespec *ts_raw,
 				   struct timespec *ts_real);
+struct correlated_ts;
+struct correlated_cs;
+extern int get_correlated_timestamp(struct correlated_ts *crt,
+				    struct correlated_cs *crs);
 
 /*
  * Persistent clock related interfaces
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index bca3667..769a04b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -312,6 +312,19 @@ static inline s64 timekeeping_get_ns(struct tk_read_base *tkr)
 	return nsec + arch_gettimeoffset();
 }
 
+static inline s64 timekeeping_convert_to_ns(struct tk_read_base *tkr,
+					    cycle_t cycles)
+{
+	cycle_t delta;
+	s64 nsec;
+
+	/* calculate the delta since the last update_wall_time */
+	delta = clocksource_delta(cycles, tkr->cycle_last, tkr->mask);
+
+	nsec = delta * tkr->mult + tkr->xtime_nsec;
+	return nsec >> tkr->shift;
+}
+
 /**
  * update_fast_timekeeper - Update the fast and NMI safe monotonic timekeeper.
  * @tkr: Timekeeping readout base from which we take the update
@@ -885,6 +898,56 @@ EXPORT_SYMBOL(getnstime_raw_and_real);
 #endif /* CONFIG_NTP_PPS */
 
 /**
+ * get_correlated_timestamp - Get a correlated timestamp
+ *
+ * Reads a timestamp from a device and correlates it to system time
+ */
+int get_correlated_timestamp(struct correlated_ts *crt,
+			     struct correlated_cs *crs)
+{
+	struct timekeeper *tk = &tk_core.timekeeper;
+	unsigned long seq;
+	cycles_t cycles;
+	ktime_t base;
+	s64 nsecs;
+	int ret;
+
+	do {
+		seq = read_seqcount_begin(&tk_core.seq);
+		/*
+		 * Verify that the correlated clocksoure is related to
+		 * the currently installed timekeeper clocksoure
+		 */
+		if (tk->tkr_mono.clock != crs->related_cs)
+			return -ENODEV;
+
+		/*
+		 * Try to get a timestamp from the device.
+		 */
+		ret = crt->get_ts(crt);
+		if (ret)
+			return ret;
+
+		/*
+		 * Convert the timestamp to timekeeper clock cycles
+		 */
+		cycles = crs->convert(crs, crt->system_ts);
+
+		/* Convert to clock realtime */
+		base = ktime_add(tk->tkr_mono.base, tk_core.timekeeper.offs_real);
+		nsecs = timekeeping_convert_to_ns(&tk->tkr_mono, cycles);
+		crt->system_real = ktime_add_ns(base, nsecs);
+
+		/* Convert to clock raw monotonic */
+		base = tk->tkr_raw.base;
+		nsecs = timekeeping_convert_to_ns(&tk->tkr_raw, cycles);
+		crt->system_raw = ktime_add_ns(base, nsecs);
+
+	} while (read_seqcount_retry(&tk_core.seq, seq));
+	return 0;
+}
+
+/**
  * do_gettimeofday - Returns the time of day in a timeval
  * @tv:		pointer to the timeval to be set
  *
-- 
1.9.1


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

* [PATCH v2 2/4] Add ART initialization code
  2015-08-07 23:01 [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher Hall
  2015-08-07 23:01 ` [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code Christopher Hall
@ 2015-08-07 23:01 ` Christopher Hall
  2015-08-07 23:01 ` [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl Christopher Hall
  2015-08-07 23:01 ` [PATCH v2 4/4] Added getsynctime64() callback Christopher Hall
  3 siblings, 0 replies; 10+ messages in thread
From: Christopher Hall @ 2015-08-07 23:01 UTC (permalink / raw)
  To: john.stultz, tglx, richardcochran, mingo, jeffrey.t.kirsher,
	john.ronciak, hpa, x86
  Cc: linux-kernel, netdev, Christopher Hall

add private struct correlated_ts member used by get_ts() code

added EXPORTs making get_correlated_timestamp() function and
	art_timestamper accessible

Add special case for denominator of 2 (art_to_tsc())
---
 arch/x86/include/asm/cpufeature.h |  3 ++-
 arch/x86/include/asm/tsc.h        |  1 +
 arch/x86/kernel/tsc.c             | 42 ++++++++++++++++++++++++++-------------
 include/linux/clocksource.h       |  8 +++++---
 kernel/time/timekeeping.c         |  1 +
 5 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 3d6606f..a9322e5 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -85,7 +85,7 @@
 #define X86_FEATURE_P4		( 3*32+ 7) /* "" P4 */
 #define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* TSC ticks at a constant rate */
 #define X86_FEATURE_UP		( 3*32+ 9) /* smp kernel running on up */
-/* free, was #define X86_FEATURE_FXSAVE_LEAK ( 3*32+10) * "" FXSAVE leaks FOP/FIP/FOP */
+#define X86_FEATURE_ART		(3*32+10) /* Platform has always running timer (ART) */
 #define X86_FEATURE_ARCH_PERFMON ( 3*32+11) /* Intel Architectural PerfMon */
 #define X86_FEATURE_PEBS	( 3*32+12) /* Precise-Event Based Sampling */
 #define X86_FEATURE_BTS		( 3*32+13) /* Branch Trace Store */
@@ -352,6 +352,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 #define cpu_has_de		boot_cpu_has(X86_FEATURE_DE)
 #define cpu_has_pse		boot_cpu_has(X86_FEATURE_PSE)
 #define cpu_has_tsc		boot_cpu_has(X86_FEATURE_TSC)
+#define cpu_has_art		boot_cpu_has(X86_FEATURE_ART)
 #define cpu_has_pge		boot_cpu_has(X86_FEATURE_PGE)
 #define cpu_has_apic		boot_cpu_has(X86_FEATURE_APIC)
 #define cpu_has_sep		boot_cpu_has(X86_FEATURE_SEP)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 94605c0..0089991 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -53,6 +53,7 @@ extern int check_tsc_disabled(void);
 extern unsigned long native_calibrate_tsc(void);
 
 extern int tsc_clocksource_reliable;
+extern struct correlated_cs art_timestamper;
 
 /*
  * Boot-time check whether the TSCs are synchronized across
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index a90aa6a..0a2f336 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1059,6 +1059,9 @@ int unsynchronized_tsc(void)
 	return 0;
 }
 
+#define ART_CPUID_LEAF (0x15)
+#define ART_MIN_DENOMINATOR (2)
+
 static u32 tsc_numerator;
 static u32 tsc_denominator;
 /*
@@ -1067,19 +1070,29 @@ static u32 tsc_denominator;
  */
 static u64 tsc_adjust;
 
-static u64 art_to_tsc(u64 cycles)
+static u64 art_to_tsc(struct correlated_cs *cs, u64 cycles)
 {
 	u64 tmp, res = tsc_adjust;
 
-	res += (cycles / tsc_denominator) * tsc_numerator;
-	tmp = (cycles % tsc_denominator) * tsc_numerator;
-	res += tmp / tsc_denominator;
+	switch (tsc_denominator) {
+	default:
+		res += (cycles / tsc_denominator) * tsc_numerator;
+		tmp = (cycles % tsc_denominator) * tsc_numerator;
+		res += tmp / tsc_denominator;
+		break;
+	case 2:
+		res += (cycles >> 1) * tsc_numerator;
+		tmp = (cycles & 0x1) * tsc_numerator;
+		res += tmp >> 1;
+		break;
+	}
 	return res;
 }
 
 struct correlated_cs art_timestamper = {
 	.convert	= art_to_tsc,
 };
+EXPORT_SYMBOL(art_timestamper);
 
 static void tsc_refine_calibration_work(struct work_struct *work);
 static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
@@ -1103,6 +1116,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 	static int hpet;
 	u64 tsc_stop, ref_stop, delta;
 	unsigned long freq;
+	unsigned int unused[2];
 
 	/* Don't bother refining TSC on unstable systems */
 	if (check_tsc_unstable())
@@ -1150,17 +1164,17 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 		(unsigned long)tsc_khz / 1000,
 		(unsigned long)tsc_khz % 1000);
 
-	/*
-	 * TODO:
-	 *
-	 * If the system has ART, initialize the art_to_tsc conversion
-	 * and set: art_timestamp.related_cs = &tsc_clocksource.
-	 *
-	 * Before that point a call to get_correlated_timestamp will
-	 * fail the clocksource match check.
-	 */
-
 out:
+	if (boot_cpu_data.cpuid_level >= ART_CPUID_LEAF) {
+		cpuid(ART_CPUID_LEAF, &tsc_denominator, &tsc_numerator, unused,
+		      unused+1);
+
+		if (tsc_denominator >= ART_MIN_DENOMINATOR) {
+			set_cpu_cap(&boot_cpu_data, X86_FEATURE_ART);
+			art_timestamper.related_cs = &clocksource_tsc;
+		}
+	}
+
 	clocksource_register_khz(&clocksource_tsc, tsc_khz);
 }
 
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 2ed3d0c..46ce54b 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -267,7 +267,8 @@ static inline void acpi_generic_timer_init(void) { }
  */
 struct correlated_cs {
 	struct clocksource	*related_cs;
-	u64			(*convert)(u64 cycles);
+	u64			(*convert)(struct correlated_cs *cs,
+					   u64 cycles);
 };
 
 struct correlated_ts;
@@ -285,7 +286,8 @@ struct correlated_ts {
 	int			(*get_ts)(struct correlated_ts *ts);
 	u64			system_ts;
 	u64			device_ts;
-	u64			system_real;
-	u64			system_raw;
+	ktime_t			system_real;
+	ktime_t			system_raw;
+	void			*private;
 };
 #endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 769a04b..abf8aec 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -946,6 +946,7 @@ int get_correlated_timestamp(struct correlated_ts *crt,
 	} while (read_seqcount_retry(&tk_core.seq, seq));
 	return 0;
 }
+EXPORT_SYMBOL(get_correlated_timestamp);
 
 /**
  * do_gettimeofday - Returns the time of day in a timeval
-- 
1.9.1


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

* [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl
  2015-08-07 23:01 [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher Hall
  2015-08-07 23:01 ` [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code Christopher Hall
  2015-08-07 23:01 ` [PATCH v2 2/4] Add ART initialization code Christopher Hall
@ 2015-08-07 23:01 ` Christopher Hall
  2015-08-10  8:53   ` Richard Cochran
  2015-08-07 23:01 ` [PATCH v2 4/4] Added getsynctime64() callback Christopher Hall
  3 siblings, 1 reply; 10+ messages in thread
From: Christopher Hall @ 2015-08-07 23:01 UTC (permalink / raw)
  To: john.stultz, tglx, richardcochran, mingo, jeffrey.t.kirsher,
	john.ronciak, hpa, x86
  Cc: linux-kernel, netdev, Christopher Hall

This patch allows system and device time ("cross-timestamp") to be
performed by the driver. Currently, the cross-timestamping is performed
in the PTP_SYS_OFFSET ioctl.  The PTP clock driver reads gettimeofday()
and the gettime64() callback provided by the driver. The cross-timestamp
is best effort where the latency between the capture of system time
(getnstimeofday()) and the device time (driver callback) may be
significant.

This patch adds an additional callback getsynctime64(). Which will be
called when the driver is able to perform a more accurate, implementation
specific cross-timestamping.  For example, future network devices that
implement PCIE PTM will be able to precisely correlate the device clock
with the system clock with virtually zero latency between captures.
This added callback can be used by the driver to expose this functionality.

The callback, getsynctime64(), will only be called when defined and
n_samples == 1 because the driver returns only 1 cross-timestamp where
multiple samples cannot be chained together.

This patch also adds to the capabilities ioctl (PTP_CLOCK_GETCAPS),
allowing applications to query whether or not drivers implement the
getsynctime callback, providing more precise cross timestamping.

Commit Details:

Added additional callback to ptp_clock_info:

* getsynctime64()

This takes 2 arguments referring to system and device time

With this callback drivers may provide both system time and device time
to ensure precise correlation

Modified PTP_SYS_OFFSET ioctl in PTP clock driver to use the above
callback if it's available

Added capability (PTP_CLOCK_GETCAPS) for checking whether driver supports
cross timestamping

Added check for cross timestamping flag to testptp.c
---
 Documentation/ptp/testptp.c      |  6 ++++--
 drivers/ptp/ptp_chardev.c        | 29 +++++++++++++++++++++--------
 include/linux/ptp_clock_kernel.h |  7 +++++++
 include/uapi/linux/ptp_clock.h   |  4 +++-
 4 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c
index 2bc8abc..8004efd 100644
--- a/Documentation/ptp/testptp.c
+++ b/Documentation/ptp/testptp.c
@@ -276,13 +276,15 @@ int main(int argc, char *argv[])
 			       "  %d external time stamp channels\n"
 			       "  %d programmable periodic signals\n"
 			       "  %d pulse per second\n"
-			       "  %d programmable pins\n",
+			       "  %d programmable pins\n"
+			       "  %d cross timestamping\n",
 			       caps.max_adj,
 			       caps.n_alarm,
 			       caps.n_ext_ts,
 			       caps.n_per_out,
 			       caps.pps,
-			       caps.n_pins);
+			       caps.n_pins,
+			       caps.cross_timestamping);
 		}
 	}
 
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index da7bae9..392ccfa 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -124,7 +124,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
 	struct ptp_clock_info *ops = ptp->info;
 	struct ptp_clock_time *pct;
-	struct timespec64 ts;
+	struct timespec64 ts, systs;
 	int enable, err = 0;
 	unsigned int i, pin_index;
 
@@ -138,6 +138,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 		caps.n_per_out = ptp->info->n_per_out;
 		caps.pps = ptp->info->pps;
 		caps.n_pins = ptp->info->n_pins;
+		caps.cross_timestamping = ptp->info->getsynctime64 != NULL;
 		if (copy_to_user((void __user *)arg, &caps, sizeof(caps)))
 			err = -EFAULT;
 		break;
@@ -196,19 +197,31 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			break;
 		}
 		pct = &sysoff->ts[0];
-		for (i = 0; i < sysoff->n_samples; i++) {
-			getnstimeofday64(&ts);
+		if (ptp->info->getsynctime64 && sysoff->n_samples == 1 &&
+		    ptp->info->getsynctime64(ptp->info, &ts, &systs) == 0) {
+			pct->sec = systs.tv_sec;
+			pct->nsec = systs.tv_nsec;
+			pct++;
 			pct->sec = ts.tv_sec;
 			pct->nsec = ts.tv_nsec;
 			pct++;
-			ptp->info->gettime64(ptp->info, &ts);
+			pct->sec = systs.tv_sec;
+			pct->nsec = systs.tv_nsec;
+		} else {
+			for (i = 0; i < sysoff->n_samples; i++) {
+				getnstimeofday64(&ts);
+				pct->sec = ts.tv_sec;
+				pct->nsec = ts.tv_nsec;
+				pct++;
+				ptp->info->gettime64(ptp->info, &ts);
+				pct->sec = ts.tv_sec;
+				pct->nsec = ts.tv_nsec;
+				pct++;
+			}
+			getnstimeofday64(&ts);
 			pct->sec = ts.tv_sec;
 			pct->nsec = ts.tv_nsec;
-			pct++;
 		}
-		getnstimeofday64(&ts);
-		pct->sec = ts.tv_sec;
-		pct->nsec = ts.tv_nsec;
 		if (copy_to_user((void __user *)arg, sysoff, sizeof(*sysoff)))
 			err = -EFAULT;
 		break;
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index b8b7306..8c43345 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -67,6 +67,11 @@ struct ptp_clock_request {
  * @gettime64:  Reads the current time from the hardware clock.
  *              parameter ts: Holds the result.
  *
+ * @getsynctime64:  Reads the current time from the hardware clock and system
+ *                  clock simultaneously.
+ *                  parameter dev: Holds the device time
+ *                  parameter sys: Holds the system time
+ *
  * @settime64:  Set the current time on the hardware clock.
  *              parameter ts: Time value to set.
  *
@@ -105,6 +110,8 @@ struct ptp_clock_info {
 	int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
 	int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);
 	int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
+	int (*getsynctime64)(struct ptp_clock_info *ptp, struct timespec64 *dev,
+			struct timespec64 *sys);
 	int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
 	int (*enable)(struct ptp_clock_info *ptp,
 		      struct ptp_clock_request *request, int on);
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index f0b7bfe..ffb2635 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -51,7 +51,9 @@ struct ptp_clock_caps {
 	int n_per_out; /* Number of programmable periodic signals. */
 	int pps;       /* Whether the clock supports a PPS callback. */
 	int n_pins;    /* Number of input/output pins. */
-	int rsv[14];   /* Reserved for future use. */
+	/* Whether the clock supports precise system-device cross timestamps */
+	int cross_timestamping;
+	int rsv[13];   /* Reserved for future use. */
 };
 
 struct ptp_extts_request {
-- 
1.9.1


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

* [PATCH v2 4/4] Added getsynctime64() callback
  2015-08-07 23:01 [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher Hall
                   ` (2 preceding siblings ...)
  2015-08-07 23:01 ` [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl Christopher Hall
@ 2015-08-07 23:01 ` Christopher Hall
  2015-08-10  8:49   ` Richard Cochran
  3 siblings, 1 reply; 10+ messages in thread
From: Christopher Hall @ 2015-08-07 23:01 UTC (permalink / raw)
  To: john.stultz, tglx, richardcochran, mingo, jeffrey.t.kirsher,
	john.ronciak, hpa, x86
  Cc: linux-kernel, netdev, Christopher Hall

Reads ART (TSC correlated clocksource), converts to realtime clock, and
	reports	cross timestamp to PTP driver
---
 drivers/net/ethernet/intel/e1000e/defines.h |  7 +++
 drivers/net/ethernet/intel/e1000e/ptp.c     | 88 +++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/e1000e/regs.h    |  4 ++
 3 files changed, 99 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 133d407..9f16269 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -527,6 +527,13 @@
 #define E1000_RXCW_C          0x20000000        /* Receive config */
 #define E1000_RXCW_SYNCH      0x40000000        /* Receive config synch */
 
+/* HH Time Sync */
+#define E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK   0x0000F000      /* max delay */
+#define E1000_TSYNCTXCTL_SYNC_COMP              0x40000000      /* sync complete
+ */
+#define E1000_TSYNCTXCTL_START_SYNC             0x80000000      /* initiate sync
+ */
+
 #define E1000_TSYNCTXCTL_VALID		0x00000001 /* Tx timestamp valid */
 #define E1000_TSYNCTXCTL_ENABLED	0x00000010 /* enable Tx timestamping */
 
diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index 25a0ad5..c3d80c4 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -25,6 +25,8 @@
  */
 
 #include "e1000.h"
+#include <asm/tsc.h>
+#include <linux/timekeeping.h>
 
 /**
  * e1000e_phc_adjfreq - adjust the frequency of the hardware clock
@@ -98,6 +100,91 @@ static int e1000e_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return 0;
 }
 
+#define HW_WAIT_COUNT (2)
+#define HW_RETRY_COUNT (2)
+
+static int e1000e_phc_get_ts(struct correlated_ts *cts)
+{
+	struct e1000_adapter *adapter = (struct e1000_adapter *) cts->private;
+	struct e1000_hw *hw = &adapter->hw;
+	int i, j;
+	u32 tsync_ctrl;
+	int ret;
+
+	if (hw->mac.type < e1000_pch_spt)
+		return -EOPNOTSUPP;
+
+	for (j = 0; j < HW_RETRY_COUNT; ++j) {
+		tsync_ctrl = er32(TSYNCTXCTL);
+		tsync_ctrl |= E1000_TSYNCTXCTL_START_SYNC |
+			E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK;
+		ew32(TSYNCTXCTL, tsync_ctrl);
+		ret = 0;
+		for (i = 0; i < HW_WAIT_COUNT; ++i) {
+			udelay(2);
+			tsync_ctrl = er32(TSYNCTXCTL);
+			if (tsync_ctrl & E1000_TSYNCTXCTL_SYNC_COMP)
+				break;
+		}
+
+		if (i == HW_WAIT_COUNT) {
+			ret = -ETIMEDOUT;
+		} else if (ret == 0) {
+			cts->system_ts = er32(PLTSTMPH);
+			cts->system_ts <<= 32;
+			cts->system_ts |= er32(PLTSTMPL);
+			cts->device_ts = er32(SYSSTMPH);
+			cts->device_ts <<= 32;
+			cts->device_ts |= er32(SYSSTMPL);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+#define SYNCTIME_RETRY_COUNT (2)
+
+static int e1000e_phc_getsynctime(struct ptp_clock_info *ptp,
+				  struct timespec64 *devts,
+				  struct timespec64 *systs)
+{
+	struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
+						     ptp_clock_info);
+	unsigned long flags;
+	u32 remainder;
+	struct correlated_ts art_correlated_ts;
+	u64 device_time;
+	int i, ret;
+
+	if (!cpu_has_art)
+		return -EOPNOTSUPP;
+
+	for (i = 0; i < SYNCTIME_RETRY_COUNT; ++i) {
+		art_correlated_ts.get_ts = e1000e_phc_get_ts;
+		art_correlated_ts.private = adapter;
+		ret = get_correlated_timestamp(&art_correlated_ts,
+					&art_timestamper);
+		if (ret != 0)
+			continue;
+
+		systs->tv_sec =
+			div_u64_rem(art_correlated_ts.system_real.tv64,
+				NSEC_PER_SEC, &remainder);
+		systs->tv_nsec = remainder;
+		spin_lock_irqsave(&adapter->systim_lock, flags);
+		device_time = timecounter_cyc2time(&adapter->tc,
+					art_correlated_ts.device_ts);
+		spin_unlock_irqrestore(&adapter->systim_lock, flags);
+		devts->tv_sec =
+			div_u64_rem(device_time, NSEC_PER_SEC, &remainder);
+		devts->tv_nsec = remainder;
+		break;
+	}
+
+	return ret;
+}
+
 /**
  * e1000e_phc_gettime - Reads the current time from the hardware clock
  * @ptp: ptp clock structure
@@ -190,6 +277,7 @@ static const struct ptp_clock_info e1000e_ptp_clock_info = {
 	.adjfreq	= e1000e_phc_adjfreq,
 	.adjtime	= e1000e_phc_adjtime,
 	.gettime64	= e1000e_phc_gettime,
+	.getsynctime64	= e1000e_phc_getsynctime,
 	.settime64	= e1000e_phc_settime,
 	.enable		= e1000e_phc_enable,
 };
diff --git a/drivers/net/ethernet/intel/e1000e/regs.h b/drivers/net/ethernet/intel/e1000e/regs.h
index b24e5fe..4dd5b54 100644
--- a/drivers/net/ethernet/intel/e1000e/regs.h
+++ b/drivers/net/ethernet/intel/e1000e/regs.h
@@ -246,6 +246,10 @@
 #define E1000_SYSTIML	0x0B600	/* System time register Low - RO */
 #define E1000_SYSTIMH	0x0B604	/* System time register High - RO */
 #define E1000_TIMINCA	0x0B608	/* Increment attributes register - RW */
+#define E1000_SYSSTMPL  0x0B648 /* HH Timesync system stamp low register */
+#define E1000_SYSSTMPH  0x0B64C /* HH Timesync system stamp hi register */
+#define E1000_PLTSTMPL  0x0B640 /* HH Timesync platform stamp low register */
+#define E1000_PLTSTMPH  0x0B644 /* HH Timesync platform stamp hi register */
 #define E1000_RXMTRL	0x0B634	/* Time sync Rx EtherType and Msg Type - RW */
 #define E1000_RXUDP	0x0B638	/* Time Sync Rx UDP Port - RW */
 
-- 
1.9.1


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

* Re: [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code
  2015-08-07 23:01 ` [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code Christopher Hall
@ 2015-08-07 23:44   ` Andy Lutomirski
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Lutomirski @ 2015-08-07 23:44 UTC (permalink / raw)
  To: Christopher Hall, john.stultz, tglx, richardcochran, mingo,
	jeffrey.t.kirsher, john.ronciak, hpa, x86
  Cc: linux-kernel, netdev

On 08/07/2015 04:01 PM, Christopher Hall wrote:
> Original patch description:
>
> Subject: ptp: Get sync timestamps
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Wed, 29 Jul 2015 10:52:06 +0200
>
> The ART stuff wants to be splitted out.
>
> ======== Changes =======
>
> Add struct correlated_cs (clocksource) with pointer to original clocksource
> 	and function pointer to convert correlated clocksource to the original
>
> Add struct correlated_ts (timestamp) with function pointer to read correlated
> 	clocksource, device and system (in terms of correlated clocksource)
> 	counter values (input) with resulting converted real and monotonic raw
> 	system times (output)
>
> Add get_correlated_timestamp() function which given specific correlated_cs
> 	and correlated_ts convert correlated counter value to system time
>
> Add art_to_tsc conversion function translated Always Running Timer (ART) to
> 	TSC value
> ---
>   arch/x86/kernel/tsc.c       | 31 ++++++++++++++++++++++
>   include/linux/clocksource.h | 30 +++++++++++++++++++++
>   include/linux/timekeeping.h |  4 +++
>   kernel/time/timekeeping.c   | 63 +++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 128 insertions(+)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 7437b41..a90aa6a 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1059,6 +1059,27 @@ int unsynchronized_tsc(void)
>   	return 0;
>   }
>
> +static u32 tsc_numerator;
> +static u32 tsc_denominator;
> +/*
> + * CHECKME: Do we need the adjust value? It should be 0, but if we run
> + * in a VM this might be a different story.
> + */
> +static u64 tsc_adjust;
> +
> +static u64 art_to_tsc(u64 cycles)
> +{
> +	u64 tmp, res = tsc_adjust;
> +
> +	res += (cycles / tsc_denominator) * tsc_numerator;
> +	tmp = (cycles % tsc_denominator) * tsc_numerator;
> +	res += tmp / tsc_denominator;
> +	return res;

Nice trick!

> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 278dd27..2ed3d0c 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -258,4 +258,34 @@ void acpi_generic_timer_init(void);
>   static inline void acpi_generic_timer_init(void) { }
>   #endif
>
> +/**
> + * struct correlated_cs - Descriptor for a clocksource correlated to another clocksource
> + * @related_cs:		Pointer to the related timekeeping clocksource
> + * @convert:		Conversion function to convert a timestamp from
> + *			the correlated clocksource to cycles of the related
> + *			timekeeping clocksource
> + */
> +struct correlated_cs {
> +	struct clocksource	*related_cs;
> +	u64			(*convert)(u64 cycles);

Should the name make it clearer which way it converts?  For example, 
convert_to_related?  We might also want convert_from_related.

--Andy

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

* Re: [PATCH v2 4/4] Added getsynctime64() callback
  2015-08-07 23:01 ` [PATCH v2 4/4] Added getsynctime64() callback Christopher Hall
@ 2015-08-10  8:49   ` Richard Cochran
  2015-08-13 21:10     ` Hall, Christopher S
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Cochran @ 2015-08-10  8:49 UTC (permalink / raw)
  To: Christopher Hall
  Cc: john.stultz, tglx, mingo, jeffrey.t.kirsher, john.ronciak, hpa,
	x86, linux-kernel, netdev

On Fri, Aug 07, 2015 at 04:01:35PM -0700, Christopher Hall wrote:
> --- a/drivers/net/ethernet/intel/e1000e/defines.h
> +++ b/drivers/net/ethernet/intel/e1000e/defines.h
> @@ -527,6 +527,13 @@
>  #define E1000_RXCW_C          0x20000000        /* Receive config */
>  #define E1000_RXCW_SYNCH      0x40000000        /* Receive config synch */
>  
> +/* HH Time Sync */
> +#define E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK   0x0000F000      /* max delay */
> +#define E1000_TSYNCTXCTL_SYNC_COMP              0x40000000      /* sync complete
> + */
> +#define E1000_TSYNCTXCTL_START_SYNC             0x80000000      /* initiate sync
> + */

Split comment looks bad.  Trim this leading space instead.   ^^^^^^

> @@ -98,6 +100,91 @@ static int e1000e_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  	return 0;
>  }
>  
> +#define HW_WAIT_COUNT (2)
> +#define HW_RETRY_COUNT (2)

A busy wait, plus a retry, ...

> +#define SYNCTIME_RETRY_COUNT (2)

plus another retry!

Seems a bit heavy handed to me.  Is the HW really that flakey?

I would expect that a reasonably long polling loop should be
sufficient.  If not, then the HW ignores certain requests, and that is
worth a comment.

In any case, I don't understand why you have two nested retry loops.

> +static int e1000e_phc_getsynctime(struct ptp_clock_info *ptp,
> +				  struct timespec64 *devts,
> +				  struct timespec64 *systs)
> +{
> +	struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
> +						     ptp_clock_info);
> +	unsigned long flags;
> +	u32 remainder;
> +	struct correlated_ts art_correlated_ts;
> +	u64 device_time;
> +	int i, ret;
> +
> +	if (!cpu_has_art)
> +		return -EOPNOTSUPP;

Perform this check before registration, setting .getsynctime64
accordingly.

Thanks,
Richard

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

* Re: [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl
  2015-08-07 23:01 ` [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl Christopher Hall
@ 2015-08-10  8:53   ` Richard Cochran
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Cochran @ 2015-08-10  8:53 UTC (permalink / raw)
  To: Christopher Hall
  Cc: john.stultz, tglx, mingo, jeffrey.t.kirsher, john.ronciak, hpa,
	x86, linux-kernel, netdev

On Fri, Aug 07, 2015 at 04:01:34PM -0700, Christopher Hall wrote:
> This patch allows system and device time ("cross-timestamp") to be
> performed by the driver. Currently, the cross-timestamping is performed
> in the PTP_SYS_OFFSET ioctl.  The PTP clock driver reads gettimeofday()
> and the gettime64() callback provided by the driver. The cross-timestamp
> is best effort where the latency between the capture of system time
> (getnstimeofday()) and the device time (driver callback) may be
> significant.

Acked-by: Richard Cochran <richardcochran@gmail.com>

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

* RE: [PATCH v2 4/4] Added getsynctime64() callback
  2015-08-10  8:49   ` Richard Cochran
@ 2015-08-13 21:10     ` Hall, Christopher S
  2015-08-14  6:32       ` Richard Cochran
  0 siblings, 1 reply; 10+ messages in thread
From: Hall, Christopher S @ 2015-08-13 21:10 UTC (permalink / raw)
  To: 'Richard Cochran'
  Cc: john.stultz, tglx, mingo, Kirsher, Jeffrey T, Ronciak, John, hpa,
	x86, linux-kernel, netdev



> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Monday, August 10, 2015 1:50 AM
> To: Hall, Christopher S
> Cc: john.stultz@linaro.org; tglx@linutronix.de; mingo@redhat.com; Kirsher,
> Jeffrey T; Ronciak, John; hpa@zytor.com; x86@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 4/4] Added getsynctime64() callback
> 
> On Fri, Aug 07, 2015 at 04:01:35PM -0700, Christopher Hall wrote:
> > --- a/drivers/net/ethernet/intel/e1000e/defines.h
> > +++ b/drivers/net/ethernet/intel/e1000e/defines.h
> > @@ -527,6 +527,13 @@
> >  #define E1000_RXCW_C          0x20000000        /* Receive config */
> >  #define E1000_RXCW_SYNCH      0x40000000        /* Receive config synch
> */
> >
> > +/* HH Time Sync */
> > +#define E1000_TSYNCTXCTL_MAX_ALLOWED_DLY_MASK   0x0000F000      /* max
> delay */
> > +#define E1000_TSYNCTXCTL_SYNC_COMP              0x40000000      /* sync
> complete
> > + */
> > +#define E1000_TSYNCTXCTL_START_SYNC             0x80000000      /*
> initiate sync
> > + */
> 
> Split comment looks bad.  Trim this leading space instead.   ^^^^^^

OK.

> 
> > @@ -98,6 +100,91 @@ static int e1000e_phc_adjtime(struct ptp_clock_info
> *ptp, s64 delta)
> >  	return 0;
> >  }
> >
> > +#define HW_WAIT_COUNT (2)
> > +#define HW_RETRY_COUNT (2)
> 
> A busy wait, plus a retry, ...
> 
> > +#define SYNCTIME_RETRY_COUNT (2)
> 
> plus another retry!
> 
> Seems a bit heavy handed to me.  Is the HW really that flakey?
> 
> I would expect that a reasonably long polling loop should be
> sufficient.  If not, then the HW ignores certain requests, and that is
> worth a comment.
> 
> In any case, I don't understand why you have two nested retry loops.

The retry in get_synctime() is a left over from the previous patch.  It's not necessary,
the current timekeeping code won't fail in a way necessitating a retry.  It's removed.

The inner retry loop is due to huge inaccuracies in udelay().  I've done some testing
and it appears udelay(2) actually results in about an 8 microsecond delay.  On
Skylake the time for completion of the cross timestamp should be about 2 microseconds.
If we eliminate the inner most loop we either spin for too long or possibly risk not
waiting long enough.  Are there any guarantees for udelay()?

As for HW_RETRY_LOOP, I will confirm whether this is necessary.  It was in reference
code I was given, but, I agree, it seems odd.

> 
> > +static int e1000e_phc_getsynctime(struct ptp_clock_info *ptp,
> > +				  struct timespec64 *devts,
> > +				  struct timespec64 *systs)
> > +{
> > +	struct e1000_adapter *adapter = container_of(ptp, struct
> e1000_adapter,
> > +						     ptp_clock_info);
> > +	unsigned long flags;
> > +	u32 remainder;
> > +	struct correlated_ts art_correlated_ts;
> > +	u64 device_time;
> > +	int i, ret;
> > +
> > +	if (!cpu_has_art)
> > +		return -EOPNOTSUPP;
> 
> Perform this check before registration, setting .getsynctime64
> accordingly.

The problem here is that ART initialization doesn't happen until we install TSC as a clocksource.  This design is per Thomas' suggestion.  That occurs after the driver is loaded (as a module).

In my somewhat limited testing, it's about 400 ms later.  The problem is the several seconds of TSC frequency refinement.  I, in principle, agree, but we either need to move the ART initialization earlier (probably split it) or defer PTP clock initialization in the driver.

> 
> Thanks,
> Richard

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

* Re: [PATCH v2 4/4] Added getsynctime64() callback
  2015-08-13 21:10     ` Hall, Christopher S
@ 2015-08-14  6:32       ` Richard Cochran
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Cochran @ 2015-08-14  6:32 UTC (permalink / raw)
  To: Hall, Christopher S
  Cc: john.stultz, tglx, mingo, Kirsher, Jeffrey T, Ronciak, John, hpa,
	x86, linux-kernel, netdev

On Thu, Aug 13, 2015 at 09:10:36PM +0000, Hall, Christopher S wrote:
> > > +	if (!cpu_has_art)
> > > +		return -EOPNOTSUPP;
> > 
> > Perform this check before registration, setting .getsynctime64
> > accordingly.
> 
> The problem here is that ART initialization doesn't happen until we
> install TSC as a clocksource.  This design is per Thomas'
> suggestion.  That occurs after the driver is loaded (as a module).

So that 'cpu_has_art' actually means 'cpu_has_art_and_has_been_initialized'?

In any case, returning EOPNOTSUPP early on, but OK later seems mean to
me.  If the clocks aren't ready yet, the error should be EBUSY so that
user space knows it can try again.

Thanks,
Richard

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

end of thread, other threads:[~2015-08-14  6:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-07 23:01 [PATCH v2 0/4] Patchset enabling hardware based cross-timestamps for next gen Intel platforms Christopher Hall
2015-08-07 23:01 ` [PATCH v2 1/4] Add generic correlated clocksource code and ART to TSC conversion code Christopher Hall
2015-08-07 23:44   ` Andy Lutomirski
2015-08-07 23:01 ` [PATCH v2 2/4] Add ART initialization code Christopher Hall
2015-08-07 23:01 ` [PATCH v2 3/4] Add support for driver cross-timestamp to PTP_SYS_OFFSET ioctl Christopher Hall
2015-08-10  8:53   ` Richard Cochran
2015-08-07 23:01 ` [PATCH v2 4/4] Added getsynctime64() callback Christopher Hall
2015-08-10  8:49   ` Richard Cochran
2015-08-13 21:10     ` Hall, Christopher S
2015-08-14  6:32       ` 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).