Changes in v3 (range-diff at the end):
- Split patch "phc2sys: break out pmc code into pmc_common.c" into more
  trivial chunks. There is zero delta compared to previous monolithic
  patch, otherwise.
- Replaced full license header in ts2phc.h with simple SPDX.
- Added usage message and manpage for '-a' option.
- Removed some useless curly braces.
- Moved ts2phc_master_get_clock() from ts2phc_phc_master.c to
  ts2phc_master.c where it should be.
- Added justification in commit message for creating struct
  ts2phc_private.
- Reordered headers alphabetically.

Changes in v2:
- Added Jacob's review tags, addressed some feedback.
- Dropped patch "pmc_common: fix potential memory leak in
  run_pmc_events()"
- Reordered patches (put the tmv helpers first)
- Fixed memory leaks

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 master, sync slave      |  |
      |  | +--------------+   +--------------+   +--------------+  |  |
      |  | |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 slave, sync master    PPS slave, sync slave     PPS slave, sync slave
+-----------------------+ +-----------------------+ +-----------------------+
|   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_threshold    0.00002
step_threshold          0.00002
ts2phc.pulsewidth       500000000
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 50000
min_neighbor_prop_delay -20000000
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.00002
tx_timestamp_timeout    20
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.

The changes should be backwards-compatible with the non-automatic mode.
I have only tested non-automatic mode with a PHC master (that's all I
have) and it still appears to work as before.

11:  e244d4901a73 ! 14:  703c3c7bc70f ts2phc: create a private data structure
    @@ Commit message
         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 master.
    +
    +    In the next patches, PPS masters will expose a struct clock, which will
    +    be synchronized from the main ts2phc.c.
    +
    +    Not all PPS masters will expose a clock, only the PHC kind will. So the
    +    current object encapsulation model needs to be loosened up little bit,
    +    because the main ts2phc.c needs to synchronize a list of clocks, list
    +    which is populated by the slaves and the masters 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 <olte...@gmail.com>
         Reviewed-by: Jacob Keller <jacob.e.kel...@intel.com>

    @@ ts2phc.c
     -  ts2phc_slave_cleanup();
     -  if (master) {
     -          ts2phc_master_destroy(master);
    -+  ts2phc_slave_cleanup(priv);
    -+  if (priv->master) {
    -+          ts2phc_master_destroy(priv->master);
    -   }
    +-  }
     -  if (cfg) {
     -          config_destroy(cfg);
    -+  if (priv->cfg) {
    +-  }
    ++  ts2phc_slave_cleanup(priv);
    ++  if (priv->master)
    ++          ts2phc_master_destroy(priv->master);
    ++  if (priv->cfg)
     +          config_destroy(priv->cfg);
    -   }
      }

    + static void usage(char *progname)
     @@ ts2phc.c: static void usage(char *progname)
      int main(int argc, char *argv[])
      {
    @@ ts2phc.h (new)
     + * @file ts2phc.h
     + * @brief Structure definitions for ts2phc
     + * @note Copyright 2020 Vladimir Oltean <olte...@gmail.com>
    -+ *
    -+ * This program is free software; you can redistribute it and/or modify
    -+ * it under the terms of the GNU General Public License as published by
    -+ * the Free Software Foundation; either version 2 of the License, or
    -+ * (at your option) any later version.
    -+ *
    -+ * This program is distributed in the hope that it will be useful,
    -+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
    -+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    -+ * GNU General Public License for more details.
    -+ *
    -+ * You should have received a copy of the GNU General Public License along
    -+ * with this program; if not, write to the Free Software Foundation, Inc.,
    -+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
    ++ * @note SPDX-License-Identifier: GPL-2.0+
     + */
    -+
     +#ifndef HAVE_TS2PHC_H
     +#define HAVE_TS2PHC_H
     +
12:  8135c7184d20 ! 15:  21813ef8fa1f ts2phc: instantiate a full clock 
structure for every slave PHC
    @@ ts2phc.c
      struct interface {
        STAILQ_ENTRY(interface) list;
     @@ ts2phc.c: static void ts2phc_cleanup(struct ts2phc_private *priv)
    -   }
    +           config_destroy(priv->cfg);
      }

     +struct servo *servo_add(struct ts2phc_private *priv, struct clock *clock)
13:  40e391e3c0dd = 16:  414256efdab0 ts2phc: instantiate a full clock 
structure for every PHC master
14:  e836e85c09ed ! 17:  6e00b1723674 ts2phc: instantiate a pmc node
    @@ makefile: FILTERS        = filter.o mave.o mmedian.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) \

    + ## ts2phc.8 ##
    +@@ ts2phc.8: 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 be a source of time and which should be a sink 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 PHC PPS master may become a time sink, and a PPS slave may 
become a
    ++time source. Other, non-PHC types of PPS masters (generic, NMEA) cannot 
become
    ++time sinks. Clocks which are not part of
    ++.B ptp4l's
    ++list of ports are not synchronized. This option is useful when the
    ++.B boundary_clock_jbod
    ++option of ptp4l is also enabled.
    ++.TP
    + .BI \-c " device|name"
    + Specifies a PHC slave clock to be synchronized.
    + The clock may be identified by its character device (like /dev/ptp0)
    +
      ## ts2phc.c ##
     @@ ts2phc.c: struct interface {

    @@ ts2phc.c: struct interface {
     +  struct port *p, *tmp;
     +
        ts2phc_slave_cleanup(priv);
    -   if (priv->master) {
    +   if (priv->master)
                ts2phc_master_destroy(priv->master);
    -@@ ts2phc.c: static void ts2phc_cleanup(struct ts2phc_private *priv)
    -   if (priv->cfg) {
    +   if (priv->cfg)
                config_destroy(priv->cfg);
    -   }
    ++
     +  close_pmc_node(&priv->node);
     +
     +  /*
    @@ ts2phc.c: void clock_destroy(struct clock *c)
      static void usage(char *progname)
      {
        fprintf(stderr,
    +           "\n"
    +           "usage: %s [options]\n\n"
    ++          " -a             turn on autoconfiguration\n"
    +           " -c [dev|name]  phc slave clock (like /dev/ptp0 or eth0)\n"
    +           "                (may be specified multiple times)\n"
    +           " -f [file]      read configuration from 'file'\n"
     @@ ts2phc.c: static void usage(char *progname)
      int main(int argc, char *argv[])
      {
15:  9897fa520d63 = 18:  de113af64419 ts2phc_slave: print master offset
16:  0888c49126ab = 19:  c395db2c3021 ts2phc: split slave poll from servo loop
17:  1255c2e8faa1 = 20:  349ea4053813 ts2phc: reconfigure sync direction by 
subscribing to ptp4l port events
18:  c18d6a1af98a ! 21:  36eb4c85fdda ts2phc: allow the PHC PPS master to be 
synchronized
    @@ ts2phc.c: int main(int argc, char *argv[])

        ts2phc_cleanup(&priv);

    + ## ts2phc_master.c ##
    +@@ ts2phc_master.c: int ts2phc_master_getppstime(struct ts2phc_master 
*master, struct timespec *ts)
    + {
    +   return master->getppstime(master, ts);
    + }
    ++
    ++struct clock *ts2phc_master_get_clock(struct ts2phc_master *m)
    ++{
    ++  if (m->get_clock)
    ++          return m->get_clock(m);
    ++
    ++  return NULL;
    ++}
    +
      ## ts2phc_master.h ##
     @@ ts2phc_master.h: void ts2phc_master_destroy(struct ts2phc_master 
*master);
       */
    @@ ts2phc_phc_master.c: struct ts2phc_master 
*ts2phc_phc_master_create(struct ts2ph

        master->clock = clock_add(priv, dev);
        if (!master->clock) {
    -@@ ts2phc_phc_master.c: struct ts2phc_master 
*ts2phc_phc_master_create(struct ts2phc_private *priv,
    -
    -   return &master->master;
    - }
    -+
    -+struct clock *ts2phc_master_get_clock(struct ts2phc_master *m)
    -+{
    -+  if (m->get_clock)
    -+          return m->get_clock(m);
    -+
    -+  return NULL;
    -+}

Vladimir Oltean (12):
  phc2sys: break long lines in the PTP management message accessors
  phc2sys: extract PMC functionality into a smaller struct pmc_node
  phc2sys: make PMC functions non-static
  phc2sys: break out pmc code into pmc_common.c
  ts2phc: create a private data structure
  ts2phc: instantiate a full clock structure for every slave PHC
  ts2phc: instantiate a full clock structure for every PHC master
  ts2phc: instantiate a pmc node
  ts2phc_slave: print master offset
  ts2phc: split slave poll from servo loop
  ts2phc: reconfigure sync direction by subscribing to ptp4l port events
  ts2phc: allow the PHC PPS master to be synchronized

 makefile                |   3 +-
 phc2sys.c               | 404 +++------------------------
 pmc_common.c            | 337 ++++++++++++++++++++++
 pmc_common.h            |  35 +++
 ts2phc.8                |  15 +
 ts2phc.c                | 604 +++++++++++++++++++++++++++++++++++++---
 ts2phc.h                |  63 +++++
 ts2phc_generic_master.c |   2 +-
 ts2phc_generic_master.h |   3 +-
 ts2phc_master.c         |  18 +-
 ts2phc_master.h         |   8 +-
 ts2phc_master_private.h |   1 +
 ts2phc_nmea_master.c    |   5 +-
 ts2phc_nmea_master.h    |   3 +-
 ts2phc_phc_master.c     |  42 +--
 ts2phc_phc_master.h     |   3 +-
 ts2phc_slave.c          | 379 ++++++++++++-------------
 ts2phc_slave.h          |  10 +-
 18 files changed, 1305 insertions(+), 630 deletions(-)
 create mode 100644 ts2phc.h

--
2.25.1



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

Reply via email to