On Fri, 23 Jun 2023 at 18:55, Jacob Keller <jacob.e.kel...@intel.com> wrote:

>
>
> On 6/23/2023 1:44 AM, Erez wrote:
> > On Fri, 23 Jun 2023 at 09:07, Jacob Keller <jacob.e.kel...@intel.com>
> wrote:
> >
> >> The phc library currently selects whether to use PTP_PIN_SETFUNC2 over
> >> PTP_PIN_SETFUNC based on whether the kernel headers it is compiled
> against
> >> have the PTP_PIN_SETFUNC2 defined.
> >>
> >> This means that the use of PTP_PIN_SETFUNC vs PTP_PIN_SETFUNC2 depends
> on
> >> whether the headers we compiled against have the appropriate definition,
> >> but not whether the running kernel supports the new ioctl.
> >>
> >> Fix this to use dynamic fallback. If PTP_PIN_SETFUNC2 returns -ENOTTY,
> then
> >> try PTP_PIN_SETFUNC. This approach will work on a wider range of kernels
> >> and doesn't need the headers to be up to date.
> >>
> >
> >
> > I look in the kernel
> >
> https://elixir.bootlin.com/linux/v6.4-rc7/source/drivers/ptp/ptp_chardev.c#L399
> > Using PTP_PIN_SETFUNC2 at the moment does not add anything.
> >
> > Perhaps in the future.
> > It is better to checkthe ptp_pin_desc structure
> >
> https://elixir.bootlin.com/linux/v6.4-rc7/source/include/uapi/linux/ptp_clock.h#L174
> > Once we have new properties there.
> > Then we can use PTP_PIN_SETFUNC2.
> >
> > Erez
> >
>
> The reason to use PTP_PIN_SETFUNC2 is that if it *does* exist we *know*
> the kernel sanitized the data structures and returned zeros in the
> reserved fields.
>

And we do not use these reserved fields.
Once the kernel uses these reserved fields, then it will be interesting.


>
> Obviously currently we aren't using any reserved fields so the behavior
> should be identical.
>
> If we use the old command the kernel sanitizes the reserved fields from
> user space as zero and forwards that to the drivers:
>
> See:
>
> >               if ((pd.rsv[0] || pd.rsv[1] || pd.rsv[2]
> >                               || pd.rsv[3] || pd.rsv[4])
> >                       && cmd == PTP_PIN_SETFUNC2) {
> >                       err = -EINVAL;
> >                       break;
> >               } else if (cmd == PTP_PIN_SETFUNC) {
> >                       pd.rsv[0] = 0;
> >                       pd.rsv[1] = 0;
> >                       pd.rsv[2] = 0;
> >                       pd.rsv[3] = 0;
> >                       pd.rsv[4] = 0;
> >               }
>
> and note the checks for PTP_PIN_SETFUNC2 and PTP_PIN_SETFUNC.
>
> This *is* a behavioral difference and I think we should prefer the
> PTP_PIN_SETFUNC2 operation where its available.
>

It is not a different behavior as we zero the structure before we call and
ignore the reserved fields after we get the reply.
It is a waste of time to use PTP_PIN_SETFUNC2 at the moment as long as the
reserved fields are not used.
The application is NOT a kernel verification tool!

Erez


> Unlike PTP_GETCAPS which is EXACTLY the same as PTP_GETCAPS2 I think its
> important to use these variants.
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to