>-----Original Message-----
>From: Miroslav Lichvar <mlich...@redhat.com>
>Sent: Tuesday, May 3, 2022 11:15 AM
>To: Kubalewski, Arkadiusz <arkadiusz.kubalew...@intel.com>
>Cc: linuxptp-devel@lists.sourceforge.net; Kwapulinski, Piotr
><piotr.kwapulin...@intel.com>; Sawula, Andrzej <andrzej.saw...@intel.com>;
>Gerasymenko, Anatolii <anatolii.gerasyme...@intel.com>
>Subject: Re: [Linuxptp-devel] [PATCH 01/11] synce4l: add config knobs for SyncE
>
>Thanks for your work on this feature. I don't have hardware to test
>it. Just some comments on the code.
Thank you very much for your comments!
>
>On Mon, May 02, 2022 at 11:05:55AM +0200, Arkadiusz Kubalewski wrote:
>> Allow config interface to parse SyncE related config files.
>>
>
>> + if (type == PTP) {
>> + ci_tab = &config_tab_ptp[0];
>> + n_items = N_CONFIG_ITEMS_PTP;
>> + } else {
>> + ci_tab = &config_tab_synce[0];
>> + n_items = N_CONFIG_ITEMS_SYNCE;
>> + }
>> + n_items = (type == PTP ? N_CONFIG_ITEMS_PTP : N_CONFIG_ITEMS_SYNCE);
>
>This looks duplicated. The last line shouldn't be there?
>
Definitely, will fix in next patch.
>
>> +# Shell command to be executed in order to obtain current DPLL status of a
>> +# device.
>> +#
>> +dpll_get_state_cmd cat /sys/class/net/enp1s0f0/device/cgu_state
>
>If I understand it correctly, this shell command is executed 50 times
>per second. Wouldn't it be sufficient to specify it as a file to be
>read directly instead instead of calling popen()?
>
That is true, but also tricky. As a matter of fact previously we have had
it hardcoded, where only sysfs was possible.
But now, we already know that some users want to control DPLL with i2c
directly, thus we tried to let users configure full command to fulfil those
needs, this approach makes SyncE logic usable by any hardware vendor.
>
>I assume this is a temporary solution until the kernel provides a
>standard ethtool/netlink API for monitoring and configuring SyncE.
>
We have work ongoing on new kernel interface, hopefully it will happen in 5.19.
Although, I am not sure if this is only temporary solution. As far as we know,
even if the interface would get into next-queue kernel, only some Linux
distros will port it to their older kernels.
>From this perspective, wouldn't it be better to leave the user with an ability
to either use in-kernel interface (once available) or product specific
interface (Out-Of-Tree driver / i2c / other external application)?
Please share us any thoughts on that matter.
Thanks you,
Arkadiusz
>--
>Miroslav Lichvar
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
