From 74ab3b3728d9d261f9e86dfdf43c5cbff0abcd58 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sat, 8 Apr 2017 20:35:58 +0200 Subject: [PATCH 1/3] tsproc: Track the validity of the filtered delay explicitly. We will want to test whether a valid filtered delay value has been calculated or not. However, we cannot simply test for zero since that is a legitimate possible delay value. This patch adds a flag that reflects the state of the filtered delay field. Signed-off-by: Richard Cochran --- tsproc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tsproc.c b/tsproc.c index cf5f0dc..1eb24a1 100644 --- a/tsproc.c +++ b/tsproc.c @@ -42,6 +42,7 @@ struct tsproc { /* Current filtered delay */ tmv_t filtered_delay; + int filtered_delay_valid; /* Delay filter */ struct filter *delay_filter; @@ -115,6 +116,7 @@ void tsproc_set_clock_rate_ratio(struct tsproc *tsp, double clock_rate_ratio) void tsproc_set_delay(struct tsproc *tsp, tmv_t delay) { tsp->filtered_delay = delay; + tsp->filtered_delay_valid = 1; } tmv_t get_raw_delay(struct tsproc *tsp) @@ -149,6 +151,7 @@ int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay) raw_delay = get_raw_delay(tsp); tsp->filtered_delay = filter_sample(tsp->delay_filter, raw_delay); + tsp->filtered_delay_valid = 1; pr_debug("delay filtered %10" PRId64 " raw %10" PRId64, tsp->filtered_delay, raw_delay); @@ -199,5 +202,6 @@ void tsproc_reset(struct tsproc *tsp, int full) if (full) { tsp->clock_rate_ratio = 1.0; filter_reset(tsp->delay_filter); + tsp->filtered_delay_valid = 0; } } From a0cbeb736750105b630ae54b599adac9b21f0e56 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sat, 8 Apr 2017 20:17:49 +0200 Subject: [PATCH 2/3] tsproc: Clarify the internal mode handling. The time stamp processor features four modes resulting from the combination of two independent Boolean options. Even though we have a proper enumerated type to represent the mode, still the code coverts the mode into two variables. The tests on the variables are currently simple enough, but soon we will want to add some more complexity. In the interest of clarity, this patch converts the paired Boolean tests into a switch/case pattern. No functional change. Signed-off-by: Richard Cochran --- tsproc.c | 64 ++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/tsproc.c b/tsproc.c index 1eb24a1..550f32c 100644 --- a/tsproc.c +++ b/tsproc.c @@ -26,8 +26,7 @@ struct tsproc { /* Processing options */ - int raw_mode; - int weighting; + enum tsproc_mode mode; /* Current ratio between remote and local clock frequency */ double clock_rate_ratio; @@ -48,6 +47,19 @@ struct tsproc { struct filter *delay_filter; }; +static int weighting(struct tsproc *tsp) +{ + switch (tsp->mode) { + case TSPROC_FILTER: + case TSPROC_RAW: + return 0; + case TSPROC_FILTER_WEIGHT: + case TSPROC_RAW_WEIGHT: + return 1; + } + return 0; +} + struct tsproc *tsproc_create(enum tsproc_mode mode, enum filter_type delay_filter, int filter_length) { @@ -59,20 +71,10 @@ struct tsproc *tsproc_create(enum tsproc_mode mode, switch (mode) { case TSPROC_FILTER: - tsp->raw_mode = 0; - tsp->weighting = 0; - break; case TSPROC_RAW: - tsp->raw_mode = 1; - tsp->weighting = 0; - break; case TSPROC_FILTER_WEIGHT: - tsp->raw_mode = 0; - tsp->weighting = 1; - break; case TSPROC_RAW_WEIGHT: - tsp->raw_mode = 1; - tsp->weighting = 1; + tsp->mode = mode; break; default: free(tsp); @@ -156,24 +158,46 @@ int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay) pr_debug("delay filtered %10" PRId64 " raw %10" PRId64, tsp->filtered_delay, raw_delay); - if (delay) - *delay = tsp->raw_mode ? raw_delay : tsp->filtered_delay; + if (!delay) { + return 0; + } + + switch (tsp->mode) { + case TSPROC_FILTER: + case TSPROC_FILTER_WEIGHT: + *delay = tsp->filtered_delay; + break; + case TSPROC_RAW: + case TSPROC_RAW_WEIGHT: + *delay = raw_delay; + break; + } return 0; } int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset, double *weight) { - tmv_t delay, raw_delay = 0; + tmv_t delay = 0, raw_delay = 0; if (tmv_is_zero(tsp->t1) || tmv_is_zero(tsp->t2) || tmv_is_zero(tsp->t3)) return -1; - if (tsp->raw_mode || tsp->weighting) + switch (tsp->mode) { + case TSPROC_FILTER: + delay = tsp->filtered_delay; + break; + case TSPROC_RAW: + case TSPROC_RAW_WEIGHT: raw_delay = get_raw_delay(tsp); - - delay = tsp->raw_mode ? raw_delay : tsp->filtered_delay; + delay = raw_delay; + break; + case TSPROC_FILTER_WEIGHT: + raw_delay = get_raw_delay(tsp); + delay = tsp->filtered_delay; + break; + } /* offset = t2 - t1 - delay */ *offset = tmv_sub(tmv_sub(tsp->t2, tsp->t1), delay); @@ -181,7 +205,7 @@ int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset, double *weight) if (!weight) return 0; - if (tsp->weighting && tsp->filtered_delay > 0 && raw_delay > 0) { + if (weighting(tsp) && tsp->filtered_delay > 0 && raw_delay > 0) { *weight = (double)tsp->filtered_delay / raw_delay; if (*weight > 1.0) *weight = 1.0; From 2d5d8a039518ec5901148f7c2d186f48af542079 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sat, 8 Apr 2017 20:36:24 +0200 Subject: [PATCH 3/3] tsproc: Allow clock synchronization immediately after jump. After a servo jump, the call to tsproc_reset() clears tsp->t3. This causes the next call to tsproc_update_offset() to fail. However the offset calculation does not necessarily depend on t3, i.e. when filtered_delay is available and neither raw nor weighting is used. This patch fixes the issue by allowing the stored filtered delay to be used when appropriate. Signed-off-by: Richard Cochran Reported-by: Burkhard Ilsen --- tsproc.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tsproc.c b/tsproc.c index 550f32c..63c989d 100644 --- a/tsproc.c +++ b/tsproc.c @@ -180,20 +180,28 @@ int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset, double *weight) { tmv_t delay = 0, raw_delay = 0; - if (tmv_is_zero(tsp->t1) || tmv_is_zero(tsp->t2) || - tmv_is_zero(tsp->t3)) + if (tmv_is_zero(tsp->t1) || tmv_is_zero(tsp->t2)) return -1; switch (tsp->mode) { case TSPROC_FILTER: + if (!tsp->filtered_delay_valid) { + return -1; + } delay = tsp->filtered_delay; break; case TSPROC_RAW: case TSPROC_RAW_WEIGHT: + if (tmv_is_zero(tsp->t3)) { + return -1; + } raw_delay = get_raw_delay(tsp); delay = raw_delay; break; case TSPROC_FILTER_WEIGHT: + if (tmv_is_zero(tsp->t3) || !tsp->filtered_delay_valid) { + return -1; + } raw_delay = get_raw_delay(tsp); delay = tsp->filtered_delay; break;