Re: [Linuxptp-devel] [PATCH v3 3/3] phc_ctl: add get_pins_cfg command to display pin functions
On Fri, 23 Jun 2023 at 19:03, Jacob Keller wrote:
>
>
> On 6/23/2023 2:08 AM, Erez wrote:
> > On Fri, 23 Jun 2023 at 09:07, Jacob Keller
> 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
> >> ---
> >> 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
Re: [Linuxptp-devel] [PATCH v3 3/3] phc_ctl: add get_pins_cfg command to display pin functions
On 6/23/2023 2:08 AM, Erez wrote:
> On Fri, 23 Jun 2023 at 09:07, Jacob Keller 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
>> ---
>> 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.
>>
>> +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.
>
>> + 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.
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v3 3/3] phc_ctl: add get_pins_cfg command to display pin functions
On Fri, 23 Jun 2023 at 09:07, Jacob Keller 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
> ---
> 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
> + if (err == -ENOTTY)
> + err = ioctl(CLOCKID_TO_FD(clkid), PTP_PIN_GETFUNC, desc);
> + if (err)
> + fprintf(stderr, "failed to set pin configuration, error:
> %m\n");
> + return err;
> +}
> +
> int phc_pin_setfunc(clockid_t clkid, struct ptp_pin_desc *desc)
> {
> int err = ioctl(CLOCKID_TO_FD(clkid), PTP_PIN_SETFUNC2, desc);
> diff --git a/phc.h b/phc.h
> index 830e002a8690..064d29bf36e0 100644
> --- a/phc.h
> +++ b/phc.h
> @@ -66,6 +66,19 @@ int phc_max_adj(clockid_t clkid);
> */
> int phc_number_pins(clockid_t clkid);
>
> +/**
> + * Reads configuration of a pin of a PTP hardware clock device.
> + *
> + * @param clkid A clock ID obtained using phc_open().
> + *
> + * @param desc Pointer to a pin descriptor with the 'index' field set.
> On
> + * success, the rest of the structure is updated with the
> + * current pin configuration.
> + *
> + * @return Zero on success, non-zero otherwise.
> + */
> +int phc_pin_getfunc(clockid_t clkid, struct ptp_pin_desc *desc);
> +
> /**
> * Configures a pin of a PTP hardware clock device.
> *
> diff --git a/phc_ctl.c b/phc_ctl.c
> index 4344184c364b..e5076f511813 100644
> --- a/phc_ctl.c
> +++ b/phc_ctl.c
> @@ -113,6 +113,7 @@ static void usage(const char *progname)
> " freq [ppb] adjust PHC frequency (default returns
> current offset)\n"
> " cmp compare PHC offset to CLOCK_REALTIME\n"
> " capsdisplay device capabilities (default if
> no command given)\n"
> + " get_pins_cfgdisplay device pins configuration\n"
> " wait pause between commands\n"
> "\n",
> progname);
> @@ -319,6 +320,71 @@ static int do_caps(clockid_t clkid, int cmdc, char
> *cmdv[])
> return 0;
> }
>
> +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.
> + 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.
> + pr_warning("CLOCK_REALTIME is not a PHC device.");
> + return 0;
> + }
> +
> + n_pins = phc_number_pins(clkid);
> + if (n_pins == 0) {
> + pr_notice("device has no configurable pins");
> + return (0);
> + }
> +
> + pr_notice("device has %d
[Linuxptp-devel] [PATCH v3 3/3] phc_ctl: add get_pins_cfg command to display pin functions
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
---
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);
+ if (err == -ENOTTY)
+ err = ioctl(CLOCKID_TO_FD(clkid), PTP_PIN_GETFUNC, desc);
+ if (err)
+ fprintf(stderr, "failed to set pin configuration, error: %m\n");
+ return err;
+}
+
int phc_pin_setfunc(clockid_t clkid, struct ptp_pin_desc *desc)
{
int err = ioctl(CLOCKID_TO_FD(clkid), PTP_PIN_SETFUNC2, desc);
diff --git a/phc.h b/phc.h
index 830e002a8690..064d29bf36e0 100644
--- a/phc.h
+++ b/phc.h
@@ -66,6 +66,19 @@ int phc_max_adj(clockid_t clkid);
*/
int phc_number_pins(clockid_t clkid);
+/**
+ * Reads configuration of a pin of a PTP hardware clock device.
+ *
+ * @param clkid A clock ID obtained using phc_open().
+ *
+ * @param desc Pointer to a pin descriptor with the 'index' field set. On
+ * success, the rest of the structure is updated with the
+ * current pin configuration.
+ *
+ * @return Zero on success, non-zero otherwise.
+ */
+int phc_pin_getfunc(clockid_t clkid, struct ptp_pin_desc *desc);
+
/**
* Configures a pin of a PTP hardware clock device.
*
diff --git a/phc_ctl.c b/phc_ctl.c
index 4344184c364b..e5076f511813 100644
--- a/phc_ctl.c
+++ b/phc_ctl.c
@@ -113,6 +113,7 @@ static void usage(const char *progname)
" freq [ppb] adjust PHC frequency (default returns
current offset)\n"
" cmp compare PHC offset to CLOCK_REALTIME\n"
" capsdisplay device capabilities (default if no
command given)\n"
+ " get_pins_cfgdisplay device pins configuration\n"
" wait pause between commands\n"
"\n",
progname);
@@ -319,6 +320,71 @@ static int do_caps(clockid_t clkid, int cmdc, char *cmdv[])
return 0;
}
+static const char *pin_func_string(unsigned int func)
+{
+ switch (func) {
+ case PTP_PF_NONE:
+ return "no function";
+ 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) {
+ pr_warning("CLOCK_REALTIME is not a PHC device.");
+ return 0;
+ }
+
+ n_pins = phc_number_pins(clkid);
+ if (n_pins == 0) {
+ pr_notice("device has no configurable pins");
+ return (0);
+ }
+
+ pr_notice("device has %d configurable input/output pins",
+ n_pins);
+
+ /* For each pin, read its configuration */
+ for (index = 0; index < n_pins; index++) {
+ memset(_desc, 0, sizeof(pin_desc));
+ pin_desc.index = index;
+
+ if (phc_pin_getfunc(clkid, _desc)) {
+ pr_warning("get pin configuration for pin %d failed:
%s",
+ index,
+ strerror(errno));
+ /* keep going */
+ continue;
+ }
+
+ if (pin_desc.func == PTP_PF_NONE) {
+ pr_notice("pin %d [%s] not configured",
+ pin_desc.index,
+ pin_desc.name);
+ } else {
+ pr_notice("pin %d [%s] %s on channel %d",
+ pin_desc.index,
+
