linuxptp-devel

[Linuxptp-devel] [PATCH RESEND v1 0/5] General improvements for linuxptp focused around phase adjustment

[Rahul R. <rra...@nv...>]

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.

Link: https://sourceforge.net/p/linuxptp/mailman/message/37860292/
Link: https://sourceforge.net/p/linuxptp/mailman/message/37854603/
Link: https://lore.kernel.org/netdev/202...@nv.../

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      | 77 ++++++++++++++++++++++++++++++++++++++------------
 port.c         | 14 ++++-----
 port_private.h |  3 +-
 servo.c        |  3 +-
 tc.c           |  6 ++--
 util.h         |  2 ++
 8 files changed, 82 insertions(+), 36 deletions(-)

-- 
2.40.1

[Linuxptp-devel] [PATCH RESEND v1 0/5] General improvements for linuxptp focused around phase adjustment

[Rahul R. <rra...@nv...>]

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.

Link: https://sourceforge.net/p/linuxptp/mailman/message/37860292/
Link: https://sourceforge.net/p/linuxptp/mailman/message/37854603/
Link: https://lore.kernel.org/netdev/202...@nv.../

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      | 77 ++++++++++++++++++++++++++++++++++++++------------
 port.c         | 14 ++++-----
 port_private.h |  3 +-
 servo.c        |  3 +-
 tc.c           |  6 ++--
 util.h         |  2 ++
 8 files changed, 82 insertions(+), 36 deletions(-)

-- 
2.40.1

[Linuxptp-devel] [PATCH RESEND v1 5/5] phc_ctl: Handle errors returned by various clockadj helpers

[Rahul R. <rra...@nv...>]

Do not print success notices if clockadj operation fails using return
values provided by the clockadj helpers.

Signed-off-by: Rahul Rameshbabu <rra...@nv...>
---
 phc_ctl.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/phc_ctl.c b/phc_ctl.c
index 34d9e0c..e174189 100644
--- a/phc_ctl.c
+++ b/phc_ctl.c
@@ -232,9 +232,8 @@ 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))
+		pr_notice("adjusted clock by %lf seconds", time_arg);
 
 	/* adjustment always consumes one argument */
 	return 1;
@@ -271,8 +270,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))
+		pr_notice("adjusted clock frequency offset to %lfppb", ppb);
 
 	/* consumed one argument to determine the frequency adjustment value */
 	return 1;
@@ -308,10 +307,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))
+		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

Re: [Linuxptp-devel] [PATCH RESEND v1 5/5] phc_ctl: Handle errors returned by various clockadj helpers

[Erez <ere...@gm...>]

On Tue, 5 Sept 2023 at 21:55, Rahul Rameshbabu via Linuxptp-devel <
lin...@li...> wrote:

> Do not print success notices if clockadj operation fails using return
> values provided by the clockadj helpers.
>

Good idea.


>
> Signed-off-by: Rahul Rameshbabu <rra...@nv...>
> ---
>  phc_ctl.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/phc_ctl.c b/phc_ctl.c
> index 34d9e0c..e174189 100644
> --- a/phc_ctl.c
> +++ b/phc_ctl.c
> @@ -232,9 +232,8 @@ 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))
>

I think a (clockadj_step(clkid, nsecs) == 0) is better.


> +               pr_notice("adjusted clock by %lf seconds", time_arg);
>
>
You can add else and return -2, clockadj_step() already prints an error
message.



>         /* adjustment always consumes one argument */
>         return 1;
> @@ -271,8 +270,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))
> +               pr_notice("adjusted clock frequency offset to %lfppb",
> ppb);
>

Same here.


>
>         /* consumed one argument to determine the frequency adjustment
> value */
>         return 1;
> @@ -308,10 +307,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))
> +               pr_notice("offset of %lf seconds provided to PHC phase
> control keyword",
> +                         offset_arg);
>

And here

Erez


>
>         /* phase offset always consumes one argument */
>         return 1;
> --
> 2.40.1
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>

[Linuxptp-devel] [PATCH RESEND v1 3/5] phc_ctl: Add maximum offset capability

[Rahul R. <rra...@nv...>]

Advertise the maximum offset that can be fed to the PHC phase control
keyword.

Signed-off-by: Rahul Rameshbabu <rra...@nv...>
---
 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

Re: [Linuxptp-devel] [PATCH RESEND v1 3/5] phc_ctl: Add maximum offset capability

[Erez <ere...@gm...>]

On Tue, 5 Sept 2023 at 21:56, Rahul Rameshbabu via Linuxptp-devel <
lin...@li...> wrote:

> Advertise the maximum offset that can be fed to the PHC phase control
> keyword.
>

Someone already sent this patch, a few months ago.
But now we can find it in the official kernel. :-)
https://elixir.bootlin.com/linux/v6.5/source/include/uapi/linux/ptp_clock.h#L204

Erez



>
> Signed-off-by: Rahul Rameshbabu <rra...@nv...>
> ---
>  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
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>

Re: [Linuxptp-devel] [PATCH RESEND v1 3/5] phc_ctl: Add maximum offset capability

[Rahul R. <rra...@nv...>]

Hi Erez,

On Tue, 05 Sep, 2023 22:15:47 +0200 Erez <ere...@gm...> wrote:
> On Tue, 5 Sept 2023 at 21:56, Rahul Rameshbabu via Linuxptp-devel <lin...@li...> wrote:
>
>  Advertise the maximum offset that can be fed to the PHC phase control
>  keyword.
>
> Someone already sent this patch, a few months ago.
> But now we can find it in the official kernel. :-)
> https://elixir.bootlin.com/linux/v6.5/source/include/uapi/linux/ptp_clock.h#L204

That someone on the mailing list was me. I also authored the kernel
patches, so I expected it to land in kernel 6.5 (which is why I was
confident in the kernel version). That said, it is definitely nice to
see the change in the official tag.

-- Rahul Rameshbabu

>
> Erez 
>
>  
>  
>  Signed-off-by: Rahul Rameshbabu <rra...@nv...>
>  ---
>   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
>  Lin...@li...
>  https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] [PATCH RESEND v1 1/5] Rename NSEC2SEC as NSEC_PER_SEC and refactor to util.h

[Rahul R. <rra...@nv...>]

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 <rra...@nv...>
---

Notes:
    Changes since RFC:
    
      * Refactored macro to live in util.h.
        + Noticed that float usage in phc_ctl.c does not need an explicit cast

 phc_ctl.c      |  8 +++-----
 port.c         | 14 +++++++-------
 port_private.h |  3 +--
 servo.c        |  3 +--
 tc.c           |  6 +++---
 util.h         |  2 ++
 6 files changed, 17 insertions(+), 19 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 1000000000.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], &ppb, -NSEC2SEC, NSEC2SEC);
+	r = get_ranged_double(cmdv[0], &ppb, -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 1000000000LL
+#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 1000000000
-
 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..7d1394c 100644
--- a/tc.c
+++ b/tc.c
@@ -256,9 +256,9 @@ static int tc_current(struct ptp_message *m, struct timespec now)
 {
 	int64_t t1, t2, tmo;
 
-	tmo = 1LL * NSEC2SEC;
-	t1 = m->ts.host.tv_sec * NSEC2SEC + m->ts.host.tv_nsec;
-	t2 = now.tv_sec * NSEC2SEC + now.tv_nsec;
+	tmo = 1LL * NSEC_PER_SEC;
+	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/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 (MAX_PRINT_BYTES * 3 + 1)
 
+#define NSEC_PER_SEC 1000000000LL
+
 /**
  * Table of human readable strings, one for each port state.
  */
-- 
2.40.1

[Linuxptp-devel] [PATCH RESEND v1 2/5] phc_ctl: Add phase command to support ADJ_OFFSET

[Rahul R. <rra...@nv...>]

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 <rra...@nv...>
---
 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"
-		"  adj  <seconds>  adjust PHC time by offset\n"
-		"  freq [ppb]      adjust PHC frequency (default returns current offset)\n"
-		"  cmp             compare PHC offset to CLOCK_REALTIME\n"
-		"  caps            display device capabilities (default if no command given)\n"
-		"  wait <seconds>  pause between commands\n"
+		"  set   [seconds]  set PHC time (defaults to time on CLOCK_REALTIME)\n"
+		"  get              get PHC time\n"
+		"  adj   <seconds>  adjust PHC time by offset\n"
+		"  freq  [ppb]      adjust PHC frequency (default returns current offset)\n"
+		"  phase <seconds>  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"
+		"  wait <seconds>   pause 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], &offset_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", &do_get },
 	{ "adj", &do_adj },
 	{ "freq", &do_freq },
+	{ "phase", &do_phase },
 	{ "cmp", &do_cmp },
 	{ "caps", &do_caps },
 	{ "wait", &do_wait },
-- 
2.40.1

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

[Rahul R. <rra...@nv...>]

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 <rra...@nv...>
---
 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

Re: [Linuxptp-devel] [PATCH RESEND v1 2/5] phc_ctl: Add phase command to support ADJ_OFFSET

[Erez <ere...@gm...>]

On Wed, 6 Sept 2023 at 00:56, Rahul Rameshbabu via Linuxptp-devel <
lin...@li...> wrote:

> 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 <rra...@nv...>
> ---
>  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"
> -               "  adj  <seconds>  adjust PHC time by offset\n"
> -               "  freq [ppb]      adjust PHC frequency (default returns
> current offset)\n"
> -               "  cmp             compare PHC offset to CLOCK_REALTIME\n"
> -               "  caps            display device capabilities (default if
> no command given)\n"
> -               "  wait <seconds>  pause between commands\n"
> +               "  set   [seconds]  set PHC time (defaults to time on
> CLOCK_REALTIME)\n"
> +               "  get              get PHC time\n"
> +               "  adj   <seconds>  adjust PHC time by offset\n"
> +               "  freq  [ppb]      adjust PHC frequency (default returns
> current offset)\n"
> +               "  phase <seconds>  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"
> +               "  wait <seconds>   pause 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], &offset_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);
>

In the other patch, you test for error here and avoid the following
'pr_notice'.
Why not do the same here?

Erez


> +
> +       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", &do_get },
>         { "adj", &do_adj },
>         { "freq", &do_freq },
> +       { "phase", &do_phase },
>         { "cmp", &do_cmp },
>         { "caps", &do_caps },
>         { "wait", &do_wait },
> --
> 2.40.1
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>

Re: [Linuxptp-devel] [PATCH RESEND v1 2/5] phc_ctl: Add phase command to support ADJ_OFFSET

[Rahul R. <rra...@nv...>]

Hi Erez,

On Wed, 06 Sep, 2023 23:05:12 +0200 Erez <ere...@gm...> wrote:
> On Wed, 6 Sept 2023 at 00:56, Rahul Rameshbabu via Linuxptp-devel <lin...@li...> wrote:
>
>  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 <rra...@nv...>
>  ---
>   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"
>  -               "  adj  <seconds>  adjust PHC time by offset\n"
>  -               "  freq [ppb]      adjust PHC frequency (default returns current offset)\n"
>  -               "  cmp             compare PHC offset to CLOCK_REALTIME\n"
>  -               "  caps            display device capabilities (default if no command given)\n"
>  -               "  wait <seconds>  pause between commands\n"
>  +               "  set   [seconds]  set PHC time (defaults to time on CLOCK_REALTIME)\n"
>  +               "  get              get PHC time\n"
>  +               "  adj   <seconds>  adjust PHC time by offset\n"
>  +               "  freq  [ppb]      adjust PHC frequency (default returns current offset)\n"
>  +               "  phase <seconds>  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"
>  +               "  wait <seconds>   pause 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], &offset_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);
>
> In the other patch, you test for error here and avoid the following 'pr_notice'.
> Why not do the same here?

I do this as part of the refactor to phc_ctl in patch 5/5 in this
series. I think this patch as-is makes sense, so phc_ctl stays
consistent no matter what reverts may occur in the history. Feel free to
let me know if you think this should be handled differently.

https://sourceforge.net/p/linuxptp/mailman/message/37892473/

-- Rahul Rameshbabu

>
> Erez
>  
>  +
>  +       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", &do_get },
>          { "adj", &do_adj },
>          { "freq", &do_freq },
>  +       { "phase", &do_phase },
>          { "cmp", &do_cmp },
>          { "caps", &do_caps },
>          { "wait", &do_wait },
>  -- 
>  2.40.1
>
>  _______________________________________________
>  Linuxptp-devel mailing list
>  Lin...@li...
>  https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

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

[Rahul R. <rra...@nv...>]

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 <rra...@nv...>
---
 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] [PATCH RESEND v1 5/5] phc_ctl: Handle errors returned by various clockadj helpers

[Rahul R. <rra...@nv...>]

Do not print success notices if clockadj operation fails using return
values provided by the clockadj helpers.

Signed-off-by: Rahul Rameshbabu <rra...@nv...>
---
 phc_ctl.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/phc_ctl.c b/phc_ctl.c
index 34d9e0c..e174189 100644
--- a/phc_ctl.c
+++ b/phc_ctl.c
@@ -232,9 +232,8 @@ 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))
+		pr_notice("adjusted clock by %lf seconds", time_arg);
 
 	/* adjustment always consumes one argument */
 	return 1;
@@ -271,8 +270,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))
+		pr_notice("adjusted clock frequency offset to %lfppb", ppb);
 
 	/* consumed one argument to determine the frequency adjustment value */
 	return 1;
@@ -308,10 +307,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))
+		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] [PATCH RESEND v1 3/5] phc_ctl: Add maximum offset capability

[Rahul R. <rra...@nv...>]

Advertise the maximum offset that can be fed to the PHC phase control
keyword.

Signed-off-by: Rahul Rameshbabu <rra...@nv...>
---
 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] [PATCH RESEND v1 2/5] phc_ctl: Add phase command to support ADJ_OFFSET

[Rahul R. <rra...@nv...>]

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 <rra...@nv...>
---
 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"
-		"  adj  <seconds>  adjust PHC time by offset\n"
-		"  freq [ppb]      adjust PHC frequency (default returns current offset)\n"
-		"  cmp             compare PHC offset to CLOCK_REALTIME\n"
-		"  caps            display device capabilities (default if no command given)\n"
-		"  wait <seconds>  pause between commands\n"
+		"  set   [seconds]  set PHC time (defaults to time on CLOCK_REALTIME)\n"
+		"  get              get PHC time\n"
+		"  adj   <seconds>  adjust PHC time by offset\n"
+		"  freq  [ppb]      adjust PHC frequency (default returns current offset)\n"
+		"  phase <seconds>  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"
+		"  wait <seconds>   pause 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], &offset_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", &do_get },
 	{ "adj", &do_adj },
 	{ "freq", &do_freq },
+	{ "phase", &do_phase },
 	{ "cmp", &do_cmp },
 	{ "caps", &do_caps },
 	{ "wait", &do_wait },
-- 
2.40.1

[Linuxptp-devel] [PATCH RESEND v1 1/5] Rename NSEC2SEC as NSEC_PER_SEC and refactor to util.h

[Rahul R. <rra...@nv...>]

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 <rra...@nv...>
---

Notes:
    Changes since RFC:
    
      * Refactored macro to live in util.h.
        + Noticed that float usage in phc_ctl.c does not need an explicit cast

 phc_ctl.c      |  8 +++-----
 port.c         | 14 +++++++-------
 port_private.h |  3 +--
 servo.c        |  3 +--
 tc.c           |  6 +++---
 util.h         |  2 ++
 6 files changed, 17 insertions(+), 19 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 1000000000.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], &ppb, -NSEC2SEC, NSEC2SEC);
+	r = get_ranged_double(cmdv[0], &ppb, -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 1000000000LL
+#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 1000000000
-
 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..7d1394c 100644
--- a/tc.c
+++ b/tc.c
@@ -256,9 +256,9 @@ static int tc_current(struct ptp_message *m, struct timespec now)
 {
 	int64_t t1, t2, tmo;
 
-	tmo = 1LL * NSEC2SEC;
-	t1 = m->ts.host.tv_sec * NSEC2SEC + m->ts.host.tv_nsec;
-	t2 = now.tv_sec * NSEC2SEC + now.tv_nsec;
+	tmo = 1LL * NSEC_PER_SEC;
+	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/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 (MAX_PRINT_BYTES * 3 + 1)
 
+#define NSEC_PER_SEC 1000000000LL
+
 /**
  * Table of human readable strings, one for each port state.
  */
-- 
2.40.1