Eliminate the ad-hoc use of global variables in the ts2phc program by
introducing one data structure that incorporates them. This might make
the code more understandable to people coming from a kernel background,
since it resembles the type of data organization used there. It is also
now closer to the data organization of phc2sys, a similar program in
both purpose and implementation.
Signed-off-by: Vladimir Oltean <olte...@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.kel...@intel.com>
---
Changes in RFC v2:
- Added Jacob's review tag.
ts2phc.c | 64 ++++++++++++-----------
ts2phc.h | 37 ++++++++++++++
ts2phc_slave.c | 136 ++++++++++++++++++++++++++++---------------------
ts2phc_slave.h | 10 ++--
4 files changed, 153 insertions(+), 94 deletions(-)
create mode 100644 ts2phc.h
diff --git a/ts2phc.c b/ts2phc.c
index 23428586ef66..0be7b18bda71 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -11,22 +11,21 @@
#include "config.h"
#include "interface.h"
#include "print.h"
-#include "ts2phc_master.h"
-#include "ts2phc_slave.h"
+#include "ts2phc.h"
#include "version.h"
struct interface {
STAILQ_ENTRY(interface) list;
};
-static void ts2phc_cleanup(struct config *cfg, struct ts2phc_master *master)
+static void ts2phc_cleanup(struct ts2phc_private *priv)
{
- ts2phc_slave_cleanup();
- if (master) {
- ts2phc_master_destroy(master);
+ ts2phc_slave_cleanup(priv);
+ if (priv->master) {
+ ts2phc_master_destroy(priv->master);
}
- if (cfg) {
- config_destroy(cfg);
+ if (priv->cfg) {
+ config_destroy(priv->cfg);
}
}
@@ -56,8 +55,8 @@ static void usage(char *progname)
int main(int argc, char *argv[])
{
int c, err = 0, have_slave = 0, index, print_level;
- struct ts2phc_master *master = NULL;
enum ts2phc_master_type pps_type;
+ struct ts2phc_private priv = {0};
char *config = NULL, *progname;
const char *pps_source = NULL;
struct config *cfg = NULL;
@@ -68,7 +67,7 @@ int main(int argc, char *argv[])
cfg = config_create();
if (!cfg) {
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
@@ -81,14 +80,14 @@ int main(int argc, char *argv[])
switch (c) {
case 0:
if (config_parse_option(cfg, opts[index].name, optarg))
{
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
break;
case 'c':
if (!config_create_interface(optarg, cfg)) {
fprintf(stderr, "failed to add slave\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
have_slave = 1;
@@ -99,7 +98,7 @@ int main(int argc, char *argv[])
case 'l':
if (get_arg_val_i(c, optarg, &print_level,
PRINT_LEVEL_MIN, PRINT_LEVEL_MAX)) {
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
config_set_int(cfg, "logging_level", print_level);
@@ -116,29 +115,29 @@ int main(int argc, char *argv[])
case 's':
if (pps_source) {
fprintf(stderr, "too many PPS sources\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
pps_source = optarg;
break;
case 'v':
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
version_show(stdout);
return 0;
case 'h':
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
case '?':
default:
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
}
}
if (config && (c = config_read(config, cfg))) {
fprintf(stderr, "failed to read config\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
print_set_progname(progname);
@@ -147,18 +146,21 @@ int main(int argc, char *argv[])
print_set_syslog(config_get_int(cfg, NULL, "use_syslog"));
print_set_level(config_get_int(cfg, NULL, "logging_level"));
+ STAILQ_INIT(&priv.slaves);
+ priv.cfg = cfg;
+
STAILQ_FOREACH(iface, &cfg->interfaces, list) {
if (1 == config_get_int(cfg, interface_name(iface),
"ts2phc.master")) {
if (pps_source) {
fprintf(stderr, "too many PPS sources\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
pps_source = interface_name(iface);
} else {
- if (ts2phc_slave_add(cfg, interface_name(iface))) {
+ if (ts2phc_slave_add(&priv, interface_name(iface))) {
fprintf(stderr, "failed to add slave\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
have_slave = 1;
@@ -166,19 +168,19 @@ int main(int argc, char *argv[])
}
if (!have_slave) {
fprintf(stderr, "no slave clocks specified\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
}
if (!pps_source) {
fprintf(stderr, "no PPS source specified\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
}
- if (ts2phc_slave_arm()) {
- fprintf(stderr, "failed to arm slaves\n");
- ts2phc_cleanup(cfg, master);
+ if (ts2phc_slaves_init(&priv)) {
+ fprintf(stderr, "failed to initialize slaves\n");
+ ts2phc_cleanup(&priv);
return -1;
}
@@ -189,21 +191,21 @@ int main(int argc, char *argv[])
} else {
pps_type = TS2PHC_MASTER_PHC;
}
- master = ts2phc_master_create(cfg, pps_source, pps_type);
- if (!master) {
+ priv.master = ts2phc_master_create(cfg, pps_source, pps_type);
+ if (!priv.master) {
fprintf(stderr, "failed to create master\n");
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return -1;
}
while (is_running()) {
- err = ts2phc_slave_poll(master);
+ err = ts2phc_slave_poll(&priv);
if (err) {
pr_err("poll failed");
break;
}
}
- ts2phc_cleanup(cfg, master);
+ ts2phc_cleanup(&priv);
return err;
}
diff --git a/ts2phc.h b/ts2phc.h
new file mode 100644
index 000000000000..a5d50fc7e3be
--- /dev/null
+++ b/ts2phc.h
@@ -0,0 +1,37 @@
+/**
+ * @file ts2phc.h
+ * @brief Structure definitions for ts2phc
+ * @note Copyright 2020 Vladimir Oltean <olte...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef HAVE_TS2PHC_H
+#define HAVE_TS2PHC_H
+
+struct ts2phc_slave_array;
+
+struct ts2phc_private {
+ struct ts2phc_master *master;
+ STAILQ_HEAD(slave_ifaces_head, ts2phc_slave) slaves;
+ unsigned int n_slaves;
+ struct ts2phc_slave_array *polling_array;
+ struct config *cfg;
+};
+
+#include "ts2phc_master.h"
+#include "ts2phc_slave.h"
+
+#endif
diff --git a/ts2phc_slave.c b/ts2phc_slave.c
index 749efe5f81ba..7b4fbfa70440 100644
--- a/ts2phc_slave.c
+++ b/ts2phc_slave.c
@@ -21,8 +21,7 @@
#include "phc.h"
#include "print.h"
#include "servo.h"
-#include "ts2phc_master.h"
-#include "ts2phc_slave.h"
+#include "ts2phc.h"
#include "util.h"
#define NS_PER_SEC 1000000000LL
@@ -47,7 +46,7 @@ struct ts2phc_slave {
struct ts2phc_slave_array {
struct ts2phc_slave **slave;
struct pollfd *pfd;
-} polling_array;
+};
struct ts2phc_source_timestamp {
struct timespec ts;
@@ -65,49 +64,55 @@ static enum extts_result ts2phc_slave_offset(struct
ts2phc_slave *slave,
int64_t *offset,
uint64_t *local_ts);
-static STAILQ_HEAD(slave_ifaces_head, ts2phc_slave) ts2phc_slaves =
- STAILQ_HEAD_INITIALIZER(ts2phc_slaves);
-
-static unsigned int ts2phc_n_slaves;
-
-static int ts2phc_slave_array_create(void)
+static int ts2phc_slave_array_create(struct ts2phc_private *priv)
{
+ struct ts2phc_slave_array *polling_array;
struct ts2phc_slave *slave;
unsigned int i;
- if (polling_array.slave) {
- return 0;
+ polling_array = malloc(sizeof(*polling_array));
+ if (!polling_array) {
+ pr_err("low memory");
+ return -1;
}
- polling_array.slave = malloc(ts2phc_n_slaves *
sizeof(*polling_array.slave));
- if (!polling_array.slave) {
+
+ polling_array->slave = malloc(priv->n_slaves *
+ sizeof(*polling_array->slave));
+ if (!polling_array->slave) {
pr_err("low memory");
return -1;
}
- polling_array.pfd = malloc(ts2phc_n_slaves *
sizeof(*polling_array.pfd));
- if (!polling_array.pfd) {
+ polling_array->pfd = malloc(priv->n_slaves *
+ sizeof(*polling_array->pfd));
+ if (!polling_array->pfd) {
pr_err("low memory");
- free(polling_array.slave);
- polling_array.slave = NULL;
+ free(polling_array->slave);
+ polling_array->slave = NULL;
return -1;
}
i = 0;
- STAILQ_FOREACH(slave, &ts2phc_slaves, list) {
- polling_array.slave[i] = slave;
+ STAILQ_FOREACH(slave, &priv->slaves, list) {
+ polling_array->slave[i] = slave;
i++;
}
- for (i = 0; i < ts2phc_n_slaves; i++) {
- polling_array.pfd[i].events = POLLIN | POLLPRI;
- polling_array.pfd[i].fd = polling_array.slave[i]->fd;
+ for (i = 0; i < priv->n_slaves; i++) {
+ polling_array->pfd[i].events = POLLIN | POLLPRI;
+ polling_array->pfd[i].fd = polling_array->slave[i]->fd;
}
+
+ priv->polling_array = polling_array;
+
return 0;
}
-static void ts2phc_slave_array_destroy(void)
+static void ts2phc_slave_array_destroy(struct ts2phc_private *priv)
{
- free(polling_array.slave);
- free(polling_array.pfd);
- polling_array.slave = NULL;
- polling_array.pfd = NULL;
+ struct ts2phc_slave_array *polling_array = priv->polling_array;
+
+ free(polling_array->slave);
+ free(polling_array->pfd);
+ polling_array->slave = NULL;
+ polling_array->pfd = NULL;
}
static int ts2phc_slave_clear_fifo(struct ts2phc_slave *slave)
@@ -143,9 +148,10 @@ static int ts2phc_slave_clear_fifo(struct ts2phc_slave
*slave)
return 0;
}
-static struct ts2phc_slave *ts2phc_slave_create(struct config *cfg, const char
*device)
+static struct ts2phc_slave *ts2phc_slave_create(struct ts2phc_private *priv,
+ const char *device)
{
- enum servo_type servo = config_get_int(cfg, NULL, "clock_servo");
+ enum servo_type servo = config_get_int(priv->cfg, NULL, "clock_servo");
int err, fadj, junk, max_adj, pulsewidth;
struct ptp_extts_request extts;
struct ts2phc_slave *slave;
@@ -161,13 +167,18 @@ static struct ts2phc_slave *ts2phc_slave_create(struct
config *cfg, const char *
free(slave);
return NULL;
}
- slave->pin_desc.index = config_get_int(cfg, device, "ts2phc.pin_index");
+ slave->pin_desc.index = config_get_int(priv->cfg, device,
+ "ts2phc.pin_index");
slave->pin_desc.func = PTP_PF_EXTTS;
- slave->pin_desc.chan = config_get_int(cfg, device, "ts2phc.channel");
- slave->polarity = config_get_int(cfg, device, "ts2phc.extts_polarity");
- slave->correction = config_get_int(cfg, device,
"ts2phc.extts_correction");
-
- pulsewidth = config_get_int(cfg, device, "ts2phc.pulsewidth");
+ slave->pin_desc.chan = config_get_int(priv->cfg, device,
+ "ts2phc.channel");
+ slave->polarity = config_get_int(priv->cfg, device,
+ "ts2phc.extts_polarity");
+ slave->correction = config_get_int(priv->cfg, device,
+ "ts2phc.extts_correction");
+
+ pulsewidth = config_get_int(priv->cfg, device,
+ "ts2phc.pulsewidth");
pulsewidth /= 2;
slave->ignore_upper = 1000000000 - pulsewidth;
slave->ignore_lower = pulsewidth;
@@ -177,7 +188,7 @@ static struct ts2phc_slave *ts2phc_slave_create(struct
config *cfg, const char *
pr_err("failed to open clock");
goto no_posix_clock;
}
- slave->no_adj = config_get_int(cfg, NULL, "free_running");
+ slave->no_adj = config_get_int(priv->cfg, NULL, "free_running");
slave->fd = CLOCKID_TO_FD(slave->clk);
pr_debug("PHC slave %s has ptp index %d", device, junk);
@@ -190,7 +201,7 @@ static struct ts2phc_slave *ts2phc_slave_create(struct
config *cfg, const char *
max_adj = phc_max_adj(slave->clk);
- slave->servo = servo_create(cfg, servo, -fadj, max_adj, 0);
+ slave->servo = servo_create(priv->cfg, servo, -fadj, max_adj, 0);
if (!slave->servo) {
pr_err("failed to create servo");
goto no_servo;
@@ -346,28 +357,28 @@ static enum extts_result ts2phc_slave_offset(struct
ts2phc_slave *slave,
/* public methods */
-int ts2phc_slave_add(struct config *cfg, const char *name)
+int ts2phc_slave_add(struct ts2phc_private *priv, const char *name)
{
struct ts2phc_slave *slave;
/* Create each interface only once. */
- STAILQ_FOREACH(slave, &ts2phc_slaves, list) {
+ STAILQ_FOREACH(slave, &priv->slaves, list) {
if (0 == strcmp(name, slave->name)) {
return 0;
}
}
- slave = ts2phc_slave_create(cfg, name);
+ slave = ts2phc_slave_create(priv, name);
if (!slave) {
pr_err("failed to create slave");
return -1;
}
- STAILQ_INSERT_TAIL(&ts2phc_slaves, slave, list);
- ts2phc_n_slaves++;
+ STAILQ_INSERT_TAIL(&priv->slaves, slave, list);
+ priv->n_slaves++;
return 0;
}
-int ts2phc_slave_arm(void)
+int ts2phc_slave_arm(struct ts2phc_private *priv)
{
struct ptp_extts_request extts;
struct ts2phc_slave *slave;
@@ -375,7 +386,7 @@ int ts2phc_slave_arm(void)
memset(&extts, 0, sizeof(extts));
- STAILQ_FOREACH(slave, &ts2phc_slaves, list) {
+ STAILQ_FOREACH(slave, &priv->slaves, list) {
extts.index = slave->pin_desc.chan;
extts.flags = slave->polarity | PTP_ENABLE_FEATURE;
err = ioctl(slave->fd, PTP_EXTTS_REQUEST2, &extts);
@@ -387,29 +398,38 @@ int ts2phc_slave_arm(void)
return 0;
}
-void ts2phc_slave_cleanup(void)
+int ts2phc_slaves_init(struct ts2phc_private *priv)
+{
+ int err;
+
+ err = ts2phc_slave_array_create(priv);
+ if (err)
+ return err;
+
+ return ts2phc_slave_arm(priv);
+}
+
+void ts2phc_slave_cleanup(struct ts2phc_private *priv)
{
struct ts2phc_slave *slave;
- ts2phc_slave_array_destroy();
+ ts2phc_slave_array_destroy(priv);
- while ((slave = STAILQ_FIRST(&ts2phc_slaves))) {
- STAILQ_REMOVE_HEAD(&ts2phc_slaves, list);
+ while ((slave = STAILQ_FIRST(&priv->slaves))) {
+ STAILQ_REMOVE_HEAD(&priv->slaves, list);
ts2phc_slave_destroy(slave);
- ts2phc_n_slaves--;
+ priv->n_slaves--;
}
}
-int ts2phc_slave_poll(struct ts2phc_master *master)
+int ts2phc_slave_poll(struct ts2phc_private *priv)
{
+ struct ts2phc_slave_array *polling_array = priv->polling_array;
struct ts2phc_source_timestamp source_ts;
unsigned int i;
int cnt, err;
- if (ts2phc_slave_array_create()) {
- return -1;
- }
- cnt = poll(polling_array.pfd, ts2phc_n_slaves, 2000);
+ cnt = poll(polling_array->pfd, priv->n_slaves, 2000);
if (cnt < 0) {
if (EINTR == errno) {
return 0;
@@ -422,12 +442,12 @@ int ts2phc_slave_poll(struct ts2phc_master *master)
return 0;
}
- err = ts2phc_master_getppstime(master, &source_ts.ts);
+ err = ts2phc_master_getppstime(priv->master, &source_ts.ts);
source_ts.valid = err ? false : true;
- for (i = 0; i < ts2phc_n_slaves; i++) {
- if (polling_array.pfd[i].revents & (POLLIN|POLLPRI)) {
- ts2phc_slave_event(polling_array.slave[i], source_ts);
+ for (i = 0; i < priv->n_slaves; i++) {
+ if (polling_array->pfd[i].revents & (POLLIN|POLLPRI)) {
+ ts2phc_slave_event(polling_array->slave[i], source_ts);
}
}
return 0;
diff --git a/ts2phc_slave.h b/ts2phc_slave.h
index 2de5ab7dd0ff..1bad9d297799 100644
--- a/ts2phc_slave.h
+++ b/ts2phc_slave.h
@@ -7,14 +7,14 @@
#ifndef HAVE_TS2PHC_SLAVE_H
#define HAVE_TS2PHC_SLAVE_H
-#include "ts2phc_master.h"
+#include "ts2phc.h"
-int ts2phc_slave_add(struct config *cfg, const char *name);
+int ts2phc_slave_add(struct ts2phc_private *priv, const char *name);
-int ts2phc_slave_arm(void);
+int ts2phc_slaves_init(struct ts2phc_private *priv);
-void ts2phc_slave_cleanup(void);
+void ts2phc_slave_cleanup(struct ts2phc_private *priv);
-int ts2phc_slave_poll(struct ts2phc_master *master);
+int ts2phc_slave_poll(struct ts2phc_private *priv);
#endif
--
2.25.1
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
