On Tue, 2022-11-15 at 16:43 -0800, Jacob Keller wrote:
> 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

Exactly, that is why I send a patch to add the new 'adjust_phase' in
do_caps(), without changing the ioctl itself :-)


> PTP_CLOCK_GETCAPS always zeroed the caps structure. However, it is
> good
> practice to use the newer version consistently.

But, it can also breaks when using the new linuxptp version build on
new  machine, but used with old kernel.
It may be reasonable in few years, when these old kernels becomes
obsolete.
But since the new PTP_CLOCK_GETCAPS2 was introduce in 2019, I think we
should wait with it.
As not everyone build linuxptp, most users use pre-build.


> 
> Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
> ---
> 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?"

There are more 12 bytes in the reserve.
I guess once we pass them, we would need a non-backward flag, but till
then, I think we can keep using the PTP_CLOCK_GETCAPS flag, and leave
the new flags in do_caps().

Erez

> 
>  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, &caps)) {
> +       if (ioctl(CLOCKID_TO_FD(clkid), PTP_CLOCK_GETCAPS2, &caps)) {
>                 pr_err("get capabilities failed: %s",
>                         strerror(errno));
>                 return -1;


_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to