[Linuxptp-devel] [PATCH v5 07/13] ts2phc: instantiate a full clock structure for every PPS source of the PHC kind

[Vladimir O. <ol...@gm...>]

This propagates the use of "struct ts2phc_private" all the way into the
PPS source API, in preparation of a new use case that will be supported
soon: some PPS sources (to be precise, the "PHC" kind) instantiate a
struct clock which could be disciplined by ts2phc.

When a PHC A emits a pulse and another PHC B timestamps it, the offset
between their precise timestamps can be used to synchronize either one
of them. So far in ts2phc, only the sink PHC (the one using extts) has
been synchronized to the source (the one using perout).

This is partly because there is no proper kernel API to report the
precise timestamp of a perout pulse. We only have the periodic API, and
that doesn't report precise timestamps either; we just use vague
approximations of what the PPS source PHC's time was, based on reading
that PHC immediately after a sink extts event was received by the
application. While this is far from ideal, it does work, and does allow
PHC A to be synchronized to B.

This is particularly useful with the yet-to-be-introduced "automatic"
mode of ts2phc (similar to '-a' of phc2sys), and the PPS distribution
tree is fixed in hardware (as opposed to port states, which in
"automatic" mode are dynamic, as the name suggests).

Signed-off-by: Vladimir Oltean <ol...@gm...>
---
 ts2phc.c                    |  2 +-
 ts2phc.h                    |  1 +
 ts2phc_generic_pps_source.c |  2 +-
 ts2phc_generic_pps_source.h |  3 ++-
 ts2phc_nmea_pps_source.c    |  6 +++---
 ts2phc_nmea_pps_source.h    |  3 ++-
 ts2phc_phc_pps_source.c     | 32 ++++++++++++++++----------------
 ts2phc_phc_pps_source.h     |  3 ++-
 ts2phc_pps_sink.c           |  2 +-
 ts2phc_pps_source.c         | 10 ++++++----
 ts2phc_pps_source.h         |  6 ++++--
 11 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/ts2phc.c b/ts2phc.c
index f63e5503637d..64472a4bb15c 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -270,7 +270,7 @@ int main(int argc, char *argv[])
 	} else {
 		pps_type = TS2PHC_PPS_SOURCE_PHC;
 	}
-	priv.src = ts2phc_pps_source_create(cfg, pps_source, pps_type);
+	priv.src = ts2phc_pps_source_create(&priv, pps_source, pps_type);
 	if (!priv.src) {
 		fprintf(stderr, "failed to create PPS source\n");
 		ts2phc_cleanup(&priv);
diff --git a/ts2phc.h b/ts2phc.h
index 043215a51609..a633492b5306 100644
--- a/ts2phc.h
+++ b/ts2phc.h
@@ -7,6 +7,7 @@
 #ifndef HAVE_TS2PHC_H
 #define HAVE_TS2PHC_H
 
+#include <stdbool.h>
 #include <sys/queue.h>
 #include <time.h>
 #include "servo.h"
diff --git a/ts2phc_generic_pps_source.c b/ts2phc_generic_pps_source.c
index 397031fe2521..6842d8e0e30a 100644
--- a/ts2phc_generic_pps_source.c
+++ b/ts2phc_generic_pps_source.c
@@ -47,7 +47,7 @@ static int ts2phc_generic_pps_source_getppstime(struct ts2phc_pps_source *src,
 	return 0;
 }
 
-struct ts2phc_pps_source *ts2phc_generic_pps_source_create(struct config *cfg,
+struct ts2phc_pps_source *ts2phc_generic_pps_source_create(struct ts2phc_private *priv,
 							   const char *dev)
 {
 	struct ts2phc_generic_pps_source *src;
diff --git a/ts2phc_generic_pps_source.h b/ts2phc_generic_pps_source.h
index e169ab380ccf..bed62740059e 100644
--- a/ts2phc_generic_pps_source.h
+++ b/ts2phc_generic_pps_source.h
@@ -6,9 +6,10 @@
 #ifndef HAVE_TS2PHC_GENERIC_PPS_SOURCE_H
 #define HAVE_TS2PHC_GENERIC_PPS_SOURCE_H
 
+#include "ts2phc.h"
 #include "ts2phc_pps_source.h"
 
-struct ts2phc_pps_source *ts2phc_generic_pps_source_create(struct config *cfg,
+struct ts2phc_pps_source *ts2phc_generic_pps_source_create(struct ts2phc_private *priv,
 							   const char *dev);
 
 #endif
diff --git a/ts2phc_nmea_pps_source.c b/ts2phc_nmea_pps_source.c
index 52a96955f3ce..23aeb3840f40 100644
--- a/ts2phc_nmea_pps_source.c
+++ b/ts2phc_nmea_pps_source.c
@@ -249,7 +249,7 @@ static int ts2phc_nmea_pps_source_getppstime(struct ts2phc_pps_source *src,
 	return lstab_error;
 }
 
-struct ts2phc_pps_source *ts2phc_nmea_pps_source_create(struct config *cfg,
+struct ts2phc_pps_source *ts2phc_nmea_pps_source_create(struct ts2phc_private *priv,
 							const char *dev)
 {
 	struct ts2phc_nmea_pps_source *s;
@@ -260,7 +260,7 @@ struct ts2phc_pps_source *ts2phc_nmea_pps_source_create(struct config *cfg,
 	if (!s) {
 		return NULL;
 	}
-	s->leapfile = config_get_string(cfg, NULL, "leapfile");
+	s->leapfile = config_get_string(priv->cfg, NULL, "leapfile");
 	s->lstab = lstab_create(s->leapfile);
 	if (!s->lstab) {
 		free(s);
@@ -277,7 +277,7 @@ struct ts2phc_pps_source *ts2phc_nmea_pps_source_create(struct config *cfg,
 	}
 	s->pps_source.destroy = ts2phc_nmea_pps_source_destroy;
 	s->pps_source.getppstime = ts2phc_nmea_pps_source_getppstime;
-	s->config = cfg;
+	s->config = priv->cfg;
 	pthread_mutex_init(&s->mutex, NULL);
 	err = pthread_create(&s->worker, NULL, monitor_nmea_status, s);
 	if (err) {
diff --git a/ts2phc_nmea_pps_source.h b/ts2phc_nmea_pps_source.h
index c14d8d741bb6..4e68e7e3d02e 100644
--- a/ts2phc_nmea_pps_source.h
+++ b/ts2phc_nmea_pps_source.h
@@ -6,8 +6,9 @@
 #ifndef HAVE_TS2PHC_NMEA_PPS_SOURCE_H
 #define HAVE_TS2PHC_NMEA_PPS_SOURCE_H
 
+#include "ts2phc.h"
 #include "ts2phc_pps_source.h"
 
-struct ts2phc_pps_source *ts2phc_nmea_pps_source_create(struct config *cfg,
+struct ts2phc_pps_source *ts2phc_nmea_pps_source_create(struct ts2phc_private *priv,
 							const char *dev);
 #endif
diff --git a/ts2phc_phc_pps_source.c b/ts2phc_phc_pps_source.c
index ffb96ef7299a..fca653f99499 100644
--- a/ts2phc_phc_pps_source.c
+++ b/ts2phc_phc_pps_source.c
@@ -12,15 +12,14 @@
 #include "phc.h"
 #include "print.h"
 #include "missing.h"
+#include "ts2phc.h"
 #include "ts2phc_pps_source_private.h"
-#include "ts2phc_phc_pps_source.h"
 #include "util.h"
 
 struct ts2phc_phc_pps_source {
 	struct ts2phc_pps_source pps_source;
-	clockid_t clkid;
+	struct ts2phc_clock *clock;
 	int channel;
-	int fd;
 };
 
 static int ts2phc_phc_pps_source_activate(struct config *cfg, const char *dev,
@@ -38,10 +37,10 @@ static int ts2phc_phc_pps_source_activate(struct config *cfg, const char *dev,
 	desc.func = PTP_PF_PEROUT;
 	desc.chan = s->channel;
 
-	if (phc_pin_setfunc(s->clkid, &desc)) {
+	if (phc_pin_setfunc(s->clock->clkid, &desc)) {
 		pr_warning("Failed to set the pin. Continuing bravely on...");
 	}
-	if (clock_gettime(s->clkid, &ts)) {
+	if (clock_gettime(s->clock->clkid, &ts)) {
 		perror("clock_gettime");
 		return -1;
 	}
@@ -52,7 +51,8 @@ static int ts2phc_phc_pps_source_activate(struct config *cfg, const char *dev,
 	perout_request.period.sec = 1;
 	perout_request.period.nsec = 0;
 
-	if (ioctl(s->fd, PTP_PEROUT_REQUEST2, &perout_request)) {
+	if (ioctl(CLOCKID_TO_FD(s->clock->clkid), PTP_PEROUT_REQUEST2,
+		  &perout_request)) {
 		pr_err(PTP_PEROUT_REQUEST_FAILED);
 		return -1;
 	}
@@ -67,10 +67,11 @@ static void ts2phc_phc_pps_source_destroy(struct ts2phc_pps_source *src)
 
 	memset(&perout_request, 0, sizeof(perout_request));
 	perout_request.index = m->channel;
-	if (ioctl(m->fd, PTP_PEROUT_REQUEST2, &perout_request)) {
+	if (ioctl(CLOCKID_TO_FD(m->clock->clkid), PTP_PEROUT_REQUEST2,
+		  &perout_request)) {
 		pr_err(PTP_PEROUT_REQUEST_FAILED);
 	}
-	posix_clock_close(m->clkid);
+	ts2phc_clock_destroy(m->clock);
 	free(m);
 }
 
@@ -79,14 +80,13 @@ static int ts2phc_phc_pps_source_getppstime(struct ts2phc_pps_source *src,
 {
 	struct ts2phc_phc_pps_source *s =
 		container_of(src, struct ts2phc_phc_pps_source, pps_source);
-	return clock_gettime(s->clkid, ts);
+	return clock_gettime(s->clock->clkid, ts);
 }
 
-struct ts2phc_pps_source *ts2phc_phc_pps_source_create(struct config *cfg,
+struct ts2phc_pps_source *ts2phc_phc_pps_source_create(struct ts2phc_private *priv,
 						       const char *dev)
 {
 	struct ts2phc_phc_pps_source *s;
-	int junk;
 
 	s = calloc(1, sizeof(*s));
 	if (!s) {
@@ -95,16 +95,16 @@ struct ts2phc_pps_source *ts2phc_phc_pps_source_create(struct config *cfg,
 	s->pps_source.destroy = ts2phc_phc_pps_source_destroy;
 	s->pps_source.getppstime = ts2phc_phc_pps_source_getppstime;
 
-	s->clkid = posix_clock_open(dev, &junk);
-	if (s->clkid == CLOCK_INVALID) {
+	s->clock = ts2phc_clock_add(priv, dev);
+	if (!s->clock) {
 		free(s);
 		return NULL;
 	}
-	s->fd = CLOCKID_TO_FD(s->clkid);
 
-	pr_debug("PHC PPS source %s has ptp index %d", dev, junk);
+	pr_debug("PHC PPS source %s has ptp index %d", dev,
+		 s->clock->phc_index);
 
-	if (ts2phc_phc_pps_source_activate(cfg, dev, s)) {
+	if (ts2phc_phc_pps_source_activate(priv->cfg, dev, s)) {
 		ts2phc_phc_pps_source_destroy(&s->pps_source);
 		return NULL;
 	}
diff --git a/ts2phc_phc_pps_source.h b/ts2phc_phc_pps_source.h
index c9ab54ecd259..d2045eddef2d 100644
--- a/ts2phc_phc_pps_source.h
+++ b/ts2phc_phc_pps_source.h
@@ -6,9 +6,10 @@
 #ifndef HAVE_TS2PHC_PHC_PPS_SOURCE_H
 #define HAVE_TS2PHC_PHC_PPS_SOURCE_H
 
+#include "ts2phc.h"
 #include "ts2phc_pps_source.h"
 
-struct ts2phc_pps_source *ts2phc_phc_pps_source_create(struct config *cfg,
+struct ts2phc_pps_source *ts2phc_phc_pps_source_create(struct ts2phc_private *priv,
 						       const char *dev);
 
 #endif
diff --git a/ts2phc_pps_sink.c b/ts2phc_pps_sink.c
index e272155eb73d..729aac8f570f 100644
--- a/ts2phc_pps_sink.c
+++ b/ts2phc_pps_sink.c
@@ -15,8 +15,8 @@
 #include <time.h>
 #include <unistd.h>
 
-#include "config.h"
 #include "clockadj.h"
+#include "config.h"
 #include "missing.h"
 #include "phc.h"
 #include "print.h"
diff --git a/ts2phc_pps_source.c b/ts2phc_pps_source.c
index ab8e899b962b..2a3300aaaf19 100644
--- a/ts2phc_pps_source.c
+++ b/ts2phc_pps_source.c
@@ -3,25 +3,27 @@
  * @note Copyright (C) 2019 Richard Cochran <ric...@gm...>
  * @note SPDX-License-Identifier: GPL-2.0+
  */
+#include "ts2phc.h"
 #include "ts2phc_generic_pps_source.h"
 #include "ts2phc_pps_source_private.h"
 #include "ts2phc_nmea_pps_source.h"
 #include "ts2phc_phc_pps_source.h"
 
-struct ts2phc_pps_source *ts2phc_pps_source_create(struct config *cfg, const char *dev,
+struct ts2phc_pps_source *ts2phc_pps_source_create(struct ts2phc_private *priv,
+						   const char *dev,
 						   enum ts2phc_pps_source_type type)
 {
 	struct ts2phc_pps_source *src = NULL;
 
 	switch (type) {
 	case TS2PHC_PPS_SOURCE_GENERIC:
-		src = ts2phc_generic_pps_source_create(cfg, dev);
+		src = ts2phc_generic_pps_source_create(priv, dev);
 		break;
 	case TS2PHC_PPS_SOURCE_NMEA:
-		src = ts2phc_nmea_pps_source_create(cfg, dev);
+		src = ts2phc_nmea_pps_source_create(priv, dev);
 		break;
 	case TS2PHC_PPS_SOURCE_PHC:
-		src = ts2phc_phc_pps_source_create(cfg, dev);
+		src = ts2phc_phc_pps_source_create(priv, dev);
 		break;
 	}
 	return src;
diff --git a/ts2phc_pps_source.h b/ts2phc_pps_source.h
index 1e1f46f70915..349c44ed0618 100644
--- a/ts2phc_pps_source.h
+++ b/ts2phc_pps_source.h
@@ -13,6 +13,7 @@ struct config;
 /**
  * Opaque type
  */
+struct ts2phc_private;
 struct ts2phc_pps_source;
 
 /**
@@ -26,12 +27,13 @@ enum ts2phc_pps_source_type {
 
 /**
  * Create a new instance of a PPS source.
- * @param cfg	Pointer to a valid configuration.
+ * @param priv	Pointer to the program's data structure.
  * @param dev   Name of the source or NULL.
  * @param type	The type of the clock to create.
  * @return	A pointer to a new PPS source on success, NULL otherwise.
  */
-struct ts2phc_pps_source *ts2phc_pps_source_create(struct config *cfg, const char *dev,
+struct ts2phc_pps_source *ts2phc_pps_source_create(struct ts2phc_private *priv,
+						   const char *dev,
 						   enum ts2phc_pps_source_type type);
 
 /**
-- 
2.25.1