On 8/1/2020 10:46 AM, Vladimir Oltean wrote:
> This propagates the use of "struct ts2phc_private" all the way into the
> master API, in preparation of a new use case that will be supported
> soon: some PPS masters (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 slave PHC (the one using extts) has
> been synchronized to the master (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 master PHC's time was, based on reading
> that PHC immediately after a slave 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.
>
Seems like this would be a good place for an extension if HW could
support it? I don't actually know if any existing HW supports this kind
of "create a pulse and timestamp it at the same time" though.
> This is particularly useful when using the "automatic" mode of ts2phc,
> and the PPS distribution tree is fixed in hardware (as opposed to port
> states).
>
> Signed-off-by: Vladimir Oltean <olte...@gmail.com>
> ---
> ts2phc.c | 2 +-
> ts2phc_generic_master.c | 2 +-
> ts2phc_generic_master.h | 3 ++-
> ts2phc_master.c | 10 ++++++----
> ts2phc_master.h | 6 ++++--
> ts2phc_nmea_master.c | 5 +++--
> ts2phc_nmea_master.h | 3 ++-
> ts2phc_phc_master.c | 33 +++++++++++++++++----------------
> ts2phc_phc_master.h | 3 ++-
> ts2phc_slave.c | 2 +-
> 10 files changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/ts2phc.c b/ts2phc.c
> index 8a4071d083e3..97e9718c999e 100644
> --- a/ts2phc.c
> +++ b/ts2phc.c
> @@ -260,7 +260,7 @@ int main(int argc, char *argv[])
> } else {
> pps_type = TS2PHC_MASTER_PHC;
> }
> - priv.master = ts2phc_master_create(cfg, pps_source, pps_type);
> + priv.master = ts2phc_master_create(&priv, pps_source, pps_type);
> if (!priv.master) {
> fprintf(stderr, "failed to create master\n");
> ts2phc_cleanup(&priv);
> diff --git a/ts2phc_generic_master.c b/ts2phc_generic_master.c
> index ad4f7f1cf1d7..05dd8f3742fc 100644
> --- a/ts2phc_generic_master.c
> +++ b/ts2phc_generic_master.c
> @@ -47,7 +47,7 @@ static int ts2phc_generic_master_getppstime(struct
> ts2phc_master *m,
> return 0;
> }
>
> -struct ts2phc_master *ts2phc_generic_master_create(struct config *cfg,
> +struct ts2phc_master *ts2phc_generic_master_create(struct ts2phc_private
> *priv,
> const char *dev)
> {
> struct ts2phc_generic_master *master;
> diff --git a/ts2phc_generic_master.h b/ts2phc_generic_master.h
> index ac0ce4f11cb8..c8fc099ad066 100644
> --- a/ts2phc_generic_master.h
> +++ b/ts2phc_generic_master.h
> @@ -6,9 +6,10 @@
> #ifndef HAVE_TS2PHC_GENERIC_MASTER_H
> #define HAVE_TS2PHC_GENERIC_MASTER_H
>
> +#include "ts2phc.h"
> #include "ts2phc_master.h"
>
> -struct ts2phc_master *ts2phc_generic_master_create(struct config *cfg,
> +struct ts2phc_master *ts2phc_generic_master_create(struct ts2phc_private
> *priv,
> const char *dev);
>
> #endif
> diff --git a/ts2phc_master.c b/ts2phc_master.c
> index 9283580df3fc..4617c4aecbe5 100644
> --- a/ts2phc_master.c
> +++ b/ts2phc_master.c
> @@ -3,25 +3,27 @@
> * @note Copyright (C) 2019 Richard Cochran <richardcoch...@gmail.com>
> * @note SPDX-License-Identifier: GPL-2.0+
> */
> +#include "ts2phc.h"
> #include "ts2phc_generic_master.h"
> #include "ts2phc_master_private.h"
> #include "ts2phc_nmea_master.h"
> #include "ts2phc_phc_master.h"
>
> -struct ts2phc_master *ts2phc_master_create(struct config *cfg, const char
> *dev,
> +struct ts2phc_master *ts2phc_master_create(struct ts2phc_private *priv,
> + const char *dev,
> enum ts2phc_master_type type)
> {
> struct ts2phc_master *master = NULL;
>
> switch (type) {
> case TS2PHC_MASTER_GENERIC:
> - master = ts2phc_generic_master_create(cfg, dev);
> + master = ts2phc_generic_master_create(priv, dev);
> break;
> case TS2PHC_MASTER_NMEA:
> - master = ts2phc_nmea_master_create(cfg, dev);
> + master = ts2phc_nmea_master_create(priv, dev);
> break;
> case TS2PHC_MASTER_PHC:
> - master = ts2phc_phc_master_create(cfg, dev);
> + master = ts2phc_phc_master_create(priv, dev);
> break;
> }
> return master;
> diff --git a/ts2phc_master.h b/ts2phc_master.h
> index 79765b696d71..a7e7186f79a1 100644
> --- a/ts2phc_master.h
> +++ b/ts2phc_master.h
> @@ -13,6 +13,7 @@ struct config;
> /**
> * Opaque type
> */
> +struct ts2phc_private;
> struct ts2phc_master;
>
> /**
> @@ -26,12 +27,13 @@ enum ts2phc_master_type {
>
> /**
> * Create a new instance of a PPS master clock.
> - * @param cfg Pointer to a valid configuration.
> + * @param priv Pointer to the program's data structure.
> * @param dev Name of the master clock or NULL.
> * @param type The type of the clock to create.
> * @return A pointer to a new PPS master clock on success, NULL otherwise.
> */
> -struct ts2phc_master *ts2phc_master_create(struct config *cfg, const char
> *dev,
> +struct ts2phc_master *ts2phc_master_create(struct ts2phc_private *priv,
> + const char *dev,
> enum ts2phc_master_type type);
>
> /**
> diff --git a/ts2phc_nmea_master.c b/ts2phc_nmea_master.c
> index 2b9af3b9f31f..70080f4fa225 100644
> --- a/ts2phc_nmea_master.c
> +++ b/ts2phc_nmea_master.c
> @@ -198,7 +198,8 @@ static int ts2phc_nmea_master_getppstime(struct
> ts2phc_master *master,
> return fix_valid ? lstab_error : -1;
> }
>
> -struct ts2phc_master *ts2phc_nmea_master_create(struct config *cfg, const
> char *dev)
> +struct ts2phc_master *ts2phc_nmea_master_create(struct ts2phc_private *priv,
> + const char *dev)
> {
> struct ts2phc_nmea_master *master;
> const char *leapfile = NULL; // TODO - read from config.
> @@ -214,7 +215,7 @@ struct ts2phc_master *ts2phc_nmea_master_create(struct
> config *cfg, const char *
> }
> master->master.destroy = ts2phc_nmea_master_destroy;
> master->master.getppstime = ts2phc_nmea_master_getppstime;
> - master->config = cfg;
> + master->config = priv->cfg;
> pthread_mutex_init(&master->mutex, NULL);
> err = pthread_create(&master->worker, NULL, monitor_nmea_status,
> master);
> if (err) {
> diff --git a/ts2phc_nmea_master.h b/ts2phc_nmea_master.h
> index 7430e20662b9..ddfbae48f83b 100644
> --- a/ts2phc_nmea_master.h
> +++ b/ts2phc_nmea_master.h
> @@ -6,8 +6,9 @@
> #ifndef HAVE_TS2PHC_NMEA_MASTER_H
> #define HAVE_TS2PHC_NMEA_MASTER_H
>
> +#include "ts2phc.h"
> #include "ts2phc_master.h"
>
> -struct ts2phc_master *ts2phc_nmea_master_create(struct config *cfg,
> +struct ts2phc_master *ts2phc_nmea_master_create(struct ts2phc_private *priv,
> const char *dev);
> #endif
> diff --git a/ts2phc_phc_master.c b/ts2phc_phc_master.c
> index 9f1837bc80eb..363085279ae3 100644
> --- a/ts2phc_phc_master.c
> +++ b/ts2phc_phc_master.c
> @@ -12,15 +12,14 @@
> #include "phc.h"
> #include "print.h"
> #include "missing.h"
> +#include "ts2phc.h"
> #include "ts2phc_master_private.h"
> -#include "ts2phc_phc_master.h"
> #include "util.h"
>
> struct ts2phc_phc_master {
> struct ts2phc_master master;
> - clockid_t clkid;
> + struct clock *clock;
> int channel;
> - int fd;
> };
>
> static int ts2phc_phc_master_activate(struct config *cfg, const char *dev,
> @@ -38,10 +37,10 @@ static int ts2phc_phc_master_activate(struct config *cfg,
> const char *dev,
> desc.func = PTP_PF_PEROUT;
> desc.chan = master->channel;
>
> - if (phc_pin_setfunc(master->clkid, &desc)) {
> + if (phc_pin_setfunc(master->clock->clkid, &desc)) {
> pr_warning("Failed to set the pin. Continuing bravely on...");
> }
> - if (clock_gettime(master->clkid, &ts)) {
> + if (clock_gettime(master->clock->clkid, &ts)) {
> perror("clock_gettime");
> return -1;
> }
> @@ -52,7 +51,8 @@ static int ts2phc_phc_master_activate(struct config *cfg,
> const char *dev,
> perout_request.period.sec = 1;
> perout_request.period.nsec = 0;
>
> - if (ioctl(master->fd, PTP_PEROUT_REQUEST2, &perout_request)) {
> + if (ioctl(CLOCKID_TO_FD(master->clock->clkid), PTP_PEROUT_REQUEST2,
> + &perout_request)) {
> pr_err(PTP_PEROUT_REQUEST_FAILED);
> return -1;
> }
> @@ -67,10 +67,11 @@ static void ts2phc_phc_master_destroy(struct
> ts2phc_master *master)
>
> memset(&perout_request, 0, sizeof(perout_request));
> perout_request.index = m->channel;
> - if (ioctl(m->fd, PTP_PEROUT_REQUEST2, &perout_request)) {
> + if (ioctl(CLOCKID_TO_FD(m->clock->clkid), PTP_PEROUT_REQUEST2,
> + &perout_request)) {
> pr_err(PTP_PEROUT_REQUEST_FAILED);
> }
> - posix_clock_close(m->clkid);
> + posix_clock_close(m->clock->clkid);
> free(m);
> }
>
> @@ -79,14 +80,13 @@ static int ts2phc_phc_master_getppstime(struct
> ts2phc_master *m,
> {
> struct ts2phc_phc_master *master =
> container_of(m, struct ts2phc_phc_master, master);
> - return clock_gettime(master->clkid, ts);
> + return clock_gettime(master->clock->clkid, ts);
> }
>
> -struct ts2phc_master *ts2phc_phc_master_create(struct config *cfg,
> +struct ts2phc_master *ts2phc_phc_master_create(struct ts2phc_private *priv,
> const char *dev)
> {
> struct ts2phc_phc_master *master;
> - int junk;
>
> master = calloc(1, sizeof(*master));
> if (!master) {
> @@ -95,16 +95,17 @@ struct ts2phc_master *ts2phc_phc_master_create(struct
> config *cfg,
> master->master.destroy = ts2phc_phc_master_destroy;
> master->master.getppstime = ts2phc_phc_master_getppstime;
>
> - master->clkid = posix_clock_open(dev, &junk);
> - if (master->clkid == CLOCK_INVALID) {
> + master->clock = clock_add(priv, dev);
> + if (!master->clock) {
> free(master);
> return NULL;
> }
> - master->fd = CLOCKID_TO_FD(master->clkid);
> + master->clock->is_destination = 0;
>
> - pr_debug("PHC master %s has ptp index %d", dev, junk);
> + pr_debug("PHC master %s has ptp index %d", dev,
> + master->clock->phc_index);
>
> - if (ts2phc_phc_master_activate(cfg, dev, master)) {
> + if (ts2phc_phc_master_activate(priv->cfg, dev, master)) {
> ts2phc_phc_master_destroy(&master->master);
> return NULL;
> }
> diff --git a/ts2phc_phc_master.h b/ts2phc_phc_master.h
> index 568df1a403d4..4ac03be7d16c 100644
> --- a/ts2phc_phc_master.h
> +++ b/ts2phc_phc_master.h
> @@ -6,9 +6,10 @@
> #ifndef HAVE_TS2PHC_PHC_MASTER_H
> #define HAVE_TS2PHC_PHC_MASTER_H
>
> +#include "ts2phc.h"
> #include "ts2phc_master.h"
>
> -struct ts2phc_master *ts2phc_phc_master_create(struct config *cfg,
> +struct ts2phc_master *ts2phc_phc_master_create(struct ts2phc_private *priv,
> const char *dev);
>
> #endif
> diff --git a/ts2phc_slave.c b/ts2phc_slave.c
> index 5ed76518ea20..7a0522fb0f86 100644
> --- a/ts2phc_slave.c
> +++ b/ts2phc_slave.c
> @@ -15,8 +15,8 @@
> #include <time.h>
> #include <unistd.h>
>
> -#include "config.h"
> #include "clockadj.h"
> +#include "config.h"
> #include "missing.h"
> #include "phc.h"
> #include "print.h"
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel