Re: [Linuxptp-devel] [PATCH 1/2] phc_ctl: explicitly check for adjust_phase definition
> -Original Message-
> From: Geva, Erez
> Sent: Thursday, November 17, 2022 9:21 AM
> To: Keller, Jacob E ; linuxptp-
> de...@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH 1/2] phc_ctl: explicitly check for
> adjust_phase definition
>
> On Tue, 2022-11-15 at 16:43 -0800, Jacob Keller wrote:
> > 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.
>
>
> This make sense.
> Thanks for the correction :-)
>
> >
> > 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
>
> Is that catch if using kernel headers?
> The location in the current kernel is
> /include/uapi/linux/ptp_clock.h
>
This is available at both /usr/include/linux/ptp_clock.h and
/include/uapi/linux/ptp_clock.h once you've finished a compilation.
> I see you add in the same way, perhaps we need to update the script?
>
>
The script already checks for existence of kernel location in common places
starting with KBUILD directory if doing a direct/targetted build, then using
/lib/modules/$(uname -r) and finally using the headers installed at /
Thanks,
Jake
> >
> > 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"
>
>
> Erez
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 1/2] phc_ctl: explicitly check for adjust_phase definition
On Tue, 2022-11-15 at 16:43 -0800, Jacob Keller wrote:
> 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.
This make sense.
Thanks for the correction :-)
>
> 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
Is that catch if using kernel headers?
The location in the current kernel is
/include/uapi/linux/ptp_clock.h
I see you add in the same way, perhaps we need to update the script?
>
> 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"
Erez
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[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
