[Linuxptp-devel] [PATCH RFC 0/2] Fix one-step regression

[Richard Cochran]

Due to a bug in the setup code refactoring (in anticipation of TC
support) in v1.7 you cannot dial one-step anymore. This series
addresses that issue.

Richard Cochran (2):
Fix regression in one-step configuration.
clock: Remove cruft from the creation method.

clock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

[Richard Cochran]

We activate on-step mode based on the "time_stamping" and
"twoStepFlag" configuration options. If twoStepFlag is false and HW
time stamping is enabled, we upgrade the time stamping mode variable
to one-step.

The code that tests the options and sets the one-step mode moved from
ptp4l.c into clock.c in commit 9b27664c ("clock: simplify the create
method."). However, that commit inadvertently moved the test after
the place where the time stamping mode is latched in a local variable.
As a result, one-step mode is not activated when configured.

This patch fixes the issue by keeping the local time stamping mode
variable up to date during the one-step test.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/clock.c b/clock.c
index 0107ef2..55f3c52 100644
--- a/clock.c
+++ b/clock.c
@@ -898,6 +898,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
"with hardware time stamping");
return NULL;
case TS_HARDWARE:
+ timestamping = TS_ONESTEP;
if (config_set_int(config, "time_stamping", TS_ONESTEP))
return NULL;
break;

[Richard Cochran]

The time stamping setup code needlessly queries the configuration data
base over and over, rather than simply using the local variable
already assigned. This patch replaces the extraneous config_get_int()
calls with the local variable.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/clock.c b/clock.c
index 55f3c52..8ca749e 100644
--- a/clock.c
+++ b/clock.c
@@ -891,7 +891,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}

if (!(c->dds.flags & DDS_TWO_STEP_FLAG)) {
- switch (config_get_int(config, NULL, "time_stamping")) {
+ switch (timestamping) {
case TS_SOFTWARE:
case TS_LEGACY_HW:
pr_err("one step is only possible "
@@ -908,7 +908,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}

/* Check the time stamping mode on each interface. */
- switch (config_get_int(config, NULL, "time_stamping")) {
+ switch (timestamping) {
case TS_SOFTWARE:
required_modes |= SOF_TIMESTAMPING_TX_SOFTWARE |
SOF_TIMESTAMPING_RX_SOFTWARE |
@@ -940,8 +940,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
/* determine PHC Clock index */
if (config_get_int(config, NULL, "free_running")) {
phc_index = -1;
- } else if (config_get_int(config, NULL, "time_stamping") == TS_SOFTWARE ||
- config_get_int(config, NULL, "time_stamping") == TS_LEGACY_HW) {
+ } else if (timestamping == TS_SOFTWARE || timestamping == TS_LEGACY_HW) {
phc_index = -1;
} else if (phc_device) {
if (1 != sscanf(phc_device, "/dev/ptp%d", &phc_index)) {