[Intel-wired-lan] [PATCH] squash! ice: enable receive hardware timestamping

Jacob Keller jacob.e.keller at intel.com
Wed Jun 2 17:31:34 UTC 2021


The ice_ptp_extend_32b_ts function is used to calculate the corrected
64bit timestamp from a 32bit timestamp and a cached copy of the PHC
time.

This function uses a local mask constant generated by GENMASK_ULL. This
value is identical to U32_MAX. Some static analyzers (such as Klocwork)
appear to incorrectly expand GENMASK_ULL(31, 0) into the wrong value.

This causes the static analyzer to report that the "delta > (mask / 2)"
calculation is redundant since delta can never be large enough to be
greater than the mask divided by 2.

We can fix this by removing the constant mask value and replacing it
with U32_MAX. Once we've done this, it's clear that cached_phc_time
& U32_MASK is the same as (u32)cached_phc_time, since casting down to
a u32 will just discard the high bits. Make this explicit by using a new
local variable, phc_time_lo.

Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 4608a69acba2..71cd5474ec86 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -329,23 +329,25 @@ static void ice_ptp_update_cached_phctime(struct ice_pf *pf)
  */
 static u64 ice_ptp_extend_32b_ts(u64 cached_phc_time, u32 in_tstamp)
 {
-	const u64 mask = GENMASK_ULL(31, 0);
-	u32 delta;
+	u32 delta, phc_time_lo;
 	u64 ns;
 
+	/* Extract the lower 32 bits of the PHC time */
+	phc_time_lo = (u32)cached_phc_time;
+
 	/* Calculate the delta between the lower 32bits of the cached PHC
 	 * time and the in_tstamp value
 	 */
-	delta = (in_tstamp - (u32)(cached_phc_time & mask));
+	delta = (in_tstamp - phc_time_lo);
 
 	/* Do not assume that the in_tstamp is always more recent than the
 	 * cached PHC time. If the delta is large, it indicates that the
 	 * in_tstamp was taken in the past, and should be converted
 	 * forward.
 	 */
-	if (delta > (mask / 2)) {
+	if (delta > (U32_MAX / 2)) {
 		/* reverse the delta calculation here */
-		delta = ((u32)(cached_phc_time & mask) - in_tstamp);
+		delta = (phc_time_lo - in_tstamp);
 		ns = cached_phc_time - delta;
 	} else {
 		ns = cached_phc_time + delta;
-- 
2.31.1.331.gb0c09ab8796f



More information about the Intel-wired-lan mailing list