From 8857ad983400d8fad62ddd95d2b10e7616df0668 Mon Sep 17 00:00:00 2001
From: Richard Cochran <richardcochran@gmail.com>
Date: Fri, 2 Mar 2018 18:59:47 -0800
Subject: [PATCH] Expand and rationalize time stamping options.

Up until now, one step operation was controlled by a combination of
options, namely time_stamping=hardware with twoStepFlag=0.  The
introduction of peer to peer one step makes the situation a bit more
complicated.

This patch adds support for setting the one step options directly by
using the "time_stamping" option.  The "twoStepFlag" is adjusted
automatically if needed.  The legacy behavior for Sync message one
step is preserved.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 clock.c  | 42 +++++++++++++++++-------------------------
 config.c | 40 ++++++++++++++++++++++++++++++++++++++++
 config.h |  2 ++
 3 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/clock.c b/clock.c
index 9f9cfdf..4bd20c5 100644
--- a/clock.c
+++ b/clock.c
@@ -848,10 +848,9 @@ static void ensure_ts_label(struct interface *iface)
 struct clock *clock_create(enum clock_type type, struct config *config,
 			   const char *phc_device)
 {
-	enum timestamp_type timestamping =
-		config_get_int(config, NULL, "time_stamping");
-	int fadj = 0, max_adj = 0, sw_ts = timestamping == TS_SOFTWARE ? 1 : 0;
 	enum servo_type servo = config_get_int(config, NULL, "clock_servo");
+	enum timestamp_type timestamping;
+	int fadj = 0, max_adj = 0, sw_ts;
 	int phc_index, required_modes = 0;
 	struct clock *c = &the_clock;
 	struct port *p;
@@ -919,12 +918,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
 	if (config_get_int(config, NULL, "slaveOnly")) {
 		c->dds.flags |= DDS_SLAVE_ONLY;
 	}
-	if (config_get_int(config, NULL, "twoStepFlag")) {
-		c->dds.flags |= DDS_TWO_STEP_FLAG;
-	}
-	c->dds.priority1 = config_get_int(config, NULL, "priority1");
-	c->dds.priority2 = config_get_int(config, NULL, "priority2");
-
 	if (!config_get_int(config, NULL, "gmCapable") &&
 	    c->dds.flags & DDS_SLAVE_ONLY) {
 		pr_err("Cannot mix 1588 slaveOnly with 802.1AS !gmCapable");
@@ -935,23 +928,22 @@ struct clock *clock_create(enum clock_type type, struct config *config,
 		c->dds.clockQuality.clockClass = 255;
 	}
 
-	if (!(c->dds.flags & DDS_TWO_STEP_FLAG)) {
-		switch (timestamping) {
-		case TS_SOFTWARE:
-		case TS_LEGACY_HW:
-			pr_err("one step is only possible "
-			       "with hardware time stamping");
-			return NULL;
-		case TS_HARDWARE:
-			timestamping = TS_ONESTEP;
-			if (config_set_int(config, "time_stamping", TS_ONESTEP))
-				return NULL;
-			break;
-		case TS_ONESTEP:
-		case TS_P2P1STEP:
-			break;
-		}
+	/* Harmonize the twoStepFlag with the time_stamping option. */
+	if (config_harmonize_onestep(config)) {
+		return NULL;
 	}
+	if (config_get_int(config, NULL, "twoStepFlag")) {
+		c->dds.flags |= DDS_TWO_STEP_FLAG;
+	}
+	timestamping = config_get_int(config, NULL, "time_stamping");
+	if (timestamping == TS_SOFTWARE) {
+		sw_ts = 1;
+	} else {
+		sw_ts = 0;
+	}
+
+	c->dds.priority1 = config_get_int(config, NULL, "priority1");
+	c->dds.priority2 = config_get_int(config, NULL, "priority2");
 
 	/* Check the time stamping mode on each interface. */
 	c->timestamping = timestamping;
diff --git a/config.c b/config.c
index 00c76c3..320cc1b 100644
--- a/config.c
+++ b/config.c
@@ -157,6 +157,8 @@ static struct config_enum timestamping_enu[] = {
 	{ "hardware", TS_HARDWARE  },
 	{ "software", TS_SOFTWARE  },
 	{ "legacy",   TS_LEGACY_HW },
+	{ "onestep",  TS_ONESTEP   },
+	{ "p2p1step", TS_P2P1STEP  },
 	{ NULL, 0 },
 };
 
@@ -757,6 +759,44 @@ char *config_get_string(struct config *cfg, const char *section,
 	return ci->val.s;
 }
 
+int config_harmonize_onestep(struct config *cfg)
+{
+	enum timestamp_type tstype = config_get_int(cfg, NULL, "time_stamping");
+	int two_step_flag = config_get_int(cfg, NULL, "twoStepFlag");
+
+	switch (tstype) {
+	case TS_SOFTWARE:
+	case TS_LEGACY_HW:
+		if (!two_step_flag) {
+			pr_err("one step is only possible "
+			       "with hardware time stamping");
+			return -1;
+		}
+		break;
+	case TS_HARDWARE:
+		if (!two_step_flag) {
+			pr_debug("upgrading to one step time stamping "
+				 "in order to match the twoStepFlag");
+			if (config_set_int(cfg, "time_stamping", TS_ONESTEP)) {
+				return -1;
+			}
+		}
+		break;
+	case TS_ONESTEP:
+	case TS_P2P1STEP:
+		if (two_step_flag) {
+			pr_debug("one step mode implies twoStepFlag=0, "
+				 "clearing twoStepFlag to match");
+			if (config_set_int(cfg, "twoStepFlag", 0)) {
+				return -1;
+			}
+		}
+		break;
+	}
+
+	return 0;
+}
+
 int config_parse_option(struct config *cfg, const char *opt, const char *val)
 {
 	enum parser_result result;
diff --git a/config.h b/config.h
index fcc1c42..b7f6592 100644
--- a/config.h
+++ b/config.h
@@ -73,6 +73,8 @@ int config_get_int(struct config *cfg, const char *section,
 char *config_get_string(struct config *cfg, const char *section,
 			const char *option);
 
+int config_harmonize_onestep(struct config *cfg);
+
 static inline struct option *config_long_options(struct config *cfg)
 {
 	return cfg->opts;