[Linuxptp-devel] [PATCH 1/2] phc_ctl: explicitly check for adjust_phase definition
The PTP_CLOCK_GETCAPS ioctl call in phc_ctl has a conditional check for
whether to report the caps.adjust_phase bit. This is always set for both
PTP_CLOCK_GETCAPS and PTP_CLOCK_GETCAPS2, and was always zero before the
bit was moved from being reserved to indicate the adjust_phase.
In some sense if we lack PTP_CLOCK_GETCAPS2 we know the kernel won't report
adjust_phase, but in reality such kernel versions did not have
adjust_phase at all.
There's no reason to make this check against PTP_CLOCK_GETCAPS2. Instead,
scan the ptp_clock.h header file and check if we have adjust_phase support.
Use this new flag instead of PTP_CLOCK_GETCAPS2.
Signed-off-by: Jacob Keller
---
Checking against PTP_CLOCK_GETCAPS2 is wrong because this is not when the
adjust_phase was actually introduced. A direct check against whats in the
header is more accurate.
incdefs.sh | 5 +
phc_ctl.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/incdefs.sh b/incdefs.sh
index 21333e5109e9..a9e94f777f6b 100755
--- a/incdefs.sh
+++ b/incdefs.sh
@@ -63,6 +63,7 @@ kernel_flags()
{
prefix=""
tstamp=/usr/include/linux/net_tstamp.h
+ ptp_clock=/usr/include/linux/ptp_clock.h
if [ "x$KBUILD_OUTPUT" != "x" ]; then
# With KBUILD_OUTPUT set, we are building against
@@ -90,6 +91,10 @@ kernel_flags()
if grep -q SOF_TIMESTAMPING_BIND_PHC ${prefix}${tstamp}; then
printf " -DHAVE_VCLOCKS"
fi
+
+ if grep -q adjust_phase ${prefix}${ptp_clock}; then
+ printf " -DHAVE_PTP_CAPS_ADJUST_PHASE"
+ fi
}
flags="$(user_flags)$(kernel_flags)"
diff --git a/phc_ctl.c b/phc_ctl.c
index 92e597c40a23..6a5c2f43d7c9 100644
--- a/phc_ctl.c
+++ b/phc_ctl.c
@@ -311,7 +311,7 @@ static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
caps.n_pins,
caps.pps ? "has" : "doesn't have",
caps.cross_timestamping ? "has" : "doesn't have",
- #ifdef PTP_CLOCK_GETCAPS2
+ #ifdef HAVE_PTP_CAPS_ADJUST_PHASE
caps.adjust_phase ? "has" : "doesn't have"
#else
"no information regarding"
--
2.38.1.420.g319605f8f00e
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH 2/2] phc_ctl: use PTP_CLOCK_GETCAPS2 ioctl if available
The PTP_CLOCK_GETCAPS2 ioctl is provided by kernel as a replacement for the
original PTP_CLOCK_GETCAPS ioctl. This was done in order to provide ioctls
which guarantee reserved fields are properly initialized. In practice the
PTP_CLOCK_GETCAPS2 and PTP_CLOCK_GETCAPS both behave identically since
PTP_CLOCK_GETCAPS always zeroed the caps structure. However, it is good
practice to use the newer version consistently.
Signed-off-by: Jacob Keller
---
Technically this is identical in current kernels, both ioctls return exactly
the same conent.. I still think its better to use the '2' variants in
principle that developer doesn't have to remember "is this variant ok to get
all the new features going forward?"
missing.h | 7 +++
phc_ctl.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/missing.h b/missing.h
index 79a87d425993..41427d3a38b2 100644
--- a/missing.h
+++ b/missing.h
@@ -98,6 +98,13 @@ enum {
#define PTP_PEROUT_REQUEST2 PTP_PEROUT_REQUEST
#endif
+#ifdef PTP_CLOCK_GETCAPS2
+#define PTP_CLOCK_GETCAPS_REQUEST_FAILED "PTP_CLOCK_GETCAPS2 failed: %m"
+#else
+#define PTP_CLOCK_GETCAPS_REQUEST_FAILED "PTP_CLOCK_GETCAPS failed: %m"
+#define PTP_CLOCK_GETCAPS2 PTP_CLOCK_GETCAPS
+#endif
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(5,8,0)
/* from upcoming Linux kernel version 5.8 */
diff --git a/phc_ctl.c b/phc_ctl.c
index 6a5c2f43d7c9..17a615f8aae6 100644
--- a/phc_ctl.c
+++ b/phc_ctl.c
@@ -288,7 +288,7 @@ static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
return 0;
}
- if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS, )) {
+ if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS2, )) {
pr_err("get capabilities failed: %s",
strerror(errno));
return -1;
--
2.38.1.420.g319605f8f00e
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH] servo: stop rounding initial frequency to nearest ppb
The interface to the Linux kernel for adjusting clock frequencies is
specified in scaled parts per million, where the frequency field is parts
per million with a 16 bit binary fractional field. (Equivalently it is
parts per ~65 billion).
The frequency adjustment is stored internally as a double, even though when
displaying we always display only a whole number adjustment. Thus ptp4l and
phc2sys already use the full range of frequency adjustments that can be
specified using a double type.
However, when initializing the servo we read the current clock frequency
and cast it to an integer, discarding the fractional part below a part per
billion.
Refactor the servo_create API to take a double value instead of an integer
value. Fix the clockadj_get_freq and clockadj_set_freq initialization to
avoid casting to an integer and thus rounding the value.
Signed-off-by: Jacob Keller
---
Noticed this while investigating whether we supported the full range of the
frequency adjustment.
clock.c | 10 ++
linreg.c | 2 +-
linreg.h | 2 +-
pi.c | 2 +-
pi.h | 2 +-
servo.c | 2 +-
servo.h | 2 +-
ts2phc.c | 5 +++--
8 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/clock.c b/clock.c
index d37bb87b6c03..7decd836feda 100644
--- a/clock.c
+++ b/clock.c
@@ -897,10 +897,11 @@ struct clock *clock_create(enum clock_type type, struct
config *config,
enum servo_type servo = config_get_int(config, NULL, "clock_servo");
char ts_label[IF_NAMESIZE], phc[32], *tmp;
enum timestamp_type timestamping;
- int fadj = 0, max_adj = 0, sw_ts;
int phc_index, conf_phc_index, required_modes = 0;
struct clock *c = _clock;
+ int max_adj = 0, sw_ts;
const char *uds_ifname;
+ double fadj = 0.0;
struct port *p;
unsigned char oui[OUI_LEN];
struct interface *iface;
@@ -1143,7 +1144,7 @@ struct clock *clock_create(enum clock_type type, struct
config *config,
c->time_flags = c->utc_timescale ? 0 : PTP_TIMESCALE;
if (c->clkid != CLOCK_INVALID) {
- fadj = (int) clockadj_get_freq(c->clkid);
+ fadj = clockadj_get_freq(c->clkid);
/* Due to a bug in older kernels, the reading may silently fail
and return 0. Set the frequency back to make sure fadj is
the actual frequency of the clock. */
@@ -1738,9 +1739,10 @@ struct tsproc *clock_get_tsproc(struct clock *c)
int clock_switch_phc(struct clock *c, int phc_index)
{
struct servo *servo;
- int fadj, max_adj;
clockid_t clkid;
char phc[32];
+ double fadj;
+ int max_adj;
snprintf(phc, sizeof(phc), "/dev/ptp%d", phc_index);
clkid = phc_open(phc);
@@ -1754,7 +1756,7 @@ int clock_switch_phc(struct clock *c, int phc_index)
phc_close(clkid);
return -1;
}
- fadj = (int) clockadj_get_freq(clkid);
+ fadj = clockadj_get_freq(clkid);
clockadj_set_freq(clkid, fadj);
servo = servo_create(c->config, c->servo_type, -fadj, max_adj, 0);
if (!servo) {
diff --git a/linreg.c b/linreg.c
index 8f354f4b0d0b..363636e16937 100644
--- a/linreg.c
+++ b/linreg.c
@@ -330,7 +330,7 @@ static void linreg_leap(struct servo *servo, int leap)
s->leap = leap;
}
-struct servo *linreg_servo_create(int fadj)
+struct servo *linreg_servo_create(double fadj)
{
struct linreg_servo *s;
diff --git a/linreg.h b/linreg.h
index 5c86ea76ebf2..3c48ce5d465a 100644
--- a/linreg.h
+++ b/linreg.h
@@ -21,6 +21,6 @@
#include "servo.h"
-struct servo *linreg_servo_create(int fadj);
+struct servo *linreg_servo_create(double fadj);
#endif
diff --git a/pi.c b/pi.c
index bfe50223d96a..4a96a5b1913b 100644
--- a/pi.c
+++ b/pi.c
@@ -177,7 +177,7 @@ static void pi_reset(struct servo *servo)
s->count = 0;
}
-struct servo *pi_servo_create(struct config *cfg, int fadj, int sw_ts)
+struct servo *pi_servo_create(struct config *cfg, double fadj, int sw_ts)
{
struct pi_servo *s;
diff --git a/pi.h b/pi.h
index feb3ebeb25b9..1e7eb4f3fd51 100644
--- a/pi.h
+++ b/pi.h
@@ -21,6 +21,6 @@
#include "servo.h"
-struct servo *pi_servo_create(struct config *cfg, int fadj, int sw_ts);
+struct servo *pi_servo_create(struct config *cfg, double fadj, int sw_ts);
#endif
diff --git a/servo.c b/servo.c
index 46042aa176d9..6ba7cb42c71e 100644
--- a/servo.c
+++ b/servo.c
@@ -31,7 +31,7 @@
#define NSEC_PER_SEC 10
struct servo *servo_create(struct config *cfg, enum servo_type type,
- int fadj, int max_ppb, int sw_ts)
+ double fadj, int max_ppb, int sw_ts)
{
double servo_first_step_threshold;
double servo_step_threshold;
diff --git a/servo.h b/servo.h
index 6c30d3341fa6..6cedb66ada03 100644
--- a/servo.h
+++ b/servo.h
@@ -77,7 +77,7 @@ enum servo_state {
* @return A pointer to a new servo on success, NULL
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> -Original Message- > From: Hal Murray > Sent: Tuesday, November 15, 2022 2:09 AM > To: Keller, Jacob E > Cc: Maciek Machnikowski ; Hal Murray > ; linuxptp-devel@lists.sourceforge.net > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo. > > >> What about rcl_sock or refclock_sock? It's used in the file linked by > Miroslav. > > Both of those sound good to me. Slight preference to refclock_sock if its > > not too > long. > > How about SOCK? > > In the ntp context, we already have SHM and PPS. Both show up in the refid > slot in packets. > > Just to make sure we are on the same wavelength... I'm looking for a term > that can be used as a handle when the context is well known for things like > "Try SOCK, it worked for me." > Yea. I just found "sock" on its own to be generic and not tell me anything about what the servo did. I don't want to get too distracted by the name though... Thanks, Jake > > -- > These are my opinions. I hate spam. > > ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
On Tue, Nov 15, 2022 at 07:29:23AM +, Keller, Jacob E wrote: > Both of those sound good to me. Slight preference to refclock_sock if its not > too long. +1 refclock_sock Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
>> What about rcl_sock or refclock_sock? It's used in the file linked by Miroslav. > Both of those sound good to me. Slight preference to refclock_sock if its not > too long. How about SOCK? In the ntp context, we already have SHM and PPS. Both show up in the refid slot in packets. Just to make sure we are on the same wavelength... I'm looking for a term that can be used as a handle when the context is well known for things like "Try SOCK, it worked for me." -- These are my opinions. I hate spam. ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
