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...@intel.com/
Thanks, Simei > -----Original Message----- > From: Thomas Monjalon <tho...@monjalon.net> > Sent: Monday, May 15, 2023 10:18 PM > To: Su, Simei <simei...@intel.com> > Cc: ferruh.yi...@amd.com; andrew.rybche...@oktetlabs.ru; Rybalchenko, > Kirill <kirill.rybalche...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; > dev@dpdk.org; Wu, Wenjun1 <wenjun1...@intel.com>; > david.march...@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 >
