Thread: Re: [Linuxptp-devel] [PATCH v3 05/11] synce4l: add synce_dev interface (Page 3)
PTP IEEE 1588 stack for Linux
Brought to you by:
rcochran
Menu
▾
▴
linuxptp-devel
|
From: Aya L. <ay...@nv...> - 2022-06-08 09:08:18
|
On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote:
> synce_dev interface allows creation, initialization, stepping and
> destroying of a single SyncE device.
>
> Newly created device must be given a device name (same as in config
> file).
>
> After creation the SyncE device is initialized with config struct,
> where device-level configuration for this device must exists.
> All ports belonging to this device are also initialized (at least one
> port configured is required).
>
> Once initialized SyncE device is ready for stepping.
> Each step will:
> - verify if device is in running state
> - acquire current state of the DPLL
> - performs actual step of a device, either as internal or external
> powered frequency provider for all the ports confgiured
>
> Destroying the SyncE device releases all its resources.
>
> Co-developed-by: Anatolii Gerasymenko <ana...@in...>
> Signed-off-by: Anatolii Gerasymenko <ana...@in...>
> Co-developed-by: Piotr Kwapulinski <pio...@in...>
> Signed-off-by: Piotr Kwapulinski <pio...@in...>
> Co-developed-by: Michal Michalik <mic...@in...>
> Signed-off-by: Michal Michalik <mic...@in...>
> Signed-off-by: Arkadiusz Kubalewski <ark...@in...>
> ---
> v2: updated license headers
> v3: rebase patch series
>
> synce_dev.c | 622 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> synce_dev.h | 64 ++++++
> 2 files changed, 686 insertions(+)
> create mode 100644 synce_dev.c
> create mode 100644 synce_dev.h
>
> diff --git a/synce_dev.c b/synce_dev.c
> new file mode 100644
> index 0000000..dfe7ff4
> --- /dev/null
> +++ b/synce_dev.c
> @@ -0,0 +1,622 @@
> +/**
> + * @file synce_dev.c
> + * @brief Interface for handling Sync-E capable devices and its ports
> + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation
> + * @note SPDX-License-Identifier: GPL-2.0+
> + */
> +#include <stdlib.h>
> +#include <sys/queue.h>
> +#include <net/if.h>
> +#include <errno.h>
> +#include "synce_dev.h"
> +#include "print.h"
> +#include "config.h"
> +#include "util.h"
> +#include "synce_port.h"
> +#include "missing.h"
> +#include "synce_dev_ctrl.h"
> +#include "synce_msg.h"
> +
> +struct interface {
> + STAILQ_ENTRY(interface) list;
> +};
> +
> +struct synce_dev_ops {
> + int (*update_ql)(struct synce_dev *dev);
> + int (*step)(struct synce_dev *dev);
> +};
> +
> +enum synce_dev_state {
> + DEVICE_UNKNOWN,
> + DEVICE_CREATED,
> + DEVICE_INITED,
> + DEVICE_RUNNING,
> + DEVICE_FAILED,
> +};
> +
> +struct synce_dev {
> + LIST_ENTRY(synce_dev) list;
> + enum synce_dev_state state;
> + char name[IF_NAMESIZE];
> + LIST_HEAD(synce_ports_head, synce_port) ports;
> + struct synce_port *best_source;
> + int num_ports;
> + int internal_input;
> + int external_input;
> + int network_option;
> + uint8_t ql;
> + uint8_t ext_ql;
> + int extended_tlv;
> + int recover_time;
> + enum dpll_state d_state;
> + enum dpll_state last_d_state;
> + struct synce_dev_ctrl *dc;
> + struct synce_dev_ops ops;
> +};
> +
> +static int add_port(struct synce_dev *dev, struct synce_port *port)
> +{
> + struct synce_port *port_iter, *last_port = NULL;
> +
> + LIST_FOREACH(port_iter, &dev->ports, list) {
> + last_port = port_iter;
> + }
> +
> + if (last_port) {
> + LIST_INSERT_AFTER(last_port, port, list);
> + } else {
> + LIST_INSERT_HEAD(&dev->ports, port, list);
> + }
> + return 0;
> +}
> +
> +static int rx_enabled(struct synce_dev *dev)
> +{
> + return (dev->external_input == 0 &&
> + dev->internal_input != 0);
> +}
> +
> +static void destroy_ports(struct synce_dev *dev)
> +{
> + struct synce_port *port, *tmp;
> +
> + LIST_FOREACH_SAFE(port, &dev->ports, list, tmp) {
> + synce_port_destroy(port);
> + LIST_REMOVE(port, list);
> + free(port);
> + }
> + dev->num_ports = 0;
> +}
> +
> +static void destroy_dev_ctrl(struct synce_dev *dev)
> +{
> + free(dev->dc);
> + dev->dc = NULL;
> +}
> +
> +static int init_ports(int *count, struct synce_dev *dev, struct config *cfg)
> +{
> + struct interface *iface;
> + struct synce_port *port;
> + const char *port_name;
> +
> + *count = 0;
> +
> + STAILQ_FOREACH(iface, &cfg->interfaces, list) {
> + /* given device takes care only of its child ports */
> + if (interface_se_has_parent_dev(iface) &&
> + (strncmp(interface_se_get_parent_dev_label(iface),
> + dev->name, sizeof(dev->name)) == 0)) {
> + port_name = interface_name(iface);
> +
> + /* If no sync mode, no need to maintain the port */
> + if (!synce_port_in_sync_mode(cfg, port_name)) {
> + continue;
> + }
> +
> + port = synce_port_create(port_name);
> + if (!port) {
> + pr_err("failed to create port %s on device %s",
> + port_name, dev->name);
> + continue;
> + }
> +
> + if (synce_port_init(port, cfg, dev->network_option,
> + dev->extended_tlv, rx_enabled(dev),
> + dev->recover_time, dev->ql,
> + dev->ext_ql)) {
> + pr_err("failed to configure port %s on device %s",
> + port_name, dev->name);
> + synce_port_destroy(port);
> + continue;
> + }
> +
> + if (add_port(dev, port)) {
> + pr_err("failed to add port %s to device %s",
> + port_name, dev->name);
> + synce_port_destroy(port);
> + continue;
> + } else {
> + (*count)++;
> + pr_debug("port %s added on device %s addr %p",
> + port_name, dev->name, port);
> + }
> + }
> + }
> +
> + if (*count == 0) {
> + pr_err("device %s has no ports configured", dev->name);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static void update_dev_state(struct synce_dev *dev)
> +{
> + struct synce_port *p;
> + int count = 0;
> +
> + LIST_FOREACH(p, &dev->ports, list) {
> + if (synce_port_thread_running(p)) {
> + count++;
> + }
> + }
> +
> + if (dev->num_ports == count) {
> + dev->state = DEVICE_RUNNING;
> + } else {
> + pr_warning("found %d ports running - %d configured on %s",
> + count, dev->num_ports, dev->name);
> + }
> +}
> +
> +static int port_set_dnu(struct synce_port *p, int extended_tlv)
> +{
> + int ret;
> +
> + if (!p) {
> + pr_err("%s port is NULL", __func__);
> + ret = -EFAULT;
> + return ret;
> + }
> +
> + ret = synce_port_set_tx_ql_dnu(p, extended_tlv);
> + if (ret) {
> + pr_err("set tx DNU fail on %s", synce_port_get_name(p));
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int port_set_ql_external_input(struct synce_port *p, int extended)
> +{
> + int ret = synce_port_set_tx_ql_forced(p, extended);
> +
> + if (ret) {
> + pr_err("set QL external failed");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int update_ql_external_input(struct synce_dev *dev)
> +{
> + struct synce_port *p;
> + int ret = 0;
> +
> + LIST_FOREACH(p, &dev->ports, list) {
> + if (dev->d_state == DPLL_HOLDOVER) {
> + ret = port_set_dnu(p, dev->extended_tlv);
> + } else if (dev->d_state == DPLL_LOCKED ||
> + dev->d_state == DPLL_LOCKED_HO_ACQ) {
> + ret = port_set_ql_external_input(p, dev->extended_tlv);
> + }
> +
> + if (ret) {
> + pr_err("update QL failed d_state %d, err:%d on %s",
> + dev->d_state, ret, dev->name);
> + break;
> + }
> +
> + }
> +
> + return ret;
> +}
> +
> +static int port_set_ql_internal_input(struct synce_dev *dev,
> + struct synce_port *p,
> + struct synce_port *best_p)
> +{
> + int ret = synce_port_set_tx_ql_from_best_input(p, best_p,
> + dev->extended_tlv);
> +
> + if (ret) {
> + pr_err("set QL failed");
> + return ret;
> + }
> +
> + if (!ret) {
> + pr_debug("%s on %s", __func__, dev->name);
> + }
> +
> + return ret;
> +}
> +
> +static int update_ql_internal_input(struct synce_dev *dev)
> +{
> + struct synce_port *p, *best_p = dev->best_source;
> + int ret = 0;
> +
> + LIST_FOREACH(p, &dev->ports, list) {
> + if (dev->d_state == DPLL_HOLDOVER) {
> + pr_debug("act on DPLL_HOLDOVER for %s",
> + synce_port_get_name(p));
> + ret = port_set_dnu(p, dev->extended_tlv);
> + if (ret) {
> + pr_err("%s set DNU failed on %s",
> + __func__, dev->name);
> + return ret;
> + }
> + } else if ((dev->d_state == DPLL_LOCKED ||
> + dev->d_state == DPLL_LOCKED_HO_ACQ) && best_p) {
> + pr_debug("act on DPLL_LOCKED/DPLL_LOCKED_HO_ACQ for %s",
> + synce_port_get_name(p));
> + /* on best port send DNU, all the others
> + * propagate what came from best source
> + */
> + if (p != best_p) {
> + ret = port_set_ql_internal_input(dev, p,
> + best_p);
> + } else {
> + ret = port_set_dnu(p, dev->extended_tlv);
> + }
> +
> + if (ret) {
> + pr_err("%s set failed on %s",
> + __func__, dev->name);
> + return ret;
> + }
> + } else {
> + pr_debug("nothing to do for %s d_state %d, best_p %p",
> + synce_port_get_name(p), dev->d_state, best_p);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void detach_port_dpll(struct synce_port *port, struct synce_dev *dev)
> +{
> + int ret = synce_port_disable_recover_clock(port); > +
> + if (ret) {
> + pr_err("disable recover clock cmd failed on %s", dev->name);
> + return;
> + } > +}
This is not abstract enough, too HW spesific
> +
> +static void force_all_dplls_detach(struct synce_dev *dev)
> +{
> + enum dpll_state state;
> + struct synce_port *p;
> +
> + LIST_FOREACH(p, &dev->ports, list) {
> + pr_debug("trying to detach DPLL RCLK for %s",
> + synce_port_get_name(p));
> + detach_port_dpll(p, dev);
> + }
> +
> + if (synce_dev_ctrl_get_state(dev->dc, &state)) {
> + pr_err("failed getting DPLL state");
> + dev->last_d_state = DPLL_UNKNOWN;
> + dev->d_state = DPLL_UNKNOWN;
> + } else {
> + dev->last_d_state = state;
> + dev->d_state = state;
> + }
> +};
> +
> +static void dev_update_ql(struct synce_dev *dev)
> +{
> + if (dev->ops.update_ql(dev)) {
> + pr_err("update QL fail on %s", dev->name);
> + }
> +}
> +
> +static int rx_ql_changed(struct synce_dev *dev)
> +{
> + struct synce_port *p;
> + int ret = 0;
> +
> + LIST_FOREACH(p, &dev->ports, list) {
> + ret = synce_port_rx_ql_changed(p);
> + if (ret) {
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static struct synce_port *find_dev_best_source(struct synce_dev *dev)
> +{
> + struct synce_port *p, *best_p = dev->best_source;
> +
> + LIST_FOREACH(p, &dev->ports, list) {
> + if (best_p != p) {
> + if (synce_port_compare_ql(best_p, p) == p) {
> + pr_debug("old best %s replaced by %s on %s",
> + synce_port_get_name(best_p),
> + synce_port_get_name(p), dev->name);
> + best_p = p;
> + }
> + }
> + }
> +
> + if (best_p) {
> + if (synce_port_is_rx_dnu(best_p)) {
> + return NULL;
> + }
> + }
> +
> + return best_p;
> +}
> +
> +static int set_input_source(struct synce_dev *dev,
> + struct synce_port *new_best_source)
> +{
> + const char *best_name = synce_port_get_name(new_best_source);
> + int ret;
> +
> + if (!best_name) {
> + pr_err("get best input name failed on %s", dev->name);
> + return -ENXIO;
> + }
> +
> + ret = synce_port_enable_recover_clock(new_best_source);
> + if (ret) {
> + pr_err("enable recover clock cmd failed on %s", dev->name);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int act_on_d_state(struct synce_dev *dev)
> +{
> + int ret = 0;
> +
> + if (dev->d_state != dev->last_d_state) {
> + ret = dev->ops.update_ql(dev);
> + if (ret) {
> + pr_err("update QL fail on %s", dev->name);
> + } else {
> + dev->last_d_state = dev->d_state;
> + pr_debug("%s QL updated on %s", __func__, dev->name);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int dev_step_external_input(struct synce_dev *dev)
> +{
> + return act_on_d_state(dev);
> +}
> +
> +static void choose_best_source(struct synce_dev *dev)
> +{
> + struct synce_port *new_best;
> +
> + new_best = find_dev_best_source(dev);
> + if (!new_best) {
> + pr_info("best source not found on %s", dev->name);
> + force_all_dplls_detach(dev);
> + dev_update_ql(dev);
> + dev->best_source = NULL;
> + } else if (new_best != dev->best_source) {
> + force_all_dplls_detach(dev);
> + if (set_input_source(dev, new_best)) {
> + pr_err("set best source failed on %s",
> + dev->name);
> + } else {
> + /* if input source is changing
> + * current input is invalid, send DNU and wait
> + * for DPLL being locked in further dev_step
> + */
> + dev_update_ql(dev);
> + /* DPLL was invalidated we can now set new
> + * best_source for further use
> + */
> + dev->best_source = new_best;
> + }
> + } else {
> + pr_info("clock source has not changed on %s", dev->name);
> + /* no port change, just update QL on TX */
> + dev_update_ql(dev);
> +
> + }
> +}
> +
> +static int dev_step_internal_input(struct synce_dev *dev)
> +{
> + int ret;
> +
> + ret = act_on_d_state(dev);
> + if (ret) {
> + pr_err("act on d_state fail on %s", dev->name);
> + return ret;
> + }
> +
> + if (rx_ql_changed(dev)) {
> + choose_best_source(dev);
> + } else if (dev->best_source) {
> + if (synce_port_rx_ql_failed(dev->best_source)) {
> + synce_port_invalidate_rx_ql(dev->best_source);
> + force_all_dplls_detach(dev);
> + dev_update_ql(dev);
> + dev->best_source = NULL;
> + choose_best_source(dev);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void init_ops(struct synce_dev *dev)
> +{
> + if (rx_enabled(dev)) {
> + dev->ops.update_ql = &update_ql_internal_input;
> + dev->ops.step = &dev_step_internal_input;
> + } else {
> + dev->ops.update_ql = &update_ql_external_input;
> + dev->ops.step = &dev_step_external_input;
> + }
> +}
> +
> +int synce_dev_init(struct synce_dev *dev, struct config *cfg)
> +{
> + const char *dpll_get_state_cmd;
> + struct dpll_state_str dss;
> + int count, ret;
> +
> + if (dev->state != DEVICE_CREATED) {
> + goto err;
> + }
> +
> + LIST_INIT(&dev->ports);
> + dev->internal_input = config_get_int(cfg, dev->name, "internal_input");
> + dev->external_input = config_get_int(cfg, dev->name, "external_input");
> + dev->ql = config_get_int(cfg, dev->name, "external_input_QL");
> + dev->ext_ql = config_get_int(cfg, dev->name, "external_input_ext_QL");
> + dev->extended_tlv = config_get_int(cfg, dev->name, "extended_tlv");
> + dev->network_option = config_get_int(cfg, dev->name, "network_option");
> + dev->recover_time = config_get_int(cfg, dev->name, "recover_time");
> + dev->best_source = NULL;
> + dpll_get_state_cmd = config_get_string(cfg, dev->name, "dpll_get_state_cmd");
> + dss.holdover = config_get_string(cfg, dev->name, "dpll_holdover_value");
> + dss.locked_ho = config_get_string(cfg, dev->name, "dpll_locked_ho_value");
> + dss.locked = config_get_string(cfg, dev->name, "dpll_locked_value");
> + dss.freerun = config_get_string(cfg, dev->name, "dpll_freerun_value");
> + dss.invalid = config_get_string(cfg, dev->name, "dpll_invalid_value");
> + dev->dc = synce_dev_ctrl_create();
> + if (!dev->dc) {
> + pr_err("device_ctrl create fail on %s", dev->name);
> + goto err;
> + }
> +
> + if (dev->external_input && dev->internal_input) {
> + pr_warning("external_input and internal_input are mutually exclusive - disabling internal_input cfg option");
> + dev->internal_input = 0;
> + } else if (!dev->external_input && !dev->internal_input) {
> + pr_err("either external_input or internal_input need to be set to 1 - aborting initialization");
> + goto err;
> + }
> +
> + ret = synce_dev_ctrl_init(dev->dc, dev->name, dpll_get_state_cmd, &dss);
> + if (ret) {
> + pr_err("synce_dev_ctrl init ret %d on %s", ret, dev->name);
> + goto err;
> + }
> +
> + if (init_ports(&count, dev, cfg))
> + goto err;
> +
> + init_ops(dev);
> + dev->num_ports = count;
> + dev->state = DEVICE_INITED;
> +
> + dev->d_state = DPLL_HOLDOVER;
> + dev->ops.update_ql(dev);
> +
> + /* in case somebody manually set recovered clock before */
> + if (dev->internal_input) {
> + force_all_dplls_detach(dev);
> + }
> + pr_info("inited num_ports %d for %s", count, dev->name);
> +
> + return 0;
> +
> +err:
> + dev->state = DEVICE_FAILED;
> + pr_err("%s failed for %s", __func__, dev->name);
> + return -ENODEV;
> +}
> +
> +struct synce_dev *synce_dev_create(const char *dev_name)
> +{
> + struct synce_dev *dev = NULL;
> +
> + if (!dev_name) {
> + return NULL;
> + }
> +
> + dev = malloc(sizeof(struct synce_dev));
> + if (!dev) {
> + return NULL;
> + }
> +
> + memcpy(dev->name, dev_name, sizeof(dev->name));
> + dev->state = DEVICE_CREATED;
> + dev->d_state = DPLL_UNKNOWN;
> + dev->last_d_state = DPLL_UNKNOWN;
> + pr_debug("created %s", dev->name);
> +
> + return dev;
> +}
> +
> +int synce_dev_step(struct synce_dev *dev)
> +{
> + int ret = -EFAULT;
> +
> + if (!dev) {
> + pr_err("%s dev is NULL", __func__);
> + return ret;
> + }
> +
> + update_dev_state(dev);
> + if (dev->state != DEVICE_RUNNING) {
> + pr_err("dev %s is not running", dev->name);
> + return ret;
> + }
> +
> + ret = synce_dev_ctrl_get_state(dev->dc, &dev->d_state);
> + if (ret) {
> + pr_warning("could not acquire dpll state on %s", dev->name);
> + return ret;
> + }
> +
> + ret = dev->ops.step(dev);
> +
> + return ret;
> +}
> +
> +const char *synce_dev_name(struct synce_dev *dev)
> +{
> + return dev->name;
> +}
> +
> +int synce_dev_is_running(struct synce_dev *dev)
> +{
> + update_dev_state(dev);
> +
> + return !!(dev->state & DEVICE_RUNNING);
> +}
> +
> +void synce_dev_destroy(struct synce_dev *dev)
> +{
> + if (!dev) {
> + pr_err("%s dev is NULL", __func__);
> + return;
> + }
> +
> + if (dev->internal_input && dev->state != DEVICE_FAILED) {
> + force_all_dplls_detach(dev);
> + }
> +
> + destroy_ports(dev);
> + destroy_dev_ctrl(dev);
> +}
> diff --git a/synce_dev.h b/synce_dev.h
> new file mode 100644
> index 0000000..6807c2b
> --- /dev/null
> +++ b/synce_dev.h
> @@ -0,0 +1,64 @@
> +/**
> + * @file synce_dev.h
> + * @brief Interface for handling Sync-E capable devices and its ports
> + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation
> + * @note SPDX-License-Identifier: GPL-2.0+
> + */
> +#ifndef HAVE_SYNCE_DEV_H
> +#define HAVE_SYNCE_DEV_H
> +
> +#include <stdint.h>
> +
> +struct config;
> +struct synce_dev;
> +
> +/**
> + * Initialize Sync-E device and its ports after device was created.
> + *
> + * @param dev Device to be initialized
> + * @param cfg Configuration struct based on SYNCE type, must hold
> + * properities of configured device ports
> + * @return 0 on success, failure otherwise
> + */
> +int synce_dev_init(struct synce_dev *dev, struct config *cfg);
> +
> +/**
> + * Alloc memory for a single Sync-E device.
> + *
> + * @param dev_name Human readable name of a device
> + * @return 0 on success, failure otherwise
> + */
> +struct synce_dev *synce_dev_create(const char *dev_name);
> +
> +/**
> + * Step a Sync-E device. Probe for events, changes and act on them.
> + *
> + * @param dev Device to be stepped
> + * @return 0 on success, failure otherwise
> + */
> +int synce_dev_step(struct synce_dev *dev);
> +
> +/**
> + * Acquire Sync-E device name.
> + *
> + * @param dev Questioned SyncE device instance
> + * @return The device name
> + */
> +const char *synce_dev_name(struct synce_dev *dev);
> +
> +/**
> + * Clean-up on memory allocated for device and its ports.
> + *
> + * @param dev SyncE device to be cleared
> + */
> +void synce_dev_destroy(struct synce_dev *dev);
> +
> +/**
> + * Check if Sync-E device is running.
> + *
> + * @param dev Questioned SyncE device
> + * @return 0 if false, 1 if true
> + */
> +int synce_dev_is_running(struct synce_dev *dev);
> +
> +#endif
|
|
From: Erez <ere...@gm...> - 2022-06-08 09:56:39
|
On Wed, 8 Jun 2022 at 11:11, Aya Levin via Linuxptp-devel <
lin...@li...> wrote:
>
>
> On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote:
> > synce_dev interface allows creation, initialization, stepping and
> > destroying of a single SyncE device.
> >
> > Newly created device must be given a device name (same as in config
> > file).
> >
> > After creation the SyncE device is initialized with config struct,
> > where device-level configuration for this device must exists.
> > All ports belonging to this device are also initialized (at least one
> > port configured is required).
> >
> > Once initialized SyncE device is ready for stepping.
> > Each step will:
> > - verify if device is in running state
> > - acquire current state of the DPLL
> > - performs actual step of a device, either as internal or external
> > powered frequency provider for all the ports confgiured
> >
> > Destroying the SyncE device releases all its resources.
> >
> > Co-developed-by: Anatolii Gerasymenko <ana...@in...>
> > Signed-off-by: Anatolii Gerasymenko <ana...@in...>
> > Co-developed-by: Piotr Kwapulinski <pio...@in...>
> > Signed-off-by: Piotr Kwapulinski <pio...@in...>
> > Co-developed-by: Michal Michalik <mic...@in...>
> > Signed-off-by: Michal Michalik <mic...@in...>
> > Signed-off-by: Arkadiusz Kubalewski <ark...@in...>
> > ---
> > v2: updated license headers
> > v3: rebase patch series
> >
> > synce_dev.c | 622 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > synce_dev.h | 64 ++++++
> > 2 files changed, 686 insertions(+)
> > create mode 100644 synce_dev.c
> > create mode 100644 synce_dev.h
> >
> > diff --git a/synce_dev.c b/synce_dev.c
> > new file mode 100644
> > index 0000000..dfe7ff4
> > --- /dev/null
> > +++ b/synce_dev.c
> > @@ -0,0 +1,622 @@
> > +/**
> > + * @file synce_dev.c
> > + * @brief Interface for handling Sync-E capable devices and its ports
> > + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation
> > + * @note SPDX-License-Identifier: GPL-2.0+
> > + */
> > +#include <stdlib.h>
> > +#include <sys/queue.h>
> > +#include <net/if.h>
> > +#include <errno.h>
> > +#include "synce_dev.h"
> > +#include "print.h"
> > +#include "config.h"
> > +#include "util.h"
> > +#include "synce_port.h"
> > +#include "missing.h"
> > +#include "synce_dev_ctrl.h"
> > +#include "synce_msg.h"
> > +
> > +struct interface {
> > + STAILQ_ENTRY(interface) list;
> > +};
> > +
> > +struct synce_dev_ops {
> > + int (*update_ql)(struct synce_dev *dev);
> > + int (*step)(struct synce_dev *dev);
> > +};
> > +
> > +enum synce_dev_state {
> > + DEVICE_UNKNOWN,
> > + DEVICE_CREATED,
> > + DEVICE_INITED,
> > + DEVICE_RUNNING,
> > + DEVICE_FAILED,
> > +};
> > +
> > +struct synce_dev {
> > + LIST_ENTRY(synce_dev) list;
> > + enum synce_dev_state state;
> > + char name[IF_NAMESIZE];
> > + LIST_HEAD(synce_ports_head, synce_port) ports;
> > + struct synce_port *best_source;
> > + int num_ports;
> > + int internal_input;
> > + int external_input;
> > + int network_option;
> > + uint8_t ql;
> > + uint8_t ext_ql;
> > + int extended_tlv;
> > + int recover_time;
> > + enum dpll_state d_state;
> > + enum dpll_state last_d_state;
> > + struct synce_dev_ctrl *dc;
> > + struct synce_dev_ops ops;
> > +};
> > +
> > +static int add_port(struct synce_dev *dev, struct synce_port *port)
> > +{
> > + struct synce_port *port_iter, *last_port = NULL;
> > +
> > + LIST_FOREACH(port_iter, &dev->ports, list) {
> > + last_port = port_iter;
> > + }
> > +
> > + if (last_port) {
> > + LIST_INSERT_AFTER(last_port, port, list);
> > + } else {
> > + LIST_INSERT_HEAD(&dev->ports, port, list);
> > + }
> > + return 0;
> > +}
> > +
> > +static int rx_enabled(struct synce_dev *dev)
> > +{
> > + return (dev->external_input == 0 &&
> > + dev->internal_input != 0);
> > +}
> > +
> > +static void destroy_ports(struct synce_dev *dev)
> > +{
> > + struct synce_port *port, *tmp;
> > +
> > + LIST_FOREACH_SAFE(port, &dev->ports, list, tmp) {
> > + synce_port_destroy(port);
> > + LIST_REMOVE(port, list);
> > + free(port);
> > + }
> > + dev->num_ports = 0;
> > +}
> > +
> > +static void destroy_dev_ctrl(struct synce_dev *dev)
> > +{
> > + free(dev->dc);
> > + dev->dc = NULL;
> > +}
> > +
> > +static int init_ports(int *count, struct synce_dev *dev, struct config
> *cfg)
> > +{
> > + struct interface *iface;
> > + struct synce_port *port;
> > + const char *port_name;
> > +
> > + *count = 0;
> > +
> > + STAILQ_FOREACH(iface, &cfg->interfaces, list) {
> > + /* given device takes care only of its child ports */
> > + if (interface_se_has_parent_dev(iface) &&
> > + (strncmp(interface_se_get_parent_dev_label(iface),
> > + dev->name, sizeof(dev->name)) == 0)) {
> > + port_name = interface_name(iface);
> > +
> > + /* If no sync mode, no need to maintain the port */
> > + if (!synce_port_in_sync_mode(cfg, port_name)) {
> > + continue;
> > + }
> > +
> > + port = synce_port_create(port_name);
> > + if (!port) {
> > + pr_err("failed to create port %s on device
> %s",
> > + port_name, dev->name);
> > + continue;
> > + }
> > +
> > + if (synce_port_init(port, cfg, dev->network_option,
> > + dev->extended_tlv,
> rx_enabled(dev),
> > + dev->recover_time, dev->ql,
> > + dev->ext_ql)) {
> > + pr_err("failed to configure port %s on
> device %s",
> > + port_name, dev->name);
> > + synce_port_destroy(port);
> > + continue;
> > + }
> > +
> > + if (add_port(dev, port)) {
> > + pr_err("failed to add port %s to device
> %s",
> > + port_name, dev->name);
> > + synce_port_destroy(port);
> > + continue;
> > + } else {
> > + (*count)++;
> > + pr_debug("port %s added on device %s addr
> %p",
> > + port_name, dev->name, port);
> > + }
> > + }
> > + }
> > +
> > + if (*count == 0) {
> > + pr_err("device %s has no ports configured", dev->name);
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void update_dev_state(struct synce_dev *dev)
> > +{
> > + struct synce_port *p;
> > + int count = 0;
> > +
> > + LIST_FOREACH(p, &dev->ports, list) {
> > + if (synce_port_thread_running(p)) {
> > + count++;
> > + }
> > + }
> > +
> > + if (dev->num_ports == count) {
> > + dev->state = DEVICE_RUNNING;
> > + } else {
> > + pr_warning("found %d ports running - %d configured on %s",
> > + count, dev->num_ports, dev->name);
> > + }
> > +}
> > +
> > +static int port_set_dnu(struct synce_port *p, int extended_tlv)
> > +{
> > + int ret;
> > +
> > + if (!p) {
> > + pr_err("%s port is NULL", __func__);
> > + ret = -EFAULT;
> > + return ret;
> > + }
> > +
> > + ret = synce_port_set_tx_ql_dnu(p, extended_tlv);
> > + if (ret) {
> > + pr_err("set tx DNU fail on %s", synce_port_get_name(p));
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int port_set_ql_external_input(struct synce_port *p, int
> extended)
> > +{
> > + int ret = synce_port_set_tx_ql_forced(p, extended);
> > +
> > + if (ret) {
> > + pr_err("set QL external failed");
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int update_ql_external_input(struct synce_dev *dev)
> > +{
> > + struct synce_port *p;
> > + int ret = 0;
> > +
> > + LIST_FOREACH(p, &dev->ports, list) {
> > + if (dev->d_state == DPLL_HOLDOVER) {
> > + ret = port_set_dnu(p, dev->extended_tlv);
> > + } else if (dev->d_state == DPLL_LOCKED ||
> > + dev->d_state == DPLL_LOCKED_HO_ACQ) {
> > + ret = port_set_ql_external_input(p,
> dev->extended_tlv);
> > + }
> > +
> > + if (ret) {
> > + pr_err("update QL failed d_state %d, err:%d on %s",
> > + dev->d_state, ret, dev->name);
> > + break;
> > + }
> > +
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int port_set_ql_internal_input(struct synce_dev *dev,
> > + struct synce_port *p,
> > + struct synce_port *best_p)
> > +{
> > + int ret = synce_port_set_tx_ql_from_best_input(p, best_p,
> > + dev->extended_tlv);
> > +
> > + if (ret) {
> > + pr_err("set QL failed");
> > + return ret;
> > + }
> > +
> > + if (!ret) {
> > + pr_debug("%s on %s", __func__, dev->name);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int update_ql_internal_input(struct synce_dev *dev)
> > +{
> > + struct synce_port *p, *best_p = dev->best_source;
> > + int ret = 0;
> > +
> > + LIST_FOREACH(p, &dev->ports, list) {
> > + if (dev->d_state == DPLL_HOLDOVER) {
> > + pr_debug("act on DPLL_HOLDOVER for %s",
> > + synce_port_get_name(p));
> > + ret = port_set_dnu(p, dev->extended_tlv);
> > + if (ret) {
> > + pr_err("%s set DNU failed on %s",
> > + __func__, dev->name);
> > + return ret;
> > + }
> > + } else if ((dev->d_state == DPLL_LOCKED ||
> > + dev->d_state == DPLL_LOCKED_HO_ACQ) && best_p) {
> > + pr_debug("act on DPLL_LOCKED/DPLL_LOCKED_HO_ACQ
> for %s",
> > + synce_port_get_name(p));
> > + /* on best port send DNU, all the others
> > + * propagate what came from best source
> > + */
> > + if (p != best_p) {
> > + ret = port_set_ql_internal_input(dev, p,
> > + best_p);
> > + } else {
> > + ret = port_set_dnu(p, dev->extended_tlv);
> > + }
> > +
> > + if (ret) {
> > + pr_err("%s set failed on %s",
> > + __func__, dev->name);
> > + return ret;
> > + }
> > + } else {
> > + pr_debug("nothing to do for %s d_state %d, best_p
> %p",
> > + synce_port_get_name(p), dev->d_state,
> best_p);
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static void detach_port_dpll(struct synce_port *port, struct synce_dev
> *dev)
> > +{
> > + int ret = synce_port_disable_recover_clock(port); > +
> > + if (ret) {
> > + pr_err("disable recover clock cmd failed on %s",
> dev->name);
> > + return;
> > + } > +}
> This is not abstract enough, too HW spesific
>
Sorry for interfering,
Could you explain what is "too hardware specific"?
What interface? What would you expect here instead?
> > +
> > +static void force_all_dplls_detach(struct synce_dev *dev)
> > +{
> > + enum dpll_state state;
> > + struct synce_port *p;
> > +
> > + LIST_FOREACH(p, &dev->ports, list) {
> > + pr_debug("trying to detach DPLL RCLK for %s",
> > + synce_port_get_name(p));
> > + detach_port_dpll(p, dev);
> > + }
> > +
> > + if (synce_dev_ctrl_get_state(dev->dc, &state)) {
> > + pr_err("failed getting DPLL state");
> > + dev->last_d_state = DPLL_UNKNOWN;
> > + dev->d_state = DPLL_UNKNOWN;
> > + } else {
> > + dev->last_d_state = state;
> > + dev->d_state = state;
> > + }
> > +};
> > +
> > +static void dev_update_ql(struct synce_dev *dev)
> > +{
> > + if (dev->ops.update_ql(dev)) {
> > + pr_err("update QL fail on %s", dev->name);
> > + }
> > +}
> > +
> > +static int rx_ql_changed(struct synce_dev *dev)
> > +{
> > + struct synce_port *p;
> > + int ret = 0;
> > +
> > + LIST_FOREACH(p, &dev->ports, list) {
> > + ret = synce_port_rx_ql_changed(p);
> > + if (ret) {
> > + break;
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static struct synce_port *find_dev_best_source(struct synce_dev *dev)
> > +{
> > + struct synce_port *p, *best_p = dev->best_source;
> > +
> > + LIST_FOREACH(p, &dev->ports, list) {
> > + if (best_p != p) {
> > + if (synce_port_compare_ql(best_p, p) == p) {
> > + pr_debug("old best %s replaced by %s on
> %s",
> > + synce_port_get_name(best_p),
> > + synce_port_get_name(p),
> dev->name);
> > + best_p = p;
> > + }
> > + }
> > + }
> > +
> > + if (best_p) {
> > + if (synce_port_is_rx_dnu(best_p)) {
> > + return NULL;
> > + }
> > + }
> > +
> > + return best_p;
> > +}
> > +
> > +static int set_input_source(struct synce_dev *dev,
> > + struct synce_port *new_best_source)
> > +{
> > + const char *best_name = synce_port_get_name(new_best_source);
> > + int ret;
> > +
> > + if (!best_name) {
> > + pr_err("get best input name failed on %s", dev->name);
> > + return -ENXIO;
> > + }
> > +
> > + ret = synce_port_enable_recover_clock(new_best_source);
> > + if (ret) {
> > + pr_err("enable recover clock cmd failed on %s", dev->name);
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int act_on_d_state(struct synce_dev *dev)
> > +{
> > + int ret = 0;
> > +
> > + if (dev->d_state != dev->last_d_state) {
> > + ret = dev->ops.update_ql(dev);
> > + if (ret) {
> > + pr_err("update QL fail on %s", dev->name);
> > + } else {
> > + dev->last_d_state = dev->d_state;
> > + pr_debug("%s QL updated on %s", __func__,
> dev->name);
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int dev_step_external_input(struct synce_dev *dev)
> > +{
> > + return act_on_d_state(dev);
> > +}
> > +
> > +static void choose_best_source(struct synce_dev *dev)
> > +{
> > + struct synce_port *new_best;
> > +
> > + new_best = find_dev_best_source(dev);
> > + if (!new_best) {
> > + pr_info("best source not found on %s", dev->name);
> > + force_all_dplls_detach(dev);
> > + dev_update_ql(dev);
> > + dev->best_source = NULL;
> > + } else if (new_best != dev->best_source) {
> > + force_all_dplls_detach(dev);
> > + if (set_input_source(dev, new_best)) {
> > + pr_err("set best source failed on %s",
> > + dev->name);
> > + } else {
> > + /* if input source is changing
> > + * current input is invalid, send DNU and wait
> > + * for DPLL being locked in further dev_step
> > + */
> > + dev_update_ql(dev);
> > + /* DPLL was invalidated we can now set new
> > + * best_source for further use
> > + */
> > + dev->best_source = new_best;
> > + }
> > + } else {
> > + pr_info("clock source has not changed on %s", dev->name);
> > + /* no port change, just update QL on TX */
> > + dev_update_ql(dev);
> > +
> > + }
> > +}
> > +
> > +static int dev_step_internal_input(struct synce_dev *dev)
> > +{
> > + int ret;
> > +
> > + ret = act_on_d_state(dev);
> > + if (ret) {
> > + pr_err("act on d_state fail on %s", dev->name);
> > + return ret;
> > + }
> > +
> > + if (rx_ql_changed(dev)) {
> > + choose_best_source(dev);
> > + } else if (dev->best_source) {
> > + if (synce_port_rx_ql_failed(dev->best_source)) {
> > + synce_port_invalidate_rx_ql(dev->best_source);
> > + force_all_dplls_detach(dev);
> > + dev_update_ql(dev);
> > + dev->best_source = NULL;
> > + choose_best_source(dev);
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static void init_ops(struct synce_dev *dev)
> > +{
> > + if (rx_enabled(dev)) {
> > + dev->ops.update_ql = &update_ql_internal_input;
> > + dev->ops.step = &dev_step_internal_input;
> > + } else {
> > + dev->ops.update_ql = &update_ql_external_input;
> > + dev->ops.step = &dev_step_external_input;
> > + }
> > +}
> > +
> > +int synce_dev_init(struct synce_dev *dev, struct config *cfg)
> > +{
> > + const char *dpll_get_state_cmd;
> > + struct dpll_state_str dss;
> > + int count, ret;
> > +
> > + if (dev->state != DEVICE_CREATED) {
> > + goto err;
> > + }
> > +
> > + LIST_INIT(&dev->ports);
> > + dev->internal_input = config_get_int(cfg, dev->name,
> "internal_input");
> > + dev->external_input = config_get_int(cfg, dev->name,
> "external_input");
> > + dev->ql = config_get_int(cfg, dev->name, "external_input_QL");
> > + dev->ext_ql = config_get_int(cfg, dev->name,
> "external_input_ext_QL");
> > + dev->extended_tlv = config_get_int(cfg, dev->name, "extended_tlv");
> > + dev->network_option = config_get_int(cfg, dev->name,
> "network_option");
> > + dev->recover_time = config_get_int(cfg, dev->name, "recover_time");
> > + dev->best_source = NULL;
> > + dpll_get_state_cmd = config_get_string(cfg, dev->name,
> "dpll_get_state_cmd");
> > + dss.holdover = config_get_string(cfg, dev->name,
> "dpll_holdover_value");
> > + dss.locked_ho = config_get_string(cfg, dev->name,
> "dpll_locked_ho_value");
> > + dss.locked = config_get_string(cfg, dev->name,
> "dpll_locked_value");
> > + dss.freerun = config_get_string(cfg, dev->name,
> "dpll_freerun_value");
> > + dss.invalid = config_get_string(cfg, dev->name,
> "dpll_invalid_value");
> > + dev->dc = synce_dev_ctrl_create();
> > + if (!dev->dc) {
> > + pr_err("device_ctrl create fail on %s", dev->name);
> > + goto err;
> > + }
> > +
> > + if (dev->external_input && dev->internal_input) {
> > + pr_warning("external_input and internal_input are mutually
> exclusive - disabling internal_input cfg option");
> > + dev->internal_input = 0;
> > + } else if (!dev->external_input && !dev->internal_input) {
> > + pr_err("either external_input or internal_input need to be
> set to 1 - aborting initialization");
> > + goto err;
> > + }
> > +
> > + ret = synce_dev_ctrl_init(dev->dc, dev->name, dpll_get_state_cmd,
> &dss);
> > + if (ret) {
> > + pr_err("synce_dev_ctrl init ret %d on %s", ret, dev->name);
> > + goto err;
> > + }
> > +
> > + if (init_ports(&count, dev, cfg))
> > + goto err;
> > +
> > + init_ops(dev);
> > + dev->num_ports = count;
> > + dev->state = DEVICE_INITED;
> > +
> > + dev->d_state = DPLL_HOLDOVER;
> > + dev->ops.update_ql(dev);
> > +
> > + /* in case somebody manually set recovered clock before */
> > + if (dev->internal_input) {
> > + force_all_dplls_detach(dev);
> > + }
> > + pr_info("inited num_ports %d for %s", count, dev->name);
> > +
> > + return 0;
> > +
> > +err:
> > + dev->state = DEVICE_FAILED;
> > + pr_err("%s failed for %s", __func__, dev->name);
> > + return -ENODEV;
> > +}
> > +
> > +struct synce_dev *synce_dev_create(const char *dev_name)
> > +{
> > + struct synce_dev *dev = NULL;
> > +
> > + if (!dev_name) {
> > + return NULL;
> > + }
> > +
> > + dev = malloc(sizeof(struct synce_dev));
> > + if (!dev) {
> > + return NULL;
> > + }
> > +
> > + memcpy(dev->name, dev_name, sizeof(dev->name));
> > + dev->state = DEVICE_CREATED;
> > + dev->d_state = DPLL_UNKNOWN;
> > + dev->last_d_state = DPLL_UNKNOWN;
> > + pr_debug("created %s", dev->name);
> > +
> > + return dev;
> > +}
> > +
> > +int synce_dev_step(struct synce_dev *dev)
> > +{
> > + int ret = -EFAULT;
> > +
> > + if (!dev) {
> > + pr_err("%s dev is NULL", __func__);
> > + return ret;
> > + }
> > +
> > + update_dev_state(dev);
> > + if (dev->state != DEVICE_RUNNING) {
> > + pr_err("dev %s is not running", dev->name);
> > + return ret;
> > + }
> > +
> > + ret = synce_dev_ctrl_get_state(dev->dc, &dev->d_state);
> > + if (ret) {
> > + pr_warning("could not acquire dpll state on %s",
> dev->name);
> > + return ret;
> > + }
> > +
> > + ret = dev->ops.step(dev);
> > +
> > + return ret;
> > +}
> > +
> > +const char *synce_dev_name(struct synce_dev *dev)
> > +{
> > + return dev->name;
> > +}
> > +
> > +int synce_dev_is_running(struct synce_dev *dev)
> > +{
> > + update_dev_state(dev);
> > +
> > + return !!(dev->state & DEVICE_RUNNING);
> > +}
> > +
> > +void synce_dev_destroy(struct synce_dev *dev)
> > +{
> > + if (!dev) {
> > + pr_err("%s dev is NULL", __func__);
> > + return;
> > + }
> > +
> > + if (dev->internal_input && dev->state != DEVICE_FAILED) {
> > + force_all_dplls_detach(dev);
> > + }
> > +
> > + destroy_ports(dev);
> > + destroy_dev_ctrl(dev);
> > +}
> > diff --git a/synce_dev.h b/synce_dev.h
> > new file mode 100644
> > index 0000000..6807c2b
> > --- /dev/null
> > +++ b/synce_dev.h
> > @@ -0,0 +1,64 @@
> > +/**
> > + * @file synce_dev.h
> > + * @brief Interface for handling Sync-E capable devices and its ports
> > + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation
> > + * @note SPDX-License-Identifier: GPL-2.0+
> > + */
> > +#ifndef HAVE_SYNCE_DEV_H
> > +#define HAVE_SYNCE_DEV_H
> > +
> > +#include <stdint.h>
> > +
> > +struct config;
> > +struct synce_dev;
> > +
> > +/**
> > + * Initialize Sync-E device and its ports after device was created.
> > + *
> > + * @param dev Device to be initialized
> > + * @param cfg Configuration struct based on SYNCE type, must hold
> > + * properities of configured device ports
> > + * @return 0 on success, failure otherwise
> > + */
> > +int synce_dev_init(struct synce_dev *dev, struct config *cfg);
> > +
> > +/**
> > + * Alloc memory for a single Sync-E device.
> > + *
> > + * @param dev_name Human readable name of a device
> > + * @return 0 on success, failure otherwise
> > + */
> > +struct synce_dev *synce_dev_create(const char *dev_name);
> > +
> > +/**
> > + * Step a Sync-E device. Probe for events, changes and act on them.
> > + *
> > + * @param dev Device to be stepped
> > + * @return 0 on success, failure otherwise
> > + */
> > +int synce_dev_step(struct synce_dev *dev);
> > +
> > +/**
> > + * Acquire Sync-E device name.
> > + *
> > + * @param dev Questioned SyncE device instance
> > + * @return The device name
> > + */
> > +const char *synce_dev_name(struct synce_dev *dev);
> > +
> > +/**
> > + * Clean-up on memory allocated for device and its ports.
> > + *
> > + * @param dev SyncE device to be cleared
> > + */
> > +void synce_dev_destroy(struct synce_dev *dev);
> > +
> > +/**
> > + * Check if Sync-E device is running.
> > + *
> > + * @param dev Questioned SyncE device
> > + * @return 0 if false, 1 if true
> > + */
> > +int synce_dev_is_running(struct synce_dev *dev);
> > +
> > +#endif
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
|
|
From: Aya L. <ay...@nv...> - 2022-06-08 11:04:29
|
On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote: > synce4l is a software implementation of Synchronous Ethernet (Sync-E) > according to ITU-T Recommendation G.8264. The design goal is to provide > logic to supported hardware by processing Ethernet Synchronization > Messaging Channel (ESMC) and control Digital Phase Locked Loop (DPLL) We should use EEC rather than DPLL (more abstract less HW related) > clock on Network Card Interface (NIC). > > Application can operate in two mutually exclusive modes. If both modes > are configured simultaneously, the external input takes precedence over > internal one (internal is disabled implicitly). AFAIU there is one selection process which regards all the sources and select according to the quality/priority. What is the benefit of two mutually exclusive modes? > > External input mode > If synce4l is configured to run in "external input" mode, then DPLL > needs to have external 1PPS source attached (GPS or another generator). > In this scenario synce4l always broadcasts clock quality level (QL) > defined in configuration file. Additionally, for this mode > incoming Sync-E frames do not participate in best source selection > algorithm for DPLL. > > Internal input mode Lets use the standard (G.8264 - Annex A) naming of traffic reference source. Internal may refer internal oscillator. > In "internal input" mode incoming Sync-E frames are processed and best > clock source is extracted from the link having the best quality level. > synce4l configures such "best quality" port as a source to recover > clock for DPLL. The recovered QL is broadcasted to all other interfaces > then. An external clock cannot be used in this mode. > > Hardware > synce4l is not bound to any specific NIC hardware. In this level HW should be hidden totally - please see my comment in patch #5 > > Theoretically any NIC could be used for enabling Sync-E device in Lets use SyncE and not Sync-E > External input mode - the NIC in this mode is in leader role (similar > to Grand Master role in PTP), providing its Quality Level information > in ESMC frames to its neighbors. How do we monitor the quality and validity of an external source? > It will not syntonize to any of its neighbor ports, since RX frames > are not monitored. Only other Sync-E enabled neighbor ports could > recover clock from that peer and syntonize to it. > Practically this mode shall be used only if the NIC PHY ports are fed > with stable and reliable frequency clock, Quality Level values that are > sent to the peers are configured by the user. This is user > responsibility to decide and set proper Quality-Level value for the > device, corresponding to the quality of frequency clock generator of > used hardware. > > When enabling Internal input mode, the NIC must be a device that > actually, supports Synchronous Ethernet, which means it is able to > syntonize frequency of all configured ports within the device > to one chosen port - clock is recovered from its RX path, then is fed > to other ports with Phase-Locked Loop (PLL) circuit powering clock of > other ports. > The NIC driver or dedicated software must provide a user with ability > to: > > obtain current state of a DPLL used to syntonize the ports > select one of the ports as a candidate for clock recovery. > Both abilities are configurable in config file - the user must provide > a shell commands - as explained in manual and descriptions inside of > example config file. > > Arkadiusz Kubalewski (11): > synce4l: add config knobs for SyncE > synce4l: add synce in interface and util code > synce4l: add esmc_socket interface > synce4l: add synce_clock interface > synce4l: add synce_dev interface > synce4l: add synce_dev_ctrl interface > synce4l: add synce_port interface > synce4l: add synce_port_ctrl interface > synce4l: add synce_msg interface > synce4l: add synce_transport interface > synce4l: add synce4l application > > .gitignore | 1 + > README.org | 2 + > config.c | 197 +++++-- > config.h | 8 + > configs/synce.cfg | 194 +++++++ > esmc_socket.c | 99 ++++ > esmc_socket.h | 42 ++ > interface.c | 29 +- > interface.h | 24 + > makefile | 9 +- > synce4l.8 | 239 ++++++++ > synce4l.c | 132 +++++ > synce4l_README.md | 194 +++++++ > synce_clock.c | 284 +++++++++ > synce_clock.h | 40 ++ > synce_dev.c | 622 ++++++++++++++++++++ > synce_dev.h | 64 ++ > synce_dev_ctrl.c | 153 +++++ > synce_dev_ctrl.h | 64 ++ > synce_msg.c | 259 +++++++++ > synce_msg.h | 174 ++++++ > synce_msg_private.h | 87 +++ > synce_port.c | 491 ++++++++++++++++ > synce_port.h | 184 ++++++ > synce_port_ctrl.c | 1161 +++++++++++++++++++++++++++++++++++++ > synce_port_ctrl.h | 165 ++++++ > synce_transport.c | 102 ++++ > synce_transport.h | 55 ++ > synce_transport_private.h | 18 + > util.c | 22 + > util.h | 8 + > 31 files changed, 5077 insertions(+), 46 deletions(-) > create mode 100644 configs/synce.cfg > create mode 100644 esmc_socket.c > create mode 100644 esmc_socket.h > create mode 100644 synce4l.8 > create mode 100644 synce4l.c > create mode 100644 synce4l_README.md > create mode 100644 synce_clock.c > create mode 100644 synce_clock.h > create mode 100644 synce_dev.c > create mode 100644 synce_dev.h > create mode 100644 synce_dev_ctrl.c > create mode 100644 synce_dev_ctrl.h > create mode 100644 synce_msg.c > create mode 100644 synce_msg.h > create mode 100644 synce_msg_private.h > create mode 100644 synce_port.c > create mode 100644 synce_port.h > create mode 100644 synce_port_ctrl.c > create mode 100644 synce_port_ctrl.h > create mode 100644 synce_transport.c > create mode 100644 synce_transport.h > create mode 100644 synce_transport_private.h > |
|
From: Aya L. <ay...@nv...> - 2022-06-08 11:09:39
|
On 6/8/2022 12:55 PM, Erez wrote: > > אינך מקבל לעתים קרובות דואר אלקטרוני מ- ere...@gm.... למד מדוע > הדבר חשוב <https://aka.ms/LearnAboutSenderIdentification> > > > > > On Wed, 8 Jun 2022 at 11:11, Aya Levin via Linuxptp-devel > <lin...@li... > <mailto:lin...@li...>> wrote: > > > > On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote: > > synce_dev interface allows creation, initialization, stepping and > > destroying of a single SyncE device. > > > > Newly created device must be given a device name (same as in config > > file). > > > > After creation the SyncE device is initialized with config struct, > > where device-level configuration for this device must exists. > > All ports belonging to this device are also initialized (at least one > > port configured is required). > > > > Once initialized SyncE device is ready for stepping. > > Each step will: > > - verify if device is in running state > > - acquire current state of the DPLL > > - performs actual step of a device, either as internal or external > > powered frequency provider for all the ports confgiured > > > > Destroying the SyncE device releases all its resources. > > > > Co-developed-by: Anatolii Gerasymenko > <ana...@in... <mailto:ana...@in...>> > > Signed-off-by: Anatolii Gerasymenko > <ana...@in... <mailto:ana...@in...>> > > Co-developed-by: Piotr Kwapulinski <pio...@in... > <mailto:pio...@in...>> > > Signed-off-by: Piotr Kwapulinski <pio...@in... > <mailto:pio...@in...>> > > Co-developed-by: Michal Michalik <mic...@in... > <mailto:mic...@in...>> > > Signed-off-by: Michal Michalik <mic...@in... > <mailto:mic...@in...>> > > Signed-off-by: Arkadiusz Kubalewski > <ark...@in... <mailto:ark...@in...>> > > --- > > v2: updated license headers > > v3: rebase patch series > > > > synce_dev.c | 622 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > synce_dev.h | 64 ++++++ > > 2 files changed, 686 insertions(+) > > create mode 100644 synce_dev.c > > create mode 100644 synce_dev.h > > > > diff --git a/synce_dev.c b/synce_dev.c > > new file mode 100644 > > index 0000000..dfe7ff4 > > --- /dev/null > > +++ b/synce_dev.c > > @@ -0,0 +1,622 @@ > > +/** > > + * @file synce_dev.c > > + * @brief Interface for handling Sync-E capable devices and its > ports > > + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation > > + * @note SPDX-License-Identifier: GPL-2.0+ > > + */ > > +#include <stdlib.h> > > +#include <sys/queue.h> > > +#include <net/if.h> > > +#include <errno.h> > > +#include "synce_dev.h" > > +#include "print.h" > > +#include "config.h" > > +#include "util.h" > > +#include "synce_port.h" > > +#include "missing.h" > > +#include "synce_dev_ctrl.h" > > +#include "synce_msg.h" > > + > > +struct interface { > > + STAILQ_ENTRY(interface) list; > > +}; > > + > > +struct synce_dev_ops { > > + int (*update_ql)(struct synce_dev *dev); > > + int (*step)(struct synce_dev *dev); > > +}; > > + > > +enum synce_dev_state { > > + DEVICE_UNKNOWN, > > + DEVICE_CREATED, > > + DEVICE_INITED, > > + DEVICE_RUNNING, > > + DEVICE_FAILED, > > +}; > > + > > +struct synce_dev { > > + LIST_ENTRY(synce_dev) list; > > + enum synce_dev_state state; > > + char name[IF_NAMESIZE]; > > + LIST_HEAD(synce_ports_head, synce_port) ports; > > + struct synce_port *best_source; > > + int num_ports; > > + int internal_input; > > + int external_input; > > + int network_option; > > + uint8_t ql; > > + uint8_t ext_ql; > > + int extended_tlv; > > + int recover_time; > > + enum dpll_state d_state; > > + enum dpll_state last_d_state; > > + struct synce_dev_ctrl *dc; > > + struct synce_dev_ops ops; > > +}; > > + > > +static int add_port(struct synce_dev *dev, struct synce_port *port) > > +{ > > + struct synce_port *port_iter, *last_port = NULL; > > + > > + LIST_FOREACH(port_iter, &dev->ports, list) { > > + last_port = port_iter; > > + } > > + > > + if (last_port) { > > + LIST_INSERT_AFTER(last_port, port, list); > > + } else { > > + LIST_INSERT_HEAD(&dev->ports, port, list); > > + } > > + return 0; > > +} > > + > > +static int rx_enabled(struct synce_dev *dev) > > +{ > > + return (dev->external_input == 0 && > > + dev->internal_input != 0); > > +} > > + > > +static void destroy_ports(struct synce_dev *dev) > > +{ > > + struct synce_port *port, *tmp; > > + > > + LIST_FOREACH_SAFE(port, &dev->ports, list, tmp) { > > + synce_port_destroy(port); > > + LIST_REMOVE(port, list); > > + free(port); > > + } > > + dev->num_ports = 0; > > +} > > + > > +static void destroy_dev_ctrl(struct synce_dev *dev) > > +{ > > + free(dev->dc); > > + dev->dc = NULL; > > +} > > + > > +static int init_ports(int *count, struct synce_dev *dev, struct > config *cfg) > > +{ > > + struct interface *iface; > > + struct synce_port *port; > > + const char *port_name; > > + > > + *count = 0; > > + > > + STAILQ_FOREACH(iface, &cfg->interfaces, list) { > > + /* given device takes care only of its child ports */ > > + if (interface_se_has_parent_dev(iface) && > > + (strncmp(interface_se_get_parent_dev_label(iface), > > + dev->name, sizeof(dev->name)) == 0)) { > > + port_name = interface_name(iface); > > + > > + /* If no sync mode, no need to maintain the > port */ > > + if (!synce_port_in_sync_mode(cfg, port_name)) { > > + continue; > > + } > > + > > + port = synce_port_create(port_name); > > + if (!port) { > > + pr_err("failed to create port %s on > device %s", > > + port_name, dev->name); > > + continue; > > + } > > + > > + if (synce_port_init(port, cfg, > dev->network_option, > > + dev->extended_tlv, > rx_enabled(dev), > > + dev->recover_time, dev->ql, > > + dev->ext_ql)) { > > + pr_err("failed to configure port %s > on device %s", > > + port_name, dev->name); > > + synce_port_destroy(port); > > + continue; > > + } > > + > > + if (add_port(dev, port)) { > > + pr_err("failed to add port %s to > device %s", > > + port_name, dev->name); > > + synce_port_destroy(port); > > + continue; > > + } else { > > + (*count)++; > > + pr_debug("port %s added on device > %s addr %p", > > + port_name, dev->name, port); > > + } > > + } > > + } > > + > > + if (*count == 0) { > > + pr_err("device %s has no ports configured", dev->name); > > + return -ENODEV; > > + } > > + > > + return 0; > > +} > > + > > +static void update_dev_state(struct synce_dev *dev) > > +{ > > + struct synce_port *p; > > + int count = 0; > > + > > + LIST_FOREACH(p, &dev->ports, list) { > > + if (synce_port_thread_running(p)) { > > + count++; > > + } > > + } > > + > > + if (dev->num_ports == count) { > > + dev->state = DEVICE_RUNNING; > > + } else { > > + pr_warning("found %d ports running - %d configured > on %s", > > + count, dev->num_ports, dev->name); > > + } > > +} > > + > > +static int port_set_dnu(struct synce_port *p, int extended_tlv) > > +{ > > + int ret; > > + > > + if (!p) { > > + pr_err("%s port is NULL", __func__); > > + ret = -EFAULT; > > + return ret; > > + } > > + > > + ret = synce_port_set_tx_ql_dnu(p, extended_tlv); > > + if (ret) { > > + pr_err("set tx DNU fail on %s", > synce_port_get_name(p)); > > + return ret; > > + } > > + > > + return ret; > > +} > > + > > +static int port_set_ql_external_input(struct synce_port *p, int > extended) > > +{ > > + int ret = synce_port_set_tx_ql_forced(p, extended); > > + > > + if (ret) { > > + pr_err("set QL external failed"); > > + return ret; > > + } > > + > > + return ret; > > +} > > + > > +static int update_ql_external_input(struct synce_dev *dev) > > +{ > > + struct synce_port *p; > > + int ret = 0; > > + > > + LIST_FOREACH(p, &dev->ports, list) { > > + if (dev->d_state == DPLL_HOLDOVER) { > > + ret = port_set_dnu(p, dev->extended_tlv); > > + } else if (dev->d_state == DPLL_LOCKED || > > + dev->d_state == DPLL_LOCKED_HO_ACQ) { > > + ret = port_set_ql_external_input(p, > dev->extended_tlv); > > + } > > + > > + if (ret) { > > + pr_err("update QL failed d_state %d, err:%d > on %s", > > + dev->d_state, ret, dev->name); > > + break; > > + } > > + > > + } > > + > > + return ret; > > +} > > + > > +static int port_set_ql_internal_input(struct synce_dev *dev, > > + struct synce_port *p, > > + struct synce_port *best_p) > > +{ > > + int ret = synce_port_set_tx_ql_from_best_input(p, best_p, > > + > dev->extended_tlv); > > + > > + if (ret) { > > + pr_err("set QL failed"); > > + return ret; > > + } > > + > > + if (!ret) { > > + pr_debug("%s on %s", __func__, dev->name); > > + } > > + > > + return ret; > > +} > > + > > +static int update_ql_internal_input(struct synce_dev *dev) > > +{ > > + struct synce_port *p, *best_p = dev->best_source; > > + int ret = 0; > > + > > + LIST_FOREACH(p, &dev->ports, list) { > > + if (dev->d_state == DPLL_HOLDOVER) { > > + pr_debug("act on DPLL_HOLDOVER for %s", > > + synce_port_get_name(p)); > > + ret = port_set_dnu(p, dev->extended_tlv); > > + if (ret) { > > + pr_err("%s set DNU failed on %s", > > + __func__, dev->name); > > + return ret; > > + } > > + } else if ((dev->d_state == DPLL_LOCKED || > > + dev->d_state == DPLL_LOCKED_HO_ACQ) && > best_p) { > > + pr_debug("act on > DPLL_LOCKED/DPLL_LOCKED_HO_ACQ for %s", > > + synce_port_get_name(p)); > > + /* on best port send DNU, all the others > > + * propagate what came from best source > > + */ > > + if (p != best_p) { > > + ret = > port_set_ql_internal_input(dev, p, > > + > best_p); > > + } else { > > + ret = port_set_dnu(p, > dev->extended_tlv); > > + } > > + > > + if (ret) { > > + pr_err("%s set failed on %s", > > + __func__, dev->name); > > + return ret; > > + } > > + } else { > > + pr_debug("nothing to do for %s d_state %d, > best_p %p", > > + synce_port_get_name(p), > dev->d_state, best_p); > > + } > > + } > > + > > + return ret; > > +} > > + > > +static void detach_port_dpll(struct synce_port *port, struct > synce_dev *dev) > > +{ > > + int ret = synce_port_disable_recover_clock(port); > + > > + if (ret) { > > + pr_err("disable recover clock cmd failed on %s", > dev->name); > > + return; > > + } > +} > This is not abstract enough, too HW spesific > > > > Sorry for interfering, > Could you explain what is "too hardware specific"? > What interface? What would you expect here instead? I'd expect some in-standard event like: move_to_holdover. Error path like failing to set quality should be handeled internally by the driver. If the DPLL needs to be reset, it is up to the driver to perform. HW should be hidden completely in hidden from SyncE demon. > > > > + > > +static void force_all_dplls_detach(struct synce_dev *dev) > > +{ > > + enum dpll_state state; > > + struct synce_port *p; > > + > > + LIST_FOREACH(p, &dev->ports, list) { > > + pr_debug("trying to detach DPLL RCLK for %s", > > + synce_port_get_name(p)); > > + detach_port_dpll(p, dev); > > + } > > + > > + if (synce_dev_ctrl_get_state(dev->dc, &state)) { > > + pr_err("failed getting DPLL state"); > > + dev->last_d_state = DPLL_UNKNOWN; > > + dev->d_state = DPLL_UNKNOWN; > > + } else { > > + dev->last_d_state = state; > > + dev->d_state = state; > > + } > > +}; > > + > > +static void dev_update_ql(struct synce_dev *dev) > > +{ > > + if (dev->ops.update_ql(dev)) { > > + pr_err("update QL fail on %s", dev->name); > > + } > > +} > > + > > +static int rx_ql_changed(struct synce_dev *dev) > > +{ > > + struct synce_port *p; > > + int ret = 0; > > + > > + LIST_FOREACH(p, &dev->ports, list) { > > + ret = synce_port_rx_ql_changed(p); > > + if (ret) { > > + break; > > + } > > + } > > + > > + return ret; > > +} > > + > > +static struct synce_port *find_dev_best_source(struct synce_dev > *dev) > > +{ > > + struct synce_port *p, *best_p = dev->best_source; > > + > > + LIST_FOREACH(p, &dev->ports, list) { > > + if (best_p != p) { > > + if (synce_port_compare_ql(best_p, p) == p) { > > + pr_debug("old best %s replaced by > %s on %s", > > + synce_port_get_name(best_p), > > + synce_port_get_name(p), > dev->name); > > + best_p = p; > > + } > > + } > > + } > > + > > + if (best_p) { > > + if (synce_port_is_rx_dnu(best_p)) { > > + return NULL; > > + } > > + } > > + > > + return best_p; > > +} > > + > > +static int set_input_source(struct synce_dev *dev, > > + struct synce_port *new_best_source) > > +{ > > + const char *best_name = synce_port_get_name(new_best_source); > > + int ret; > > + > > + if (!best_name) { > > + pr_err("get best input name failed on %s", dev->name); > > + return -ENXIO; > > + } > > + > > + ret = synce_port_enable_recover_clock(new_best_source); > > + if (ret) { > > + pr_err("enable recover clock cmd failed on %s", > dev->name); > > + return ret; > > + } > > + > > + return ret; > > +} > > + > > +static int act_on_d_state(struct synce_dev *dev) > > +{ > > + int ret = 0; > > + > > + if (dev->d_state != dev->last_d_state) { > > + ret = dev->ops.update_ql(dev); > > + if (ret) { > > + pr_err("update QL fail on %s", dev->name); > > + } else { > > + dev->last_d_state = dev->d_state; > > + pr_debug("%s QL updated on %s", __func__, > dev->name); > > + } > > + } > > + > > + return ret; > > +} > > + > > +static int dev_step_external_input(struct synce_dev *dev) > > +{ > > + return act_on_d_state(dev); > > +} > > + > > +static void choose_best_source(struct synce_dev *dev) > > +{ > > + struct synce_port *new_best; > > + > > + new_best = find_dev_best_source(dev); > > + if (!new_best) { > > + pr_info("best source not found on %s", dev->name); > > + force_all_dplls_detach(dev); > > + dev_update_ql(dev); > > + dev->best_source = NULL; > > + } else if (new_best != dev->best_source) { > > + force_all_dplls_detach(dev); > > + if (set_input_source(dev, new_best)) { > > + pr_err("set best source failed on %s", > > + dev->name); > > + } else { > > + /* if input source is changing > > + * current input is invalid, send DNU and wait > > + * for DPLL being locked in further dev_step > > + */ > > + dev_update_ql(dev); > > + /* DPLL was invalidated we can now set new > > + * best_source for further use > > + */ > > + dev->best_source = new_best; > > + } > > + } else { > > + pr_info("clock source has not changed on %s", > dev->name); > > + /* no port change, just update QL on TX */ > > + dev_update_ql(dev); > > + > > + } > > +} > > + > > +static int dev_step_internal_input(struct synce_dev *dev) > > +{ > > + int ret; > > + > > + ret = act_on_d_state(dev); > > + if (ret) { > > + pr_err("act on d_state fail on %s", dev->name); > > + return ret; > > + } > > + > > + if (rx_ql_changed(dev)) { > > + choose_best_source(dev); > > + } else if (dev->best_source) { > > + if (synce_port_rx_ql_failed(dev->best_source)) { > > + synce_port_invalidate_rx_ql(dev->best_source); > > + force_all_dplls_detach(dev); > > + dev_update_ql(dev); > > + dev->best_source = NULL; > > + choose_best_source(dev); > > + } > > + } > > + > > + return ret; > > +} > > + > > +static void init_ops(struct synce_dev *dev) > > +{ > > + if (rx_enabled(dev)) { > > + dev->ops.update_ql = &update_ql_internal_input; > > + dev->ops.step = &dev_step_internal_input; > > + } else { > > + dev->ops.update_ql = &update_ql_external_input; > > + dev->ops.step = &dev_step_external_input; > > + } > > +} > > + > > +int synce_dev_init(struct synce_dev *dev, struct config *cfg) > > +{ > > + const char *dpll_get_state_cmd; > > + struct dpll_state_str dss; > > + int count, ret; > > + > > + if (dev->state != DEVICE_CREATED) { > > + goto err; > > + } > > + > > + LIST_INIT(&dev->ports); > > + dev->internal_input = config_get_int(cfg, dev->name, > "internal_input"); > > + dev->external_input = config_get_int(cfg, dev->name, > "external_input"); > > + dev->ql = config_get_int(cfg, dev->name, "external_input_QL"); > > + dev->ext_ql = config_get_int(cfg, dev->name, > "external_input_ext_QL"); > > + dev->extended_tlv = config_get_int(cfg, dev->name, > "extended_tlv"); > > + dev->network_option = config_get_int(cfg, dev->name, > "network_option"); > > + dev->recover_time = config_get_int(cfg, dev->name, > "recover_time"); > > + dev->best_source = NULL; > > + dpll_get_state_cmd = config_get_string(cfg, dev->name, > "dpll_get_state_cmd"); > > + dss.holdover = config_get_string(cfg, dev->name, > "dpll_holdover_value"); > > + dss.locked_ho = config_get_string(cfg, dev->name, > "dpll_locked_ho_value"); > > + dss.locked = config_get_string(cfg, dev->name, > "dpll_locked_value"); > > + dss.freerun = config_get_string(cfg, dev->name, > "dpll_freerun_value"); > > + dss.invalid = config_get_string(cfg, dev->name, > "dpll_invalid_value"); > > + dev->dc = synce_dev_ctrl_create(); > > + if (!dev->dc) { > > + pr_err("device_ctrl create fail on %s", dev->name); > > + goto err; > > + } > > + > > + if (dev->external_input && dev->internal_input) { > > + pr_warning("external_input and internal_input are > mutually exclusive - disabling internal_input cfg option"); > > + dev->internal_input = 0; > > + } else if (!dev->external_input && !dev->internal_input) { > > + pr_err("either external_input or internal_input > need to be set to 1 - aborting initialization"); > > + goto err; > > + } > > + > > + ret = synce_dev_ctrl_init(dev->dc, dev->name, > dpll_get_state_cmd, &dss); > > + if (ret) { > > + pr_err("synce_dev_ctrl init ret %d on %s", ret, > dev->name); > > + goto err; > > + } > > + > > + if (init_ports(&count, dev, cfg)) > > + goto err; > > + > > + init_ops(dev); > > + dev->num_ports = count; > > + dev->state = DEVICE_INITED; > > + > > + dev->d_state = DPLL_HOLDOVER; > > + dev->ops.update_ql(dev); > > + > > + /* in case somebody manually set recovered clock before */ > > + if (dev->internal_input) { > > + force_all_dplls_detach(dev); > > + } > > + pr_info("inited num_ports %d for %s", count, dev->name); > > + > > + return 0; > > + > > +err: > > + dev->state = DEVICE_FAILED; > > + pr_err("%s failed for %s", __func__, dev->name); > > + return -ENODEV; > > +} > > + > > +struct synce_dev *synce_dev_create(const char *dev_name) > > +{ > > + struct synce_dev *dev = NULL; > > + > > + if (!dev_name) { > > + return NULL; > > + } > > + > > + dev = malloc(sizeof(struct synce_dev)); > > + if (!dev) { > > + return NULL; > > + } > > + > > + memcpy(dev->name, dev_name, sizeof(dev->name)); > > + dev->state = DEVICE_CREATED; > > + dev->d_state = DPLL_UNKNOWN; > > + dev->last_d_state = DPLL_UNKNOWN; > > + pr_debug("created %s", dev->name); > > + > > + return dev; > > +} > > + > > +int synce_dev_step(struct synce_dev *dev) > > +{ > > + int ret = -EFAULT; > > + > > + if (!dev) { > > + pr_err("%s dev is NULL", __func__); > > + return ret; > > + } > > + > > + update_dev_state(dev); > > + if (dev->state != DEVICE_RUNNING) { > > + pr_err("dev %s is not running", dev->name); > > + return ret; > > + } > > + > > + ret = synce_dev_ctrl_get_state(dev->dc, &dev->d_state); > > + if (ret) { > > + pr_warning("could not acquire dpll state on %s", > dev->name); > > + return ret; > > + } > > + > > + ret = dev->ops.step(dev); > > + > > + return ret; > > +} > > + > > +const char *synce_dev_name(struct synce_dev *dev) > > +{ > > + return dev->name; > > +} > > + > > +int synce_dev_is_running(struct synce_dev *dev) > > +{ > > + update_dev_state(dev); > > + > > + return !!(dev->state & DEVICE_RUNNING); > > +} > > + > > +void synce_dev_destroy(struct synce_dev *dev) > > +{ > > + if (!dev) { > > + pr_err("%s dev is NULL", __func__); > > + return; > > + } > > + > > + if (dev->internal_input && dev->state != DEVICE_FAILED) { > > + force_all_dplls_detach(dev); > > + } > > + > > + destroy_ports(dev); > > + destroy_dev_ctrl(dev); > > +} > > diff --git a/synce_dev.h b/synce_dev.h > > new file mode 100644 > > index 0000000..6807c2b > > --- /dev/null > > +++ b/synce_dev.h > > @@ -0,0 +1,64 @@ > > +/** > > + * @file synce_dev.h > > + * @brief Interface for handling Sync-E capable devices and its > ports > > + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation > > + * @note SPDX-License-Identifier: GPL-2.0+ > > + */ > > +#ifndef HAVE_SYNCE_DEV_H > > +#define HAVE_SYNCE_DEV_H > > + > > +#include <stdint.h> > > + > > +struct config; > > +struct synce_dev; > > + > > +/** > > + * Initialize Sync-E device and its ports after device was created. > > + * > > + * @param dev Device to be initialized > > + * @param cfg Configuration struct based on SYNCE type, > must hold > > + * properities of configured device ports > > + * @return 0 on success, failure otherwise > > + */ > > +int synce_dev_init(struct synce_dev *dev, struct config *cfg); > > + > > +/** > > + * Alloc memory for a single Sync-E device. > > + * > > + * @param dev_name Human readable name of a device > > + * @return 0 on success, failure otherwise > > + */ > > +struct synce_dev *synce_dev_create(const char *dev_name); > > + > > +/** > > + * Step a Sync-E device. Probe for events, changes and act on them. > > + * > > + * @param dev Device to be stepped > > + * @return 0 on success, failure otherwise > > + */ > > +int synce_dev_step(struct synce_dev *dev); > > + > > +/** > > + * Acquire Sync-E device name. > > + * > > + * @param dev Questioned SyncE device instance > > + * @return The device name > > + */ > > +const char *synce_dev_name(struct synce_dev *dev); > > + > > +/** > > + * Clean-up on memory allocated for device and its ports. > > + * > > + * @param dev SyncE device to be cleared > > + */ > > +void synce_dev_destroy(struct synce_dev *dev); > > + > > +/** > > + * Check if Sync-E device is running. > > + * > > + * @param dev Questioned SyncE device > > + * @return 0 if false, 1 if true > > + */ > > +int synce_dev_is_running(struct synce_dev *dev); > > + > > +#endif > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > <mailto:Lin...@li...> > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Flinuxptp-devel&data=05%7C01%7Cayal%40nvidia.com%7C4c0b644ed27c41b756f208da49352af0%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637902789967187353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IF99oKWG0qqF%2B33TAo5IRaTse5hB4x3rl8%2F9BDKwib4%3D&reserved=0> > |
|
From: Kubalewski, A. <ark...@in...> - 2022-06-09 19:12:37
|
> -----Original Message----- > From: Aya Levin via Linuxptp-devel <lin...@li...> > Sent: Wednesday, June 8, 2022 12:54 PM > > On 6/8/2022 12:55 PM, Erez wrote: > > > > אינך מקבל לעתים קרובות דואר אלקטרוני מ- ere...@gm.... למד מדוע > > הדבר חשוב <https://aka.ms/LearnAboutSenderIdentification> > > > > > > > > > > On Wed, 8 Jun 2022 at 11:11, Aya Levin via Linuxptp-devel > > <lin...@li... > > <mailto:lin...@li...>> wrote: > > > > > > > > On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote: > > > synce_dev interface allows creation, initialization, stepping and > > > destroying of a single SyncE device. > > > > > > Newly created device must be given a device name (same as in config > > > file). > > > > > > After creation the SyncE device is initialized with config struct, > > > where device-level configuration for this device must exists. > > > All ports belonging to this device are also initialized (at least one > > > port configured is required). > > > > > > Once initialized SyncE device is ready for stepping. > > > Each step will: > > > - verify if device is in running state > > > - acquire current state of the DPLL > > > - performs actual step of a device, either as internal or external > > > powered frequency provider for all the ports confgiured > > > > > > Destroying the SyncE device releases all its resources. > > > > > > Co-developed-by: Anatolii Gerasymenko > > <ana...@in... <mailto:ana...@in...>> > > > Signed-off-by: Anatolii Gerasymenko > > <ana...@in... <mailto:ana...@in...>> > > > Co-developed-by: Piotr Kwapulinski <pio...@in... > > <mailto:pio...@in...>> > > > Signed-off-by: Piotr Kwapulinski <pio...@in... > > <mailto:pio...@in...>> > > > Co-developed-by: Michal Michalik <mic...@in... > > <mailto:mic...@in...>> > > > Signed-off-by: Michal Michalik <mic...@in... > > <mailto:mic...@in...>> > > > Signed-off-by: Arkadiusz Kubalewski > > <ark...@in... <mailto:ark...@in...>> > > > --- > > > v2: updated license headers > > > v3: rebase patch series > > > > > > synce_dev.c | 622 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > synce_dev.h | 64 ++++++ > > > 2 files changed, 686 insertions(+) > > > create mode 100644 synce_dev.c > > > create mode 100644 synce_dev.h > > > > > > diff --git a/synce_dev.c b/synce_dev.c > > > new file mode 100644 > > > index 0000000..dfe7ff4 > > > --- /dev/null > > > +++ b/synce_dev.c > > > @@ -0,0 +1,622 @@ > > > +/** > > > + * @file synce_dev.c > > > + * @brief Interface for handling Sync-E capable devices and its > > ports > > > + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation > > > + * @note SPDX-License-Identifier: GPL-2.0+ > > > + */ > > > +#include <stdlib.h> > > > +#include <sys/queue.h> > > > +#include <net/if.h> > > > +#include <errno.h> > > > +#include "synce_dev.h" > > > +#include "print.h" > > > +#include "config.h" > > > +#include "util.h" > > > +#include "synce_port.h" > > > +#include "missing.h" > > > +#include "synce_dev_ctrl.h" > > > +#include "synce_msg.h" > > > + > > > +struct interface { > > > + STAILQ_ENTRY(interface) list; > > > +}; > > > + > > > +struct synce_dev_ops { > > > + int (*update_ql)(struct synce_dev *dev); > > > + int (*step)(struct synce_dev *dev); > > > +}; > > > + > > > +enum synce_dev_state { > > > + DEVICE_UNKNOWN, > > > + DEVICE_CREATED, > > > + DEVICE_INITED, > > > + DEVICE_RUNNING, > > > + DEVICE_FAILED, > > > +}; > > > + > > > +struct synce_dev { > > > + LIST_ENTRY(synce_dev) list; > > > + enum synce_dev_state state; > > > + char name[IF_NAMESIZE]; > > > + LIST_HEAD(synce_ports_head, synce_port) ports; > > > + struct synce_port *best_source; > > > + int num_ports; > > > + int internal_input; > > > + int external_input; > > > + int network_option; > > > + uint8_t ql; > > > + uint8_t ext_ql; > > > + int extended_tlv; > > > + int recover_time; > > > + enum dpll_state d_state; > > > + enum dpll_state last_d_state; > > > + struct synce_dev_ctrl *dc; > > > + struct synce_dev_ops ops; > > > +}; > > > + > > > +static int add_port(struct synce_dev *dev, struct synce_port *port) > > > +{ > > > + struct synce_port *port_iter, *last_port = NULL; > > > + > > > + LIST_FOREACH(port_iter, &dev->ports, list) { > > > + last_port = port_iter; > > > + } > > > + > > > + if (last_port) { > > > + LIST_INSERT_AFTER(last_port, port, list); > > > + } else { > > > + LIST_INSERT_HEAD(&dev->ports, port, list); > > > + } > > > + return 0; > > > +} > > > + > > > +static int rx_enabled(struct synce_dev *dev) > > > +{ > > > + return (dev->external_input == 0 && > > > + dev->internal_input != 0); > > > +} > > > + > > > +static void destroy_ports(struct synce_dev *dev) > > > +{ > > > + struct synce_port *port, *tmp; > > > + > > > + LIST_FOREACH_SAFE(port, &dev->ports, list, tmp) { > > > + synce_port_destroy(port); > > > + LIST_REMOVE(port, list); > > > + free(port); > > > + } > > > + dev->num_ports = 0; > > > +} > > > + > > > +static void destroy_dev_ctrl(struct synce_dev *dev) > > > +{ > > > + free(dev->dc); > > > + dev->dc = NULL; > > > +} > > > + > > > +static int init_ports(int *count, struct synce_dev *dev, struct > > config *cfg) > > > +{ > > > + struct interface *iface; > > > + struct synce_port *port; > > > + const char *port_name; > > > + > > > + *count = 0; > > > + > > > + STAILQ_FOREACH(iface, &cfg->interfaces, list) { > > > + /* given device takes care only of its child ports */ > > > + if (interface_se_has_parent_dev(iface) && > > > + (strncmp(interface_se_get_parent_dev_label(iface), > > > + dev->name, sizeof(dev->name)) == 0)) { > > > + port_name = interface_name(iface); > > > + > > > + /* If no sync mode, no need to maintain the > > port */ > > > + if (!synce_port_in_sync_mode(cfg, port_name)) { > > > + continue; > > > + } > > > + > > > + port = synce_port_create(port_name); > > > + if (!port) { > > > + pr_err("failed to create port %s on > > device %s", > > > + port_name, dev->name); > > > + continue; > > > + } > > > + > > > + if (synce_port_init(port, cfg, > > dev->network_option, > > > + dev->extended_tlv, > > rx_enabled(dev), > > > + dev->recover_time, dev->ql, > > > + dev->ext_ql)) { > > > + pr_err("failed to configure port %s > > on device %s", > > > + port_name, dev->name); > > > + synce_port_destroy(port); > > > + continue; > > > + } > > > + > > > + if (add_port(dev, port)) { > > > + pr_err("failed to add port %s to > > device %s", > > > + port_name, dev->name); > > > + synce_port_destroy(port); > > > + continue; > > > + } else { > > > + (*count)++; > > > + pr_debug("port %s added on device > > %s addr %p", > > > + port_name, dev->name, port); > > > + } > > > + } > > > + } > > > + > > > + if (*count == 0) { > > > + pr_err("device %s has no ports configured", dev->name); > > > + return -ENODEV; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static void update_dev_state(struct synce_dev *dev) > > > +{ > > > + struct synce_port *p; > > > + int count = 0; > > > + > > > + LIST_FOREACH(p, &dev->ports, list) { > > > + if (synce_port_thread_running(p)) { > > > + count++; > > > + } > > > + } > > > + > > > + if (dev->num_ports == count) { > > > + dev->state = DEVICE_RUNNING; > > > + } else { > > > + pr_warning("found %d ports running - %d configured > > on %s", > > > + count, dev->num_ports, dev->name); > > > + } > > > +} > > > + > > > +static int port_set_dnu(struct synce_port *p, int extended_tlv) > > > +{ > > > + int ret; > > > + > > > + if (!p) { > > > + pr_err("%s port is NULL", __func__); > > > + ret = -EFAULT; > > > + return ret; > > > + } > > > + > > > + ret = synce_port_set_tx_ql_dnu(p, extended_tlv); > > > + if (ret) { > > > + pr_err("set tx DNU fail on %s", > > synce_port_get_name(p)); > > > + return ret; > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int port_set_ql_external_input(struct synce_port *p, int > > extended) > > > +{ > > > + int ret = synce_port_set_tx_ql_forced(p, extended); > > > + > > > + if (ret) { > > > + pr_err("set QL external failed"); > > > + return ret; > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int update_ql_external_input(struct synce_dev *dev) > > > +{ > > > + struct synce_port *p; > > > + int ret = 0; > > > + > > > + LIST_FOREACH(p, &dev->ports, list) { > > > + if (dev->d_state == DPLL_HOLDOVER) { > > > + ret = port_set_dnu(p, dev->extended_tlv); > > > + } else if (dev->d_state == DPLL_LOCKED || > > > + dev->d_state == DPLL_LOCKED_HO_ACQ) { > > > + ret = port_set_ql_external_input(p, > > dev->extended_tlv); > > > + } > > > + > > > + if (ret) { > > > + pr_err("update QL failed d_state %d, err:%d > > on %s", > > > + dev->d_state, ret, dev->name); > > > + break; > > > + } > > > + > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int port_set_ql_internal_input(struct synce_dev *dev, > > > + struct synce_port *p, > > > + struct synce_port *best_p) > > > +{ > > > + int ret = synce_port_set_tx_ql_from_best_input(p, best_p, > > > + > > dev->extended_tlv); > > > + > > > + if (ret) { > > > + pr_err("set QL failed"); > > > + return ret; > > > + } > > > + > > > + if (!ret) { > > > + pr_debug("%s on %s", __func__, dev->name); > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int update_ql_internal_input(struct synce_dev *dev) > > > +{ > > > + struct synce_port *p, *best_p = dev->best_source; > > > + int ret = 0; > > > + > > > + LIST_FOREACH(p, &dev->ports, list) { > > > + if (dev->d_state == DPLL_HOLDOVER) { > > > + pr_debug("act on DPLL_HOLDOVER for %s", > > > + synce_port_get_name(p)); > > > + ret = port_set_dnu(p, dev->extended_tlv); > > > + if (ret) { > > > + pr_err("%s set DNU failed on %s", > > > + __func__, dev->name); > > > + return ret; > > > + } > > > + } else if ((dev->d_state == DPLL_LOCKED || > > > + dev->d_state == DPLL_LOCKED_HO_ACQ) && > > best_p) { > > > + pr_debug("act on > > DPLL_LOCKED/DPLL_LOCKED_HO_ACQ for %s", > > > + synce_port_get_name(p)); > > > + /* on best port send DNU, all the others > > > + * propagate what came from best source > > > + */ > > > + if (p != best_p) { > > > + ret = > > port_set_ql_internal_input(dev, p, > > > + > > best_p); > > > + } else { > > > + ret = port_set_dnu(p, > > dev->extended_tlv); > > > + } > > > + > > > + if (ret) { > > > + pr_err("%s set failed on %s", > > > + __func__, dev->name); > > > + return ret; > > > + } > > > + } else { > > > + pr_debug("nothing to do for %s d_state %d, > > best_p %p", > > > + synce_port_get_name(p), > > dev->d_state, best_p); > > > + } > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static void detach_port_dpll(struct synce_port *port, struct > > synce_dev *dev) > > > +{ > > > + int ret = synce_port_disable_recover_clock(port); > + > > > + if (ret) { > > > + pr_err("disable recover clock cmd failed on %s", > > dev->name); > > > + return; > > > + } > +} > > This is not abstract enough, too HW spesific > > > > > > > > Sorry for interfering, > > Could you explain what is "too hardware specific"? > > What interface? What would you expect here instead? > I'd expect some in-standard event like: move_to_holdover. Error path > like failing to set quality should be handeled internally by the driver. > If the DPLL needs to be reset, it is up to the driver to perform. > HW should be hidden completely in hidden from SyncE demon. Sure, I agree G.8264 standard doesn't mention DPLL. As answered on patch 0/11 mail thread, we will try to fix naming scheme, using EEC instead of DPLL. Thank you, Arkadiusz > > > > > > > > + > > > +static void force_all_dplls_detach(struct synce_dev *dev) > > > +{ > > > + enum dpll_state state; > > > + struct synce_port *p; > > > + > > > + LIST_FOREACH(p, &dev->ports, list) { > > > + pr_debug("trying to detach DPLL RCLK for %s", > > > + synce_port_get_name(p)); > > > + detach_port_dpll(p, dev); > > > + } > > > + > > > + if (synce_dev_ctrl_get_state(dev->dc, &state)) { > > > + pr_err("failed getting DPLL state"); > > > + dev->last_d_state = DPLL_UNKNOWN; > > > + dev->d_state = DPLL_UNKNOWN; > > > + } else { > > > + dev->last_d_state = state; > > > + dev->d_state = state; > > > + } > > > +}; > > > + > > > +static void dev_update_ql(struct synce_dev *dev) > > > +{ > > > + if (dev->ops.update_ql(dev)) { > > > + pr_err("update QL fail on %s", dev->name); > > > + } > > > +} > > > + > > > +static int rx_ql_changed(struct synce_dev *dev) > > > +{ > > > + struct synce_port *p; > > > + int ret = 0; > > > + > > > + LIST_FOREACH(p, &dev->ports, list) { > > > + ret = synce_port_rx_ql_changed(p); > > > + if (ret) { > > > + break; > > > + } > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static struct synce_port *find_dev_best_source(struct synce_dev > > *dev) > > > +{ > > > + struct synce_port *p, *best_p = dev->best_source; > > > + > > > + LIST_FOREACH(p, &dev->ports, list) { > > > + if (best_p != p) { > > > + if (synce_port_compare_ql(best_p, p) == p) { > > > + pr_debug("old best %s replaced by > > %s on %s", > > > + synce_port_get_name(best_p), > > > + synce_port_get_name(p), > > dev->name); > > > + best_p = p; > > > + } > > > + } > > > + } > > > + > > > + if (best_p) { > > > + if (synce_port_is_rx_dnu(best_p)) { > > > + return NULL; > > > + } > > > + } > > > + > > > + return best_p; > > > +} > > > + > > > +static int set_input_source(struct synce_dev *dev, > > > + struct synce_port *new_best_source) > > > +{ > > > + const char *best_name = synce_port_get_name(new_best_source); > > > + int ret; > > > + > > > + if (!best_name) { > > > + pr_err("get best input name failed on %s", dev->name); > > > + return -ENXIO; > > > + } > > > + > > > + ret = synce_port_enable_recover_clock(new_best_source); > > > + if (ret) { > > > + pr_err("enable recover clock cmd failed on %s", > > dev->name); > > > + return ret; > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int act_on_d_state(struct synce_dev *dev) > > > +{ > > > + int ret = 0; > > > + > > > + if (dev->d_state != dev->last_d_state) { > > > + ret = dev->ops.update_ql(dev); > > > + if (ret) { > > > + pr_err("update QL fail on %s", dev->name); > > > + } else { > > > + dev->last_d_state = dev->d_state; > > > + pr_debug("%s QL updated on %s", __func__, > > dev->name); > > > + } > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int dev_step_external_input(struct synce_dev *dev) > > > +{ > > > + return act_on_d_state(dev); > > > +} > > > + > > > +static void choose_best_source(struct synce_dev *dev) > > > +{ > > > + struct synce_port *new_best; > > > + > > > + new_best = find_dev_best_source(dev); > > > + if (!new_best) { > > > + pr_info("best source not found on %s", dev->name); > > > + force_all_dplls_detach(dev); > > > + dev_update_ql(dev); > > > + dev->best_source = NULL; > > > + } else if (new_best != dev->best_source) { > > > + force_all_dplls_detach(dev); > > > + if (set_input_source(dev, new_best)) { > > > + pr_err("set best source failed on %s", > > > + dev->name); > > > + } else { > > > + /* if input source is changing > > > + * current input is invalid, send DNU and wait > > > + * for DPLL being locked in further dev_step > > > + */ > > > + dev_update_ql(dev); > > > + /* DPLL was invalidated we can now set new > > > + * best_source for further use > > > + */ > > > + dev->best_source = new_best; > > > + } > > > + } else { > > > + pr_info("clock source has not changed on %s", > > dev->name); > > > + /* no port change, just update QL on TX */ > > > + dev_update_ql(dev); > > > + > > > + } > > > +} > > > + > > > +static int dev_step_internal_input(struct synce_dev *dev) > > > +{ > > > + int ret; > > > + > > > + ret = act_on_d_state(dev); > > > + if (ret) { > > > + pr_err("act on d_state fail on %s", dev->name); > > > + return ret; > > > + } > > > + > > > + if (rx_ql_changed(dev)) { > > > + choose_best_source(dev); > > > + } else if (dev->best_source) { > > > + if (synce_port_rx_ql_failed(dev->best_source)) { > > > + synce_port_invalidate_rx_ql(dev->best_source); > > > + force_all_dplls_detach(dev); > > > + dev_update_ql(dev); > > > + dev->best_source = NULL; > > > + choose_best_source(dev); > > > + } > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static void init_ops(struct synce_dev *dev) > > > +{ > > > + if (rx_enabled(dev)) { > > > + dev->ops.update_ql = &update_ql_internal_input; > > > + dev->ops.step = &dev_step_internal_input; > > > + } else { > > > + dev->ops.update_ql = &update_ql_external_input; > > > + dev->ops.step = &dev_step_external_input; > > > + } > > > +} > > > + > > > +int synce_dev_init(struct synce_dev *dev, struct config *cfg) > > > +{ > > > + const char *dpll_get_state_cmd; > > > + struct dpll_state_str dss; > > > + int count, ret; > > > + > > > + if (dev->state != DEVICE_CREATED) { > > > + goto err; > > > + } > > > + > > > + LIST_INIT(&dev->ports); > > > + dev->internal_input = config_get_int(cfg, dev->name, > > "internal_input"); > > > + dev->external_input = config_get_int(cfg, dev->name, > > "external_input"); > > > + dev->ql = config_get_int(cfg, dev->name, "external_input_QL"); > > > + dev->ext_ql = config_get_int(cfg, dev->name, > > "external_input_ext_QL"); > > > + dev->extended_tlv = config_get_int(cfg, dev->name, > > "extended_tlv"); > > > + dev->network_option = config_get_int(cfg, dev->name, > > "network_option"); > > > + dev->recover_time = config_get_int(cfg, dev->name, > > "recover_time"); > > > + dev->best_source = NULL; > > > + dpll_get_state_cmd = config_get_string(cfg, dev->name, > > "dpll_get_state_cmd"); > > > + dss.holdover = config_get_string(cfg, dev->name, > > "dpll_holdover_value"); > > > + dss.locked_ho = config_get_string(cfg, dev->name, > > "dpll_locked_ho_value"); > > > + dss.locked = config_get_string(cfg, dev->name, > > "dpll_locked_value"); > > > + dss.freerun = config_get_string(cfg, dev->name, > > "dpll_freerun_value"); > > > + dss.invalid = config_get_string(cfg, dev->name, > > "dpll_invalid_value"); > > > + dev->dc = synce_dev_ctrl_create(); > > > + if (!dev->dc) { > > > + pr_err("device_ctrl create fail on %s", dev->name); > > > + goto err; > > > + } > > > + > > > + if (dev->external_input && dev->internal_input) { > > > + pr_warning("external_input and internal_input are > > mutually exclusive - disabling internal_input cfg option"); > > > + dev->internal_input = 0; > > > + } else if (!dev->external_input && !dev->internal_input) { > > > + pr_err("either external_input or internal_input > > need to be set to 1 - aborting initialization"); > > > + goto err; > > > + } > > > + > > > + ret = synce_dev_ctrl_init(dev->dc, dev->name, > > dpll_get_state_cmd, &dss); > > > + if (ret) { > > > + pr_err("synce_dev_ctrl init ret %d on %s", ret, > > dev->name); > > > + goto err; > > > + } > > > + > > > + if (init_ports(&count, dev, cfg)) > > > + goto err; > > > + > > > + init_ops(dev); > > > + dev->num_ports = count; > > > + dev->state = DEVICE_INITED; > > > + > > > + dev->d_state = DPLL_HOLDOVER; > > > + dev->ops.update_ql(dev); > > > + > > > + /* in case somebody manually set recovered clock before */ > > > + if (dev->internal_input) { > > > + force_all_dplls_detach(dev); > > > + } > > > + pr_info("inited num_ports %d for %s", count, dev->name); > > > + > > > + return 0; > > > + > > > +err: > > > + dev->state = DEVICE_FAILED; > > > + pr_err("%s failed for %s", __func__, dev->name); > > > + return -ENODEV; > > > +} > > > + > > > +struct synce_dev *synce_dev_create(const char *dev_name) > > > +{ > > > + struct synce_dev *dev = NULL; > > > + > > > + if (!dev_name) { > > > + return NULL; > > > + } > > > + > > > + dev = malloc(sizeof(struct synce_dev)); > > > + if (!dev) { > > > + return NULL; > > > + } > > > + > > > + memcpy(dev->name, dev_name, sizeof(dev->name)); > > > + dev->state = DEVICE_CREATED; > > > + dev->d_state = DPLL_UNKNOWN; > > > + dev->last_d_state = DPLL_UNKNOWN; > > > + pr_debug("created %s", dev->name); > > > + > > > + return dev; > > > +} > > > + > > > +int synce_dev_step(struct synce_dev *dev) > > > +{ > > > + int ret = -EFAULT; > > > + > > > + if (!dev) { > > > + pr_err("%s dev is NULL", __func__); > > > + return ret; > > > + } > > > + > > > + update_dev_state(dev); > > > + if (dev->state != DEVICE_RUNNING) { > > > + pr_err("dev %s is not running", dev->name); > > > + return ret; > > > + } > > > + > > > + ret = synce_dev_ctrl_get_state(dev->dc, &dev->d_state); > > > + if (ret) { > > > + pr_warning("could not acquire dpll state on %s", > > dev->name); > > > + return ret; > > > + } > > > + > > > + ret = dev->ops.step(dev); > > > + > > > + return ret; > > > +} > > > + > > > +const char *synce_dev_name(struct synce_dev *dev) > > > +{ > > > + return dev->name; > > > +} > > > + > > > +int synce_dev_is_running(struct synce_dev *dev) > > > +{ > > > + update_dev_state(dev); > > > + > > > + return !!(dev->state & DEVICE_RUNNING); > > > +} > > > + > > > +void synce_dev_destroy(struct synce_dev *dev) > > > +{ > > > + if (!dev) { > > > + pr_err("%s dev is NULL", __func__); > > > + return; > > > + } > > > + > > > + if (dev->internal_input && dev->state != DEVICE_FAILED) { > > > + force_all_dplls_detach(dev); > > > + } > > > + > > > + destroy_ports(dev); > > > + destroy_dev_ctrl(dev); > > > +} > > > diff --git a/synce_dev.h b/synce_dev.h > > > new file mode 100644 > > > index 0000000..6807c2b > > > --- /dev/null > > > +++ b/synce_dev.h > > > @@ -0,0 +1,64 @@ > > > +/** > > > + * @file synce_dev.h > > > + * @brief Interface for handling Sync-E capable devices and its > > ports > > > + * @note SPDX-FileCopyrightText: Copyright 2022 Intel Corporation > > > + * @note SPDX-License-Identifier: GPL-2.0+ > > > + */ > > > +#ifndef HAVE_SYNCE_DEV_H > > > +#define HAVE_SYNCE_DEV_H > > > + > > > +#include <stdint.h> > > > + > > > +struct config; > > > +struct synce_dev; > > > + > > > +/** > > > + * Initialize Sync-E device and its ports after device was created. > > > + * > > > + * @param dev Device to be initialized > > > + * @param cfg Configuration struct based on SYNCE type, > > must hold > > > + * properities of configured device ports > > > + * @return 0 on success, failure otherwise > > > + */ > > > +int synce_dev_init(struct synce_dev *dev, struct config *cfg); > > > + > > > +/** > > > + * Alloc memory for a single Sync-E device. > > > + * > > > + * @param dev_name Human readable name of a device > > > + * @return 0 on success, failure otherwise > > > + */ > > > +struct synce_dev *synce_dev_create(const char *dev_name); > > > + > > > +/** > > > + * Step a Sync-E device. Probe for events, changes and act on them. > > > + * > > > + * @param dev Device to be stepped > > > + * @return 0 on success, failure otherwise > > > + */ > > > +int synce_dev_step(struct synce_dev *dev); > > > + > > > +/** > > > + * Acquire Sync-E device name. > > > + * > > > + * @param dev Questioned SyncE device instance > > > + * @return The device name > > > + */ > > > +const char *synce_dev_name(struct synce_dev *dev); > > > + > > > +/** > > > + * Clean-up on memory allocated for device and its ports. > > > + * > > > + * @param dev SyncE device to be cleared > > > + */ > > > +void synce_dev_destroy(struct synce_dev *dev); > > > + > > > +/** > > > + * Check if Sync-E device is running. > > > + * > > > + * @param dev Questioned SyncE device > > > + * @return 0 if false, 1 if true > > > + */ > > > +int synce_dev_is_running(struct synce_dev *dev); > > > + > > > +#endif > > > > |
|
From: Kubalewski, A. <ark...@in...> - 2022-06-09 19:12:18
|
-----Original Message----- From: Aya Levin via Linuxptp-devel <lin...@li...> Sent: Wednesday, June 8, 2022 10:31 AM > On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote: > > synce4l is a software implementation of Synchronous Ethernet (Sync-E) > > according to ITU-T Recommendation G.8264. The design goal is to provide > > logic to supported hardware by processing Ethernet Synchronization > > Messaging Channel (ESMC) and control Digital Phase Locked Loop (DPLL) > We should use EEC rather than DPLL (more abstract less HW related) Sure, graet idea. Will fix it. > > clock on Network Card Interface (NIC). > > > > Application can operate in two mutually exclusive modes. If both modes > > are configured simultaneously, the external input takes precedence over > > internal one (internal is disabled implicitly). > AFAIU there is one selection process which regards all the sources and > select according to the quality/priority. > What is the benefit of two mutually exclusive modes? Yes that is true, but SyncE chain has to start somewhere, currently: - internal mode - the NIC is monitoring its ports and selects one to recover the clock from - external mode - ports are not monitored at all, user provides external signal which drives the EEC, once EEC-DPLL is in locked state synce4l starts broadcasting its QL to the neighbour configured ports. (QL value also user-configured, as only user knows QL of external signal used to drive the EEC), Basically, external mode here was similar to Grandmaster in PTP, at least that was our understanding of A.2 from Annex A of G.8264 I think this needs to be renamed to make it clear for everyone. Annex A also says: "Synchronous Ethernet equipment shall support a selection mechanism that allows synchronization to be derived from the line (i.e., traffic-carrying interfaces), from external synchronization interfaces (i.e., provided by co-located equipment) or internally from the EEC." So in fact, we shall have 3 mutually exclusive modes for a device. We will change this config item to a name "input_mode", with possible string values: - "line" - "external" - "internal" Altough new modes: "internal" and "external", will not differ in implementation from current "external_mode", as this requires preconfiguring device and explicitly defining the QL by the user (at least before proper in-kernel interface for controlling EEC-alike integrated circuits is introduced). Does it make sense? > > > > External input mode > > If synce4l is configured to run in "external input" mode, then DPLL > > needs to have external 1PPS source attached (GPS or another generator). > > In this scenario synce4l always broadcasts clock quality level (QL) > > defined in configuration file. Additionally, for this mode > > incoming Sync-E frames do not participate in best source selection > > algorithm for DPLL. > > > > Internal input mode > Lets use the standard (G.8264 - Annex A) naming of traffic reference > source. Internal may refer internal oscillator. Sure, above answer is sufficient? > > In "internal input" mode incoming Sync-E frames are processed and best > > clock source is extracted from the link having the best quality level. > > synce4l configures such "best quality" port as a source to recover > > clock for DPLL. The recovered QL is broadcasted to all other interfaces > > then. An external clock cannot be used in this mode. > > > > Hardware > > synce4l is not bound to any specific NIC hardware. > In this level HW should be hidden totally - please see my comment in > patch #5 I understand you are refering to the name of "DPLL", will change it to EEC. > > > > Theoretically any NIC could be used for enabling Sync-E device in > Lets use SyncE and not Sync-E Sure, can do that. > > External input mode - the NIC in this mode is in leader role (similar > > to Grand Master role in PTP), providing its Quality Level information > > in ESMC frames to its neighbors. > How do we monitor the quality and validity of an external source? Right now (we already know that DPLL naming all over the code and documentation shall change in future patches), validity is checked as a state of dpll (user configures a shell command "get_dpll_state_cmd" and its possible return values). If it is not "locked", the DNU QL is broadcasted through the configured ports, once "locked" the user-configured QL starts to be broadcasted. The quality itself depends on "external" sources, so this is user-configurable. Thank you, Arkadiusz > > It will not syntonize to any of its neighbor ports, since RX frames > > are not monitored. Only other Sync-E enabled neighbor ports could > > recover clock from that peer and syntonize to it. > > Practically this mode shall be used only if the NIC PHY ports are fed > > with stable and reliable frequency clock, Quality Level values that are > > sent to the peers are configured by the user. This is user > > responsibility to decide and set proper Quality-Level value for the > > device, corresponding to the quality of frequency clock generator of > > used hardware. > > > > When enabling Internal input mode, the NIC must be a device that > > actually, supports Synchronous Ethernet, which means it is able to > > syntonize frequency of all configured ports within the device > > to one chosen port - clock is recovered from its RX path, then is fed > > to other ports with Phase-Locked Loop (PLL) circuit powering clock of > > other ports. > > The NIC driver or dedicated software must provide a user with ability > > to: > > > > obtain current state of a DPLL used to syntonize the ports > > select one of the ports as a candidate for clock recovery. > > Both abilities are configurable in config file - the user must provide > > a shell commands - as explained in manual and descriptions inside of > > example config file. > > > > Arkadiusz Kubalewski (11): > > synce4l: add config knobs for SyncE > > synce4l: add synce in interface and util code > > synce4l: add esmc_socket interface > > synce4l: add synce_clock interface > > synce4l: add synce_dev interface > > synce4l: add synce_dev_ctrl interface > > synce4l: add synce_port interface > > synce4l: add synce_port_ctrl interface > > synce4l: add synce_msg interface > > synce4l: add synce_transport interface > > synce4l: add synce4l application > > > > .gitignore | 1 + > > README.org | 2 + > > config.c | 197 +++++-- > > config.h | 8 + > > configs/synce.cfg | 194 +++++++ > > esmc_socket.c | 99 ++++ > > esmc_socket.h | 42 ++ > > interface.c | 29 +- > > interface.h | 24 + > > makefile | 9 +- > > synce4l.8 | 239 ++++++++ > > synce4l.c | 132 +++++ > > synce4l_README.md | 194 +++++++ > > synce_clock.c | 284 +++++++++ > > synce_clock.h | 40 ++ > > synce_dev.c | 622 ++++++++++++++++++++ > > synce_dev.h | 64 ++ > > synce_dev_ctrl.c | 153 +++++ > > synce_dev_ctrl.h | 64 ++ > > synce_msg.c | 259 +++++++++ > > synce_msg.h | 174 ++++++ > > synce_msg_private.h | 87 +++ > > synce_port.c | 491 ++++++++++++++++ > > synce_port.h | 184 ++++++ > > synce_port_ctrl.c | 1161 +++++++++++++++++++++++++++++++++++++ > > synce_port_ctrl.h | 165 ++++++ > > synce_transport.c | 102 ++++ > > synce_transport.h | 55 ++ > > synce_transport_private.h | 18 + > > util.c | 22 + > > util.h | 8 + > > 31 files changed, 5077 insertions(+), 46 deletions(-) > > create mode 100644 configs/synce.cfg > > create mode 100644 esmc_socket.c > > create mode 100644 esmc_socket.h > > create mode 100644 synce4l.8 > > create mode 100644 synce4l.c > > create mode 100644 synce4l_README.md > > create mode 100644 synce_clock.c > > create mode 100644 synce_clock.h > > create mode 100644 synce_dev.c > > create mode 100644 synce_dev.h > > create mode 100644 synce_dev_ctrl.c > > create mode 100644 synce_dev_ctrl.h > > create mode 100644 synce_msg.c > > create mode 100644 synce_msg.h > > create mode 100644 synce_msg_private.h > > create mode 100644 synce_port.c > > create mode 100644 synce_port.h > > create mode 100644 synce_port_ctrl.c > > create mode 100644 synce_port_ctrl.h > > create mode 100644 synce_transport.c > > create mode 100644 synce_transport.h > > create mode 100644 synce_transport_private.h > > > |
|
From: Aya L. <ay...@nv...> - 2022-06-19 16:50:16
|
Hi, Please elaborate regarding kernel API. I understood from the submission that you plan to start off with some vendor specific sysfs declared in the config file. But I believe this is not the final goal, correct? Do you have kernel patches for review? Thanks, Aya On 6/9/2022 10:12 PM, Kubalewski, Arkadiusz wrote: > -----Original Message----- > From: Aya Levin via Linuxptp-devel <lin...@li...> > Sent: Wednesday, June 8, 2022 10:31 AM > >> On 5/30/2022 10:34 PM, Arkadiusz Kubalewski wrote: >>> synce4l is a software implementation of Synchronous Ethernet (Sync-E) >>> according to ITU-T Recommendation G.8264. The design goal is to provide >>> logic to supported hardware by processing Ethernet Synchronization >>> Messaging Channel (ESMC) and control Digital Phase Locked Loop (DPLL) >> We should use EEC rather than DPLL (more abstract less HW related) > > Sure, graet idea. Will fix it. > >>> clock on Network Card Interface (NIC). >>> >>> Application can operate in two mutually exclusive modes. If both modes >>> are configured simultaneously, the external input takes precedence over >>> internal one (internal is disabled implicitly). >> AFAIU there is one selection process which regards all the sources and >> select according to the quality/priority. >> What is the benefit of two mutually exclusive modes? > > Yes that is true, but SyncE chain has to start somewhere, currently: > - internal mode - the NIC is monitoring its ports and selects one to > recover the clock from > - external mode - ports are not monitored at all, user provides > external signal which drives the EEC, once EEC-DPLL is in locked state > synce4l starts broadcasting its QL to the neighbour configured ports. > (QL value also user-configured, as only user knows QL of external signal used > to drive the EEC), > > Basically, external mode here was similar to Grandmaster in PTP, > at least that was our understanding of A.2 from Annex A of G.8264 > > I think this needs to be renamed to make it clear for everyone. > > Annex A also says: > "Synchronous Ethernet equipment shall support a selection mechanism that allows > synchronization to be derived from the line (i.e., traffic-carrying > interfaces), from external synchronization interfaces (i.e., provided by > co-located equipment) or internally from the EEC." > > So in fact, we shall have 3 mutually exclusive modes for a device. > > We will change this config item to a name "input_mode", with possible string > values: > - "line" > - "external" > - "internal" > > Altough new modes: "internal" and "external", will not differ in implementation > from current "external_mode", as this requires preconfiguring device and > explicitly defining the QL by the user (at least before proper in-kernel > interface for controlling EEC-alike integrated circuits is introduced). > > Does it make sense? > >>> >>> External input mode >>> If synce4l is configured to run in "external input" mode, then DPLL >>> needs to have external 1PPS source attached (GPS or another generator). >>> In this scenario synce4l always broadcasts clock quality level (QL) >>> defined in configuration file. Additionally, for this mode >>> incoming Sync-E frames do not participate in best source selection >>> algorithm for DPLL. >>> >>> Internal input mode >> Lets use the standard (G.8264 - Annex A) naming of traffic reference >> source. Internal may refer internal oscillator. > > Sure, above answer is sufficient? > >>> In "internal input" mode incoming Sync-E frames are processed and best >>> clock source is extracted from the link having the best quality level. >>> synce4l configures such "best quality" port as a source to recover >>> clock for DPLL. The recovered QL is broadcasted to all other interfaces >>> then. An external clock cannot be used in this mode. >>> >>> Hardware >>> synce4l is not bound to any specific NIC hardware. >> In this level HW should be hidden totally - please see my comment in >> patch #5 > > I understand you are refering to the name of "DPLL", will change it to EEC. > >>> >>> Theoretically any NIC could be used for enabling Sync-E device in >> Lets use SyncE and not Sync-E > > Sure, can do that. > >>> External input mode - the NIC in this mode is in leader role (similar >>> to Grand Master role in PTP), providing its Quality Level information >>> in ESMC frames to its neighbors. >> How do we monitor the quality and validity of an external source? > > Right now (we already know that DPLL naming all over the code and documentation > shall change in future patches), validity is checked as a state of dpll (user > configures a shell command "get_dpll_state_cmd" and its possible return > values). If it is not "locked", the DNU QL is broadcasted through the > configured ports, once "locked" the user-configured QL starts to be > broadcasted. > > The quality itself depends on "external" sources, so this is user-configurable. > > Thank you, > Arkadiusz > >>> It will not syntonize to any of its neighbor ports, since RX frames >>> are not monitored. Only other Sync-E enabled neighbor ports could >>> recover clock from that peer and syntonize to it. >>> Practically this mode shall be used only if the NIC PHY ports are fed >>> with stable and reliable frequency clock, Quality Level values that are >>> sent to the peers are configured by the user. This is user >>> responsibility to decide and set proper Quality-Level value for the >>> device, corresponding to the quality of frequency clock generator of >>> used hardware. >>> >>> When enabling Internal input mode, the NIC must be a device that >>> actually, supports Synchronous Ethernet, which means it is able to >>> syntonize frequency of all configured ports within the device >>> to one chosen port - clock is recovered from its RX path, then is fed >>> to other ports with Phase-Locked Loop (PLL) circuit powering clock of >>> other ports. >>> The NIC driver or dedicated software must provide a user with ability >>> to: >>> >>> obtain current state of a DPLL used to syntonize the ports >>> select one of the ports as a candidate for clock recovery. >>> Both abilities are configurable in config file - the user must provide >>> a shell commands - as explained in manual and descriptions inside of >>> example config file. >>> >>> Arkadiusz Kubalewski (11): >>> synce4l: add config knobs for SyncE >>> synce4l: add synce in interface and util code >>> synce4l: add esmc_socket interface >>> synce4l: add synce_clock interface >>> synce4l: add synce_dev interface >>> synce4l: add synce_dev_ctrl interface >>> synce4l: add synce_port interface >>> synce4l: add synce_port_ctrl interface >>> synce4l: add synce_msg interface >>> synce4l: add synce_transport interface >>> synce4l: add synce4l application >>> >>> .gitignore | 1 + >>> README.org | 2 + >>> config.c | 197 +++++-- >>> config.h | 8 + >>> configs/synce.cfg | 194 +++++++ >>> esmc_socket.c | 99 ++++ >>> esmc_socket.h | 42 ++ >>> interface.c | 29 +- >>> interface.h | 24 + >>> makefile | 9 +- >>> synce4l.8 | 239 ++++++++ >>> synce4l.c | 132 +++++ >>> synce4l_README.md | 194 +++++++ >>> synce_clock.c | 284 +++++++++ >>> synce_clock.h | 40 ++ >>> synce_dev.c | 622 ++++++++++++++++++++ >>> synce_dev.h | 64 ++ >>> synce_dev_ctrl.c | 153 +++++ >>> synce_dev_ctrl.h | 64 ++ >>> synce_msg.c | 259 +++++++++ >>> synce_msg.h | 174 ++++++ >>> synce_msg_private.h | 87 +++ >>> synce_port.c | 491 ++++++++++++++++ >>> synce_port.h | 184 ++++++ >>> synce_port_ctrl.c | 1161 +++++++++++++++++++++++++++++++++++++ >>> synce_port_ctrl.h | 165 ++++++ >>> synce_transport.c | 102 ++++ >>> synce_transport.h | 55 ++ >>> synce_transport_private.h | 18 + >>> util.c | 22 + >>> util.h | 8 + >>> 31 files changed, 5077 insertions(+), 46 deletions(-) >>> create mode 100644 configs/synce.cfg >>> create mode 100644 esmc_socket.c >>> create mode 100644 esmc_socket.h >>> create mode 100644 synce4l.8 >>> create mode 100644 synce4l.c >>> create mode 100644 synce4l_README.md >>> create mode 100644 synce_clock.c >>> create mode 100644 synce_clock.h >>> create mode 100644 synce_dev.c >>> create mode 100644 synce_dev.h >>> create mode 100644 synce_dev_ctrl.c >>> create mode 100644 synce_dev_ctrl.h >>> create mode 100644 synce_msg.c >>> create mode 100644 synce_msg.h >>> create mode 100644 synce_msg_private.h >>> create mode 100644 synce_port.c >>> create mode 100644 synce_port.h >>> create mode 100644 synce_port_ctrl.c >>> create mode 100644 synce_port_ctrl.h >>> create mode 100644 synce_transport.c >>> create mode 100644 synce_transport.h >>> create mode 100644 synce_transport_private.h >>> >> |
|
From: Kubalewski, A. <ark...@in...> - 2022-06-21 15:25:23
|
-----Original Message----- From: Aya Levin <ay...@nv...> Sent: Sunday, June 19, 2022 6:35 PM > >Hi, > >Please elaborate regarding kernel API. I understood from the submission >that you plan to start off with some vendor specific sysfs declared in >the config file. >But I believe this is not the final goal, correct? >Do you have kernel patches for review? > >Thanks, >Aya Hi, SyncE standard needs two abilities from the Network Interface Card: - check the status of frequency loop, if it is locked/holdover/etc - select one of the PHY ports as a signal source. Currently in sync4l both of those abilities are provided in form of shell commands, so it can be used with any user-space tool. For the future, the plan is to have generic netlink based interface/subsystem in the kernel. Designed for all devices capable of syntonizing its clock to an external source (I am working on it right now), probably also equipped with generic sysfs for manual control. Thank you, Arkadiusz |
|
From: Aya L. <ay...@nv...> - 2022-06-22 13:42:35
|
On 6/21/2022 6:25 PM, Kubalewski, Arkadiusz wrote: > -----Original Message----- > From: Aya Levin <ay...@nv...> > Sent: Sunday, June 19, 2022 6:35 PM >> >> Hi, >> >> Please elaborate regarding kernel API. I understood from the submission >> that you plan to start off with some vendor specific sysfs declared in >> the config file. >> But I believe this is not the final goal, correct? >> Do you have kernel patches for review? >> >> Thanks, >> Aya > > Hi, > > SyncE standard needs two abilities from the Network Interface Card: > - check the status of frequency loop, if it is locked/holdover/etc > - select one of the PHY ports as a signal source. What about set status - when SyncE state machine needs to move to holdover? > > Currently in sync4l both of those abilities are provided in form of shell > commands, so it can be used with any user-space tool. > > For the future, the plan is to have generic netlink based interface/subsystem > in the kernel. Designed for all devices capable of syntonizing its clock to an > external source (I am working on it right now), probably also equipped with > generic sysfs for manual control. > > Thank you, > Arkadiusz |
|
From: Kubalewski, A. <ark...@in...> - 2022-07-04 10:41:07
|
-----Original Message----- From: Aya Levin <ay...@nv...> Sent: Wednesday, June 22, 2022 3:42 PM > >On 6/21/2022 6:25 PM, Kubalewski, Arkadiusz wrote: >> -----Original Message----- >> From: Aya Levin <ay...@nv...> >> Sent: Sunday, June 19, 2022 6:35 PM >>> >>> Hi, >>> >>> Please elaborate regarding kernel API. I understood from the submission >>> that you plan to start off with some vendor specific sysfs declared in >>> the config file. >>> But I believe this is not the final goal, correct? >>> Do you have kernel patches for review? >>> >>> Thanks, >>> Aya >> >> Hi, >> >> SyncE standard needs two abilities from the Network Interface Card: >> - check the status of frequency loop, if it is locked/holdover/etc >> - select one of the PHY ports as a signal source. >What about set status - when SyncE state machine needs to move to holdover? Sorry, I missed this question. Yes, definitely we need it both, ability to enable and disable clock recovery on a net-device PHY port. Thanks, Arkadiusz >> >> Currently in sync4l both of those abilities are provided in form of shell >> commands, so it can be used with any user-space tool. >> >> For the future, the plan is to have generic netlink based interface/subsystem >> in the kernel. Designed for all devices capable of syntonizing its clock to an >> external source (I am working on it right now), probably also equipped with >> generic sysfs for manual control. >> >> Thank you, >> Arkadiusz > |
|
From: Miroslav L. <mli...@re...> - 2022-05-31 14:06:57
|
On Thu, May 26, 2022 at 10:51:01AM -0700, Richard Cochran wrote: > On Mon, May 02, 2022 at 11:05:54AM +0200, Arkadiusz Kubalewski wrote: > > synce4l is a software implementation of Synchronous Ethernet (Sync-E) > > according to ITU-T Recommendation G.8264. The design goal is to provide > > logic to supported hardware by processing Ethernet Synchronization > > Messaging Channel (ESMC) and control Digital Phase Locked Loop (DPLL) > > clock on Network Card Interface (NIC). > > The bulk of this is a new, stand alone program. One comment that I > received off list questioned whether this program should be part of > linuxptp, or does it deserve its own project? synce4l as submitted doesn't share much with the rest of linuxptp, but that could change with support for the L1_SYNC TLV from 1588-2019. I think it would need to be handled by ptp4l itself, or at least by something communicating with ptp4l over UDS. -- Miroslav Lichvar |
|
From: Kubalewski, A. <ark...@in...> - 2022-08-31 21:23:35
|
>-----Original Message----- >From: Miroslav Lichvar mli...@re... >Sent: Thursday, August 4, 2022 11:08 AM > >On Wed, Jul 20, 2022 at 03:50:04AM +0000, Greg Armstrong wrote: >> To address this concern, Renesas has submitted our open implementation of the ESMC protocol to github, also under the same name: https://github.com/renesas/synce4l. This is an open project under GPLv2, and we welcome contributions to it. > >I looked at the project to better understand the differences between >this and the Intel implementation. Here are my observations. > >The code doesn't build on its own. It requires "clockmatrix api", >which doesn't seem to be available without registration. > >There are some HW-specific ioctls. > >I don't see an abstraction which would allow one binary to support >different types of hardware. > >About 20% of the code comes from linuxptp (per copyright). > >-- >Miroslav Lichvar > Thanks Miroslav! I assume that silence on this thread (from the rest of the members), same as silence on any other community mailing list thread, is actually a quiet approval (or at least no one has any objections). Greg, You are saying that your project is open source, and you would welcome contribution to it, but at the same time seems that your code: - requires third party proprietary module for compilation, - uses HW specific i2c to configure your NIC/timecard. Is that true? If so, do you think that it is proper way to cooperate with an open source community? Excuses me but I hardly understand what you would like to achieve except blocking delivery of this solution. As I see it, your only argument is that SyncE is not a part of 1588. Well sure, SyncE is only mentioned in appendices, but you know, Telecom Profiles are not part of 1588, they are only mentioned in appendices. Should support for them be removed from linuxptp? In my humble opinion, it wouldn't make any sense. Maybe SyncE without PTP does make some sense, but for telecoms it makes most sense to use them both (if hardware supports it). Following this idea, it makes most sense to have both in the same package, as for full support of "Annex L" (1588), the SyncE daemon shall provide data to the PTP daemon. It is easiest to maintain both applications in one repository, this prevents any incompatibility issues between shared interfaces. Richard, I know our solution in its current state is far from being perfect. It communicates with EEC by shell commands specified by the user, but at least it is not specific for any particular hardware. Although it doesn't configure EEC, as it shall be preconfigured before using our solution. Our next steps would be: - introduce in-kernel interface which allows control of EEC, - implement a user-space tool for configuration of EEC using new in-kernel interface, - add support for EEC "get lock status"/"set source" in our solution and Replace shell commands - add support for getting EEC info for Telecom profiles in our solution. Could you please decide if there is a room for this solution in linuxptp, assuming we will review and ACK all the future patches on the SyncE part, and help to maintain it as long as it needs support? We are planning to help extending ptp4l with the interface to interoperate with synce4l (and other similar tools, including Renesas implementation). Also, if you think that it would be better to have it in separated project, please let us know, as we need to upstream it somewhere due to customer requests. Thank you, Arkadiusz |
|
From: Richard C. <ric...@gm...> - 2022-09-05 14:05:01
|
On Wed, Aug 31, 2022 at 09:23:25PM +0000, Kubalewski, Arkadiusz wrote: > Richard, > I know our solution in its current state is far from being perfect. > It communicates with EEC by shell commands specified by the user, but at least > it is not specific for any particular hardware. > Although it doesn't configure EEC, as it shall be preconfigured before using our > solution. I'm not able to judge between the two new projects based on technical merits, because I simply have no time to do so. WRT integration into linuxptp, seeing as there is no agreement on a path forward, I am not going to merge any major new SyncE services into linuxptp. May I suggest that both SyncE projects publish their code on GH/whatever and build their communities. Competition is a good thing in the OSS world! For the interaction between SyncE services and ptp4l, let's discuss proposals for new management messages here in this channel. I'm sure we can come with message formats that will work for any SyncE use case. Thanks, Richard |
|
From: Kubalewski, A. <ark...@in...> - 2022-09-05 19:17:17
|
>On Wed, Aug 31, 2022 at 09:23:25PM +0000, Kubalewski, Arkadiusz wrote: >> Richard, >> I know our solution in its current state is far from being perfect. >> It communicates with EEC by shell commands specified by the user, but at least >> it is not specific for any particular hardware. >> Although it doesn't configure EEC, as it shall be preconfigured before using our >> solution. > >I'm not able to judge between the two new projects based on technical >merits, because I simply have no time to do so. > >WRT integration into linuxptp, seeing as there is no agreement on a >path forward, I am not going to merge any major new SyncE services >into linuxptp. > >May I suggest that both SyncE projects publish their code on >GH/whatever and build their communities. Competition is a good thing >in the OSS world! > >For the interaction between SyncE services and ptp4l, let's discuss >proposals for new management messages here in this channel. I'm sure >we can come with message formats that will work for any SyncE use >case. > >Thanks, >Richard > Thank you very much Richard! BR, Arkadiusz |
|
From: Greg A. <gre...@re...> - 2022-05-31 14:42:16
|
This is the reason synce4l (based on an ITU-T physical layer protocol, G.8264) should be separate from linuxptp project and be it's own standalone project; so that it does NOT get confused with L1_SYNC TLV from IEEE 1588-2019 (if/when it gets added to linuxptp). The HA profile's use of the physical layer clock is not the same as ITU-T definition of SyncE. This is not to say there is no interaction between synce4l and ptp4l to support the ITU-T profiles that have physical layer assistance, but the two protocols themselves are mutually exclusive. Also, similar to the PHC to abstract the hardware for user space programs, I would suspect a similar SyncE Hardware Clock infrastructure will be needed in Linux to allow for abstraction of hardware used by synce4l (i.e. as this project matures to meet the full recommendations in G.8264 and, as required, G.781). Greg Greg Armstrong Principal System Architect, Timing Products Division Renesas Electronics Canada Limited Mobile: 1-613-218-9373 -----Original Message----- From: Miroslav Lichvar <mli...@re...> Sent: May 31, 2022 10:07 AM To: Richard Cochran <ric...@gm...> Cc: pio...@in...; ana...@in...; and...@in...; lin...@li... Subject: Re: [Linuxptp-devel] [PATCH 00/11] synce4l: add software for Synchronous Ethernet On Thu, May 26, 2022 at 10:51:01AM -0700, Richard Cochran wrote: > On Mon, May 02, 2022 at 11:05:54AM +0200, Arkadiusz Kubalewski wrote: > > synce4l is a software implementation of Synchronous Ethernet > > (Sync-E) according to ITU-T Recommendation G.8264. The design goal > > is to provide logic to supported hardware by processing Ethernet > > Synchronization Messaging Channel (ESMC) and control Digital Phase > > Locked Loop (DPLL) clock on Network Card Interface (NIC). > > The bulk of this is a new, stand alone program. One comment that I > received off list questioned whether this program should be part of > linuxptp, or does it deserve its own project? synce4l as submitted doesn't share much with the rest of linuxptp, but that could change with support for the L1_SYNC TLV from 1588-2019. I think it would need to be handled by ptp4l itself, or at least by something communicating with ptp4l over UDS. -- Miroslav Lichvar _______________________________________________ Linuxptp-devel mailing list Lin...@li... https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Flinuxptp-devel&data=05%7C01%7Cgreg.armstrong.uw%40renesas.com%7Cece84de7784b43f32e4908da430f2242%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637896029522150943%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=9fqPCkzVsPbjxXg%2Bn2%2FwKztjJiWq5RXsRUEXmHTC4JA%3D&reserved=0 |
|
From: Miroslav L. <mli...@re...> - 2022-06-01 07:33:28
|
On Tue, May 31, 2022 at 02:42:00PM +0000, Greg Armstrong wrote: > This is the reason synce4l (based on an ITU-T physical layer protocol, G.8264) should be separate from linuxptp project and be it's own standalone project; so that it does NOT get confused with L1_SYNC TLV from IEEE 1588-2019 (if/when it gets added to linuxptp). The HA profile's use of the physical layer clock is not the same as ITU-T definition of SyncE. If synce4l remained specific to G.8264, the confusion could be avoided by renaming it to something else. (I don't have a suggestion at the moment.) There are plenty of conflicting options in linuxptp. The advantage of having both supported in linuxptp, no matter if the actual HW support is for both in synce4l or separately in ptp4l and synce4l, is that the support code can be shared. The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. -- Miroslav Lichvar |
|
From: Greg A. <gre...@re...> - 2022-06-01 13:18:25
|
> The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. This is my concern, as this statement is false. SyncE describes an ITU-T network synchronization method for the distribution of frequency over a transport network, using the L1 physical layer clock (Ethernet or OTN). Details of the network limits, equipment clock requirements, protocol and architecture are contained in numerous ITU-T recommendations (G.826x). The use of the SyncE clock to assist PTP is described in G.8273.2 for T-BC/T-TSC (and also in G.8273.4 as a option for T-BC-P/T-TSC-P). In this case, the SyncE clock does not need to be coherent or congruent with the PTP clock. The L1_SYNC TLV is a Layer-1 based synchronization performance enhancement for PTP (described in Annex L of IEEE 1588-2019). The key aspect of the scheme illustrated by the high accuracy model is that the physical clock signal and the time of the PTP Instance are coherent. Yes, both methods can be used to syntonize the Timestamping Clocks in all PTP Instances to within the required tolerance with respect to the Grandmaster Clock, but how to use SyncE vs L1_SYNC in the protocol (ptp4l) is different - one is managed with the protocol (L1_SYNC) the other by it's own protocol (synce4l). I'm not arguing there could not be shared support code, but to the uninformed developer, synce4l could be mistaken for L1_SYNC (i.e. mistakenly use the PHC for "SyncE" clock management). Also, as Richard highlighted, the development of synce4l and ptp4l are mutual exclusive, so it does not make sense for the synce4l code submissions to be reviewed by the linuxptp development community, as they are not the right audience (as unlikely familiar with the ITU-T recommendations that synce4l must follow). Greg Greg Armstrong Principal System Architect, Timing Products Division Renesas Electronics Canada Limited Mobile: 1-613-218-9373 -----Original Message----- From: Miroslav Lichvar <mli...@re...> Sent: June 1, 2022 3:33 AM To: Greg Armstrong <gre...@re...> Cc: Richard Cochran <ric...@gm...>; pio...@in...; ana...@in...; and...@in...; lin...@li... Subject: Re: [Linuxptp-devel] [PATCH 00/11] synce4l: add software for Synchronous Ethernet On Tue, May 31, 2022 at 02:42:00PM +0000, Greg Armstrong wrote: > This is the reason synce4l (based on an ITU-T physical layer protocol, G.8264) should be separate from linuxptp project and be it's own standalone project; so that it does NOT get confused with L1_SYNC TLV from IEEE 1588-2019 (if/when it gets added to linuxptp). The HA profile's use of the physical layer clock is not the same as ITU-T definition of SyncE. If synce4l remained specific to G.8264, the confusion could be avoided by renaming it to something else. (I don't have a suggestion at the moment.) There are plenty of conflicting options in linuxptp. The advantage of having both supported in linuxptp, no matter if the actual HW support is for both in synce4l or separately in ptp4l and synce4l, is that the support code can be shared. The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. -- Miroslav Lichvar |
|
From: Kubalewski, A. <ark...@in...> - 2022-06-01 14:43:55
|
-----Original Message----- From: Greg Armstrong <gre...@re...> Sent: Wednesday, June 1, 2022 3:18 PM > > The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. > > This is my concern, as this statement is false. SyncE describes an ITU-T network synchronization method for the distribution of frequency over a transport network, using the L1 physical layer clock (Ethernet or OTN). Details of the network limits, equipment clock requirements, protocol and architecture are contained in numerous ITU-T recommendations (G.826x). The use of the SyncE clock to assist PTP is described in G.8273.2 for T-BC/T-TSC (and also in G.8273.4 as a option for T-BC-P/T-TSC-P). In this case, the SyncE clock does not need to be coherent or congruent with the PTP clock. > > The L1_SYNC TLV is a Layer-1 based synchronization performance enhancement for PTP (described in Annex L of IEEE 1588-2019). The key aspect of the scheme illustrated by the high accuracy model is that the physical clock signal and the time of the PTP Instance are coherent. > > Yes, both methods can be used to syntonize the Timestamping Clocks in all PTP Instances to within the required tolerance with respect to the Grandmaster Clock, but how to use SyncE vs L1_SYNC in the protocol (ptp4l) is different - one is managed with the protocol (L1_SYNC) the other by it's own protocol (synce4l). According to the Annex L, L.1 General, L1Sync is optional feature that can be used to support cooperation between PTP and physical layer (L1) based syntonization, also example of Synchronous Ethernet is given. My understanding: First there must be physical layer (L1) based synchronization, then PTP protocol can support L1Sync. Or in other words, the one will not be able to support optional PTP L1Sync, without physical layer (L1) based syntonization (e.g. SyncE) Am I wrong somewhere? Thanks, Arkadiusz > > I'm not arguing there could not be shared support code, but to the uninformed developer, synce4l could be mistaken for L1_SYNC (i.e. mistakenly use the PHC for "SyncE" clock management). Also, as Richard highlighted, the development of synce4l and ptp4l are mutual exclusive, so it does not make sense for the synce4l code submissions to be reviewed by the linuxptp development community, as they are not the right audience (as unlikely familiar with the ITU-T recommendations that synce4l must follow). > > Greg > > Greg Armstrong > Principal System Architect, Timing Products Division > Renesas Electronics Canada Limited > Mobile: 1-613-218-9373 > > -----Original Message----- > From: Miroslav Lichvar <mli...@re...> > Sent: June 1, 2022 3:33 AM > To: Greg Armstrong <gre...@re...> > Cc: Richard Cochran <ric...@gm...>; pio...@in...; ana...@in...; and...@in...; lin...@li... > Subject: Re: [Linuxptp-devel] [PATCH 00/11] synce4l: add software for Synchronous Ethernet > > On Tue, May 31, 2022 at 02:42:00PM +0000, Greg Armstrong wrote: > > This is the reason synce4l (based on an ITU-T physical layer protocol, G.8264) should be separate from linuxptp project and be it's own standalone project; so that it does NOT get confused with L1_SYNC TLV from IEEE 1588-2019 (if/when it gets added to linuxptp). The HA profile's use of the physical layer clock is not the same as ITU-T definition of SyncE. > > If synce4l remained specific to G.8264, the confusion could be avoided by renaming it to something else. (I don't have a suggestion at the > moment.) > > There are plenty of conflicting options in linuxptp. > > The advantage of having both supported in linuxptp, no matter if the actual HW support is for both in synce4l or separately in ptp4l and synce4l, is that the support code can be shared. The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. > > -- > Miroslav Lichvar > > > |
|
From: Greg A. <gre...@re...> - 2022-06-01 15:14:59
|
Again, I'm not arguing there is no interface between synce4l and ptp4l; but the development of synce4l, which is a implementation of the ITU-T G.8264 ESMC protocol and interfaces the synchronous Ethernet Equipment Clock (EEC), should not be under the linuxptp project, which is an implementation of the IEEE 1588 protocol which interfaces the Local PTP Clock (via the Linux PHC). Below is a snip from the Network Time Foundation: The Linux PTP Project is a Linux-focused software implementation of the Precision Time Protocol (PTP) specification that meets or exceeds the IEEE 1588 Standard. Its dual design goals are to provide the highest possible performance levels and be a thoroughly robust implementation of the PTP Standard. Thus, I would be more supportive of get NTF to start a Linux ESMC Project (synce4l) and allow your submission to be there. This will also make sure one does not "break" either protocol to support L1 syntonization in PTP (i.e. make sure the interfaces are vetted properly, and that there is no undesired influences on either clock by the other protocol). Greg -----Original Message----- From: Kubalewski, Arkadiusz <ark...@in...> Sent: June 1, 2022 10:44 AM To: Greg Armstrong <gre...@re...>; Miroslav Lichvar <mli...@re...> Cc: Kwapulinski, Piotr <pio...@in...>; Sawula, Andrzej <and...@in...>; lin...@li...; Gerasymenko, Anatolii <ana...@in...> Subject: RE: [Linuxptp-devel] [PATCH 00/11] synce4l: add software for Synchronous Ethernet -----Original Message----- From: Greg Armstrong <gre...@re...> Sent: Wednesday, June 1, 2022 3:18 PM > > The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. > > This is my concern, as this statement is false. SyncE describes an ITU-T network synchronization method for the distribution of frequency over a transport network, using the L1 physical layer clock (Ethernet or OTN). Details of the network limits, equipment clock requirements, protocol and architecture are contained in numerous ITU-T recommendations (G.826x). The use of the SyncE clock to assist PTP is described in G.8273.2 for T-BC/T-TSC (and also in G.8273.4 as a option for T-BC-P/T-TSC-P). In this case, the SyncE clock does not need to be coherent or congruent with the PTP clock. > > The L1_SYNC TLV is a Layer-1 based synchronization performance enhancement for PTP (described in Annex L of IEEE 1588-2019). The key aspect of the scheme illustrated by the high accuracy model is that the physical clock signal and the time of the PTP Instance are coherent. > > Yes, both methods can be used to syntonize the Timestamping Clocks in all PTP Instances to within the required tolerance with respect to the Grandmaster Clock, but how to use SyncE vs L1_SYNC in the protocol (ptp4l) is different - one is managed with the protocol (L1_SYNC) the other by it's own protocol (synce4l). According to the Annex L, L.1 General, L1Sync is optional feature that can be used to support cooperation between PTP and physical layer (L1) based syntonization, also example of Synchronous Ethernet is given. My understanding: First there must be physical layer (L1) based synchronization, then PTP protocol can support L1Sync. Or in other words, the one will not be able to support optional PTP L1Sync, without physical layer (L1) based syntonization (e.g. SyncE) Am I wrong somewhere? Thanks, Arkadiusz > > I'm not arguing there could not be shared support code, but to the uninformed developer, synce4l could be mistaken for L1_SYNC (i.e. mistakenly use the PHC for "SyncE" clock management). Also, as Richard highlighted, the development of synce4l and ptp4l are mutual exclusive, so it does not make sense for the synce4l code submissions to be reviewed by the linuxptp development community, as they are not the right audience (as unlikely familiar with the ITU-T recommendations that synce4l must follow). > > Greg > > Greg Armstrong > Principal System Architect, Timing Products Division Renesas > Electronics Canada Limited > Mobile: 1-613-218-9373 > > -----Original Message----- > From: Miroslav Lichvar <mli...@re...> > Sent: June 1, 2022 3:33 AM > To: Greg Armstrong <gre...@re...> > Cc: Richard Cochran <ric...@gm...>; > pio...@in...; ana...@in...; > and...@in...; lin...@li... > Subject: Re: [Linuxptp-devel] [PATCH 00/11] synce4l: add software for > Synchronous Ethernet > > On Tue, May 31, 2022 at 02:42:00PM +0000, Greg Armstrong wrote: > > This is the reason synce4l (based on an ITU-T physical layer protocol, G.8264) should be separate from linuxptp project and be it's own standalone project; so that it does NOT get confused with L1_SYNC TLV from IEEE 1588-2019 (if/when it gets added to linuxptp). The HA profile's use of the physical layer clock is not the same as ITU-T definition of SyncE. > > If synce4l remained specific to G.8264, the confusion could be avoided > by renaming it to something else. (I don't have a suggestion at the > moment.) > > There are plenty of conflicting options in linuxptp. > > The advantage of having both supported in linuxptp, no matter if the actual HW support is for both in synce4l or separately in ptp4l and synce4l, is that the support code can be shared. The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. > > -- > Miroslav Lichvar > > > |
|
From: Miroslav L. <mli...@re...> - 2022-06-01 15:19:44
|
On Wed, Jun 01, 2022 at 01:18:09PM +0000, Greg Armstrong wrote: > > The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. > > This is my concern, as this statement is false. SyncE describes an ITU-T network synchronization method for the distribution of frequency over a transport network, using the L1 physical layer clock (Ethernet or OTN). Details of the network limits, equipment clock requirements, protocol and architecture are contained in numerous ITU-T recommendations (G.826x). The use of the SyncE clock to assist PTP is described in G.8273.2 for T-BC/T-TSC (and also in G.8273.4 as a option for T-BC-P/T-TSC-P). In this case, the SyncE clock does not need to be coherent or congruent with the PTP clock. > > The L1_SYNC TLV is a Layer-1 based synchronization performance enhancement for PTP (described in Annex L of IEEE 1588-2019). The key aspect of the scheme illustrated by the high accuracy model is that the physical clock signal and the time of the PTP Instance are coherent. The HA profile (corresponding to White Rabbit) requires that, but not the L1 support in PTP in general. That's the point of the L1_SYNC TLV to communicate the requirements for coherency and congruency between the peers. > Yes, both methods can be used to syntonize the Timestamping Clocks in all PTP Instances to within the required tolerance with respect to the Grandmaster Clock, but how to use SyncE vs L1_SYNC in the protocol (ptp4l) is different - one is managed with the protocol (L1_SYNC) the other by it's own protocol (synce4l). Right. But consider what the L1 support is in essence. The main point is to configure and monitor the L1 properties of the hardware. A proper kernel interface doesn't exist yet. It might be ethtool netlink, which is not very easy to use (at least for me). > I'm not arguing there could not be shared support code, but to the uninformed developer, synce4l could be mistaken for L1_SYNC (i.e. mistakenly use the PHC for "SyncE" clock management). Also, as Richard highlighted, the development of synce4l and ptp4l are mutual exclusive, so it does not make sense for the synce4l code submissions to be reviewed by the linuxptp development community, as they are not the right audience (as unlikely familiar with the ITU-T recommendations that synce4l must follow). As I said, the confusion could be avoided by different naming if necessary. To me that doesn't look like a big concern. linuxptp includes utilities like phc2sys and phc_ctl that don't have much to do with the PTP protocol, but they are useful when PTP is used for synchronization. Same applies to synce4l. There already is support for some telco profiles, so I'd say it's a very good fit for the project. Whether Richard has or doesn't have time to review/maintain the code, I think that is a different matter. -- Miroslav Lichvar |
|
From: Greg A. <gre...@re...> - 2022-06-01 15:45:01
|
I see synce4l as an implementation of the ESMC protocol, thus, the name fits. As you highlighted, there will be challenges for synce4l to configure and monitor the L1 properties of the hardware, and also interface the EEC hardware. If this wasn't the intent of synce4l, then I agree, the name needs to change to remove "synce", which is an ITU-T term. For the implementation of L1_SYNC, it may or may not have the same interfaces to hardware - especially when looking at how HA profile uses it. Also, the ITU-T 1588 profiles do not use L1_SYNC to manage the physical layer - I'm only aware of the HA profile using it right now. Greg -----Original Message----- From: Miroslav Lichvar <mli...@re...> Sent: June 1, 2022 11:19 AM To: Greg Armstrong <gre...@re...> Cc: Richard Cochran <ric...@gm...>; pio...@in...; ana...@in...; and...@in...; lin...@li... Subject: Re: [Linuxptp-devel] [PATCH 00/11] synce4l: add software for Synchronous Ethernet On Wed, Jun 01, 2022 at 01:18:09PM +0000, Greg Armstrong wrote: > > The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite small when compared to the rest. > > This is my concern, as this statement is false. SyncE describes an ITU-T network synchronization method for the distribution of frequency over a transport network, using the L1 physical layer clock (Ethernet or OTN). Details of the network limits, equipment clock requirements, protocol and architecture are contained in numerous ITU-T recommendations (G.826x). The use of the SyncE clock to assist PTP is described in G.8273.2 for T-BC/T-TSC (and also in G.8273.4 as a option for T-BC-P/T-TSC-P). In this case, the SyncE clock does not need to be coherent or congruent with the PTP clock. > > The L1_SYNC TLV is a Layer-1 based synchronization performance enhancement for PTP (described in Annex L of IEEE 1588-2019). The key aspect of the scheme illustrated by the high accuracy model is that the physical clock signal and the time of the PTP Instance are coherent. The HA profile (corresponding to White Rabbit) requires that, but not the L1 support in PTP in general. That's the point of the L1_SYNC TLV to communicate the requirements for coherency and congruency between the peers. > Yes, both methods can be used to syntonize the Timestamping Clocks in all PTP Instances to within the required tolerance with respect to the Grandmaster Clock, but how to use SyncE vs L1_SYNC in the protocol (ptp4l) is different - one is managed with the protocol (L1_SYNC) the other by it's own protocol (synce4l). Right. But consider what the L1 support is in essence. The main point is to configure and monitor the L1 properties of the hardware. A proper kernel interface doesn't exist yet. It might be ethtool netlink, which is not very easy to use (at least for me). > I'm not arguing there could not be shared support code, but to the uninformed developer, synce4l could be mistaken for L1_SYNC (i.e. mistakenly use the PHC for "SyncE" clock management). Also, as Richard highlighted, the development of synce4l and ptp4l are mutual exclusive, so it does not make sense for the synce4l code submissions to be reviewed by the linuxptp development community, as they are not the right audience (as unlikely familiar with the ITU-T recommendations that synce4l must follow). As I said, the confusion could be avoided by different naming if necessary. To me that doesn't look like a big concern. linuxptp includes utilities like phc2sys and phc_ctl that don't have much to do with the PTP protocol, but they are useful when PTP is used for synchronization. Same applies to synce4l. There already is support for some telco profiles, so I'd say it's a very good fit for the project. Whether Richard has or doesn't have time to review/maintain the code, I think that is a different matter. -- Miroslav Lichvar |
|
From: Erez <ere...@gm...> - 2022-06-01 15:37:54
|
On Wed, 1 Jun 2022 at 17:22, Miroslav Lichvar <mli...@re...> wrote: > On Wed, Jun 01, 2022 at 01:18:09PM +0000, Greg Armstrong wrote: > > > The difference in the protocol (L1_SYNC TLV vs SSM) seems to me quite > small when compared to the rest. > > > > This is my concern, as this statement is false. SyncE describes an ITU-T > network synchronization method for the distribution of frequency over a > transport network, using the L1 physical layer clock (Ethernet or OTN). > Details of the network limits, equipment clock requirements, protocol and > architecture are contained in numerous ITU-T recommendations (G.826x). The > use of the SyncE clock to assist PTP is described in G.8273.2 for > T-BC/T-TSC (and also in G.8273.4 as a option for T-BC-P/T-TSC-P). In this > case, the SyncE clock does not need to be coherent or congruent with the > PTP clock. > > > > The L1_SYNC TLV is a Layer-1 based synchronization performance > enhancement for PTP (described in Annex L of IEEE 1588-2019). The key > aspect of the scheme illustrated by the high accuracy model is that the > physical clock signal and the time of the PTP Instance are coherent. > > The HA profile (corresponding to White Rabbit) requires that, but not > the L1 support in PTP in general. That's the point of the L1_SYNC TLV > to communicate the requirements for coherency and congruency between > the peers. > > > Yes, both methods can be used to syntonize the Timestamping Clocks in > all PTP Instances to within the required tolerance with respect to the > Grandmaster Clock, but how to use SyncE vs L1_SYNC in the protocol (ptp4l) > is different - one is managed with the protocol (L1_SYNC) the other by it's > own protocol (synce4l). > > Right. But consider what the L1 support is in essence. The main point > is to configure and monitor the L1 properties of the hardware. A > proper kernel interface doesn't exist yet. It might be ethtool > netlink, which is not very easy to use (at least for me). > > > I'm not arguing there could not be shared support code, but to the > uninformed developer, synce4l could be mistaken for L1_SYNC (i.e. > mistakenly use the PHC for "SyncE" clock management). Also, as Richard > highlighted, the development of synce4l and ptp4l are mutual exclusive, so > it does not make sense for the synce4l code submissions to be reviewed by > the linuxptp development community, as they are not the right audience (as > unlikely familiar with the ITU-T recommendations that synce4l must follow). > > As I said, the confusion could be avoided by different naming if > necessary. To me that doesn't look like a big concern. > > linuxptp includes utilities like phc2sys and phc_ctl that don't have > much to do with the PTP protocol, but they are useful when PTP is used > for synchronization. Same applies to synce4l. There already is support > for some telco profiles, so I'd say it's a very good fit for the > project. > I agree that phc2sys and phc_ctl do not do much with PTP. I think Richard adds them and the pmc tool, since he needs them for debugging and no one else provides the infrastructure to communicate with the ptp4l or with Linux PHC clocks. I do hope my project https://sf.net/p/libptpmgmt/, does help to fill this gap and helps with reducing the overhead from this project. phc2sys can be implemented with libptpmgmt. I already cloned pmc with libptpmgmt. Maybe it is time that this project focuses on ptp4l and let other applications have their own projects. > > Whether Richard has or doesn't have time to review/maintain the code, > I think that is a different matter. > > -- > Miroslav Lichvar > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > |
|
From: Miroslav L. <mli...@re...> - 2022-06-02 10:23:30
|
On Wed, Jun 01, 2022 at 05:37:10PM +0200, Erez wrote: > I do hope my project https://sf.net/p/libptpmgmt/, does help to fill this > gap and helps with reducing the overhead from this project. > phc2sys can be implemented with libptpmgmt. > I already cloned pmc with libptpmgmt. > > Maybe it is time that this project focuses on ptp4l and let other > applications have their own projects. phc2sys shares quite a lot of code with ptp4l, it's not just the PTP management support. It it was a separate project, it would need to duplicate the code, or linuxptp would need to provide and maintain a library. If some components had a different maintainer, it doesn't mean it has to be a separate project or repository. -- Miroslav Lichvar |
|
From: Aya L. <ay...@nv...> - 2022-06-08 09:09:50
|
On 5/2/2022 12:05 PM, Arkadiusz Kubalewski wrote: > synce_clock interface allows creation, polling and destruction of > SyncE clock, designed to step and drive SyncE logic on all configured > SyncE devices. > > SyncE clock is created with a SyncE-type configuration, where > SyncE-type configuration is prepared by parsing SyncE config file. > > Poll of a SyncE clock, will step each configured SyncE device. > > Destruction of SyncE clock releases all resources obtained by SyncE > clock and devices. > > Co-developed-by: Piotr Kwapulinski <pio...@in...> > Signed-off-by: Piotr Kwapulinski <pio...@in...> > Co-developed-by: Michal Michalik <mic...@in...> > Signed-off-by: Michal Michalik <mic...@in...> > Signed-off-by: Arkadiusz Kubalewski <ark...@in...> > --- > synce_clock.c | 296 ++++++++++++++++++++++++++++++++++++++++++++++++++ > synce_clock.h | 52 +++++++++ > 2 files changed, 348 insertions(+) > create mode 100644 synce_clock.c > create mode 100644 synce_clock.h > > diff --git a/synce_clock.c b/synce_clock.c > new file mode 100644 > index 000000000000..8007fa390bbd > --- /dev/null > +++ b/synce_clock.c > @@ -0,0 +1,296 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/** > + * @file synce_clock.c > + * @brief Implements a SYNCE clock interface. > + * @note Copyright (C) 2022 Intel Corporation > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2, as published > + * by the Free Software Foundation. > + * > + * 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, see <http://www.gnu.org/licenses/>. > + */ > +#include <stdlib.h> > +#include <errno.h> > +#include <unistd.h> > +#include <sys/queue.h> > + > +#include "synce_clock.h" > +#include "interface.h" > +#include "synce_dev.h" > +#include "print.h" > +#include "config.h" > +#include "missing.h" > + > +#define SYNCE_CLOCK_DELAY_USEC 20000 > +#define SYNCE_CLOCK_INIT_DELAY_USEC 200000 > +#define SYNCE_CLOCK_INIT_N_TRIES 10 > + > +struct interface { > + STAILQ_ENTRY(interface) list; > +}; > + > +struct synce_dev { > + LIST_ENTRY(synce_dev) list; > +}; > + > +enum synce_clock_state { > + SYNCE_CLK_UNKNOWN = 0, > + SYNCE_CLK_INITED, > + SYNCE_CLK_DEV_RDY, > + SYNCE_CLK_DEV_INITED, > + SYNCE_CLK_RUNNING, > + SYNCE_CLK_FAILED, > +}; > + > +struct synce_clock { > + int num_devices; > + int state; > + LIST_HEAD(devices_head, synce_dev) devices; > +}; > + > +static struct synce_clock *create_clock(void) Please rename to create_synce_clock() > +{ > + static struct synce_clock clk; > + > + if (clk.state != SYNCE_CLK_UNKNOWN) { > + synce_clock_destroy(&clk); > + pr_info("old synce_clock destroyed"); > + } > + clk.state = SYNCE_CLK_INITED; > + > + return &clk; > +} > + > +static void add_device(struct synce_clock *clk, struct synce_dev *dev) > +{ > + struct synce_dev *dev_iter, *last_dev = NULL; > + > + LIST_FOREACH(dev_iter, &clk->devices, list) { > + last_dev = dev_iter; > + } > + > + if (last_dev) { > + LIST_INSERT_AFTER(last_dev, dev, list); > + } else { > + LIST_INSERT_HEAD(&clk->devices, dev, list); > + } > +} > + > +static int create_synce_devices(struct synce_clock *clk, struct config *cfg) > +{ > + struct interface *iface; > + struct synce_dev *dev; > + const char *dev_name; > + int count = 0; > + > + if (clk->state != SYNCE_CLK_INITED) { > + goto err; > + } > + > + LIST_INIT(&clk->devices); > + STAILQ_FOREACH(iface, &cfg->interfaces, list) { > + /* only parent devices shall be addresed */ > + if (!interface_se_has_parent_dev(iface)) { > + dev_name = interface_name(iface); > + dev = synce_dev_create(dev_name); > + if (!dev) { > + pr_err("failed to create device %s", dev_name); > + continue; > + } > + > + pr_debug("device init %s addr %p", dev_name, dev); > + add_device(clk, dev); > + count++; > + } > + } > + > + if (!count) { > + pr_err("no devices created"); > + goto err; > + } > + > + pr_info("created num_devices: %d", count); > + clk->num_devices = count; > + clk->state = SYNCE_CLK_DEV_RDY; > + > + return 0; > +err: > + clk->state = SYNCE_CLK_FAILED; > + return -EINVAL; > +} > + > +static int init_synce_devices(struct synce_clock *clk, struct config *cfg) > +{ > + struct synce_dev *dev, *tmp; > + int count = 0; > + > + if (clk->state != SYNCE_CLK_DEV_RDY) { > + goto err; > + } > + > + LIST_FOREACH_SAFE(dev, &clk->devices, list, tmp) { > + /* Each parent device will init its ports */ > + if (synce_dev_init(dev, cfg)) { > + pr_err("failed to init device %s", > + synce_dev_name(dev)); > + synce_dev_destroy(dev); > + LIST_REMOVE(dev, list); > + free(dev); > + continue; > + } else { > + pr_debug("device inited %s", synce_dev_name(dev)); > + count++; > + } > + } > + > + if (count == 0) { > + pr_err("no Sync-E devices initialized"); > + goto err; > + } else if (count != clk->num_devices) { > + pr_warning("initialized only %d from %d Sync-E devices", > + count, clk->num_devices); > + clk->num_devices = count; > + } > + clk->state = SYNCE_CLK_DEV_INITED; > + > + return 0; > +err: > + clk->state = SYNCE_CLK_FAILED; > + return -EINVAL; > +} > + > +static void remove_failed_devices(struct synce_clock *clk) > +{ > + struct synce_dev *dev, *tmp; > + int failed_cnt = 0; > + > + LIST_FOREACH_SAFE(dev, &clk->devices, list, tmp) { > + if (!synce_dev_is_running(dev)) { > + synce_dev_destroy(dev); > + LIST_REMOVE(dev, list); > + free(dev); > + failed_cnt++; > + } > + } > + clk->num_devices -= failed_cnt; > + pr_warning("Found dead devices: %d", failed_cnt); > + pr_info("devices still running: %d", clk->num_devices); > +} > + > +static int verify_clock_state(struct synce_clock *clk) > +{ > + int i, running, timeout = SYNCE_CLOCK_INIT_N_TRIES; > + struct synce_dev *dev; > + > + if (clk->state < SYNCE_CLK_DEV_INITED) { > + return -ENODEV; > + } > + > + /* let threads get running */ > + for (i = 0; i < timeout; ++i) { > + running = 0; > + LIST_FOREACH(dev, &clk->devices, list) { > + if (synce_dev_is_running(dev)) > + running++; > + } > + > + if (running == clk->num_devices) { > + clk->state = SYNCE_CLK_RUNNING; > + break; > + } > + usleep(SYNCE_CLOCK_INIT_DELAY_USEC); > + } > + > + pr_debug("running num_devices %d configured %d", > + running, clk->num_devices); > + > + /* If at least one dev is running we leave clock running > + * while removing failed devices. > + * Previous traces shall indicate which ones have failed. > + */ > + if (!running) { > + pr_err("no device is running"); > + return -ENODEV; > + } else if (running != clk->num_devices) { > + remove_failed_devices(clk); > + } > + > + return 0; > +} > + > +struct synce_clock *synce_clock_create(struct config *cfg) > +{ > + struct synce_clock *clk; > + int err; > + > + if (!cfg) { > + pr_err("%s cfg is NULL", __func__); > + return NULL; > + } > + > + clk = create_clock(); > + if (!clk) { > + return NULL; > + } > + err = create_synce_devices(clk, cfg); > + if (err) { > + goto destroy; > + } > + err = init_synce_devices(clk, cfg); > + if (err) { > + goto destroy; > + } > + err = verify_clock_state(clk); > + if (err) { > + goto destroy; > + } > + > + return clk; > + > +destroy: > + synce_clock_destroy(clk); > + > + return NULL; > +} > + > +void synce_clock_destroy(struct synce_clock *clk) > +{ > + struct synce_dev *dev, *tmp; > + > + pr_debug("%s", __func__); > + > + LIST_FOREACH_SAFE(dev, &clk->devices, list, tmp) { > + synce_dev_destroy(dev); > + LIST_REMOVE(dev, list); > + free(dev); > + } > + clk->num_devices = 0; > + clk->state = SYNCE_CLK_UNKNOWN; > + > + return; > +} > + > +int synce_clock_poll(struct synce_clock *clk) > +{ > + struct synce_dev *dev; > + int ret = -ENODEV; > + > + if (clk->state == SYNCE_CLK_RUNNING) { > + LIST_FOREACH(dev, &clk->devices, list) { > + ret = synce_dev_step(dev); > + if (ret) { > + pr_err("dev_step fail"); > + } > + } > + } > + usleep(SYNCE_CLOCK_DELAY_USEC); > + > + return ret; > +} > diff --git a/synce_clock.h b/synce_clock.h > new file mode 100644 > index 000000000000..e5559a4cc9cc > --- /dev/null > +++ b/synce_clock.h > @@ -0,0 +1,52 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/** > + * @file synce_clock.h > + * @brief Implements a Sync-E clock behavior. > + * @note Copyright (C) 2022 Intel Corporation > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2, as published > + * by the Free Software Foundation. > + * > + * 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, see <http://www.gnu.org/licenses/>. > + */ > +#ifndef HAVE_SYNCE_CLOCK_H > +#define HAVE_SYNCE_CLOCK_H > + > +#include "config.h" > + > +/* Opaque type */ > +struct synce_clock; > + > +/** > + * Create a Sync-E clock instance. There can only be one synce_clock in a > + * system so subsequent calls will destroy the previous clock instance. > + * > + * @param cfg Pointer to the SYNCE-type configuration database > + * @return Pointer to the single global Sync-E clock instance > + */ > +struct synce_clock *synce_clock_create(struct config *cfg); > + > +/** > + * Destroy resources associated with the synce clock. > + * > + * @param clk Pointer to synce_clock instance > + */ > +void synce_clock_destroy(struct synce_clock *clk); > + > +/** > + * Poll for synce events and dispatch them. > + * > + * @param clk A pointer to a synce_clock instance obtained with > + * synce_clock_create(). > + * @return Zero on success, non-zero otherwise > + */ > +int synce_clock_poll(struct synce_clock *clk); > + > +#endif |
|
From: Kubalewski, A. <ark...@in...> - 2022-06-09 19:11:34
|
> -----Original Message----- > From: Aya Levin <ay...@nv...> > Sent: Wednesday, June 8, 2022 11:10 AM > > On 5/2/2022 12:05 PM, Arkadiusz Kubalewski wrote: > > synce_clock interface allows creation, polling and destruction of > > SyncE clock, designed to step and drive SyncE logic on all configured > > SyncE devices. > > > > SyncE clock is created with a SyncE-type configuration, where > > SyncE-type configuration is prepared by parsing SyncE config file. > > > > Poll of a SyncE clock, will step each configured SyncE device. > > > > Destruction of SyncE clock releases all resources obtained by SyncE > > clock and devices. > > > > Co-developed-by: Piotr Kwapulinski <pio...@in...> > > Signed-off-by: Piotr Kwapulinski <pio...@in...> > > Co-developed-by: Michal Michalik <mic...@in...> > > Signed-off-by: Michal Michalik <mic...@in...> > > Signed-off-by: Arkadiusz Kubalewski <ark...@in...> > > --- > > synce_clock.c | 296 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > synce_clock.h | 52 +++++++++ > > 2 files changed, 348 insertions(+) > > create mode 100644 synce_clock.c > > create mode 100644 synce_clock.h > > > > diff --git a/synce_clock.c b/synce_clock.c > > new file mode 100644 > > index 000000000000..8007fa390bbd > > --- /dev/null > > +++ b/synce_clock.c > > @@ -0,0 +1,296 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/** > > + * @file synce_clock.c > > + * @brief Implements a SYNCE clock interface. > > + * @note Copyright (C) 2022 Intel Corporation > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms of the GNU General Public License version 2, as published > > + * by the Free Software Foundation. > > + * > > + * 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, see <http://www.gnu.org/licenses/>. > > + */ > > +#include <stdlib.h> > > +#include <errno.h> > > +#include <unistd.h> > > +#include <sys/queue.h> > > + > > +#include "synce_clock.h" > > +#include "interface.h" > > +#include "synce_dev.h" > > +#include "print.h" > > +#include "config.h" > > +#include "missing.h" > > + > > +#define SYNCE_CLOCK_DELAY_USEC 20000 > > +#define SYNCE_CLOCK_INIT_DELAY_USEC 200000 > > +#define SYNCE_CLOCK_INIT_N_TRIES 10 > > + > > +struct interface { > > + STAILQ_ENTRY(interface) list; > > +}; > > + > > +struct synce_dev { > > + LIST_ENTRY(synce_dev) list; > > +}; > > + > > +enum synce_clock_state { > > + SYNCE_CLK_UNKNOWN = 0, > > + SYNCE_CLK_INITED, > > + SYNCE_CLK_DEV_RDY, > > + SYNCE_CLK_DEV_INITED, > > + SYNCE_CLK_RUNNING, > > + SYNCE_CLK_FAILED, > > +}; > > + > > +struct synce_clock { > > + int num_devices; > > + int state; > > + LIST_HEAD(devices_head, synce_dev) devices; > > +}; > > + > > +static struct synce_clock *create_clock(void) > Please rename to create_synce_clock() Sure, makes sense, I will fix that. Thank you, Arkadiusz > > +{ > > + static struct synce_clock clk; > > + > > + if (clk.state != SYNCE_CLK_UNKNOWN) { > > + synce_clock_destroy(&clk); > > + pr_info("old synce_clock destroyed"); > > + } > > + clk.state = SYNCE_CLK_INITED; > > + > > + return &clk; > > +} > > + > > +static void add_device(struct synce_clock *clk, struct synce_dev *dev) > > +{ > > + struct synce_dev *dev_iter, *last_dev = NULL; > > + > > + LIST_FOREACH(dev_iter, &clk->devices, list) { > > + last_dev = dev_iter; > > + } > > + > > + if (last_dev) { > > + LIST_INSERT_AFTER(last_dev, dev, list); > > + } else { > > + LIST_INSERT_HEAD(&clk->devices, dev, list); > > + } > > +} > > + > > +static int create_synce_devices(struct synce_clock *clk, struct config *cfg) > > +{ > > + struct interface *iface; > > + struct synce_dev *dev; > > + const char *dev_name; > > + int count = 0; > > + > > + if (clk->state != SYNCE_CLK_INITED) { > > + goto err; > > + } > > + > > + LIST_INIT(&clk->devices); > > + STAILQ_FOREACH(iface, &cfg->interfaces, list) { > > + /* only parent devices shall be addresed */ > > + if (!interface_se_has_parent_dev(iface)) { > > + dev_name = interface_name(iface); > > + dev = synce_dev_create(dev_name); > > + if (!dev) { > > + pr_err("failed to create device %s", dev_name); > > + continue; > > + } > > + > > + pr_debug("device init %s addr %p", dev_name, dev); > > + add_device(clk, dev); > > + count++; > > + } > > + } > > + > > + if (!count) { > > + pr_err("no devices created"); > > + goto err; > > + } > > + > > + pr_info("created num_devices: %d", count); > > + clk->num_devices = count; > > + clk->state = SYNCE_CLK_DEV_RDY; > > + > > + return 0; > > +err: > > + clk->state = SYNCE_CLK_FAILED; > > + return -EINVAL; > > +} > > + > > +static int init_synce_devices(struct synce_clock *clk, struct config *cfg) > > +{ > > + struct synce_dev *dev, *tmp; > > + int count = 0; > > + > > + if (clk->state != SYNCE_CLK_DEV_RDY) { > > + goto err; > > + } > > + > > + LIST_FOREACH_SAFE(dev, &clk->devices, list, tmp) { > > + /* Each parent device will init its ports */ > > + if (synce_dev_init(dev, cfg)) { > > + pr_err("failed to init device %s", > > + synce_dev_name(dev)); > > + synce_dev_destroy(dev); > > + LIST_REMOVE(dev, list); > > + free(dev); > > + continue; > > + } else { > > + pr_debug("device inited %s", synce_dev_name(dev)); > > + count++; > > + } > > + } > > + > > + if (count == 0) { > > + pr_err("no Sync-E devices initialized"); > > + goto err; > > + } else if (count != clk->num_devices) { > > + pr_warning("initialized only %d from %d Sync-E devices", > > + count, clk->num_devices); > > + clk->num_devices = count; > > + } > > + clk->state = SYNCE_CLK_DEV_INITED; > > + > > + return 0; > > +err: > > + clk->state = SYNCE_CLK_FAILED; > > + return -EINVAL; > > +} > > + > > +static void remove_failed_devices(struct synce_clock *clk) > > +{ > > + struct synce_dev *dev, *tmp; > > + int failed_cnt = 0; > > + > > + LIST_FOREACH_SAFE(dev, &clk->devices, list, tmp) { > > + if (!synce_dev_is_running(dev)) { > > + synce_dev_destroy(dev); > > + LIST_REMOVE(dev, list); > > + free(dev); > > + failed_cnt++; > > + } > > + } > > + clk->num_devices -= failed_cnt; > > + pr_warning("Found dead devices: %d", failed_cnt); > > + pr_info("devices still running: %d", clk->num_devices); > > +} > > + > > +static int verify_clock_state(struct synce_clock *clk) > > +{ > > + int i, running, timeout = SYNCE_CLOCK_INIT_N_TRIES; > > + struct synce_dev *dev; > > + > > + if (clk->state < SYNCE_CLK_DEV_INITED) { > > + return -ENODEV; > > + } > > + > > + /* let threads get running */ > > + for (i = 0; i < timeout; ++i) { > > + running = 0; > > + LIST_FOREACH(dev, &clk->devices, list) { > > + if (synce_dev_is_running(dev)) > > + running++; > > + } > > + > > + if (running == clk->num_devices) { > > + clk->state = SYNCE_CLK_RUNNING; > > + break; > > + } > > + usleep(SYNCE_CLOCK_INIT_DELAY_USEC); > > + } > > + > > + pr_debug("running num_devices %d configured %d", > > + running, clk->num_devices); > > + > > + /* If at least one dev is running we leave clock running > > + * while removing failed devices. > > + * Previous traces shall indicate which ones have failed. > > + */ > > + if (!running) { > > + pr_err("no device is running"); > > + return -ENODEV; > > + } else if (running != clk->num_devices) { > > + remove_failed_devices(clk); > > + } > > + > > + return 0; > > +} > > + > > +struct synce_clock *synce_clock_create(struct config *cfg) > > +{ > > + struct synce_clock *clk; > > + int err; > > + > > + if (!cfg) { > > + pr_err("%s cfg is NULL", __func__); > > + return NULL; > > + } > > + > > + clk = create_clock(); > > + if (!clk) { > > + return NULL; > > + } > > + err = create_synce_devices(clk, cfg); > > + if (err) { > > + goto destroy; > > + } > > + err = init_synce_devices(clk, cfg); > > + if (err) { > > + goto destroy; > > + } > > + err = verify_clock_state(clk); > > + if (err) { > > + goto destroy; > > + } > > + > > + return clk; > > + > > +destroy: > > + synce_clock_destroy(clk); > > + > > + return NULL; > > +} > > + > > +void synce_clock_destroy(struct synce_clock *clk) > > +{ > > + struct synce_dev *dev, *tmp; > > + > > + pr_debug("%s", __func__); > > + > > + LIST_FOREACH_SAFE(dev, &clk->devices, list, tmp) { > > + synce_dev_destroy(dev); > > + LIST_REMOVE(dev, list); > > + free(dev); > > + } > > + clk->num_devices = 0; > > + clk->state = SYNCE_CLK_UNKNOWN; > > + > > + return; > > +} > > + > > +int synce_clock_poll(struct synce_clock *clk) > > +{ > > + struct synce_dev *dev; > > + int ret = -ENODEV; > > + > > + if (clk->state == SYNCE_CLK_RUNNING) { > > + LIST_FOREACH(dev, &clk->devices, list) { > > + ret = synce_dev_step(dev); > > + if (ret) { > > + pr_err("dev_step fail"); > > + } > > + } > > + } > > + usleep(SYNCE_CLOCK_DELAY_USEC); > > + > > + return ret; > > +} > > diff --git a/synce_clock.h b/synce_clock.h > > new file mode 100644 > > index 000000000000..e5559a4cc9cc > > --- /dev/null > > +++ b/synce_clock.h > > @@ -0,0 +1,52 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/** > > + * @file synce_clock.h > > + * @brief Implements a Sync-E clock behavior. > > + * @note Copyright (C) 2022 Intel Corporation > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms of the GNU General Public License version 2, as published > > + * by the Free Software Foundation. > > + * > > + * 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, see <http://www.gnu.org/licenses/>. > > + */ > > +#ifndef HAVE_SYNCE_CLOCK_H > > +#define HAVE_SYNCE_CLOCK_H > > + > > +#include "config.h" > > + > > +/* Opaque type */ > > +struct synce_clock; > > + > > +/** > > + * Create a Sync-E clock instance. There can only be one synce_clock in a > > + * system so subsequent calls will destroy the previous clock instance. > > + * > > + * @param cfg Pointer to the SYNCE-type configuration database > > + * @return Pointer to the single global Sync-E clock instance > > + */ > > +struct synce_clock *synce_clock_create(struct config *cfg); > > + > > +/** > > + * Destroy resources associated with the synce clock. > > + * > > + * @param clk Pointer to synce_clock instance > > + */ > > +void synce_clock_destroy(struct synce_clock *clk); > > + > > +/** > > + * Poll for synce events and dispatch them. > > + * > > + * @param clk A pointer to a synce_clock instance obtained with > > + * synce_clock_create(). > > + * @return Zero on success, non-zero otherwise > > + */ > > +int synce_clock_poll(struct synce_clock *clk); > > + > > +#endif > |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X