Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
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
Date: Thu, 21 Jul 2022 14:29:58 -0700	[thread overview]
Message-ID: <20220721213001.2483596-6-jacob.e.keller@intel.com> (raw)
In-Reply-To: <20220721213001.2483596-1-jacob.e.keller@intel.com>

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(-)

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);
 
-- 
2.35.1.456.ga9c7032d4631

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

  parent reply	other threads:[~2022-07-21 21:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-21 21:29 [Intel-wired-lan] [net-next PATCH 0/7] intel: ptp: convert .adjfreq to .adjfine Jacob Keller
2022-07-21 21:29 ` [Intel-wired-lan] [net-next PATCH 1/7] ice: implement adjfine with mul_u64_u64_div_u64 Jacob Keller
2022-07-26 12:48   ` G, GurucharanX
2022-07-21 21:29 ` [Intel-wired-lan] [net-next PATCH 1/1] igb: convert .adjfreq to .adjfine Jacob Keller
2022-07-21 21:29 ` [Intel-wired-lan] [net-next PATCH 2/7] e1000e: remove unnecessary range check in e1000e_phc_adjfreq Jacob Keller
2022-07-25  4:44   ` naamax.meir
2022-07-21 21:29 ` [Intel-wired-lan] [net-next PATCH 3/7] e1000e: convert .adjfreq to .adjfine Jacob Keller
2022-07-25 11:37   ` naamax.meir
2022-07-21 21:29 ` Jacob Keller [this message]
2022-07-26 12:48   ` [Intel-wired-lan] [net-next PATCH 4/7] i40e: use mul_u64_u64_div_u64 for PTP frequency calculation G, GurucharanX
2022-07-21 21:29 ` [Intel-wired-lan] [net-next PATCH 5/7] i40e: convert .adjfreq to .adjfine Jacob Keller
2022-07-27  8:18   ` G, GurucharanX
2022-07-21 21:30 ` [Intel-wired-lan] [net-next PATCH 6/7] ixgbe: " Jacob Keller
2022-07-28  9:51   ` G, GurucharanX
2022-07-21 21:30 ` [Intel-wired-lan] [net-next PATCH 7/7] igb: " Jacob Keller
2022-07-28  9:52   ` G, GurucharanX

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220721213001.2483596-6-jacob.e.keller@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox