On Fri, 23 Jun 2023 at 19:03, Jacob Keller <jacob.e.kel...@intel.com> wrote:
>
>
> On 6/23/2023 2:08 AM, Erez wrote:
> > On Fri, 23 Jun 2023 at 09:07, Jacob Keller <jacob.e.kel...@intel.com>
> wrote:
> >
> >> Add a new function to phc_ctl to display the devices pin configuration
> >> data. First, obtain the device capabilities to determine the number of
> >> pins. Then, for each pin, print the name, function, and channel
> >> information.
> >>
> >> Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
> >> ---
> >> missing.h | 5 +++++
> >> phc.c | 10 +++++++++
> >> phc.h | 13 +++++++++++
> >> phc_ctl.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 95 insertions(+)
> >>
> >> diff --git a/missing.h b/missing.h
> >> index 9b37dc258c0f..4a71307169ad 100644
> >> --- a/missing.h
> >> +++ b/missing.h
> >> @@ -240,10 +240,15 @@ struct ptp_pin_desc {
> >> unsigned int rsv[5];
> >> };
> >>
> >> +#define PTP_PIN_GETFUNC _IOWR(PTP_CLK_MAGIC, 6, struct ptp_pin_desc)
> >> #define PTP_PIN_SETFUNC _IOW(PTP_CLK_MAGIC, 7, struct ptp_pin_desc)
> >>
> >> #endif /*!PTP_PIN_SETFUNC*/
> >>
> >> +#ifndef PTP_PIN_GETFUNC2
> >> +#define PTP_PIN_GETFUNC2 _IOWR(PTP_CLK_MAGIC, 15, struct
> ptp_pin_desc)
> >> +#endif
> >> +
> >> #ifndef PTP_PIN_SETFUNC2
> >> #define PTP_PIN_SETFUNC2 _IOW(PTP_CLK_MAGIC, 16, struct ptp_pin_desc)
> >> #endif
> >> diff --git a/phc.c b/phc.c
> >> index fe8c5eccabda..879a008bd392 100644
> >> --- a/phc.c
> >> +++ b/phc.c
> >> @@ -108,6 +108,16 @@ int phc_number_pins(clockid_t clkid)
> >> return caps.n_pins;
> >> }
> >>
> >> +int phc_pin_getfunc(clockid_t clkid, struct ptp_pin_desc *desc)
> >> +{
> >> + int err = ioctl(CLOCKID_TO_FD(clkid), PTP_PIN_GETFUNC2, desc);
> >>
> >
> > At the moment PTP_PIN_GETFUNC2 do not provide any additional information,
> > We can skip it
> >
> >
>
> Using PTP_PIN_GETFUNC2 enforces that we get zeros for reserved fields
> where as using PTP_PIN_GETFUNC would give us potential garbage data in
> the reserved fields. I think its worth using PTP_PIN_GETFUNC2 now for
> that reason alone, even if we don't rely on this.
>
> We will fall back to using PTP_PIN_GETFUNC on older kernels at a slight
> increase on cost here but that should be negligible.
>
> Yes right now the two behave (mostly) identically with PTP_PIN_GETFUNC2
> enforcing reserved fields are zero, so it would cause an error if we
> didn't already memset the desc structure, where as PTP_PIN_GETFUNC would
> silently zero out those fields for us automatically.
>
> I can drop this part if everyone agrees that its not worth it, but I
> felt that enabling this now would make it easier to use the new versions
> in the future when necessary.
>
As I wrote in the other mail.
The application is NOT a kernel verification tool.
If the reserve fields are not used.
I see no point in using PTP_PIN_GETFUNC2/PTP_PIN_SETFUNC2 at the moment.
>
> >>
> >> +static const char *pin_func_string(unsigned int func)
> >> +{
> >> + switch (func) {
> >> + case PTP_PF_NONE:
> >> + return "no function";
> >>
> >
> > We already filter PTP_PF_NONE in do_get_pins_cfg().
> > As it is a static function, you can skip it here.
> > The default tag will catch missing values for the compiler. So no
> > compilation warnings.
> > Simply leave a note here, that we already filter it.
> >
>
> I guess no one else uses this function so thats reasonable.
>
+1
>
> >
> >> + case PTP_PF_EXTTS:
> >> + return "external timestamping";
> >> + case PTP_PF_PEROUT:
> >> + return "periodic output";
> >> + case PTP_PF_PHYSYNC:
> >> + return "phy sync";
> >> + default:
> >> + return "unknown function";
> >> + }
> >> +}
> >> +
> >> +static int do_get_pins_cfg(clockid_t clkid, int cmdc, char *cmdv[])
> >> +{
> >> + struct ptp_pin_desc pin_desc;
> >> + unsigned int index;
> >> + int n_pins;
> >> +
> >> + if (clkid == CLOCK_REALTIME) {
> >>
> >
> > All PHC are negative values, while the system clock uses positive values
> > (for the different variations).
> > It is better to check "(clkid >= 0)".
> > As the file description to clock ID formula is a public formula, this
> will
> > not change.
> > We might use other system clock variants in the future.
> >
> >
>
> Fair.
>
+1
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
