[Linuxptp-devel] [PATCH v3 4/5] phc_ctl: Use pr_notice instead of pr_err for displaying adjusted frequency

2023-09-25 Thread Rahul Rameshbabu via Linuxptp-devel
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

2023-09-25 Thread Rahul Rameshbabu via Linuxptp-devel
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

2023-09-25 Thread Rahul Rameshbabu via Linuxptp-devel
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

2023-09-25 Thread Rahul Rameshbabu via Linuxptp-devel
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

2023-09-25 Thread Rahul Rameshbabu via Linuxptp-devel
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

2023-09-25 Thread Rahul Rameshbabu via Linuxptp-devel
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.

2023-09-25 Thread Miroslav Lichvar
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