> -----Original Message----- > From: Polchlopek, Mateusz <[email protected]> > Sent: Monday, April 22, 2024 2:23 AM > To: Rahul Rameshbabu <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; > Nguyen, Anthony L <[email protected]>; Keller, Jacob E > <[email protected]>; Drewek, Wojciech <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 08/12] iavf: periodically > cache > PHC time > > > > On 4/18/2024 9:51 PM, Rahul Rameshbabu wrote: > > On Thu, 18 Apr, 2024 01:24:56 -0400 Mateusz Polchlopek > <[email protected]> wrote: > >> From: Jacob Keller <[email protected]> > >> > >> The Rx timestamps reported by hardware may only have 32 bits of storage > >> for nanosecond time. These timestamps cannot be directly reported to the > >> Linux stack, as it expects 64bits of time. > >> > >> To handle this, the timestamps must be extended using an algorithm that > >> calculates the corrected 64bit timestamp by comparison between the PHC > >> time and the timestamp. This algorithm requires the PHC time to be > >> captured within ~2 seconds of when the timestamp was captured. > >> > >> Instead of trying to read the PHC time in the Rx hotpath, the algorithm > >> relies on a cached value that is periodically updated. > >> > >> Keep this cached time up to date by using the PTP .do_aux_work kthread > >> function. > > > > Seems reasonable. > > > >> > >> The iavf_ptp_do_aux_work will reschedule itself about twice a second, > >> and will check whether or not the cached PTP time needs to be updated. > >> If so, it issues a VIRTCHNL_OP_1588_PTP_GET_TIME to request the time > >> from the PF. The jitter and latency involved with this command aren't > >> important, because the cached time just needs to be kept up to date > >> within about ~2 seconds. > >> > >> Reviewed-by: Wojciech Drewek <[email protected]> > >> Signed-off-by: Jacob Keller <[email protected]> > >> Co-developed-by: Mateusz Polchlopek <[email protected]> > >> Signed-off-by: Mateusz Polchlopek <[email protected]> > >> --- > >> drivers/net/ethernet/intel/iavf/iavf_ptp.c | 52 ++++++++++++++++++++++ > >> drivers/net/ethernet/intel/iavf/iavf_ptp.h | 1 + > >> 2 files changed, 53 insertions(+) > >> > >> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.c > b/drivers/net/ethernet/intel/iavf/iavf_ptp.c > > <snip> > >> +/** > >> + * iavf_ptp_do_aux_work - Perform periodic work required for PTP support > >> + * @ptp: PTP clock info structure > >> + * > >> + * Handler to take care of periodic work required for PTP operation. This > >> + * includes the following tasks: > >> + * > >> + * 1) updating cached_phc_time > >> + * > >> + * cached_phc_time is used by the Tx and Rx timestamp flows in order > >> to > >> + * perform timestamp extension, by carefully comparing the timestamp > >> + * 32bit nanosecond timestamps and determining the corrected 64bit > >> + * timestamp value to report to userspace. This algorithm only works > >> if > >> + * the cached_phc_time is within ~1 second of the Tx or Rx timestamp > >> + * event. This task periodically reads the PHC time and stores it, to > >> + * ensure that timestamp extension operates correctly. > >> + * > >> + * Returns: time in jiffies until the periodic task should be > >> re-scheduled. > >> + */ > >> +long iavf_ptp_do_aux_work(struct ptp_clock_info *ptp) > >> +{ > >> + struct iavf_adapter *adapter = clock_to_adapter(ptp); > >> + > >> + iavf_ptp_cache_phc_time(adapter); > >> + > >> + /* Check work about twice a second */ > >> + return msecs_to_jiffies(500); > > > > HZ / 2 might be more intuitive? > >
I've always found it more intuitive to think in terms of msecs myself, but HZ / 2 is ok if other folks agree. Thanks, Jake
