• Thomas Gleixner's avatar
    net: stmmac: Fix signed/unsigned wreckage · 3751c3d3
    Thomas Gleixner authored
    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 accidentally
    disimproved. Return an u32 from that inline as the result is guaranteed
    to fit which lets the compiler optimize the substraction.
    
    Cc: stable@vger.kernel.org
    Fixes: 3600be5f ("net: stmmac: add timestamp correction to rid CDC sync error")
    Reported-by: default avatarBenedikt Spranger <b.spranger@linutronix.de>
    Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Tested-by: default avatarBenedikt Spranger <b.spranger@linutronix.de>
    Tested-by: Kurt Kanzenbach <kurt@linutronix.de> # Intel EHL
    Link: https://lore.kernel.org/r/87mtm578cs.ffs@tglxSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    3751c3d3
stmmac_main.c 196 KB