| Message ID | 20240418052500.50678-9-mateusz.polchlopek@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Rx timestamping for both ice and iavf drivers. | expand |
On Thu, 18 Apr, 2024 01:24:56 -0400 Mateusz Polchlopek <mateusz.polchlopek@intel.com> wrote: > From: Jacob Keller <jacob.e.keller@intel.com> > > 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 <wojciech.drewek@intel.com> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > --- > 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? > +} > + <snip>
On 4/18/2024 9:51 PM, Rahul Rameshbabu wrote: > On Thu, 18 Apr, 2024 01:24:56 -0400 Mateusz Polchlopek <mateusz.polchlopek@intel.com> wrote: >> From: Jacob Keller <jacob.e.keller@intel.com> >> >> 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 <wojciech.drewek@intel.com> >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> >> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >> --- >> 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? > Makes sense, will take a look on that and if applicable then will send in the next version. Thanks >> +} >> + > <snip>
> -----Original Message----- > From: Polchlopek, Mateusz <mateusz.polchlopek@intel.com> > Sent: Monday, April 22, 2024 2:23 AM > To: Rahul Rameshbabu <rrameshbabu@nvidia.com> > Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; horms@kernel.org; > Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E > <jacob.e.keller@intel.com>; Drewek, Wojciech <wojciech.drewek@intel.com> > 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 > <mateusz.polchlopek@intel.com> wrote: > >> From: Jacob Keller <jacob.e.keller@intel.com> > >> > >> 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 <wojciech.drewek@intel.com> > >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > >> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > >> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > >> --- > >> 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
On 4/25/24 00:03, Keller, Jacob E wrote: > > >> -----Original Message----- >> From: Polchlopek, Mateusz <mateusz.polchlopek@intel.com> >> Sent: Monday, April 22, 2024 2:23 AM >> To: Rahul Rameshbabu <rrameshbabu@nvidia.com> >> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; horms@kernel.org; >> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E >> <jacob.e.keller@intel.com>; Drewek, Wojciech <wojciech.drewek@intel.com> >> 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 >> <mateusz.polchlopek@intel.com> wrote: >>>> From: Jacob Keller <jacob.e.keller@intel.com> >>>> >>>> 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 <wojciech.drewek@intel.com> >>>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> >>>> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >>>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >>>> --- >>>> 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. HZ/2 or HZ/3 as a timer period could be understood without thinking, but the same stands for 400ms. Problems starts when one thinks about it ;) For me HZ, which could be both literally and colloquially understood as "per second" should not mean 1000ms (just evaluate to). 2Hz is a frequency with half second period, but 2*HZ evaluates to 2000ms which is 4 times more :/ > > Thanks, > Jake >
> -----Original Message----- > From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com> > Sent: Thursday, April 25, 2024 3:52 AM > To: Keller, Jacob E <jacob.e.keller@intel.com>; Polchlopek, Mateusz > <mateusz.polchlopek@intel.com>; Rahul Rameshbabu > <rrameshbabu@nvidia.com> > Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; horms@kernel.org; > Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Drewek, Wojciech > <wojciech.drewek@intel.com> > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 08/12] iavf: periodically cache > PHC time > > On 4/25/24 00:03, Keller, Jacob E wrote: > > > > > >> -----Original Message----- > >> From: Polchlopek, Mateusz <mateusz.polchlopek@intel.com> > >> Sent: Monday, April 22, 2024 2:23 AM > >> To: Rahul Rameshbabu <rrameshbabu@nvidia.com> > >> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; > horms@kernel.org; > >> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E > >> <jacob.e.keller@intel.com>; Drewek, Wojciech <wojciech.drewek@intel.com> > >> 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 > >> <mateusz.polchlopek@intel.com> wrote: > >>>> From: Jacob Keller <jacob.e.keller@intel.com> > >>>> > >>>> 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 <wojciech.drewek@intel.com> > >>>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > >>>> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > >>>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > >>>> --- > >>>> 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. > > HZ/2 or HZ/3 as a timer period could be understood without thinking, but > the same stands for 400ms. Problems starts when one thinks about it ;) > > For me HZ, which could be both literally and colloquially understood as > "per second" should not mean 1000ms (just evaluate to). > 2Hz is a frequency with half second period, but 2*HZ evaluates to 2000ms > which is 4 times more :/ > That’s part of why I switched ice over from using HZ generally to using jiffies_to_msec in a lot of cases. It really depends on what you personally find intuitive. Those used to seeing and reading HZ may find it easier. Thanks, Jake > > > > > Thanks, > > Jake > >
On Thu, 25 Apr, 2024 16:28:22 +0000 "Keller, Jacob E" <jacob.e.keller@intel.com> wrote: >> -----Original Message----- >> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com> >> Sent: Thursday, April 25, 2024 3:52 AM >> To: Keller, Jacob E <jacob.e.keller@intel.com>; Polchlopek, Mateusz >> <mateusz.polchlopek@intel.com>; Rahul Rameshbabu >> <rrameshbabu@nvidia.com> >> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; horms@kernel.org; >> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Drewek, Wojciech >> <wojciech.drewek@intel.com> >> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 08/12] iavf: periodically cache >> PHC time >> >> On 4/25/24 00:03, Keller, Jacob E wrote: >> > >> > >> >> -----Original Message----- >> >> From: Polchlopek, Mateusz <mateusz.polchlopek@intel.com> >> >> Sent: Monday, April 22, 2024 2:23 AM >> >> To: Rahul Rameshbabu <rrameshbabu@nvidia.com> >> >> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; >> horms@kernel.org; >> >> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E >> >> <jacob.e.keller@intel.com>; Drewek, Wojciech <wojciech.drewek@intel.com> >> >> 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 >> >> <mateusz.polchlopek@intel.com> wrote: >> >>>> From: Jacob Keller <jacob.e.keller@intel.com> >> >>>> >> >>>> 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 <wojciech.drewek@intel.com> >> >>>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> >> >>>> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >> >>>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >> >>>> --- >> >>>> 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. >> >> HZ/2 or HZ/3 as a timer period could be understood without thinking, but >> the same stands for 400ms. Problems starts when one thinks about it ;) >> >> For me HZ, which could be both literally and colloquially understood as >> "per second" should not mean 1000ms (just evaluate to). >> 2Hz is a frequency with half second period, but 2*HZ evaluates to 2000ms >> which is 4 times more :/ >> > > That’s part of why I switched ice over from using HZ generally to using > jiffies_to_msec in a lot of cases. It really depends on what you personally find > intuitive. Those used to seeing and reading HZ may find it easier. > Makes sense to stick with the same if ice is using jiffies_to_msec in general. I, recently, was re-reading the Linux Device Drivers book, which has a section that elaborates on HZ a bit. -- Thanks, Rahul Rameshbabu
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.c b/drivers/net/ethernet/intel/iavf/iavf_ptp.c index 0d30979616a0..8f6b1a8df5df 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_ptp.c +++ b/drivers/net/ethernet/intel/iavf/iavf_ptp.c @@ -169,6 +169,55 @@ static int iavf_ptp_gettimex64(struct ptp_clock_info *ptp, return iavf_read_phc_indirect(adapter, ts, sts); } +/** + * iavf_ptp_cache_phc_time - Cache PHC time for performing timestamp extension + * @adapter: private adapter structure + * + * Periodically cache the PHC time in order to allow for timestamp extension. + * This is required because the Tx and Rx timestamps only contain 32bits of + * nanoseconds. Timestamp extension allows calculating the corrected 64bit + * timestamp. This algorithm relies on the cached time being within ~1 second + * of the timestamp. + */ +static void iavf_ptp_cache_phc_time(struct iavf_adapter *adapter) +{ + if (time_is_before_jiffies(adapter->ptp.cached_phc_updated + HZ)) { + /* The response from virtchnl will store the time into + * cached_phc_time + */ + iavf_send_phc_read(adapter); + } +} + +/** + * 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); +} + /** * iavf_ptp_register_clock - Register a new PTP for userspace * @adapter: private adapter structure @@ -189,6 +238,7 @@ static int iavf_ptp_register_clock(struct iavf_adapter *adapter) dev_name(dev)); ptp_info->owner = THIS_MODULE; ptp_info->gettimex64 = iavf_ptp_gettimex64; + ptp_info->do_aux_work = iavf_ptp_do_aux_work; adapter->ptp.clock = ptp_clock_register(ptp_info, dev); if (IS_ERR(adapter->ptp.clock)) @@ -228,6 +278,8 @@ void iavf_ptp_init(struct iavf_adapter *adapter) return; } + ptp_schedule_worker(adapter->ptp.clock, 0); + adapter->ptp.initialized = true; } diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.h b/drivers/net/ethernet/intel/iavf/iavf_ptp.h index 4f84416743e1..7a25647980f3 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_ptp.h +++ b/drivers/net/ethernet/intel/iavf/iavf_ptp.h @@ -34,5 +34,6 @@ void iavf_ptp_release(struct iavf_adapter *adapter); void iavf_ptp_process_caps(struct iavf_adapter *adapter); bool iavf_ptp_cap_supported(struct iavf_adapter *adapter, u32 cap); void iavf_virtchnl_send_ptp_cmd(struct iavf_adapter *adapter); +long iavf_ptp_do_aux_work(struct ptp_clock_info *ptp); #endif /* _IAVF_PTP_H_ */