Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
On Mon, Sep 17, 2018 at 03:43:01PM +, FUSTE Emmanuel wrote:
> diff --git a/phc.h b/phc.h
> index 154e35e..2fe33c4 100644
> --- a/phc.h
> +++ b/phc.h
The diff does not apply (using git am) to the current master. Can you
please clean it up and re-submit?
Thanks,
Richard
> @@ -21,6 +21,19 @@
>
> #include "missing.h"
>
> +/* from linux kernel v4.15 */
> +struct compat_caps {
> +int max_adj; /* Maximum frequency adjustment in parts per billon.
> */
> +int n_alarm; /* Number of programmable alarms. */
> +int n_ext_ts; /* Number of external time stamp channels. */
> +int n_per_out; /* Number of programmable periodic signals. */
> +int pps; /* Whether the clock supports a PPS callback. */
> +int n_pins;/* Number of input/output pins. */
> +/* Whether the clock supports precise system-device cross timestamps
> */
> +int cross_timestamping;
> +int rsv[13]; /* Reserved for future use. */
> +};
> +
> /**
>* Opens a PTP hardware clock device.
>*
> diff --git a/phc_ctl.c b/phc_ctl.c
> index 4a78a19..d33ff1f 100644
> --- a/phc_ctl.c
> +++ b/phc_ctl.c
> @@ -334,14 +334,17 @@ static int do_freq(clockid_t clkid, int cmdc, char
> *cmdv[])
>
> static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
> {
> - struct ptp_clock_caps caps;
> + union {
> + struct ptp_clock_caps caps;
> + struct compat_caps compat;
> + } u;
>
> if (clkid == CLOCK_REALTIME) {
> pr_warning("CLOCK_REALTIME is not a PHC device.");
> return 0;
> }
>
> - if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS, )) {
> + if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS, )) {
> pr_err("get capabilities failed: %s",
> strerror(errno));
> return -1;
> @@ -353,12 +356,14 @@ static int do_caps(clockid_t clkid, int cmdc, char
> *cmdv[])
> " %d programable alarms\n"
> " %d external time stamp channels\n"
> " %d programmable periodic signals\n"
> - " %s pulse per second support",
> - caps.max_adj,
> - caps.n_alarm,
> - caps.n_ext_ts,
> - caps.n_per_out,
> - caps.pps ? "has" : "doesn't have");
> + " %s pulse per second support\n"
> + " %s precise system-device cross timestamps support",
> + u.caps.max_adj,
> + u.caps.n_alarm,
> + u.caps.n_ext_ts,
> + u.caps.n_per_out,
> + u.caps.pps ? "has" : "doesn't have",
> + u.compat.cross_timestamping ? "has" : "doesn't have");
> return 0;
> }
>
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
Le 17/09/2018 à 17:12, Richard Cochran a écrit :
> On Mon, Sep 17, 2018 at 08:14:04AM +, FUSTE Emmanuel wrote:
>> Running the command compiled without PTP_SYS_OFFSET_PRECISE support on a
>> kernel (and perhaps the hardware) with PTP_SYS_OFFSET_PRECISE will
>> return a definitively wrong answer.
> Yeah, this isn't ideal.
>
> Saying "unknown" is also a bit weird. People will wonder, why can't
> you tell?
>
> How about this:
>
> /* from linux kernel v4.19-rc4 */
> struct compat_caps {
> int max_adj; /* Maximum frequency adjustment in parts per billon. */
> int n_alarm; /* Number of programmable alarms. */
> int n_ext_ts; /* Number of external time stamp channels. */
> int n_per_out; /* Number of programmable periodic signals. */
> int pps; /* Whether the clock supports a PPS callback. */
> int n_pins;/* Number of input/output pins. */
> /* Whether the clock supports precise system-device cross timestamps */
> int cross_timestamping;
> int rsv[13]; /* Reserved for future use. */
> };
>
> and then
>
> static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
> {
> union {
> struct ptp_clock_caps caps;
> struct compat_caps compat;
> } u;
> ...
> // check u.compat.cross_timestamping
> }
>
Yes, clever and more future proof. Damm, it was a long time since I did
real C... |-)
diff --git a/phc.h b/phc.h
index 154e35e..2fe33c4 100644
--- a/phc.h
+++ b/phc.h
@@ -21,6 +21,19 @@
#include "missing.h"
+/* from linux kernel v4.15 */
+struct compat_caps {
+int max_adj; /* Maximum frequency adjustment in parts per billon. */
+int n_alarm; /* Number of programmable alarms. */
+int n_ext_ts; /* Number of external time stamp channels. */
+int n_per_out; /* Number of programmable periodic signals. */
+int pps; /* Whether the clock supports a PPS callback. */
+int n_pins;/* Number of input/output pins. */
+/* Whether the clock supports precise system-device cross timestamps */
+int cross_timestamping;
+int rsv[13]; /* Reserved for future use. */
+};
+
/**
* Opens a PTP hardware clock device.
*
diff --git a/phc_ctl.c b/phc_ctl.c
index 4a78a19..d33ff1f 100644
--- a/phc_ctl.c
+++ b/phc_ctl.c
@@ -334,14 +334,17 @@ static int do_freq(clockid_t clkid, int cmdc, char
*cmdv[])
static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
{
- struct ptp_clock_caps caps;
+ union {
+ struct ptp_clock_caps caps;
+ struct compat_caps compat;
+ } u;
if (clkid == CLOCK_REALTIME) {
pr_warning("CLOCK_REALTIME is not a PHC device.");
return 0;
}
- if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS, )) {
+ if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS, )) {
pr_err("get capabilities failed: %s",
strerror(errno));
return -1;
@@ -353,12 +356,14 @@ static int do_caps(clockid_t clkid, int cmdc, char
*cmdv[])
" %d programable alarms\n"
" %d external time stamp channels\n"
" %d programmable periodic signals\n"
- " %s pulse per second support",
- caps.max_adj,
- caps.n_alarm,
- caps.n_ext_ts,
- caps.n_per_out,
- caps.pps ? "has" : "doesn't have");
+ " %s pulse per second support\n"
+ " %s precise system-device cross timestamps support",
+ u.caps.max_adj,
+ u.caps.n_alarm,
+ u.caps.n_ext_ts,
+ u.caps.n_per_out,
+ u.caps.pps ? "has" : "doesn't have",
+ u.compat.cross_timestamping ? "has" : "doesn't have");
return 0;
}
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
On Mon, Sep 17, 2018 at 08:14:04AM +, FUSTE Emmanuel wrote:
> Running the command compiled without PTP_SYS_OFFSET_PRECISE support on a
> kernel (and perhaps the hardware) with PTP_SYS_OFFSET_PRECISE will
> return a definitively wrong answer.
Yeah, this isn't ideal.
Saying "unknown" is also a bit weird. People will wonder, why can't
you tell?
How about this:
/* from linux kernel v4.19-rc4 */
struct compat_caps {
int max_adj; /* Maximum frequency adjustment in parts per billon. */
int n_alarm; /* Number of programmable alarms. */
int n_ext_ts; /* Number of external time stamp channels. */
int n_per_out; /* Number of programmable periodic signals. */
int pps; /* Whether the clock supports a PPS callback. */
int n_pins;/* Number of input/output pins. */
/* Whether the clock supports precise system-device cross timestamps */
int cross_timestamping;
int rsv[13]; /* Reserved for future use. */
};
and then
static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
{
union {
struct ptp_clock_caps caps;
struct compat_caps compat;
} u;
...
// check u.compat.cross_timestamping
}
Hm?
Thanks,
Richard
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
Le 14/09/2018 à 18:50, Keller, Jacob E a écrit : > >> -Original Message- >> From: FUSTE Emmanuel [mailto:emmanuel.fu...@thalesgroup.com] >> Sent: Friday, September 14, 2018 8:57 AM >> To: Richard Cochran >> Cc: linuxptp-devel@lists.sourceforge.net >> Subject: Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE >> support to phc_ctl >> >> Le 14/09/2018 à 06:37, Richard Cochran a écrit : >>> On Thu, Aug 16, 2018 at 03:22:51PM +, FUSTE Emmanuel wrote: >>>> diff --git a/phc_ctl.c b/phc_ctl.c >>>> index 4a78a19..f6234da 100644 >>>> --- a/phc_ctl.c >>>> +++ b/phc_ctl.c >>>> @@ -353,12 +353,14 @@ static int do_caps(clockid_t clkid, int cmdc, char >> *cmdv[]) >>>>" %d programable alarms\n" >>>>" %d external time stamp channels\n" >>>>" %d programmable periodic signals\n" >>>> - " %s pulse per second support", >>>> + " %s pulse per second support\n" >>>> + " %s precise system-device cross timestamps support", >>>>caps.max_adj, >>>>caps.n_alarm, >>>>caps.n_ext_ts, >>>>caps.n_per_out, >>>> - caps.pps ? "has" : "doesn't have"); >>>> + caps.pps ? "has" : "doesn't have", >>>> + caps.cross_timestamping ? "has" : "doesn't have"); >>>>return 0; >>>>} >>> This will need a compile time check for PTP_SYS_OFFSET_PRECISE, otherwise >> you get >>> /home/richard/git/linuxptp/phc_ctl.c: In function 'do_caps': >>> /home/richard/git/linuxptp/phc_ctl.c:350:2: error: 'struct ptp_clock_caps' >>> has >> no member named 'cross_timestamping' >>> make: *** [phc_ctl.o] Error 1 >>> make: *** Waiting for unfinished jobs >>> richard@hoboy:~/git/linuxptp$ uname -a >>> Linux hoboy 4.9.0-8-amd64 #1 SMP Debian 4.9.110-3+deb9u4 (2018-08-21) >> x86_64 GNU/Linux >>> Thanks, >>> Richard >>> >> Something like that ? >> >> diff --git a/phc_ctl.c b/phc_ctl.c >> index 4a78a19..fdaeb08 100644 >> --- a/phc_ctl.c >> +++ b/phc_ctl.c >> @@ -353,12 +353,18 @@ static int do_caps(clockid_t clkid, int cmdc, char >> *cmdv[]) >> " %d programable alarms\n" >> " %d external time stamp channels\n" >> " %d programmable periodic signals\n" >> - " %s pulse per second support", >> + " %s pulse per second support\n" >> + " %s precise system-device cross timestamps support", >> caps.max_adj, >> caps.n_alarm, >> caps.n_ext_ts, >> caps.n_per_out, >> - caps.pps ? "has" : "doesn't have"); >> + caps.pps ? "has" : "doesn't have", >> +#ifdef PTP_SYS_OFFSET_PRECISE >> + caps.cross_timestamping ? "has" : "doesn't have"); >> +#else >> + "unknown"); >> +#endif > If PTP_SYS_OFFSET_PRECISE is undefined, I think we can just list it as > "doesn't have", no? Running the command compiled without PTP_SYS_OFFSET_PRECISE support on a kernel (and perhaps the hardware) with PTP_SYS_OFFSET_PRECISE will return a definitively wrong answer. But I don't care much, I will rely on Richard's judgment to choose. Regards, Emmanuel. ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
> -Original Message- > From: FUSTE Emmanuel [mailto:emmanuel.fu...@thalesgroup.com] > Sent: Friday, September 14, 2018 8:57 AM > To: Richard Cochran > Cc: linuxptp-devel@lists.sourceforge.net > Subject: Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE > support to phc_ctl > > Le 14/09/2018 à 06:37, Richard Cochran a écrit : > > On Thu, Aug 16, 2018 at 03:22:51PM +, FUSTE Emmanuel wrote: > >> diff --git a/phc_ctl.c b/phc_ctl.c > >> index 4a78a19..f6234da 100644 > >> --- a/phc_ctl.c > >> +++ b/phc_ctl.c > >> @@ -353,12 +353,14 @@ static int do_caps(clockid_t clkid, int cmdc, char > *cmdv[]) > >>" %d programable alarms\n" > >>" %d external time stamp channels\n" > >>" %d programmable periodic signals\n" > >> - " %s pulse per second support", > >> + " %s pulse per second support\n" > >> + " %s precise system-device cross timestamps support", > >>caps.max_adj, > >>caps.n_alarm, > >>caps.n_ext_ts, > >>caps.n_per_out, > >> - caps.pps ? "has" : "doesn't have"); > >> + caps.pps ? "has" : "doesn't have", > >> + caps.cross_timestamping ? "has" : "doesn't have"); > >>return 0; > >> } > > This will need a compile time check for PTP_SYS_OFFSET_PRECISE, otherwise > you get > > > > /home/richard/git/linuxptp/phc_ctl.c: In function 'do_caps': > > /home/richard/git/linuxptp/phc_ctl.c:350:2: error: 'struct ptp_clock_caps' > > has > no member named 'cross_timestamping' > > make: *** [phc_ctl.o] Error 1 > > make: *** Waiting for unfinished jobs > > richard@hoboy:~/git/linuxptp$ uname -a > > Linux hoboy 4.9.0-8-amd64 #1 SMP Debian 4.9.110-3+deb9u4 (2018-08-21) > x86_64 GNU/Linux > > > > Thanks, > > Richard > > > Something like that ? > > diff --git a/phc_ctl.c b/phc_ctl.c > index 4a78a19..fdaeb08 100644 > --- a/phc_ctl.c > +++ b/phc_ctl.c > @@ -353,12 +353,18 @@ static int do_caps(clockid_t clkid, int cmdc, char > *cmdv[]) > " %d programable alarms\n" > " %d external time stamp channels\n" > " %d programmable periodic signals\n" > - " %s pulse per second support", > + " %s pulse per second support\n" > + " %s precise system-device cross timestamps support", > caps.max_adj, > caps.n_alarm, > caps.n_ext_ts, > caps.n_per_out, > - caps.pps ? "has" : "doesn't have"); > + caps.pps ? "has" : "doesn't have", > +#ifdef PTP_SYS_OFFSET_PRECISE > + caps.cross_timestamping ? "has" : "doesn't have"); > +#else > + "unknown"); > +#endif If PTP_SYS_OFFSET_PRECISE is undefined, I think we can just list it as "doesn't have", no? Regards, Jake > return 0; > } > > Emmanuel. > > ___ > 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
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
Le 14/09/2018 à 06:37, Richard Cochran a écrit : > On Thu, Aug 16, 2018 at 03:22:51PM +, FUSTE Emmanuel wrote: >> diff --git a/phc_ctl.c b/phc_ctl.c >> index 4a78a19..f6234da 100644 >> --- a/phc_ctl.c >> +++ b/phc_ctl.c >> @@ -353,12 +353,14 @@ static int do_caps(clockid_t clkid, int cmdc, char >> *cmdv[]) >> " %d programable alarms\n" >> " %d external time stamp channels\n" >> " %d programmable periodic signals\n" >> -" %s pulse per second support", >> +" %s pulse per second support\n" >> +" %s precise system-device cross timestamps support", >> caps.max_adj, >> caps.n_alarm, >> caps.n_ext_ts, >> caps.n_per_out, >> -caps.pps ? "has" : "doesn't have"); >> +caps.pps ? "has" : "doesn't have", >> +caps.cross_timestamping ? "has" : "doesn't have"); >> return 0; >> } > This will need a compile time check for PTP_SYS_OFFSET_PRECISE, otherwise you > get > > /home/richard/git/linuxptp/phc_ctl.c: In function 'do_caps': > /home/richard/git/linuxptp/phc_ctl.c:350:2: error: 'struct ptp_clock_caps' > has no member named 'cross_timestamping' > make: *** [phc_ctl.o] Error 1 > make: *** Waiting for unfinished jobs > richard@hoboy:~/git/linuxptp$ uname -a > Linux hoboy 4.9.0-8-amd64 #1 SMP Debian 4.9.110-3+deb9u4 (2018-08-21) x86_64 > GNU/Linux > > Thanks, > Richard > Something like that ? diff --git a/phc_ctl.c b/phc_ctl.c index 4a78a19..fdaeb08 100644 --- a/phc_ctl.c +++ b/phc_ctl.c @@ -353,12 +353,18 @@ static int do_caps(clockid_t clkid, int cmdc, char *cmdv[]) " %d programable alarms\n" " %d external time stamp channels\n" " %d programmable periodic signals\n" - " %s pulse per second support", + " %s pulse per second support\n" + " %s precise system-device cross timestamps support", caps.max_adj, caps.n_alarm, caps.n_ext_ts, caps.n_per_out, - caps.pps ? "has" : "doesn't have"); + caps.pps ? "has" : "doesn't have", +#ifdef PTP_SYS_OFFSET_PRECISE + caps.cross_timestamping ? "has" : "doesn't have"); +#else + "unknown"); +#endif return 0; } Emmanuel. ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
On Thu, Aug 16, 2018 at 03:22:51PM +, FUSTE Emmanuel wrote: > diff --git a/phc_ctl.c b/phc_ctl.c > index 4a78a19..f6234da 100644 > --- a/phc_ctl.c > +++ b/phc_ctl.c > @@ -353,12 +353,14 @@ static int do_caps(clockid_t clkid, int cmdc, char > *cmdv[]) > " %d programable alarms\n" > " %d external time stamp channels\n" > " %d programmable periodic signals\n" > - " %s pulse per second support", > + " %s pulse per second support\n" > + " %s precise system-device cross timestamps support", > caps.max_adj, > caps.n_alarm, > caps.n_ext_ts, > caps.n_per_out, > - caps.pps ? "has" : "doesn't have"); > + caps.pps ? "has" : "doesn't have", > + caps.cross_timestamping ? "has" : "doesn't have"); > return 0; > } This will need a compile time check for PTP_SYS_OFFSET_PRECISE, otherwise you get /home/richard/git/linuxptp/phc_ctl.c: In function 'do_caps': /home/richard/git/linuxptp/phc_ctl.c:350:2: error: 'struct ptp_clock_caps' has no member named 'cross_timestamping' make: *** [phc_ctl.o] Error 1 make: *** Waiting for unfinished jobs richard@hoboy:~/git/linuxptp$ uname -a Linux hoboy 4.9.0-8-amd64 #1 SMP Debian 4.9.110-3+deb9u4 (2018-08-21) x86_64 GNU/Linux Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
On Mon, Aug 20, 2018 at 09:00:33AM +0200, Miroslav Lichvar wrote: > Do we? In this case the string is printed by pr_notice(), which should > add a newline automatically when printing to terminal. Oh, you're right. Never mind! Thanks, Richard -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
On Sat, Aug 18, 2018 at 10:07:59AM -0400, Richard Cochran wrote: > On Thu, Aug 16, 2018 at 03:22:51PM +, FUSTE Emmanuel wrote: > > diff --git a/phc_ctl.c b/phc_ctl.c > > index 4a78a19..f6234da 100644 > > --- a/phc_ctl.c > > +++ b/phc_ctl.c > > @@ -353,12 +353,14 @@ static int do_caps(clockid_t clkid, int cmdc, char > > *cmdv[]) > > " %d programable alarms\n" > > " %d external time stamp channels\n" > > " %d programmable periodic signals\n" > > - " %s pulse per second support", > > + " %s pulse per second support\n" > > + " %s precise system-device cross timestamps support", > > This patch looks okay, but we want a newline here --^ Do we? In this case the string is printed by pr_notice(), which should add a newline automatically when printing to terminal. -- Miroslav Lichvar -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] add report of PTP_SYS_OFFSET_PRECISE support to phc_ctl
On Thu, Aug 16, 2018 at 03:22:51PM +, FUSTE Emmanuel wrote: > diff --git a/phc_ctl.c b/phc_ctl.c > index 4a78a19..f6234da 100644 > --- a/phc_ctl.c > +++ b/phc_ctl.c > @@ -353,12 +353,14 @@ static int do_caps(clockid_t clkid, int cmdc, char > *cmdv[]) > " %d programable alarms\n" > " %d external time stamp channels\n" > " %d programmable periodic signals\n" > - " %s pulse per second support", > + " %s pulse per second support\n" > + " %s precise system-device cross timestamps support", This patch looks okay, but we want a newline here --^ > caps.max_adj, > caps.n_alarm, > caps.n_ext_ts, > caps.n_per_out, > - caps.pps ? "has" : "doesn't have"); > + caps.pps ? "has" : "doesn't have", > + caps.cross_timestamping ? "has" : "doesn't have"); > return 0; > } Thanks, Richard -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
