| Message ID | 20220721213001.2483596-6-jacob.e.keller@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | intel: ptp: convert .adjfreq to .adjfine | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Jacob Keller > Sent: Friday, July 22, 2022 3:00 AM > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org> > Subject: [Intel-wired-lan] [net-next PATCH 4/7] i40e: use > mul_u64_u64_div_u64 for PTP frequency calculation > > The i40e device has a different clock rate depending on the current link > speed. This requires using a different increment rate for the PTP clock > registers. For slower link speeds, the base increment value is larger. > Directly multiplying the larger increment value by the parts per billion > adjustment might overflow. > > To avoid this, the i40e implementation defaults to using the lower increment > value and then multiplying the adjustment afterwards. This causes a loss of > precision for lower link speeds. > > We can fix this by using mul_u64_u64_div_u64 instead of performing the > multiplications using standard C operations. On X86, this will use special > instructions that perform the multiplication and division with 128bit > intermediate values. For other architectures, the fallback implementation will > limit the loss of precision for large values. Small adjustments don't overflow > anyways and won't lose precision at all. > > This allows first multiplying the base increment value and then performing > the adjustment calculation, since we no longer fear overflowing. It also > makes it easier to convert to the even more precise .adjfine implementation > in a following change. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > --- > drivers/net/ethernet/intel/i40e/i40e_ptp.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c index 57a71fa17ed5..15de918abc41 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c @@ -353,25 +353,16 @@ static int i40e_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb) ppb = -ppb; } - freq = I40E_PTP_40GB_INCVAL; - freq *= ppb; - diff = div_u64(freq, 1000000000ULL); + smp_mb(); /* Force any pending update before accessing. */ + freq = I40E_PTP_40GB_INCVAL * READ_ONCE(pf->ptp_adj_mult); + diff = mul_u64_u64_div_u64(freq, (u64)ppb, + 1000000000ULL); if (neg_adj) adj = I40E_PTP_40GB_INCVAL - diff; else adj = I40E_PTP_40GB_INCVAL + diff; - /* At some link speeds, the base incval is so large that directly - * multiplying by ppb would result in arithmetic overflow even when - * using a u64. Avoid this by instead calculating the new incval - * always in terms of the 40GbE clock rate and then multiplying by the - * link speed factor afterwards. This does result in slightly lower - * precision at lower link speeds, but it is fairly minor. - */ - smp_mb(); /* Force any pending update before accessing. */ - adj *= READ_ONCE(pf->ptp_adj_mult); - wr32(hw, I40E_PRTTSYN_INC_L, adj & 0xFFFFFFFF); wr32(hw, I40E_PRTTSYN_INC_H, adj >> 32);
The i40e device has a different clock rate depending on the current link speed. This requires using a different increment rate for the PTP clock registers. For slower link speeds, the base increment value is larger. Directly multiplying the larger increment value by the parts per billion adjustment might overflow. To avoid this, the i40e implementation defaults to using the lower increment value and then multiplying the adjustment afterwards. This causes a loss of precision for lower link speeds. We can fix this by using mul_u64_u64_div_u64 instead of performing the multiplications using standard C operations. On X86, this will use special instructions that perform the multiplication and division with 128bit intermediate values. For other architectures, the fallback implementation will limit the loss of precision for large values. Small adjustments don't overflow anyways and won't lose precision at all. This allows first multiplying the base increment value and then performing the adjustment calculation, since we no longer fear overflowing. It also makes it easier to convert to the even more precise .adjfine implementation in a following change. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- drivers/net/ethernet/intel/i40e/i40e_ptp.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)