Subject: Re: [PATCH v3 net-next 2/4] net: phy: nxp-c45-tja11xx: express timestamp wraparound interval in terms of TS_SEC_MASK

From: "Russell King (Oracle)" <[email-masked]>
Date: Mon, 14 Jun 2021 15:14:05 +0100

On Mon, Jun 14, 2021 at 04:44:39PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <[email-masked]>
> 
> nxp_c45_reconstruct_ts() takes a partial hardware timestamp in @hwts,
> with 2 bits of the 'seconds' portion, and a full PTP time in @ts.
> 
> It patches in the lower bits of @hwts into @ts, and to ensure that the
> reconstructed timestamp is correct, it checks whether the lower 2 bits
> of @hwts are not in fact higher than the lower 2 bits of @ts. This is
> not logically possible because, according to the calling convention, @ts
> was collected later in time than @hwts, but due to two's complement
> arithmetic it can actually happen, because the current PTP time might
> have wrapped around between when @hwts was collected and when @ts was,
> yielding the lower 2 bits of @ts smaller than those of @hwts.
> 
> To correct for that situation which is expected to happen under normal
> conditions, the driver subtracts exactly one wraparound interval from
> the reconstructed timestamp, since the upper bits of that need to
> correspond to what the upper bits of @hwts were, not to what the upper
> bits of @ts were.
> 
> Readers might be confused because the driver denotes the amount of bits
> that the partial hardware timestamp has to offer as TS_SEC_MASK
> (timestamp mask for seconds). But it subtracts a seemingly unrelated
> BIT(2), which is in fact more subtle: if the hardware timestamp provides
> 2 bits of partial 'seconds' timestamp, then the wraparound interval is
> 2^2 == BIT(2).
> 
> But nonetheless, it is better to express the wraparound interval in
> terms of a definition we already have, so replace BIT(2) with
> 1 + GENMASK(1, 0) which produces the same result but is clearer.
> 
> Suggested-by: Russell King (Oracle) <[email-masked]>
> Cc: Richard Cochran <[email-masked]>
> Signed-off-by: Vladimir Oltean <[email-masked]>

Thanks!

Reviewed-by: Russell King (Oracle) <[email-masked]>

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!