On Wed, 07 Jun, 2023 18:04:08 +0200 Maciek Machnikowski <mac...@machnikowski.net> wrote: > On 6/7/2023 17:03, Erez wrote: >> >> >> On Wed, 7 Jun 2023 at 00:09, Rahul Rameshbabu via Linuxptp-devel >> <linuxptp-devel@lists.sourceforge.net >> <mailto:linuxptp-devel@lists.sourceforge.net>> wrote: >> >> 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. >> >> >> +1 >> >> And follow Linux kernel. >> >> Erez > > +1 as well, but with comments :) > > NSEC_PER_SEC is a uint64 in the kernel, this value is a float/double in > the phc_ctl. > Maybe NSEC_PER_SEC_F would work better? Or just add the cast in one > place that requires float?
I am leaning more towards the cast. > > Also - wouldn't it be better to move it to the util.h and have it in one > place? I agree with this. Will do this in a follow-up submission which I can either send as an independent submission not gated by the kernel patch series mentioned or as a follow-up submission to this patch series. I would prefer the latter just because I expect the kernel patch series to converge soon. -- Rahul Rameshbabu > > Thanks, > Maciek > >> >> >> Signed-off-by: Rahul Rameshbabu <rrameshb...@nvidia.com >> <mailto:rrameshb...@nvidia.com>> >> --- >> phc_ctl.c | 8 ++++---- >> port.c | 14 +++++++------- >> port_private.h | 2 +- >> tc.c | 6 +++--- >> 4 files changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/phc_ctl.c b/phc_ctl.c >> index 92e597c..50948e0 100644 >> --- a/phc_ctl.c >> +++ b/phc_ctl.c >> @@ -48,7 +48,7 @@ >> #include "util.h" >> #include "version.h" >> >> -#define NSEC2SEC 1000000000.0 >> +#define NSEC_PER_SEC 1000000000.0 >> >> /* trap the alarm signal so that pause() will wake up on receipt */ >> static void handle_alarm(int s) >> @@ -68,7 +68,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 +230,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 +257,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..c5f55a4 100644 >> --- a/port_private.h >> +++ b/port_private.h >> @@ -29,7 +29,7 @@ >> #include "power_profile.h" >> #include "tmv.h" >> >> -#define NSEC2SEC 1000000000LL >> +#define NSEC_PER_SEC 1000000000LL >> >> enum syfu_state { >> SF_EMPTY, >> 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; >> } >> -- >> 2.38.5 >> >> >> >> _______________________________________________ >> Linuxptp-devel mailing list >> Linuxptp-devel@lists.sourceforge.net >> <mailto:Linuxptp-devel@lists.sourceforge.net> >> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel >> <https://lists.sourceforge.net/lists/listinfo/linuxptp-devel> >> >> >> >> _______________________________________________ >> Linuxptp-devel mailing list >> Linuxptp-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > > > _______________________________________________ > Linuxptp-devel mailing list > Linuxptp-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
