[Linuxptp-devel] [PATCH v3 4/5] phc_ctl: Use pr_notice instead of pr_err for displaying adjusted frequency
Adjusted frequency value displayed by do_freq is not an error, but a notication to the user. pr_err should not be used for providing notices with information about successful operations. Fixes: bdb6a35883b0 ("linuxptp: add phc_ctl program to help debug PHC devices") Signed-off-by: Rahul Rameshbabu --- phc_ctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phc_ctl.c b/phc_ctl.c index a814648..34d9e0c 100644 --- a/phc_ctl.c +++ b/phc_ctl.c @@ -249,7 +249,7 @@ static int do_freq(clockid_t clkid, int cmdc, char *cmdv[]) if (cmdc < 1 || name_is_a_command(cmdv[0])) { ppb = clockadj_get_freq(clkid); - pr_err("clock frequency offset is %lfppb", ppb); + pr_notice("clock frequency offset is %lfppb", ppb); /* no argument was used */ return 0; @@ -272,7 +272,7 @@ static int do_freq(clockid_t clkid, int cmdc, char *cmdv[]) } clockadj_set_freq(clkid, ppb); - pr_err("adjusted clock frequency offset to %lfppb", ppb); + pr_notice("adjusted clock frequency offset to %lfppb", ppb); /* consumed one argument to determine the frequency adjustment value */ return 1; -- 2.40.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH v3 0/5] General improvements for linuxptp focused around phase adjustment
The main focus of this submission is adding support for testing ADJ_OFFSET with phc_ctl and querying the maximum supported ADJ_OFFSET adjustment that a device is capable of. Some other minor cleanups are also included in the submission. The patch that introduces support for querying the maximum offset supported by ADJ_OFFSET depends on a kernel patch series (linked below) that is targeted for kernel 6.5. Previously, sent this series out as an RFC to inquire feedback early on. Have incorporated that feedback into this submission. Changes: patch 1: v2->v3: - Eliminate unnecessary tmo variable due to refactor in tc.c. patch 5: v1->v2: - Accounted for conditional formatting changes suggested by Erez. - Added pre-emptive return on error since clockadj_step prints an error message as suggested by Erez. Link: https://sourceforge.net/p/linuxptp/mailman/message/38267194/ Link: https://sourceforge.net/p/linuxptp/mailman/message/37871081/ Link: https://sourceforge.net/p/linuxptp/mailman/message/37860292/ Link: https://sourceforge.net/p/linuxptp/mailman/message/37854603/ Link: https://lore.kernel.org/netdev/20230612211500.309075-1-rrameshb...@nvidia.com/ Rahul Rameshbabu (5): Rename NSEC2SEC as NSEC_PER_SEC and refactor to util.h phc_ctl: Add phase command to support ADJ_OFFSET phc_ctl: Add maximum offset capability phc_ctl: Use pr_notice instead of pr_err for displaying adjusted frequency phc_ctl: Handle errors returned by various clockadj helpers missing.h | 9 +++--- phc_ctl.8 | 4 +++ phc_ctl.c | 79 ++ port.c | 14 - port_private.h | 3 +- servo.c| 3 +- tc.c | 9 +++--- util.h | 2 ++ 8 files changed, 85 insertions(+), 38 deletions(-) -- 2.40.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH v3 2/5] phc_ctl: Add phase command to support ADJ_OFFSET
Take double precision floating point representation of an offset value in seconds. Feed this value to the PHC's phase control keyword. Signed-off-by: Rahul Rameshbabu --- phc_ctl.8 | 4 phc_ctl.c | 55 --- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/phc_ctl.8 b/phc_ctl.8 index 650e661..b10566e 100644 --- a/phc_ctl.8 +++ b/phc_ctl.8 @@ -62,6 +62,10 @@ Adjust the frequency of the PHC clock by the specified parts per billion. If no argument is provided, it will attempt to read the current frequency and report it. .TP +.BI phase " seconds" +Pass an amount in seconds to the PHC clock's phase control keyword. This +argument is required. +.TP .BI cmp Compare the PHC clock device to CLOCK_REALTIME, using the best method available. .TP diff --git a/phc_ctl.c b/phc_ctl.c index db89f01..c5430d8 100644 --- a/phc_ctl.c +++ b/phc_ctl.c @@ -106,13 +106,14 @@ static void usage(const char *progname) " specify commands with arguments. Can specify multiple\n" " commands to be executed in order. Seconds are read as\n" " double precision floating point values.\n" - " set [seconds] set PHC time (defaults to time on CLOCK_REALTIME)\n" - " get get PHC time\n" - " adjadjust PHC time by offset\n" - " freq [ppb] adjust PHC frequency (default returns current offset)\n" - " cmp compare PHC offset to CLOCK_REALTIME\n" - " capsdisplay device capabilities (default if no command given)\n" - " wait pause between commands\n" + " set [seconds] set PHC time (defaults to time on CLOCK_REALTIME)\n" + " get get PHC time\n" + " adj adjust PHC time by offset\n" + " freq [ppb] adjust PHC frequency (default returns current offset)\n" + " phase pass offset to PHC phase control keyword\n" + " cmp compare PHC offset to CLOCK_REALTIME\n" + " caps display device capabilities (default if no command given)\n" + " waitpause between commands\n" "\n", progname); } @@ -277,6 +278,45 @@ static int do_freq(clockid_t clkid, int cmdc, char *cmdv[]) return 1; } +static int do_phase(clockid_t clkid, int cmdc, char *cmdv[]) +{ + double offset_arg; + long nsecs; + enum parser_result r; + + if (cmdc < 1 || name_is_a_command(cmdv[0])) { + pr_err("phase: missing required time argument"); + return -2; + } + + /* parse the double time offset argument */ + r = get_ranged_double(cmdv[0], _arg, -DBL_MAX, DBL_MAX); + switch (r) { + case PARSED_OK: + break; + case MALFORMED: + pr_err("phase: '%s' is not a valid double", cmdv[0]); + return -2; + case OUT_OF_RANGE: + pr_err("phase: '%s' is out of range.", cmdv[0]); + return -2; + default: + pr_err("phase: couldn't process '%s'", cmdv[0]); + return -2; + } + + nsecs = (long)(NSEC_PER_SEC * offset_arg); + + clockadj_init(clkid); + clockadj_set_phase(clkid, nsecs); + + pr_notice("offset of %lf seconds provided to PHC phase control keyword", + offset_arg); + + /* phase offset always consumes one argument */ + return 1; +} + static int do_caps(clockid_t clkid, int cmdc, char *cmdv[]) { struct ptp_clock_caps caps; @@ -399,6 +439,7 @@ static const struct cmd_t all_commands[] = { { "get", _get }, { "adj", _adj }, { "freq", _freq }, + { "phase", _phase }, { "cmp", _cmp }, { "caps", _caps }, { "wait", _wait }, -- 2.40.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH v3 5/5] phc_ctl: Handle errors returned by various clockadj helpers
Do not print success notices if clockadj operation fails using return values provided by the clockadj helpers. Signed-off-by: Rahul Rameshbabu --- Notes: Changes: v1->v2: - Accounted for conditional formatting changes suggested by Erez. - Added pre-emptive return on error since clockadj_step prints an error message as suggested by Erez. phc_ctl.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/phc_ctl.c b/phc_ctl.c index 34d9e0c..47fea8f 100644 --- a/phc_ctl.c +++ b/phc_ctl.c @@ -232,9 +232,10 @@ static int do_adj(clockid_t clkid, int cmdc, char *cmdv[]) nsecs = (int64_t)(NSEC_PER_SEC * time_arg); clockadj_init(clkid); - clockadj_step(clkid, nsecs); - - pr_notice("adjusted clock by %lf seconds", time_arg); + if (clockadj_step(clkid, nsecs) == 0) + pr_notice("adjusted clock by %lf seconds", time_arg); + else + return -2; /* adjustment always consumes one argument */ return 1; @@ -271,8 +272,8 @@ static int do_freq(clockid_t clkid, int cmdc, char *cmdv[]) return -2; } - clockadj_set_freq(clkid, ppb); - pr_notice("adjusted clock frequency offset to %lfppb", ppb); + if (clockadj_set_freq(clkid, ppb) == 0) + pr_notice("adjusted clock frequency offset to %lfppb", ppb); /* consumed one argument to determine the frequency adjustment value */ return 1; @@ -308,10 +309,9 @@ static int do_phase(clockid_t clkid, int cmdc, char *cmdv[]) nsecs = (long)(NSEC_PER_SEC * offset_arg); clockadj_init(clkid); - clockadj_set_phase(clkid, nsecs); - - pr_notice("offset of %lf seconds provided to PHC phase control keyword", - offset_arg); + if (clockadj_set_phase(clkid, nsecs) == 0) + pr_notice("offset of %lf seconds provided to PHC phase control keyword", + offset_arg); /* phase offset always consumes one argument */ return 1; -- 2.40.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH v3 1/5] Rename NSEC2SEC as NSEC_PER_SEC and refactor to util.h
The name NSEC2SEC implies converting nanoseconds to seconds, but the value used for the macro converts seconds to nanoseconds. NSEC_PER_SEC is the accurate name for this macro. Move macro to common location in util.h. Signed-off-by: Rahul Rameshbabu --- Notes: Changes: v2->v3: - Eliminate unnecessary tmo variable due to refactor in tc.c. phc_ctl.c | 8 +++- port.c | 14 +++--- port_private.h | 3 +-- servo.c| 3 +-- tc.c | 9 - util.h | 2 ++ 6 files changed, 18 insertions(+), 21 deletions(-) diff --git a/phc_ctl.c b/phc_ctl.c index 92e597c..db89f01 100644 --- a/phc_ctl.c +++ b/phc_ctl.c @@ -48,8 +48,6 @@ #include "util.h" #include "version.h" -#define NSEC2SEC 10.0 - /* trap the alarm signal so that pause() will wake up on receipt */ static void handle_alarm(int s) { @@ -68,7 +66,7 @@ static void double_to_timespec(double d, struct timespec *ts) * value by our fractional component. This results in a correct * timespec from the double representing seconds. */ - ts->tv_nsec = (long)(NSEC2SEC * fraction); + ts->tv_nsec = (long)(NSEC_PER_SEC * fraction); } static int install_handler(int signum, void(*handler)(int)) @@ -230,7 +228,7 @@ static int do_adj(clockid_t clkid, int cmdc, char *cmdv[]) return -2; } - nsecs = (int64_t)(NSEC2SEC * time_arg); + nsecs = (int64_t)(NSEC_PER_SEC * time_arg); clockadj_init(clkid); clockadj_step(clkid, nsecs); @@ -257,7 +255,7 @@ static int do_freq(clockid_t clkid, int cmdc, char *cmdv[]) } /* parse the double ppb argument */ - r = get_ranged_double(cmdv[0], , -NSEC2SEC, NSEC2SEC); + r = get_ranged_double(cmdv[0], , -NSEC_PER_SEC, NSEC_PER_SEC); switch (r) { case PARSED_OK: break; diff --git a/port.c b/port.c index d551bef..b75f850 100644 --- a/port.c +++ b/port.c @@ -138,17 +138,17 @@ static int msg_current(struct ptp_message *m, struct timespec now) { int64_t t1, t2, tmo; - t1 = m->ts.host.tv_sec * NSEC2SEC + m->ts.host.tv_nsec; - t2 = now.tv_sec * NSEC2SEC + now.tv_nsec; + t1 = m->ts.host.tv_sec * NSEC_PER_SEC + m->ts.host.tv_nsec; + t2 = now.tv_sec * NSEC_PER_SEC + now.tv_nsec; if (m->header.logMessageInterval <= -31) { tmo = 0; } else if (m->header.logMessageInterval >= 31) { tmo = INT64_MAX; } else if (m->header.logMessageInterval < 0) { - tmo = 4LL * NSEC2SEC / (1 << -m->header.logMessageInterval); + tmo = 4LL * NSEC_PER_SEC / (1 << -m->header.logMessageInterval); } else { - tmo = 4LL * (1 << m->header.logMessageInterval) * NSEC2SEC; + tmo = 4LL * (1 << m->header.logMessageInterval) * NSEC_PER_SEC; } return t2 - t1 < tmo; @@ -340,10 +340,10 @@ static void fc_prune(struct foreign_clock *fc) static int delay_req_current(struct ptp_message *m, struct timespec now) { - int64_t t1, t2, tmo = 5 * NSEC2SEC; + int64_t t1, t2, tmo = 5 * NSEC_PER_SEC; - t1 = m->ts.host.tv_sec * NSEC2SEC + m->ts.host.tv_nsec; - t2 = now.tv_sec * NSEC2SEC + now.tv_nsec; + t1 = m->ts.host.tv_sec * NSEC_PER_SEC + m->ts.host.tv_nsec; + t2 = now.tv_sec * NSEC_PER_SEC + now.tv_nsec; return t2 - t1 < tmo; } diff --git a/port_private.h b/port_private.h index 3b02d2f..d922a3d 100644 --- a/port_private.h +++ b/port_private.h @@ -28,8 +28,7 @@ #include "msg.h" #include "power_profile.h" #include "tmv.h" - -#define NSEC2SEC 10LL +#include "util.h" enum syfu_state { SF_EMPTY, diff --git a/servo.c b/servo.c index daaa41c..a68c04a 100644 --- a/servo.c +++ b/servo.c @@ -26,11 +26,10 @@ #include "pi.h" #include "refclock_sock.h" #include "servo_private.h" +#include "util.h" #include "print.h" -#define NSEC_PER_SEC 10 - struct servo *servo_create(struct config *cfg, enum servo_type type, double fadj, int max_ppb, int sw_ts) { diff --git a/tc.c b/tc.c index 1847041..20717c1 100644 --- a/tc.c +++ b/tc.c @@ -254,13 +254,12 @@ static void tc_complete(struct port *q, struct port *p, static int tc_current(struct ptp_message *m, struct timespec now) { - int64_t t1, t2, tmo; + int64_t t1, t2; - tmo = 1LL * NSEC2SEC; - t1 = m->ts.host.tv_sec * NSEC2SEC + m->ts.host.tv_nsec; - t2 = now.tv_sec * NSEC2SEC + now.tv_nsec; + t1 = m->ts.host.tv_sec * NSEC_PER_SEC + m->ts.host.tv_nsec; + t2 = now.tv_sec * NSEC_PER_SEC + now.tv_nsec; - return t2 - t1 < tmo; + return t2 - t1 < NSEC_PER_SEC; } static int tc_fwd_event(struct port *q, struct ptp_message *msg) diff --git a/util.h b/util.h index 2bbde71..1cbd9b6 100644 --- a/util.h +++ b/util.h @@ -33,6 +33,8 @@ #define MAX_PRINT_BYTES 16 #define BIN_BUF_SIZE
[Linuxptp-devel] [PATCH v3 3/5] phc_ctl: Add maximum offset capability
Advertise the maximum offset that can be fed to the PHC phase control keyword. Signed-off-by: Rahul Rameshbabu --- missing.h | 9 + phc_ctl.c | 4 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/missing.h b/missing.h index 79a87d4..165a297 100644 --- a/missing.h +++ b/missing.h @@ -98,9 +98,9 @@ enum { #define PTP_PEROUT_REQUEST2 PTP_PEROUT_REQUEST #endif -#if LINUX_VERSION_CODE < KERNEL_VERSION(5,8,0) +#if LINUX_VERSION_CODE < KERNEL_VERSION(6,5,0) -/* from upcoming Linux kernel version 5.8 */ +/* from upcoming Linux kernel version 6.5 */ struct compat_ptp_clock_caps { int max_adj; /* Maximum frequency adjustment in parts per billon. */ int n_alarm; /* Number of programmable alarms. */ @@ -112,12 +112,13 @@ struct compat_ptp_clock_caps { int cross_timestamping; /* Whether the clock supports adjust phase */ int adjust_phase; - int rsv[12]; /* Reserved for future use. */ + int max_phase_adj; + int rsv[11]; /* Reserved for future use. */ }; #define ptp_clock_caps compat_ptp_clock_caps -#endif /*LINUX_VERSION_CODE < 5.8*/ +#endif /*LINUX_VERSION_CODE < 6.5*/ /* * Bits of the ptp_perout_request.flags field: diff --git a/phc_ctl.c b/phc_ctl.c index c5430d8..a814648 100644 --- a/phc_ctl.c +++ b/phc_ctl.c @@ -355,6 +355,10 @@ static int do_caps(clockid_t clkid, int cmdc, char *cmdv[]) "no information regarding" #endif ); + + if (caps.max_phase_adj) + pr_notice(" %d maximum offset adjustment (ns)\n", caps.max_phase_adj); + return 0; } -- 2.40.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH] clock: Downgrade log message about failed uds forward.
If multiple management clients are used in the network and ptp4l responded at least once over UDS, it will try to forward all management responses received from network to the last UDS client. ptp4l doesn't track the messages and doesn't know if they are responses to the UDS client or other clients in the network. If the UDS client is no longer running (receiving messages on its address), ptp4l logs "uds port: management forward failed" error message. With frequent management requests in the network this can lead to flooding of the system log. Downgrade the error message to debug to disable it in the default log level. Signed-off-by: Miroslav Lichvar --- clock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clock.c b/clock.c index 5a64613..74911fd 100644 --- a/clock.c +++ b/clock.c @@ -1585,7 +1585,7 @@ static void clock_forward_mgmt_msg(struct clock *c, struct port *p, struct ptp_m port_log_name(piter)); } if (clock_do_forward_mgmt(c, p, c->uds_rw_port, msg, _ready)) - pr_err("uds port: management forward failed"); + pr_debug("uds port: management forward failed"); if (msg_ready) { msg_post_recv(msg, pdulen); msg->management.boundaryHops++; -- 2.41.0 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel