Re: [Linuxptp-devel] [PATCH 2/2] util: fix dangling file descriptors on the error path of posix_clock_open

[Keller, Jacob E]

> -Original Message-
> From: Vladimir Oltean 
> Sent: Monday, November 22, 2021 4:38 PM
> To: richardcoch...@gmail.com
> Cc: linuxptp-devel@lists.sourceforge.net
> Subject: [Linuxptp-devel] [PATCH 2/2] util: fix dangling file descriptors on 
> the
> error path of posix_clock_open
> 
> When we determine that the "device" argument represents a path to a char
> device, we call phc_open() and then we attempt to see, if the device
> real path starts with "/dev/ptp", what number comes afterwards. If that
> fails, we should close the char device we've just opened.
> 
> Fixes: 380d023abb1f ("posix_clock_open: derive PHC index from device name if
> possible")
> Signed-off-by: Vladimir Oltean 


Does this really need to be a separate commit? It seems like we could have 
squashed this into the same fix as the series.

Thanks,
Jake

> ---
>  util.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/util.c b/util.c
> index 686ed5e2f110..34b0bc8e109c 100644
> --- a/util.c
> +++ b/util.c
> @@ -226,7 +226,8 @@ clockid_t posix_clock_open(const char *device, int
> *phc_index)
>   fprintf(stderr,
>   "failed to parse PHC index from %s\n",
>   phc_device_path);
> - return -1;
> + phc_close(clkid);
> + return CLOCK_INVALID;
>   }
>   }
>   return clkid;
> --
> 2.25.1
> 
> 
> 
> ___
> Linuxptp-devel mailing list
> Linuxptp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] [PATCH 2/2] util: fix dangling file descriptors on the error path of posix_clock_open

[Keller, Jacob E]

> -Original Message-
> From: Keller, Jacob E
> Sent: Monday, November 22, 2021 5:23 PM
> To: Vladimir Oltean ; richardcoch...@gmail.com
> Cc: linuxptp-devel@lists.sourceforge.net
> Subject: RE: [Linuxptp-devel] [PATCH 2/2] util: fix dangling file descriptors 
> on the
> error path of posix_clock_open
> 
> 
> 
> > -Original Message-
> > From: Vladimir Oltean 
> > Sent: Monday, November 22, 2021 4:38 PM
> > To: richardcoch...@gmail.com
> > Cc: linuxptp-devel@lists.sourceforge.net
> > Subject: [Linuxptp-devel] [PATCH 2/2] util: fix dangling file descriptors 
> > on the
> > error path of posix_clock_open
> >
> > When we determine that the "device" argument represents a path to a char
> > device, we call phc_open() and then we attempt to see, if the device
> > real path starts with "/dev/ptp", what number comes afterwards. If that
> > fails, we should close the char device we've just opened.
> >
> > Fixes: 380d023abb1f ("posix_clock_open: derive PHC index from device name if
> > possible")
> > Signed-off-by: Vladimir Oltean 
> 
> 
> Does this really need to be a separate commit? It seems like we could have
> squashed this into the same fix as the series.
> 

If you really want it to be separate, I suggest doing this one first in the 
series.

Thanks,
Jake



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] [PATCH 2/2] util: fix dangling file descriptors on the error path of posix_clock_open

[Vladimir Oltean]

When we determine that the "device" argument represents a path to a char
device, we call phc_open() and then we attempt to see, if the device
real path starts with "/dev/ptp", what number comes afterwards. If that
fails, we should close the char device we've just opened.

Fixes: 380d023abb1f ("posix_clock_open: derive PHC index from device name if 
possible")
Signed-off-by: Vladimir Oltean 
---
 util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util.c b/util.c
index 686ed5e2f110..34b0bc8e109c 100644
--- a/util.c
+++ b/util.c
@@ -226,7 +226,8 @@ clockid_t posix_clock_open(const char *device, int 
*phc_index)
fprintf(stderr,
"failed to parse PHC index from %s\n",
phc_device_path);
-   return -1;
+   phc_close(clkid);
+   return CLOCK_INVALID;
}
}
return clkid;
-- 
2.25.1



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] [PATCH 1/2] util: attempt to resolve symlinks to the PHC device in posix_clock_open

[Vladimir Oltean]

Ed uses the ptp_kvm driver with phc2sys, and Debian 10 has an udev rule
which creates a symlink for this device called /dev/ptp_kvm.

/lib/udev/rules.d/50-udev-default.rules:

SUBSYSTEM=="ptp", ATTR{clock_name}=="KVM virtual PTP", SYMLINK += "ptp_kvm"

Since the blamed patch, posix_clock_open(), called from a variety of
places including the phc2sys clock_add(), returns an error which states
that it cannot deduce the PHC index from this char device symlink.

It appears that phc2sys supports, to some extent, synchronizing PTP
clocks specified by path to the char device, for which the PHC index is
not known. In fact, there are some places in phc2sys where clocks are
compared by PHC number, but almost miraculously, it manages to not trip
over the fact that the PHC number is missing.

For example:

- find_dst_clock() finds clocks by PHC index, but it is only called from
  the "automatic" (phc2sys -a) code path. In that mode, phc2sys finds
  the PHC through this algorithm:
  - send a PORT_PROPERTIES_NP management TLV to ptp4l which contains the
port interface name as string ("eth0" etc)
  - retrieve the PHC device through ethtool ioctls and compute the device
path as /dev/ptpN as discussed
  So it manages to find the char device path in the reverse direction
  (the PHC number is already known).

- do_loop() has this logic:

/* don't try to synchronize the clock to itself */
if (clock->clkid == priv->master->clkid ||
(clock->phc_index >= 0 &&
 clock->phc_index == priv->master->phc_index) ||
!strcmp(clock->device, priv->master->device))
continue;

  which again, magically manages to not trip over a comparison between
  the priv->master_phc_index (CLOCK_REALTIME, has phc_index == -1) and
  clock->phc_index (/dev/ptp_kvm, also has phc_index == -1), simply
  because the slave clock first has to have a valid phc_index before the
  comparison is even been done.

Otherwise said, for phc2sys it is an actual regression to require that
the path to the PTP char device be in the "/dev/ptpN" format. It
doesn't strictly require having a PHC index, but leaving the data
structures like that, now that we are aware of this corner case, is
prone to breakage introduced by future changes.

To fix the problem, we maintain that the PTP clock char device must
contain a number, if its path begins with /dev/ptp. But this time, we
follow any symbolic links that might exist, before attempting to deduce
the number from the path string.

Fixes: 380d023abb1f ("posix_clock_open: derive PHC index from device name if 
possible")
Reported-by: Ed Branch 
Signed-off-by: Vladimir Oltean 
---
 util.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/util.c b/util.c
index cc4f11008be2..686ed5e2f110 100644
--- a/util.c
+++ b/util.c
@@ -18,6 +18,7 @@
  */
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -200,6 +201,7 @@ void posix_clock_close(clockid_t clock)
 
 clockid_t posix_clock_open(const char *device, int *phc_index)
 {
+   char phc_device_path[PATH_MAX];
struct sk_ts_info ts_info;
char phc_device[19];
int clkid;
@@ -208,21 +210,28 @@ clockid_t posix_clock_open(const char *device, int 
*phc_index)
if (!strcasecmp(device, "CLOCK_REALTIME")) {
return CLOCK_REALTIME;
}
-   /* check if device is valid phc device */
-   clkid = phc_open(device);
-   if (clkid != CLOCK_INVALID) {
-   if (!strncmp(device, "/dev/ptp", strlen("/dev/ptp"))) {
-   int r = get_ranged_int(device + strlen("/dev/ptp"),
+
+   /* if the device name resolves so a plausible filesystem path, we
+* assume it is the path to a PHC char device, and treat it as such
+*/
+   if (realpath(device, phc_device_path)) {
+   clkid = phc_open(device);
+   if (clkid == CLOCK_INVALID)
+   return clkid;
+
+   if (!strncmp(phc_device_path, "/dev/ptp", strlen("/dev/ptp"))) {
+   int r = get_ranged_int(phc_device_path + 
strlen("/dev/ptp"),
   phc_index, 0, 65535);
if (r) {
fprintf(stderr,
"failed to parse PHC index from %s\n",
-   device);
+   phc_device_path);
return -1;
}
}
return clkid;
}
+
/* check if device is a valid ethernet device */
if (sk_get_ts_info(device, _info) || !ts_info.valid) {
pr_err("unknown clock %s: %m", device);
-- 
2.25.1



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] [PATCH v5 12/13] ts2phc: allow PHC PPS sources to be synchronized

[Vladimir Oltean]

Now that we are registering a clock even for the PPS source when it
supports that (i.e. when it is a PHC), introduce a new API to retrieve
its clock in order to add timestamps to it.

The timestamps are a mere approximation. We configure the kernel to emit
periodic output and then never hear back from that PHC again. We just
assume that it emits periodic output as promised, and that each pulse is
emitted at the beginning of each second. We rely on the PPS sinks to
report an extts event first, and we know who generated that - the PPS
source, of course. So then we proceed to read the PPS source's PHC time,
and round that to what is the most plausible integer second in its time
base. We believe that to be the 'timestamp'. This is fed into the servo
algorithm.

The PHC PPS source can be synchronized to the extts events of a PHC
slave, when in automatic mode.

Signed-off-by: Vladimir Oltean 
---
v4->v5:
- rebase on top of data structure renames
v3->v4:
Add one more paragraph to commit message.
v2->v3:
Implement ts2phc_master_get_clock() as part of ts2phc_master.c instead
of ts2phc_phc_master.c.

 ts2phc.c| 45 -
 ts2phc_phc_pps_source.c |  9 
 ts2phc_pps_source.c |  8 +++
 ts2phc_pps_source.h |  2 ++
 ts2phc_pps_source_private.h |  1 +
 5 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/ts2phc.c b/ts2phc.c
index 0968ef28ca73..25c8bb3a5fa6 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -482,6 +482,42 @@ static void ts2phc_synchronize_clocks(struct 
ts2phc_private *priv, int autocfg)
}
 }
 
+static int ts2phc_collect_pps_source_tstamp(struct ts2phc_private *priv)
+{
+   struct ts2phc_clock *pps_src_clock;
+   struct timespec source_ts;
+   int err;
+
+   pps_src_clock = ts2phc_pps_source_get_clock(priv->src);
+   /*
+* PPS source isn't a PHC (it may be a generic or a GPS PPS source),
+* don't error out, just don't do anything. If it doesn't have a PHC,
+* there is nothing to synchronize, which is the only point of
+* collecting its perout timestamp in the first place.
+*/
+   if (!pps_src_clock)
+   return 0;
+
+   err = ts2phc_pps_source_getppstime(priv->src, _ts);
+   if (err < 0) {
+   pr_err("source ts not valid");
+   return err;
+   }
+
+   /*
+* As long as the kernel doesn't support a proper API for reporting
+* a precise perout timestamp, we'll have to use this crude
+* approximation.
+*/
+   if (source_ts.tv_nsec > NS_PER_SEC / 2)
+   source_ts.tv_sec++;
+   source_ts.tv_nsec = 0;
+
+   ts2phc_clock_add_tstamp(pps_src_clock, timespec_to_tmv(source_ts));
+
+   return 0;
+}
+
 static void usage(char *progname)
 {
fprintf(stderr,
@@ -702,8 +738,15 @@ int main(int argc, char *argv[])
pr_err("poll failed");
break;
}
-   if (err > 0)
+   if (err > 0) {
+   err = ts2phc_collect_pps_source_tstamp();
+   if (err) {
+   pr_err("failed to collect PPS source tstamp");
+   break;
+   }
+
ts2phc_synchronize_clocks(, autocfg);
+   }
}
 
ts2phc_cleanup();
diff --git a/ts2phc_phc_pps_source.c b/ts2phc_phc_pps_source.c
index 4a4bfb2569b9..d9e00f3b525d 100644
--- a/ts2phc_phc_pps_source.c
+++ b/ts2phc_phc_pps_source.c
@@ -83,6 +83,14 @@ static int ts2phc_phc_pps_source_getppstime(struct 
ts2phc_pps_source *src,
return clock_gettime(s->clock->clkid, ts);
 }
 
+struct ts2phc_clock *ts2phc_phc_pps_source_get_clock(struct ts2phc_pps_source 
*src)
+{
+   struct ts2phc_phc_pps_source *s =
+   container_of(src, struct ts2phc_phc_pps_source, pps_source);
+
+   return s->clock;
+}
+
 struct ts2phc_pps_source *ts2phc_phc_pps_source_create(struct ts2phc_private 
*priv,
   const char *dev)
 {
@@ -94,6 +102,7 @@ struct ts2phc_pps_source 
*ts2phc_phc_pps_source_create(struct ts2phc_private *pr
}
s->pps_source.destroy = ts2phc_phc_pps_source_destroy;
s->pps_source.getppstime = ts2phc_phc_pps_source_getppstime;
+   s->pps_source.get_clock = ts2phc_phc_pps_source_get_clock;
 
s->clock = ts2phc_clock_add(priv, dev);
if (!s->clock) {
diff --git a/ts2phc_pps_source.c b/ts2phc_pps_source.c
index 2a3300aaaf19..53aaf207f566 100644
--- a/ts2phc_pps_source.c
+++ b/ts2phc_pps_source.c
@@ -38,3 +38,11 @@ int ts2phc_pps_source_getppstime(struct ts2phc_pps_source 
*src, struct timespec
 {
return src->getppstime(src, ts);
 }
+
+struct ts2phc_clock *ts2phc_pps_source_get_clock(struct ts2phc_pps_source *src)
+{
+   if (src->get_clock)
+   return src->get_clock(src);
+
+   

[Linuxptp-devel] [PATCH v5 10/13] ts2phc: split PPS sink poll from servo loop

[Vladimir Oltean]

The previous patches have established that:

- a PPS sink deals with extts events
- a clock deals with synchronization via a servo loop

Therefore, the code for synchronization should not be part of the
implementation of a PPS sink. Move it to the main ts2phc.c.
This allows it to be used by a PPS source as well (the PHC kind).

Signed-off-by: Vladimir Oltean 
---
v4->v5:
- Rebase on top of the variable renaming.
v3->v4:
- Use bool for boolean types.
v2->v3:
- None.

 ts2phc.c  |  94 +-
 ts2phc.h  |   3 +
 ts2phc_pps_sink.c | 199 ++
 3 files changed, 190 insertions(+), 106 deletions(-)

diff --git a/ts2phc.c b/ts2phc.c
index 1c577af7768e..f84950947712 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -21,6 +21,9 @@
 #include "ts2phc.h"
 #include "version.h"
 
+#define NS_PER_SEC 10LL
+#define SAMPLE_WEIGHT  1.0
+
 struct interface {
STAILQ_ENTRY(interface) list;
 };
@@ -154,6 +157,30 @@ static struct servo *ts2phc_servo_create(struct 
ts2phc_private *priv,
return servo;
 }
 
+void ts2phc_clock_add_tstamp(struct ts2phc_clock *clock, tmv_t t)
+{
+   struct timespec ts = tmv_to_timespec(t);
+
+   pr_debug("adding tstamp %ld.%09ld to clock %s",
+ts.tv_sec, ts.tv_nsec, clock->name);
+   clock->last_ts = t;
+   clock->is_ts_available = true;
+}
+
+static int ts2phc_clock_get_tstamp(struct ts2phc_clock *clock, tmv_t *ts)
+{
+   if (!clock->is_ts_available)
+   return 0;
+   clock->is_ts_available = false;
+   *ts = clock->last_ts;
+   return 1;
+}
+
+static void ts2phc_clock_flush_tstamp(struct ts2phc_clock *clock)
+{
+   clock->is_ts_available = false;
+}
+
 struct ts2phc_clock *ts2phc_clock_add(struct ts2phc_private *priv,
  const char *device)
 {
@@ -303,6 +330,64 @@ static int auto_init_ports(struct ts2phc_private *priv)
return 0;
 }
 
+static void ts2phc_synchronize_clocks(struct ts2phc_private *priv)
+{
+   struct timespec source_ts;
+   tmv_t source_tmv;
+   struct ts2phc_clock *c;
+   int valid, err;
+
+   err = ts2phc_pps_source_getppstime(priv->src, _ts);
+   if (err < 0) {
+   pr_err("source ts not valid");
+   return;
+   }
+   if (source_ts.tv_nsec > NS_PER_SEC / 2)
+   source_ts.tv_sec++;
+   source_ts.tv_nsec = 0;
+
+   source_tmv = timespec_to_tmv(source_ts);
+
+   LIST_FOREACH(c, >clocks, list) {
+   int64_t offset;
+   double adj;
+   tmv_t ts;
+
+   valid = ts2phc_clock_get_tstamp(c, );
+   if (!valid) {
+   pr_debug("%s timestamp not valid, skipping", c->name);
+   continue;
+   }
+
+   offset = tmv_to_nanoseconds(tmv_sub(ts, source_tmv));
+
+   if (c->no_adj) {
+   pr_info("%s offset %10" PRId64, c->name,
+   offset);
+   continue;
+   }
+
+   adj = servo_sample(c->servo, offset, tmv_to_nanoseconds(ts),
+  SAMPLE_WEIGHT, >servo_state);
+
+   pr_info("%s offset %10" PRId64 " s%d freq %+7.0f",
+   c->name, offset, c->servo_state, adj);
+
+   switch (c->servo_state) {
+   case SERVO_UNLOCKED:
+   break;
+   case SERVO_JUMP:
+   clockadj_set_freq(c->clkid, -adj);
+   clockadj_step(c->clkid, -offset);
+   break;
+   case SERVO_LOCKED:
+   case SERVO_LOCKED_STABLE:
+   clockadj_set_freq(c->clkid, -adj);
+   break;
+   }
+   }
+}
+
 static void usage(char *progname)
 {
fprintf(stderr,
@@ -501,11 +586,18 @@ int main(int argc, char *argv[])
}
 
while (is_running()) {
+   struct ts2phc_clock *c;
+
+   LIST_FOREACH(c, , list)
+   ts2phc_clock_flush_tstamp(c);
+
err = ts2phc_pps_sink_poll();
-   if (err) {
+   if (err < 0) {
pr_err("poll failed");
break;
}
+   if (err > 0)
+   ts2phc_synchronize_clocks();
}
 
ts2phc_cleanup();
diff --git a/ts2phc.h b/ts2phc.h
index 1fab09dbf130..f52a2f3d3cae 100644
--- a/ts2phc.h
+++ b/ts2phc.h
@@ -27,6 +27,8 @@ struct ts2phc_clock {
enum servo_state servo_state;
char *name;
bool no_adj;
+   bool is_ts_available;
+   tmv_t last_ts;
 };
 
 struct ts2phc_port {
@@ -50,6 +52,7 @@ struct ts2phc_private {
 struct ts2phc_clock *ts2phc_clock_add(struct ts2phc_private *priv,
  const char *device);
 void 

[Linuxptp-devel] [PATCH v5 09/13] ts2phc_slave: print offset to the source clock

[Vladimir Oltean]

Make this information more visible by default, since it is the key
output of this program.

Signed-off-by: Vladimir Oltean 
Reviewed-by: Jacob Keller 
---
v4->v5: rebase on top of the variable renames
v1->v4: none

 ts2phc_pps_sink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ts2phc_pps_sink.c b/ts2phc_pps_sink.c
index 729aac8f570f..69cc97179e36 100644
--- a/ts2phc_pps_sink.c
+++ b/ts2phc_pps_sink.c
@@ -260,8 +260,8 @@ static int ts2phc_pps_sink_event(struct ts2phc_pps_sink 
*sink,
adj = servo_sample(sink->clock->servo, offset, extts_ts,
   SAMPLE_WEIGHT, >clock->servo_state);
 
-   pr_debug("%s source offset %10" PRId64 " s%d freq %+7.0f",
-sink->name, offset, sink->clock->servo_state, adj);
+   pr_info("%s source offset %10" PRId64 " s%d freq %+7.0f",
+   sink->name, offset, sink->clock->servo_state, adj);
 
switch (sink->clock->servo_state) {
case SERVO_UNLOCKED:
-- 
2.25.1



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] [PATCH v5 00/13] Dynamic sync direction for ts2phc

[Vladimir Oltean]

As discussed in this email thread:
https://sourceforge.net/p/linuxptp/mailman/message/37047555/
there is a desire to synchronize multiple DSA switches in a
boundary_clock_jbod setup, using a PPS signal, and the ts2phc program
already offers a solid base.

This patch series extends the ts2phc program to cater for that use case
by introducing a new '-a' option and friends ('--transportSpecific').
This makes it quite similar to phc2sys which has the same ability, just
that ts2phc can give much better performance if the hardware can assist
with that.

The overall board design for my use case is that there's an SoC with an
embedded DSA switch, and hanging off of 3 ports of that embedded switch
are 3 external switches. Every networking device (the DSA master for the
embedded switch, the embedded switch, as well as each individual
external switch) has a PTP clock. The topology for PPS signal
distribution is fixed - that is given by hardware ability - the
/dev/ptp1 clock can emit a valid PPS, and all external switches can
timestamp that PPS (it is connected in a sort-of simplex "bus" design),
and it cannot be any other way around. It looks like this:

  +---+
  | LS1028A   /dev/ptp0 (unused)  |
  |   +--+|
  |   |  DSA master for Felix||
  |   |(internal ENETC port 2: eno2))||
  |  ++--+-+  |
  |  | Felix embedded L2 switch  /dev/ptp1 |  |
  |  | PPS source, sync target |  |
  |  | +--+   +--+   +--+  |  |
  |  | |DSA master for|   |DSA master for|   |DSA master for|  |  |
  |  | |  SJA1105 1   |   |  SJA1105 2   |   |  SJA1105 3   |  |  |
  |  | |(Felix port 1)|   |(Felix port 2)|   |(Felix port 3)|  |  |
  +--+-+--+---+--+---+--+--+--+

/dev/ptp2/dev/ptp3/dev/ptp4
 PPS sink, sync reference   PPS sink, sync target PPS sink, sync target
+---+ +---+ +---+
|   SJA1105 switch 1| |   SJA1105 switch 2| |   SJA1105 switch 3|
+-+-+-+-+ +-+-+-+-+ +-+-+-+-+
|sw1p0|sw1p1|sw1p2|sw1p3| |sw2p0|sw2p1|sw2p2|sw2p3| |sw3p0|sw3p1|sw3p2|sw3p3|
+-+-+-+-+ +-+-+-+-+ +-+-+-+-+
   ^
   |
GM connected here

In text, it would be described as this:

cat ts2phc.cfg
[global]
first_step_threshold0.2
step_threshold  0.2
ts2phc.pulsewidth   5
ts2phc.perout_phase 0

# Felix
[/dev/ptp1]
ts2phc.master   1

# SJA1105 switch 1
[/dev/ptp2]
ts2phc.channel  0
ts2phc.extts_polarity   both

# SJA1105 switch 2
[/dev/ptp3]
ts2phc.channel  0
ts2phc.extts_polarity   both

# SJA1105 switch 3
[/dev/ptp4]
ts2phc.channel  0
ts2phc.extts_polarity   both

cat gPTP.cfg
[global]
gmCapable   1
priority1   248
priority2   248
logAnnounceInterval 0
logSyncInterval -3
syncReceiptTimeout  3
neighborPropDelayThresh 5
min_neighbor_prop_delay -2000
assume_two_step 1
path_trace_enabled  1
follow_up_info  1
transportSpecific   0x1
ptp_dst_mac 01:80:C2:00:00:0E
network_transport   L2
delay_mechanism P2P
step_threshold  0.2
tx_timestamp_timeout20
boundary_clock_jbod 1

[sw1p0]
[sw1p1]
[sw1p2]
[sw1p3]
[sw2p0]
[sw2p1]
[sw2p2]
[sw2p3]
[sw3p0]
[sw3p1]
[sw3p2]
[sw3p3]

At a high level, what I have done is:
- I moved the PMC related code from phc2sys into pmc_common.c, for
  ts2phc reuse
- I created an extra abstraction in ts2phc as "struct clock" that would
  represent what's synchronizable. The "master" and "slave" concepts
  retain their meaning, which is: "master" == the device that emits PPS,
  and "slave" == the device that timestamps PPS.
- I added support for telling the PHC master to start emitting periodic
  output using just phase information, and not an absolute time. This is
  required for the kernel drivers I am working with.
- I added support for configuring the pulse width, not just hoping that
  the value specified in ts2phc.cfg is representative of what the
  hardware actually uses.

The changes should be 

[Linuxptp-devel] [PATCH v5 03/13] ts2phc: rename "slave clocks" to "PPS sinks"

[Vladimir Oltean]

The ts2phc program will introduce clock synchronization which is
orthogonal to the direction in which PPS is emitted (from the master
to the slave).

To have a more consistent terminology, we can avoid using the
ultra-generic term "slave" and replace it with "PPS sink", which
describes the role of the data structure in the new interpretation of
the program in a much clearer way.

Signed-off-by: Vladimir Oltean 
---
v4->v5: patch is new

 ts2phc.c  |  24 ++---
 ts2phc_pps_sink.c | 255 +++---
 ts2phc_pps_sink.h |  12 +--
 3 files changed, 146 insertions(+), 145 deletions(-)

diff --git a/ts2phc.c b/ts2phc.c
index 8f2039085812..812ac78b4875 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -21,7 +21,7 @@ struct interface {
 
 static void ts2phc_cleanup(struct config *cfg, struct ts2phc_master *master)
 {
-   ts2phc_slave_cleanup();
+   ts2phc_pps_sink_cleanup();
if (master) {
ts2phc_master_destroy(master);
}
@@ -55,7 +55,7 @@ static void usage(char *progname)
 
 int main(int argc, char *argv[])
 {
-   int c, err = 0, have_slave = 0, index, print_level;
+   int c, err = 0, have_sink = 0, index, print_level;
struct ts2phc_master *master = NULL;
enum ts2phc_master_type pps_type;
char *config = NULL, *progname;
@@ -87,11 +87,11 @@ int main(int argc, char *argv[])
break;
case 'c':
if (!config_create_interface(optarg, cfg)) {
-   fprintf(stderr, "failed to add slave\n");
+   fprintf(stderr, "failed to add PPS sink\n");
ts2phc_cleanup(cfg, master);
return -1;
}
-   have_slave = 1;
+   have_sink = 1;
break;
case 'f':
config = optarg;
@@ -156,16 +156,16 @@ int main(int argc, char *argv[])
}
pps_source = interface_name(iface);
} else {
-   if (ts2phc_slave_add(cfg, interface_name(iface))) {
-   fprintf(stderr, "failed to add slave\n");
+   if (ts2phc_pps_sink_add(cfg, interface_name(iface))) {
+   fprintf(stderr, "failed to add PPS sink\n");
ts2phc_cleanup(cfg, master);
return -1;
}
-   have_slave = 1;
+   have_sink = 1;
}
}
-   if (!have_slave) {
-   fprintf(stderr, "no slave clocks specified\n");
+   if (!have_sink) {
+   fprintf(stderr, "no PPS sinks specified\n");
ts2phc_cleanup(cfg, master);
usage(progname);
return -1;
@@ -176,8 +176,8 @@ int main(int argc, char *argv[])
usage(progname);
return -1;
}
-   if (ts2phc_slave_arm()) {
-   fprintf(stderr, "failed to arm slaves\n");
+   if (ts2phc_pps_sink_arm()) {
+   fprintf(stderr, "failed to arm PPS sinks\n");
ts2phc_cleanup(cfg, master);
return -1;
}
@@ -197,7 +197,7 @@ int main(int argc, char *argv[])
}
 
while (is_running()) {
-   err = ts2phc_slave_poll(master);
+   err = ts2phc_pps_sink_poll(master);
if (err) {
pr_err("poll failed");
break;
diff --git a/ts2phc_pps_sink.c b/ts2phc_pps_sink.c
index a2245e62d58c..7da850c718eb 100644
--- a/ts2phc_pps_sink.c
+++ b/ts2phc_pps_sink.c
@@ -1,5 +1,5 @@
 /**
- * @file ts2phc_slave.c
+ * @file ts2phc_pps_sink.c
  * @brief Utility program to synchronize the PHC clock to external events
  * @note Copyright (C) 2019 Balint Ferencz 
  * @note SPDX-License-Identifier: GPL-2.0+
@@ -29,9 +29,9 @@
 #define SAMPLE_WEIGHT  1.0
 #define SERVO_SYNC_INTERVAL1.0
 
-struct ts2phc_slave {
+struct ts2phc_pps_sink {
char *name;
-   STAILQ_ENTRY(ts2phc_slave) list;
+   STAILQ_ENTRY(ts2phc_pps_sink) list;
struct ptp_pin_desc pin_desc;
enum servo_state state;
unsigned int polarity;
@@ -44,8 +44,8 @@ struct ts2phc_slave {
int fd;
 };
 
-struct ts2phc_slave_array {
-   struct ts2phc_slave **slave;
+struct ts2phc_sink_array {
+   struct ts2phc_pps_sink **sink;
struct pollfd *pfd;
 } polling_array;
 
@@ -60,61 +60,61 @@ enum extts_result {
EXTTS_IGNORE= 1,
 };
 
-static enum extts_result ts2phc_slave_offset(struct ts2phc_slave *slave,
-struct ts2phc_source_timestamp ts,
-int64_t *offset,
-uint64_t *local_ts);
+static enum 

[Linuxptp-devel] [PATCH v5 05/13] ts2phc: create a private data structure

[Vladimir Oltean]

Eliminate the ad-hoc use of global variables in the ts2phc program by
introducing one data structure that incorporates them. This might make
the code more understandable to people coming from a kernel background,
since it resembles the type of data organization used there. It is also
now closer to the data organization of phc2sys, a similar program in
both purpose and implementation.

The reason why this is needed has to do with the ts2phc polymorphism for
a PPS source.

In the next patches, PPS sources will expose a clock structure, which
will be synchronized from the main ts2phc.c.

Not all PPS sources will expose a clock, only the PHC kind will. So the
current object encapsulation model needs to be loosened up a little bit,
because the main ts2phc.c needs to synchronize a list of clocks, list
which is populated by the PPS sinks and by the subset of PPS sources
which are capable of being synchronized.

So instead of having the translation modules of ts2phc communicate
through global variables, let's make struct ts2phc_private the common
working space for the entire program, which is a paradigm that is more
natural.

Signed-off-by: Vladimir Oltean 
Reviewed-by: Jacob Keller 
---
v4->v5:
- Rebased on top of data structure renames
v3->v4:
- Rebased, no conflicts
v3->v2:
- Amended commit message.
- Replaced full license header in ts2phc.h with simple SPDX.

 ts2phc.c  |  66 +--
 ts2phc.h  |  23 ++
 ts2phc_pps_sink.c | 113 ++
 ts2phc_pps_sink.h |  10 ++--
 4 files changed, 125 insertions(+), 87 deletions(-)
 create mode 100644 ts2phc.h

diff --git a/ts2phc.c b/ts2phc.c
index 6004e04effdd..f228351bae3a 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -11,23 +11,20 @@
 #include "config.h"
 #include "interface.h"
 #include "print.h"
-#include "ts2phc_pps_source.h"
-#include "ts2phc_pps_sink.h"
+#include "ts2phc.h"
 #include "version.h"
 
 struct interface {
STAILQ_ENTRY(interface) list;
 };
 
-static void ts2phc_cleanup(struct config *cfg, struct ts2phc_pps_source *src)
+static void ts2phc_cleanup(struct ts2phc_private *priv)
 {
-   ts2phc_pps_sink_cleanup();
-   if (src) {
-   ts2phc_pps_source_destroy(src);
-   }
-   if (cfg) {
-   config_destroy(cfg);
-   }
+   ts2phc_pps_sink_cleanup(priv);
+   if (priv->src)
+   ts2phc_pps_source_destroy(priv->src);
+   if (priv->cfg)
+   config_destroy(priv->cfg);
 }
 
 static void usage(char *progname)
@@ -56,8 +53,8 @@ static void usage(char *progname)
 int main(int argc, char *argv[])
 {
int c, err = 0, have_sink = 0, index, print_level;
-   struct ts2phc_pps_source *src = NULL;
enum ts2phc_pps_source_type pps_type;
+   struct ts2phc_private priv = {0};
char *config = NULL, *progname;
const char *pps_source = NULL;
struct config *cfg = NULL;
@@ -68,7 +65,7 @@ int main(int argc, char *argv[])
 
cfg = config_create();
if (!cfg) {
-   ts2phc_cleanup(cfg, src);
+   ts2phc_cleanup();
return -1;
}
 
@@ -81,14 +78,14 @@ int main(int argc, char *argv[])
switch (c) {
case 0:
if (config_parse_option(cfg, opts[index].name, optarg)) 
{
-   ts2phc_cleanup(cfg, src);
+   ts2phc_cleanup();
return -1;
}
break;
case 'c':
if (!config_create_interface(optarg, cfg)) {
fprintf(stderr, "failed to add PPS sink\n");
-   ts2phc_cleanup(cfg, src);
+   ts2phc_cleanup();
return -1;
}
have_sink = 1;
@@ -99,7 +96,7 @@ int main(int argc, char *argv[])
case 'l':
if (get_arg_val_i(c, optarg, _level,
  PRINT_LEVEL_MIN, PRINT_LEVEL_MAX)) {
-   ts2phc_cleanup(cfg, src);
+   ts2phc_cleanup();
return -1;
}
config_set_int(cfg, "logging_level", print_level);
@@ -116,29 +113,29 @@ int main(int argc, char *argv[])
case 's':
if (pps_source) {
fprintf(stderr, "too many PPS sources\n");
-   ts2phc_cleanup(cfg, src);
+   ts2phc_cleanup();
return -1;
}
pps_source = optarg;
break;
case 'v':
-   ts2phc_cleanup(cfg, src);
+   ts2phc_cleanup();
   

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

[Vladimir Oltean]

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 
---
 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(, pps_source, pps_type);
if (!priv.src) {
fprintf(stderr, "failed to create PPS source\n");
ts2phc_cleanup();
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 
 #include 
 #include 
 #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(>mutex, NULL);
  

[Linuxptp-devel] [PATCH v5 02/13] ts2phc: rename source code files ("master" to "source", "slave" to "sink")

[Vladimir Oltean]

At the moment, a clock which emits PPS is not synchronizable, and
therefore called "master", and a clock which timestamps PPS is
synchronizable, and therefore called "slave".

The ts2phc program is preparing to decouple the concept of clock
synchronization from the concept of who emits PPS and who timestamps
PPS.

When that will happen, the "master" and "slave" terminology will become
vague. Rename what is today a "master" to a "PPS source" and a "slave"
to a "PPS sink". Temporarily, the clock representing the time reference
still coincides with the "PPS source", and the target clocks for
synchronization still coincide with the "PPS sinks".

The change only renames files and their occurrences in the source code
(include paths and makefile). No functional change intended.

Suggested-by: Richard Cochran 
Signed-off-by: Vladimir Oltean 
---
v4->v5: patch is new

 makefile   | 4 ++--
 ts2phc.c   | 4 ++--
 ts2phc_generic_master.c => ts2phc_generic_pps_source.c | 4 ++--
 ts2phc_generic_master.h => ts2phc_generic_pps_source.h | 4 ++--
 ts2phc_nmea_master.c => ts2phc_nmea_pps_source.c   | 6 +++---
 ts2phc_nmea_master.h => ts2phc_nmea_pps_source.h   | 4 ++--
 ts2phc_phc_master.c => ts2phc_phc_pps_source.c | 6 +++---
 ts2phc_phc_master.h => ts2phc_phc_pps_source.h | 4 ++--
 ts2phc_slave.c => ts2phc_pps_sink.c| 4 ++--
 ts2phc_slave.h => ts2phc_pps_sink.h| 4 ++--
 ts2phc_master.c => ts2phc_pps_source.c | 8 
 ts2phc_master.h => ts2phc_pps_source.h | 2 +-
 ts2phc_master_private.h => ts2phc_pps_source_private.h | 4 ++--
 13 files changed, 29 insertions(+), 29 deletions(-)
 rename ts2phc_generic_master.c => ts2phc_generic_pps_source.c (94%)
 rename ts2phc_generic_master.h => ts2phc_generic_pps_source.h (81%)
 rename ts2phc_nmea_master.c => ts2phc_nmea_pps_source.c (98%)
 rename ts2phc_nmea_master.h => ts2phc_nmea_pps_source.h (81%)
 rename ts2phc_phc_master.c => ts2phc_phc_pps_source.c (96%)
 rename ts2phc_phc_master.h => ts2phc_phc_pps_source.h (82%)
 rename ts2phc_slave.c => ts2phc_pps_sink.c (99%)
 rename ts2phc_slave.h => ts2phc_pps_sink.h (88%)
 rename ts2phc_master.c => ts2phc_pps_source.c (84%)
 rename ts2phc_master.h => ts2phc_pps_source.h (97%)
 rename ts2phc_master_private.h => ts2phc_pps_source_private.h (86%)

diff --git a/makefile b/makefile
index 33e7ca0f038b..5295b60b5dac 100644
--- a/makefile
+++ b/makefile
@@ -26,8 +26,8 @@ PRG   = ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster 
ts2phc
 FILTERS= filter.o mave.o mmedian.o
 SERVOS = linreg.o ntpshm.o nullf.o pi.o servo.o
 TRANSP = raw.o transport.o udp.o udp6.o uds.o
-TS2PHC = ts2phc.o lstab.o nmea.o serial.o sock.o ts2phc_generic_master.o \
- ts2phc_master.o ts2phc_phc_master.o ts2phc_nmea_master.o ts2phc_slave.o
+TS2PHC = ts2phc.o lstab.o nmea.o serial.o sock.o ts2phc_generic_pps_source.o \
+ ts2phc_nmea_pps_source.o ts2phc_phc_pps_source.o ts2phc_pps_sink.o 
ts2phc_pps_source.o
 OBJ= bmc.o clock.o clockadj.o clockcheck.o config.o designated_fsm.o \
  e2e_tc.o fault.o $(FILTERS) fsm.o hash.o interface.o monitor.o msg.o phc.o \
  port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o rtnl.o $(SERVOS) \
diff --git a/ts2phc.c b/ts2phc.c
index bc410415dfc6..8f2039085812 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -11,8 +11,8 @@
 #include "config.h"
 #include "interface.h"
 #include "print.h"
-#include "ts2phc_master.h"
-#include "ts2phc_slave.h"
+#include "ts2phc_pps_source.h"
+#include "ts2phc_pps_sink.h"
 #include "version.h"
 
 struct interface {
diff --git a/ts2phc_generic_master.c b/ts2phc_generic_pps_source.c
similarity index 94%
rename from ts2phc_generic_master.c
rename to ts2phc_generic_pps_source.c
index ad4f7f1cf1d7..0afea8ec1594 100644
--- a/ts2phc_generic_master.c
+++ b/ts2phc_generic_pps_source.c
@@ -8,8 +8,8 @@
 
 #include "missing.h"
 #include "print.h"
-#include "ts2phc_generic_master.h"
-#include "ts2phc_master_private.h"
+#include "ts2phc_generic_pps_source.h"
+#include "ts2phc_pps_source_private.h"
 #include "util.h"
 
 struct ts2phc_generic_master {
diff --git a/ts2phc_generic_master.h b/ts2phc_generic_pps_source.h
similarity index 81%
rename from ts2phc_generic_master.h
rename to ts2phc_generic_pps_source.h
index ac0ce4f11cb8..94ab29b30bcb 100644
--- a/ts2phc_generic_master.h
+++ b/ts2phc_generic_pps_source.h
@@ -1,12 +1,12 @@
 /**
- * @file ts2phc_generic_master.h
+ * @file ts2phc_generic_pps_source.h
  * @note Copyright (C) 2019 Richard Cochran 
  * @note SPDX-License-Identifier: GPL-2.0+
  */
 #ifndef HAVE_TS2PHC_GENERIC_MASTER_H
 #define HAVE_TS2PHC_GENERIC_MASTER_H
 
-#include "ts2phc_master.h"
+#include "ts2phc_pps_source.h"
 
 struct ts2phc_master *ts2phc_generic_master_create(struct config *cfg,
   const char *dev);
diff --git a/ts2phc_nmea_master.c b/ts2phc_nmea_pps_source.c

[Linuxptp-devel] [PATCH v5 04/13] ts2phc: rename "master" to "source"

[Vladimir Oltean]

The ts2phc program will introduce clock synchronization which is
orthogonal to the direction in which PPS is emitted (from the master
to the slave).

To have a more consistent terminology, we can avoid using the
ultra-generic term "master" and replace it with "PPS source", which
describes the role of the data structure in the new interpretation of
the program in a much clearer way.

Signed-off-by: Vladimir Oltean 
---
v4->v5: patch is new

 ts2phc.c|  56 +-
 ts2phc_generic_pps_source.c |  32 +-
 ts2phc_generic_pps_source.h |   8 +--
 ts2phc_nmea_pps_source.c| 113 ++--
 ts2phc_nmea_pps_source.h|   8 +--
 ts2phc_phc_pps_source.c |  66 ++---
 ts2phc_phc_pps_source.h |   8 +--
 ts2phc_pps_sink.c   |  10 ++--
 ts2phc_pps_sink.h   |   2 +-
 ts2phc_pps_source.c |  30 +-
 ts2phc_pps_source.h |  36 ++--
 ts2phc_pps_source_private.h |  10 ++--
 12 files changed, 190 insertions(+), 189 deletions(-)

diff --git a/ts2phc.c b/ts2phc.c
index 812ac78b4875..6004e04effdd 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -19,11 +19,11 @@ struct interface {
STAILQ_ENTRY(interface) list;
 };
 
-static void ts2phc_cleanup(struct config *cfg, struct ts2phc_master *master)
+static void ts2phc_cleanup(struct config *cfg, struct ts2phc_pps_source *src)
 {
ts2phc_pps_sink_cleanup();
-   if (master) {
-   ts2phc_master_destroy(master);
+   if (src) {
+   ts2phc_pps_source_destroy(src);
}
if (cfg) {
config_destroy(cfg);
@@ -56,8 +56,8 @@ static void usage(char *progname)
 int main(int argc, char *argv[])
 {
int c, err = 0, have_sink = 0, index, print_level;
-   struct ts2phc_master *master = NULL;
-   enum ts2phc_master_type pps_type;
+   struct ts2phc_pps_source *src = NULL;
+   enum ts2phc_pps_source_type pps_type;
char *config = NULL, *progname;
const char *pps_source = NULL;
struct config *cfg = NULL;
@@ -68,7 +68,7 @@ int main(int argc, char *argv[])
 
cfg = config_create();
if (!cfg) {
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
return -1;
}
 
@@ -81,14 +81,14 @@ int main(int argc, char *argv[])
switch (c) {
case 0:
if (config_parse_option(cfg, opts[index].name, optarg)) 
{
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
return -1;
}
break;
case 'c':
if (!config_create_interface(optarg, cfg)) {
fprintf(stderr, "failed to add PPS sink\n");
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
return -1;
}
have_sink = 1;
@@ -99,7 +99,7 @@ int main(int argc, char *argv[])
case 'l':
if (get_arg_val_i(c, optarg, _level,
  PRINT_LEVEL_MIN, PRINT_LEVEL_MAX)) {
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
return -1;
}
config_set_int(cfg, "logging_level", print_level);
@@ -116,29 +116,29 @@ int main(int argc, char *argv[])
case 's':
if (pps_source) {
fprintf(stderr, "too many PPS sources\n");
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
return -1;
}
pps_source = optarg;
break;
case 'v':
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
version_show(stdout);
return 0;
case 'h':
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
usage(progname);
return -1;
case '?':
default:
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
usage(progname);
return -1;
}
}
if (config && (c = config_read(config, cfg))) {
fprintf(stderr, "failed to read config\n");
-   ts2phc_cleanup(cfg, master);
+   ts2phc_cleanup(cfg, src);
return -1;
}

[Linuxptp-devel] [PATCH v5 11/13] ts2phc: reconfigure sync direction by subscribing to ptp4l port events

[Vladimir Oltean]

Monitor the port state change events from ptp4l, and use that
information to determine the "reference" clock.

Then synchronize all other clocks in our list to that source, by feeding
into their respective servo loop an offset equal to the delta between
their timestamp and the timestamp of the reference clock. All timestamps
are representative of the same event, which is the most recent perout
pulse of the PPS source.

Signed-off-by: Vladimir Oltean 
---
v4->v5:
- Rename source clock to reference clock, destination clock to target
  clock
- Rebase on top of earlier data structure renaming (master -> PPS
  source, slave -> PPS sink)
v3->v4:
- Use bool for boolean types.
v2->v3:
- None.

 ts2phc.c| 130 
 ts2phc.h|   3 +
 ts2phc_phc_pps_source.c |   1 +
 ts2phc_pps_sink.c   |   1 +
 4 files changed, 123 insertions(+), 12 deletions(-)

diff --git a/ts2phc.c b/ts2phc.c
index f84950947712..0968ef28ca73 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -123,6 +123,7 @@ static int ts2phc_recv_subscribed(void *context, struct 
ptp_message *msg,
state = ts2phc_clock_compute_state(priv, clock);
if (clock->state != state || clock->new_state) {
clock->new_state = state;
+   priv->state_changed = true;
}
}
return 1;
@@ -326,33 +327,126 @@ static int auto_init_ports(struct ts2phc_private *priv)
LIST_FOREACH(clock, >clocks, list) {
clock->new_state = ts2phc_clock_compute_state(priv, clock);
}
+   priv->state_changed = true;
 
return 0;
 }
 
-static void ts2phc_synchronize_clocks(struct ts2phc_private *priv)
+static void ts2phc_reconfigure(struct ts2phc_private *priv)
+{
+   struct ts2phc_clock *c, *ref_clk = NULL, *last = NULL;
+   int num_ref_clocks = 0, num_target_clocks = 0;
+
+   pr_info("reconfiguring after port state change");
+   priv->state_changed = false;
+
+   LIST_FOREACH(c, >clocks, list) {
+   if (c->new_state) {
+   c->state = c->new_state;
+   c->new_state = 0;
+   }
+
+   switch (c->state) {
+   case PS_FAULTY:
+   case PS_DISABLED:
+   case PS_LISTENING:
+   case PS_PRE_MASTER:
+   case PS_MASTER:
+   case PS_PASSIVE:
+   if (!c->is_target) {
+   pr_info("selecting %s for synchronization",
+   c->name);
+   c->is_target = true;
+   }
+   num_target_clocks++;
+   break;
+   case PS_UNCALIBRATED:
+   num_ref_clocks++;
+   break;
+   case PS_SLAVE:
+   ref_clk = c;
+   num_ref_clocks++;
+   break;
+   default:
+   break;
+   }
+   last = c;
+   }
+   if (num_target_clocks >= 1 && !ref_clk) {
+   priv->ref_clock = last;
+   priv->ref_clock->is_target = false;
+   /* Reset to original state in next reconfiguration. */
+   priv->ref_clock->new_state = priv->ref_clock->state;
+   priv->ref_clock->state = PS_SLAVE;
+   pr_info("no reference clock, selecting %s by default",
+   last->name);
+   return;
+   }
+   if (num_ref_clocks > 1) {
+   pr_info("multiple reference clocks available, postponing 
sync...");
+   priv->ref_clock = NULL;
+   return;
+   }
+   if (num_ref_clocks > 0 && !ref_clk) {
+   pr_info("reference clock not ready, waiting...");
+   priv->ref_clock = NULL;
+   return;
+   }
+   if (!num_ref_clocks && !num_target_clocks) {
+   pr_info("no PHC ready, waiting...");
+   priv->ref_clock = NULL;
+   return;
+   }
+   if (!num_ref_clocks) {
+   pr_info("nothing to synchronize");
+   priv->ref_clock = NULL;
+   return;
+   }
+   ref_clk->is_target = false;
+   priv->ref_clock = ref_clk;
+   pr_info("selecting %s as the reference clock", ref_clk->name);
+}
+
+static void ts2phc_synchronize_clocks(struct ts2phc_private *priv, int autocfg)
 {
-   struct timespec source_ts;
tmv_t source_tmv;
struct ts2phc_clock *c;
int valid, err;
 
-   err = ts2phc_pps_source_getppstime(priv->src, _ts);
-   if (err < 0) {
-   pr_err("source ts not valid");
-   return;
-   }
-   if (source_ts.tv_nsec > NS_PER_SEC / 2)
-   source_ts.tv_sec++;
-   

[Linuxptp-devel] [PATCH v5 06/13] ts2phc: instantiate a full clock structure for every PPS sink

[Vladimir Oltean]

Currently in ts2phc, PPS sinks are also the only candidates for the
clocks that get synchronized. There are 2 aspects that make this too
restrictive:

- Not all PPS sinks may be target clocks for synchronization. Consider a
  dynamic environment of multiple DSA switches using boundary_clock_jbod,
  and only one port is in the PS_SLAVE state. In that case, the clock of
  that port should be the reference for synchronization, regardless of
  whether it supports the extts API or not.

- Not all target clocks for synchronization may be PPS sinks.
  Specifically, the "PHC" type of PPS master in ts2phc can also be,
  fundamentally, treated as a target clock, and disciplined.

The code base should be prepared to handle the distinction that exists
between these concepts, by recognizing that things that can be
disciplined by a servo should be represented by a "struct ts2phc_clock",
and things that can timestamp external events should be represented by a
"struct ts2phc_pps_sink".

Signed-off-by: Vladimir Oltean 
---
v4->v5:
- Rebased on top of data structure renames
v3->v4:
- rebased, needed to pass priv->cfg in ts2phc_nmea_master_create
- setting clock->is_destination can be delayed to a future patch
v2->v3:
None.

 ts2phc.c  | 81 ++
 ts2phc.h  | 21 +++
 ts2phc_pps_sink.c | 90 ++-
 3 files changed, 137 insertions(+), 55 deletions(-)

diff --git a/ts2phc.c b/ts2phc.c
index f228351bae3a..f63e5503637d 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -6,10 +6,15 @@
  * @note Copyright (C) 2012 Richard Cochran 
  * @note SPDX-License-Identifier: GPL-2.0+
  */
+#include 
 #include 
+#include 
+#include 
 
+#include "clockadj.h"
 #include "config.h"
 #include "interface.h"
+#include "phc.h"
 #include "print.h"
 #include "ts2phc.h"
 #include "version.h"
@@ -27,6 +32,82 @@ static void ts2phc_cleanup(struct ts2phc_private *priv)
config_destroy(priv->cfg);
 }
 
+static struct servo *ts2phc_servo_create(struct ts2phc_private *priv,
+struct ts2phc_clock *clock)
+{
+   enum servo_type type = config_get_int(priv->cfg, NULL, "clock_servo");
+   struct servo *servo;
+   int fadj, max_adj;
+
+   fadj = (int) clockadj_get_freq(clock->clkid);
+   /* Due to a bug in older kernels, the reading may silently fail
+* and return 0. Set the frequency back to make sure fadj is
+* the actual frequency of the clock.
+*/
+   if (!clock->no_adj) {
+   clockadj_set_freq(clock->clkid, fadj);
+   }
+
+   max_adj = phc_max_adj(clock->clkid);
+
+   servo = servo_create(priv->cfg, type, -fadj, max_adj, 0);
+   if (!servo)
+   return NULL;
+
+   servo_sync_interval(servo, SERVO_SYNC_INTERVAL);
+
+   return servo;
+}
+
+struct ts2phc_clock *ts2phc_clock_add(struct ts2phc_private *priv,
+ const char *device)
+{
+   clockid_t clkid = CLOCK_INVALID;
+   struct ts2phc_clock *c;
+   int phc_index = -1;
+   int err;
+
+   clkid = posix_clock_open(device, _index);
+   if (clkid == CLOCK_INVALID)
+   return NULL;
+
+   LIST_FOREACH(c, >clocks, list) {
+   if (c->phc_index == phc_index) {
+   /* Already have the clock, don't add it again */
+   posix_clock_close(clkid);
+   return c;
+   }
+   }
+
+   c = calloc(1, sizeof(*c));
+   if (!c) {
+   pr_err("failed to allocate memory for a clock");
+   return NULL;
+   }
+   c->clkid = clkid;
+   c->phc_index = phc_index;
+   c->servo_state = SERVO_UNLOCKED;
+   c->servo = ts2phc_servo_create(priv, c);
+   c->no_adj = config_get_int(priv->cfg, NULL, "free_running");
+   err = asprintf(>name, "/dev/ptp%d", phc_index);
+   if (err < 0) {
+   free(c);
+   posix_clock_close(clkid);
+   return NULL;
+   }
+
+   LIST_INSERT_HEAD(>clocks, c, list);
+   return c;
+}
+
+void ts2phc_clock_destroy(struct ts2phc_clock *c)
+{
+   servo_destroy(c->servo);
+   posix_clock_close(c->clkid);
+   free(c->name);
+   free(c);
+}
+
 static void usage(char *progname)
 {
fprintf(stderr,
diff --git a/ts2phc.h b/ts2phc.h
index 14cb2b0c21a3..043215a51609 100644
--- a/ts2phc.h
+++ b/ts2phc.h
@@ -7,16 +7,37 @@
 #ifndef HAVE_TS2PHC_H
 #define HAVE_TS2PHC_H
 
+#include 
+#include 
+#include "servo.h"
+
 struct ts2phc_sink_array;
 
+#define SERVO_SYNC_INTERVAL1.0
+
+struct ts2phc_clock {
+   LIST_ENTRY(ts2phc_clock) list;
+   clockid_t clkid;
+   int phc_index;
+   struct servo *servo;
+   enum servo_state servo_state;
+   char *name;
+   bool no_adj;
+};
+
 struct ts2phc_private {
struct ts2phc_pps_source *src;
STAILQ_HEAD(sink_ifaces_head, ts2phc_pps_sink) 

[Linuxptp-devel] [PATCH v5 08/13] ts2phc: instantiate a pmc agent

[Vladimir Oltean]

This introduces the '-a' option in ts2phc, an option inspired from
phc2sys that puts the clocks in "automatic" mode. In this mode, ts2phc
listens, as a PMC, to port state change events from ptp4l, and detects
which port state machine, if any, has transitioned to PS_SLAVE. That
port's clock will become the synchronization reference for the clock
hierarchy described by ts2phc.

The use case is a multi-switch DSA setup with boundary_clock_jbod, where
there is only one grandmaster, connected to one switch's port. The other
switches, connected together through a PPS signal, must adapt themselves
to this time reference, while the switch connected to the GM must not be
synchronized by ts2phc because it is already synchronized by ptp4l.

Graphically, the setup looks like this:

  +---+
  | LS1028A   /dev/ptp0 (unused)  |
  |   +--+|
  |   |  DSA master for Felix||
  |   |(internal ENETC port 2: eno2))||
  |  ++--+-+  |
  |  | Felix embedded L2 switch  /dev/ptp1 |  |
  |  | PPS source, sync target |  |
  |  | +--+   +--+   +--+  |  |
  |  | |DSA master for|   |DSA master for|   |DSA master for|  |  |
  |  | |  SJA1105 1   |   |  SJA1105 2   |   |  SJA1105 3   |  |  |
  |  | |(Felix port 1)|   |(Felix port 2)|   |(Felix port 3)|  |  |
  +--+-+--+---+--+---+--+--+--+

/dev/ptp2/dev/ptp3/dev/ptp4
 PPS sink, sync reference   PPS sink, sync target PPS sink, sync target
+---+ +---+ +---+
|   SJA1105 switch 1| |   SJA1105 switch 2| |   SJA1105 switch 3|
+-+-+-+-+ +-+-+-+-+ +-+-+-+-+
|sw1p0|sw1p1|sw1p2|sw1p3| |sw2p0|sw2p1|sw2p2|sw2p3| |sw3p0|sw3p1|sw3p2|sw3p3|
+-+-+-+-+ +-+-+-+-+ +-+-+-+-+
   ^
   |
GM connected here

In the described state, the Felix embedded L2 switch is statically
configured to emit a 1PPS signal on a sort of "simplex bus", and SJA1105
switches 1 to 3 are statically configured to record it in their
respective time bases. Because the GM is connected to one of the ports
of SJA1105 switch 1, that port is in PS_SLAVE and is the source of
ts2phc time. So the embedded L2 switch as well as the other SJA1105
switches synchronize to the SJA1105.

When the GM migrates to a port belonging to a different PHC (say sw2p0),
the PPS distribution tree remains unchanged but the port reported by
ptp4l as being in PS_SLAVE changes. So ts2phc must reconfigure itself to
use /dev/ptp3 as time reference, and the other PHC devices turn into
target clocks.

Signed-off-by: Vladimir Oltean 
---
v4->v5:
- rebased on top of the variable renaming
v3->v4:
- use the new pmc agent API
- add more info to the commit message
- priv->state_changed can be delayed to a future patch
- use enum port_state instead of int
v2->v3:
- Added man page and usage verbiage.
- Removed some useless curly braces in ts2phc_cleanup().

 makefile |   5 +-
 ts2phc.8 |  15 
 ts2phc.c | 225 ++-
 ts2phc.h |  12 +++
 4 files changed, 254 insertions(+), 3 deletions(-)

diff --git a/makefile b/makefile
index 5295b60b5dac..9aed38318803 100644
--- a/makefile
+++ b/makefile
@@ -68,8 +68,9 @@ phc_ctl: phc_ctl.o phc.o sk.o util.o clockadj.o sysoff.o 
print.o version.o
 
 timemaster: phc.o print.o rtnl.o sk.o timemaster.o util.o version.o
 
-ts2phc: config.o clockadj.o hash.o interface.o phc.o print.o $(SERVOS) sk.o \
- $(TS2PHC) util.o version.o
+ts2phc: config.o clockadj.o hash.o interface.o msg.o phc.o pmc_agent.o \
+ pmc_common.o print.o $(SERVOS) sk.o $(TS2PHC) tlv.o transport.o raw.o \
+ udp.o udp6.o uds.o util.o version.o
 
 version.o: .version version.sh $(filter-out version.d,$(DEPEND))
 
diff --git a/ts2phc.8 b/ts2phc.8
index 690c4627f9f2..36d56cce0270 100644
--- a/ts2phc.8
+++ b/ts2phc.8
@@ -26,6 +26,21 @@ A single source may be used to distribute time to one or 
more PHC devices.
 
 .SH OPTIONS
 .TP
+.BI \-a
+Adjust the direction of synchronization automatically. The program determines
+which PHC should the reference clock for time distribution and which should
+be the destinations by querying the port states from the running instance of
+.B ptp4l.
+Note that using this option, the PPS signal distribution hierarchy still
+remains fixed as per the configuration file. This implies that using this
+option, a PPS source of the PHC kind may become a target clock, and a PPS sink
+may become a reference clock. Other, non-PHC types of PPS sources (generic,
+NMEA) cannot 

[Linuxptp-devel] [PATCH v5 13/13] ts2phc_phc_pps_source: make use of new kernel API for perout waveform

[Vladimir Oltean]

This API was introduced for 2 reasons:

1. Some hardware can emit PPS signals but not starting from arbitrary
   absolute times, but rather just emit at a certain phase offset from
   the beginning of each second. We _could_ patch ts2phc to always
   specify a start time of 0.0 to PTP_PEROUT_REQUEST, and in
   theory that should then become the kernel's responsibility to advance
   that time in the past by an integer number of seconds while keeping
   the phase untouched, but in practice, we would never know whether
   that would actually work with all in-kernel PHC drivers, since it
   wasn't enforced as a requirement before. So there was a need for a
   new flag that only specifies the phase of the periodic signal, and
   not the absolute start time.

2. Some hardware can, rather unfortunately, not distinguish between a
   rising and a falling extts edge. And, since whatever rises also has
   to fall before rising again, the strategy in ts2phc is to set a
   'large' pulse width (half the period) and ignore the extts event
   corresponding to the mid-way between one second and another. This is
   all fine, but currently, ts2phc.pulsewidth is a read-only property in
   the config file. The kernel is not instructed in any way to use this
   value, it is simply that must be configured based on prior knowledge
   of the PHC's implementation. This API changes that.

The introduction of a phase adjustment for the PHC kind of PPS sources
means we have to adjust our approximation of the precise perout
timestamp. We put that code into a common function and convert all call
sites to call that. We also need to do the same thing for the edge
ignoring logic.

Signed-off-by: Vladimir Oltean 
---
v4->v5: rebase on top of variable renames
v3->v4: patch is new.

 config.c|  1 +
 missing.h   | 52 +
 ts2phc.8| 17 +++-
 ts2phc.c| 86 +++--
 ts2phc.h|  1 +
 ts2phc_phc_pps_source.c | 44 ++---
 ts2phc_pps_sink.c   | 16 +++-
 7 files changed, 180 insertions(+), 37 deletions(-)

diff --git a/config.c b/config.c
index f3c52baff765..760d0e12b0b6 100644
--- a/config.c
+++ b/config.c
@@ -321,6 +321,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_STR("ts2phc.nmea_remote_host", ""),
GLOB_ITEM_STR("ts2phc.nmea_remote_port", ""),
GLOB_ITEM_STR("ts2phc.nmea_serialport", "/dev/ttyS0"),
+   PORT_ITEM_INT("ts2phc.perout_phase", -1, 0, 9),
PORT_ITEM_INT("ts2phc.pin_index", 0, 0, INT_MAX),
GLOB_ITEM_INT("ts2phc.pulsewidth", 5, 100, 99900),
PORT_ITEM_ENU("tsproc_mode", TSPROC_FILTER, tsproc_enu),
diff --git a/missing.h b/missing.h
index 89cb51360ef7..7f06da3220f2 100644
--- a/missing.h
+++ b/missing.h
@@ -97,6 +97,58 @@ struct compat_ptp_clock_caps {
 
 #endif /*LINUX_VERSION_CODE < 5.8*/
 
+/*
+ * Bits of the ptp_perout_request.flags field:
+ */
+
+#ifndef PTP_PEROUT_ONE_SHOT
+#define PTP_PEROUT_ONE_SHOT(1<<0)
+#endif
+
+#ifndef PTP_PEROUT_DUTY_CYCLE
+#define PTP_PEROUT_DUTY_CYCLE  (1<<1)
+#endif
+
+#ifndef PTP_PEROUT_PHASE
+#define PTP_PEROUT_PHASE   (1<<2)
+#endif
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5,9,0)
+
+struct compat_ptp_perout_request {
+   union {
+   /*
+* Absolute start time.
+* Valid only if (flags & PTP_PEROUT_PHASE) is unset.
+*/
+   struct ptp_clock_time start;
+   /*
+* Phase offset. The signal should start toggling at an
+* unspecified integer multiple of the period, plus this value.
+* The start time should be "as soon as possible".
+* Valid only if (flags & PTP_PEROUT_PHASE) is set.
+*/
+   struct ptp_clock_time phase;
+   };
+   struct ptp_clock_time period; /* Desired period, zero means disable. */
+   unsigned int index;   /* Which channel to configure. */
+   unsigned int flags;
+   union {
+   /*
+* The "on" time of the signal.
+* Must be lower than the period.
+* Valid only if (flags & PTP_PEROUT_DUTY_CYCLE) is set.
+*/
+   struct ptp_clock_time on;
+   /* Reserved for future use. */
+   unsigned int rsv[4];
+   };
+};
+
+#define ptp_perout_request compat_ptp_perout_request
+
+#endif /* LINUX_VERSION_CODE < 5.9 */
+
 #ifndef PTP_MAX_SAMPLES
 #define PTP_MAX_SAMPLES 25 /* Maximum allowed offset measurement samples. */
 #endif /* PTP_MAX_SAMPLES */
diff --git a/ts2phc.8 b/ts2phc.8
index 36d56cce0270..ded6f9ac8afb 100644
--- a/ts2phc.8
+++ b/ts2phc.8
@@ -176,10 +176,23 @@ connection will be used in preference to the configured 
serial port.
 The default serial port is "/dev/ttyS0".
 The default baudrate is 9600 bps.
 

[Linuxptp-devel] [PATCH v5 01/13] pmc_agent: make pmc_agent_query_port_properties take an enum port_state argument

[Vladimir Oltean]

Enforce type correctness and make all callers of pmc_agent_query_port_properties
pass an enum port_state argument instead of plain int.

Signed-off-by: Vladimir Oltean 
---
v4->v5: patch is new

 phc2sys.c   | 6 --
 pmc_agent.c | 2 +-
 pmc_agent.h | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 6815c3dee8a0..786831cd3112 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -292,7 +292,8 @@ static struct port *port_add(struct phc2sys_private *priv, 
unsigned int number,
 static void clock_reinit(struct phc2sys_private *priv, struct clock *clock,
 int new_state)
 {
-   int err = -1, phc_index = -1, phc_switched = 0, state, timestamping;
+   int err = -1, phc_index = -1, phc_switched = 0, timestamping;
+   enum port_state state;
struct port *p;
struct sk_ts_info ts_info;
char iface[IFNAMSIZ];
@@ -844,7 +845,8 @@ static int phc2sys_recv_subscribed(void *context, struct 
ptp_message *msg,
 
 static int auto_init_ports(struct phc2sys_private *priv, int add_rt)
 {
-   int err, number_ports, state, timestamping;
+   int err, number_ports, timestamping;
+   enum port_state state;
char iface[IFNAMSIZ];
struct clock *clock;
struct port *port;
diff --git a/pmc_agent.c b/pmc_agent.c
index 86350d8e6fd1..0dc99a4a83b5 100644
--- a/pmc_agent.c
+++ b/pmc_agent.c
@@ -303,7 +303,7 @@ int pmc_agent_query_dds(struct pmc_agent *node, int timeout)
 }
 
 int pmc_agent_query_port_properties(struct pmc_agent *node, int timeout,
-   unsigned int port, int *state,
+   unsigned int port, enum port_state *state,
int *tstamping, char *iface)
 {
struct port_properties_np *ppn;
diff --git a/pmc_agent.h b/pmc_agent.h
index 11b93479fb0b..43738fa71fe5 100644
--- a/pmc_agent.h
+++ b/pmc_agent.h
@@ -24,6 +24,7 @@
 
 #include 
 
+#include "fsm.h"
 #include "pmc_common.h"
 
 struct pmc_agent;
@@ -107,7 +108,7 @@ int pmc_agent_query_dds(struct pmc_agent *agent, int 
timeout);
  * @return   Zero on success, negative error code otherwise.
  */
 int pmc_agent_query_port_properties(struct pmc_agent *agent, int timeout,
-   unsigned int port, int *state,
+   unsigned int port, enum port_state *state,
int *tstamping, char *iface);
 
 /**
-- 
2.25.1



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] phc2sys S1 phase

[ramesh t via Linuxptp-devel]

hi,

step_threshold default config is 0.0. Hence it was not doing time-sync when 
moving from s0 to s2 via s1. After changing value of step_threshold, time-sync 
is happening.

Question: Any reason as to why step_threshold is set 0.0? What should be the 
proper value?
Also first_step_threshold value is set to 20 microseconds.

Please suggest.

regards,
Ramesh 

On Tuesday, November 23, 2021, 12:00:53 AM GMT+5:30, ramesh t via 
Linuxptp-devel  wrote: 





hi,

phc2sys has 3 phases
1) unlock (s0, SERVO_UNLOCKED 
2) time-sync (s1, SERVO_JUMP)
3) freq-sync (s2, SERVO_LOCKED)

Based on the log capture below:
Nov 10 03:49:05 phc2sys: [21956.824] CLOCK_REALTIME phc offset         8 s2 
freq   +9775 delay   2252
Nov  4 12:50:40 ptp4l: [21957.406] rms    3 max    5 freq  +2780 +/-   5 delay  
  86 +/-   1
Nov  6 05:55:10 phc2sys: [21957.824] clockcheck: clock jumped backward or 
running slower than expected!
Nov  6 05:55:10 phc2sys: [21957.824] CLOCK_REALTIME phc offset -338035766568736 
s0 freq   +9775 delay   2461
Nov  6 07:39:56 ptp4l: [21958.399] rms    2 max    3 freq  +2783 +/-   3 delay  
  85 +/-   1
Nov  9 21:52:19 phc2sys: [21958.824] CLOCK_REALTIME phc offset -21408060435248 
s0 freq   +9775 delay   2468
Nov  9 21:52:19 ptp4l: [21959.393] rms    3 max    6 freq  +2784 +/-   4 delay  
  84 +/-   1
Nov  9 21:52:20 phc2sys: [21959.825] CLOCK_REALTIME phc offset -21408060435245 
s2 freq   +9772 delay   2472

Actual time as received by ptp4l from GM is Nov 10 03:49 (1636516145) But 
system time is somehow got changed to Nov 9 21:52 (1636494740)

Offset between two (ptp4l and system time) is 21408 seconds.

So when the phc2sys moves from s0 to s2 via s1 phase, it should have set the 
system time to proper value based on below code (clockadj_step).
        case SERVO_JUMP:
                clockadj_step(clock->clkid, -offset);
                if (clock->sanity_check)
                        clockcheck_step(clock->sanity_check, -offset);
                /* Fall through. */
        case SERVO_LOCKED:

But it still remained on Nov 9 21:52. What is wrong here?
Nov  9 21:52:20  ptp4l: [21960.382] rms    3 max    6 freq  +2776 +/-   2 delay 
   86 +/-   1
Nov  9 21:52:21 phc2sys: [21960.825] CLOCK_REALTIME phc offset -21408060435252 
s2 freq -1 delay   2488
Nov  9 21:52:21 ptp4l: [21961.429] rms    3 max    6 freq  +2778 +/-   5 delay  
  86 +/-   0
Nov  9 21:52:22 phc2sys: [21961.825] CLOCK_REALTIME phc offset -21407969493862 
s2 freq -1 delay   2709
Nov  9 21:52:22 ptp4l: [21962.520] rms    3 max    7 freq  +2784 +/-   4 delay  
  85 +/-   1

Please suggest.

regards,
Ramesh


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] phc2sys S1 phase

[ramesh t via Linuxptp-devel]

hi,

phc2sys has 3 phases
1) unlock (s0, SERVO_UNLOCKED 
2) time-sync (s1, SERVO_JUMP)
3) freq-sync (s2, SERVO_LOCKED)

Based on the log capture below:
Nov 10 03:49:05 phc2sys: [21956.824] CLOCK_REALTIME phc offset         8 s2 
freq   +9775 delay   2252
Nov  4 12:50:40 ptp4l: [21957.406] rms    3 max    5 freq  +2780 +/-   5 delay  
  86 +/-   1
Nov  6 05:55:10 phc2sys: [21957.824] clockcheck: clock jumped backward or 
running slower than expected!
Nov  6 05:55:10 phc2sys: [21957.824] CLOCK_REALTIME phc offset -338035766568736 
s0 freq   +9775 delay   2461
Nov  6 07:39:56 ptp4l: [21958.399] rms    2 max    3 freq  +2783 +/-   3 delay  
  85 +/-   1
Nov  9 21:52:19 phc2sys: [21958.824] CLOCK_REALTIME phc offset -21408060435248 
s0 freq   +9775 delay   2468
Nov  9 21:52:19 ptp4l: [21959.393] rms    3 max    6 freq  +2784 +/-   4 delay  
  84 +/-   1
Nov  9 21:52:20 phc2sys: [21959.825] CLOCK_REALTIME phc offset -21408060435245 
s2 freq   +9772 delay   2472

Actual time as received by ptp4l from GM is Nov 10 03:49 (1636516145) But 
system time is somehow got changed to Nov 9 21:52 (1636494740)

Offset between two (ptp4l and system time) is 21408 seconds.

So when the phc2sys moves from s0 to s2 via s1 phase, it should have set the 
system time to proper value based on below code (clockadj_step).
        case SERVO_JUMP:
                clockadj_step(clock->clkid, -offset);
                if (clock->sanity_check)
                        clockcheck_step(clock->sanity_check, -offset);
                /* Fall through. */
        case SERVO_LOCKED:

But it still remained on Nov 9 21:52. What is wrong here?
Nov  9 21:52:20  ptp4l: [21960.382] rms    3 max    6 freq  +2776 +/-   2 delay 
   86 +/-   1
Nov  9 21:52:21 phc2sys: [21960.825] CLOCK_REALTIME phc offset -21408060435252 
s2 freq -1 delay   2488
Nov  9 21:52:21 ptp4l: [21961.429] rms    3 max    6 freq  +2778 +/-   5 delay  
  86 +/-   0
Nov  9 21:52:22 phc2sys: [21961.825] CLOCK_REALTIME phc offset -21407969493862 
s2 freq -1 delay   2709
Nov  9 21:52:22 ptp4l: [21962.520] rms    3 max    7 freq  +2784 +/-   4 delay  
  85 +/-   1

Please suggest.

regards,
Ramesh


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel