RE: [RFC v2 1/3] ethdev: add frequency adjustment API

["Su, Simei" <simei.su@intel.com>]

Hi Thomas,

Thanks a lot for your review. I have already sent RFC v3 patch based on your comments.
https://patchwork.dpdk.org/project/dpdk/cover/20230522132332.102030-1-simei.su@intel.com/

Thanks,
Simei

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, May 15, 2023 10:18 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; Rybalchenko,
> Kirill <kirill.rybalchenko@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> dev@dpdk.org; Wu, Wenjun1 <wenjun1.wu@intel.com>;
> david.marchand@redhat.com
> Subject: Re: [RFC v2 1/3] ethdev: add frequency adjustment API
> 
> Hello,
> 
> 03/04/2023 11:22, Simei Su:
> > This patch introduces a new timesync API "rte_eth_timesync_adjust_freq"
> > which enables frequency adjustment during PTP timesync.
> 
> You should explain how it compares with existing functions like
> rte_eth_timesync_adjust_time().
> 
> [...]
> >  /**
> > + * Adjust the clock increment rate on an Ethernet device.
> > + *
> > + * This is usually used in conjunction with other Ethdev timesync
> > + functions to
> > + * synchronize the device time using the IEEE1588/802.1AS protocol.
> 
> All timesync functions have this sentence, but it does not help to understand
> how to combine them.
> 
> > + *
> > + * @param port_id
> > + *  The port identifier of the Ethernet device.
> > + * @param ppm
> > + *  Parts per million with 16-bit fractional field
> 
> Sorry I don't understand what this parameter is about.
> Probably because I'm not an expert of IEEE1588/802.1AS protocol.
> Please make it possible to understand for basic users.
> 
> > + *
> > + * @return
> > + *   - 0: Success.
> > + *   - -ENODEV: The port ID is invalid.
> > + *   - -EIO: if device is removed.
> > + *   - -ENOTSUP: The function is not supported by the Ethernet driver.
> > + */
> > +int rte_eth_timesync_adjust_freq(uint16_t port_id, int64_t ppm);
> 
> In general, I think there is an effort to provide in order to make the timesync
> API better understandable.
> Example of something to explain: how Rx and Tx timestamps of a port are
> differents? Is it different of rte_eth_timesync_read_time()?
> 
> The function rte_eth_read_clock() was provided with a better explanation.
> 
> Please make improvements in API documentation as part of this API, I don't
> want to get more functions without improving explanation of older functions.
> Thank you
>