bugfix: correctionField conversion

I initially looked at int64_to_TimeInternal to swap arguments, since to_TimeInternal as timeinternal (the output) as first argument, and they appeared in pairs. However, int64_to_TimeInternal is only used for correction fields, and they are scaled nanoseconds (page 126), not nanoseconds. Thus, this commit fixes the conversion and renames the function to "cField_to_TimeInternal" to better tell the reader what it is. Signed-off-by: Alessandro Rubini <rubini@gnudd.com>

bugfix: correctionField conversion
I initially looked at int64_to_TimeInternal to swap arguments, since to_TimeInternal as timeinternal (the output) as first argument, and they appeared in pairs. However, int64_to_TimeInternal is only used for correction fields, and they are scaled nanoseconds (page 126), not nanoseconds. Thus, this commit fixes the conversion and renames the function to "cField_to_TimeInternal" to better tell the reader what it is. Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
a1b8d474 · Alessandro Rubini · d0452c0d · a1b8d474 · a1b8d474 · a1b8d474
Commit a1b8d474 authored Mar 17, 2013 by Alessandro Rubini
5 changed files
--- a/include/ppsi/ppsi.h
+++ b/include/ppsi/ppsi.h
@@ -263,7 +263,7 @@ extern int msg_issue_delay_resp(struct pp_instance *ppi, TimeInternal *time);
 /* Functions for timestamp handling (internal to protocol format conversion*/
 /* FIXME: add prefix in function name? */
-extern void int64_to_TimeInternal(Integer64 bigint, TimeInternal *internal);
+extern void cField_to_TimeInternal(TimeInternal *internal, Integer64 bigint);
 extern int from_TimeInternal(TimeInternal *internal, Timestamp *external);
 extern int to_TimeInternal(TimeInternal *internal, Timestamp *external);
 extern void add_TimeInternal(TimeInternal *r, TimeInternal *x, TimeInternal *y);

--- a/proto-ext-whiterabbit/hooks.c
+++ b/proto-ext-whiterabbit/hooks.c
@@ -80,7 +80,8 @@ static int wr_update_delay(struct pp_instance *ppi)
 	MsgHeader *hdr = &ppi->received_ptp_header;
 	TimeInternal correction_field;
-	int64_to_TimeInternal(hdr->correctionfield, &correction_field);
+	/* FIXME: check sub-nano relevance of correction filed */
+	cField_to_TimeInternal(&correction_field, hdr->correctionfield);
 	/* If no WR mode is on, run normal code */
 	if (!WR_DSPOR(ppi)->wrModeOn) {

--- a/proto-standard/arith.c
+++ b/proto-standard/arith.c
@@ -6,19 +6,29 @@
 #include <limits.h>
 #include <ppsi/ppsi.h>
-void int64_to_TimeInternal(Integer64 bigint, TimeInternal *internal)
+void cField_to_TimeInternal(TimeInternal *internal, Integer64 cField)
 {
-	uint64_t bigint_val;
+	uint64_t i64;
-	if (bigint.msb < 0)
+	i64 = cField.lsb;
-		PP_PRINTF("BUG: %s doesn't support negatives\n", __func__);
+	i64 |= ((int64_t)cField.msb) << 32;
-	bigint_val = bigint.lsb;
+	if ((int32_t)cField.msb < 0)
-	bigint_val += ((int64_t)bigint.msb) << 32;
+		pp_error("BUG: %s doesn't support negatives\n", __func__);
+	/*
+	 * the correctionField is nanoseconds scaled by 16 bits.
+	 * It is updated by transparent clocks and may be used to count
+	 * for asymmetry. Since we support no better than nanosecond with
+	 * the standard protocol and WR (which is better than nanosecond)
+	 * doesn't use this field, just approximate to nanoseconds.
+	 * and the WR extension uses its own methods for asymmetry,
+	 */
+	i64 += 0x8000;
+	i64 >>= 16;
 	/* Use __div64_32 from library, to avoid libgcc on small targets */
-	internal->nanoseconds = __div64_32(&bigint_val, PP_NSEC_PER_SEC);
+	internal->nanoseconds = __div64_32(&i64, PP_NSEC_PER_SEC);
-	internal->seconds = bigint_val;
+	internal->seconds = i64;
 }
 int from_TimeInternal(TimeInternal *internal, Timestamp *external)

--- a/proto-standard/common-fun.c
+++ b/proto-standard/common-fun.c
@@ -151,15 +151,13 @@ int st_com_slave_handle_sync(struct pp_instance *ppi, unsigned char *buf,
 		ppi->waiting_for_follow = TRUE;
 		ppi->recv_sync_sequence_id = hdr->sequenceId;
 		/* Save correctionField of Sync message */
-		int64_to_TimeInternal(
+		cField_to_TimeInternal(&ppi->last_sync_corr_field,
-			hdr->correctionfield,
+				      hdr->correctionfield);
-			&ppi->last_sync_corr_field);
 		return 0;
 	}
 	msg_unpack_sync(buf, &sync);
-	int64_to_TimeInternal(
+	cField_to_TimeInternal(&correction_field,
-		ppi->received_ptp_header.correctionfield,
+			      ppi->received_ptp_header.correctionfield);
-		&correction_field);
 	display_TimeInternal("Correction field",
 			     &correction_field);
@@ -207,9 +205,8 @@ int st_com_slave_handle_followup(struct pp_instance *ppi, unsigned char *buf,
 	ppi->waiting_for_follow = FALSE;
 	to_TimeInternal(&ppi->t1, &follow.preciseOriginTimestamp);
-	int64_to_TimeInternal(ppi->received_ptp_header.correctionfield,
+	cField_to_TimeInternal(&correction_field,
-					&correction_field);
+			      ppi->received_ptp_header.correctionfield);
 	add_TimeInternal(&correction_field, &correction_field,
 		&ppi->last_sync_corr_field);

--- a/proto-standard/state-slave.c
+++ b/proto-standard/state-slave.c
@@ -71,9 +71,8 @@ int pp_slave(struct pp_instance *ppi, unsigned char *pkt, int plen)
 			to_TimeInternal(&ppi->t4, &resp.receiveTimestamp);
-			int64_to_TimeInternal(
+			cField_to_TimeInternal(&correction_field,
-				hdr->correctionfield,
+					      hdr->correctionfield);
-				&correction_field);
 			if (pp_hooks.update_delay)
 				e = pp_hooks.update_delay(ppi);