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.
>>
>> +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
