On 22/04/2021 09:18, Luigi 'Comio' Mantellini wrote:
> With this patch we introduce the twoStepFlag evaluation at port level.
>
> ---
> config.c | 2 +-
> port.c | 35 ++++++++++++++++++++++++++++++++++-
> ptp4l.8 | 9 ++++-----
> 3 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/config.c b/config.c
> index 4472d3d..f0e1e07 100644
> --- a/config.c
> +++ b/config.c
> @@ -322,7 +322,7 @@ struct config_item config_tab[] = {
> PORT_ITEM_INT("ts2phc.pin_index", 0, 0, INT_MAX),
> GLOB_ITEM_INT("ts2phc.pulsewidth", 500000000, 1000000, 999000000),
> PORT_ITEM_ENU("tsproc_mode", TSPROC_FILTER, tsproc_enu),
> - GLOB_ITEM_INT("twoStepFlag", 1, 0, 1),
> + PORT_ITEM_INT("twoStepFlag", 1, 0, 1),
> GLOB_ITEM_INT("tx_timestamp_timeout", 1, 1, INT_MAX),
> PORT_ITEM_INT("udp_ttl", 1, 1, 255),
> PORT_ITEM_INT("udp6_scope", 0x0E, 0x00, 0x0F),
> diff --git a/port.c b/port.c
> index 10bb9e1..b700ff3 100644
> --- a/port.c
> +++ b/port.c
> @@ -3028,6 +3028,37 @@ err:
> msg_put(msg);
> }
>
> +static enum timestamp_type port_harmonize_onestep(struct port *p, enum
> timestamp_type timestamping, int twoStepFlag)
> +{
> + switch (timestamping) {
> + case TS_SOFTWARE:
> + case TS_LEGACY_HW:
> + if (!twoStepFlag) {
> + pr_err("%s: one step is only possible "
> + "with hardware time stamping", p->log_name);
> + return timestamping;
> + }
> + break;
> + case TS_HARDWARE:
> + if (!twoStepFlag) {
> + pr_debug("%s: upgrading to one step time stamping "
> + "in order to match the port.twoStepFlag",
> p->log_name);
> + return TS_ONESTEP;
> + }
> + break;
> + case TS_ONESTEP:
> + case TS_P2P1STEP:
> + if (twoStepFlag) {
> + pr_debug("%s: degrading to two step time stamping, "
> + "in order to match the port.twoStepFlag",
> p->log_name);
> + return TS_HARDWARE;
> + }
> + break;
> + }
> +
> + return timestamping;
> +}
> +
As user do configure the time_stamping, I think that changing of timestamping
require a flag that permit it, or alternatively generate error.
I think that automatic probing of timestamping without proper configuration may
confuse users.
We should allow time_stamping per port.
Just because I choose one step or two steps, does not guarantee I like the
resulting timestamping.
In addition, the driver might not support the probed timestamping.
I do like probing. I just think it requires a flag that the user can decide to
use it or not.
> struct port *port_open(const char *phc_device,
> int phc_index,
> enum timestamp_type timestamping,
> @@ -3039,6 +3070,7 @@ struct port *port_open(const char *phc_device,
> struct config *cfg = clock_config(clock);
> struct port *p = malloc(sizeof(*p));
> int i;
> + int twoStepFlag;
>
> if (!p) {
> return NULL;
> @@ -3134,7 +3166,8 @@ struct port *port_open(const char *phc_device,
> p->tx_timestamp_offset <<= 16;
> p->link_status = LINK_UP;
> p->clock = clock;
> - p->timestamping = timestamping;
> + twoStepFlag = config_get_int(cfg, p->name, "twoStepFlag");
> + p->timestamping = port_harmonize_onestep(p, timestamping, twoStepFlag);
> p->portIdentity.clockIdentity = clock_identity(clock);
> p->portIdentity.portNumber = number;
> p->state = PS_INITIALIZING;
> diff --git a/ptp4l.8 b/ptp4l.8
> index fe9e150..bea4dae 100644
> --- a/ptp4l.8
> +++ b/ptp4l.8
> @@ -143,6 +143,10 @@ See UNICAST DISCOVERY OPTIONS, below.
> .SH PORT OPTIONS
>
> .TP
> +.B twoStepFlag
> +Enable two-step mode for sync messages. One-step mode can be used only with
> +hardware time stamping.
> +The default is 1 (enabled).
> .B delayAsymmetry
> The time difference in nanoseconds of the transmit and receive
> paths. This value should be positive when the server-to-client
> @@ -378,11 +382,6 @@ to the same subnet.
>
> .SH PROGRAM AND CLOCK OPTIONS
>
> -.TP
> -.B twoStepFlag
> -Enable two-step mode for sync messages. One-step mode can be used only with
> -hardware time stamping.
> -The default is 1 (enabled).
> .TP
> .B clientOnly
> The local clock is a client-only clock if enabled. The default is 0
> (disabled).
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
