+ BL On Mon Nov 15 2021, Thomas Gleixner wrote: > The recent addition of timestamp correction to compensate the CDC error > introduced a subtle signed/unsigned bug in stmmac_get_tx_hwtstamp() while > it managed for some obscure reason to avoid that in stmmac_get_rx_hwtstamp(). > > The issue is: > > s64 adjust = 0; > u64 ns; > > adjust += -(2 * (NSEC_PER_SEC / priv->plat->clk_ptp_rate)); > ns += adjust; > > works by chance on 64bit, but falls apart on 32bit because the compiler > knows that adjust fits into 32bit and then treats the addition as a u64 + > u32 resulting in an off by ~2 seconds failure. > > The RX variant uses an u64 for adjust and does the adjustment via > > ns -= adjust; > > because consistency is obviously overrated. > > Get rid of the pointless zero initialized adjust variable and do: > > ns -= (2 * NSEC_PER_SEC) / priv->plat->clk_ptp_rate; > > which is obviously correct and spares the adjust obfuscation. Aside of that > it yields a more accurate result because the multiplication takes place > before the integer divide truncation and not afterwards. > > Stick the calculation into an inline so it can't be accidentaly > disimproved. Return an u32 from that inline as the result is guaranteed > to fit which lets the compiler optimize the substraction. > > Fixes: 3600be5f58c1 ("net: stmmac: add timestamp correction to rid CDC sync error") > Reported-by: Benedikt Spranger > Signed-off-by: Thomas Gleixner > Tested-by: Benedikt Spranger > Cc: stable@vger.kernel.org Thanks! Tested-by: Kurt Kanzenbach # Intel EHL