Message ID | 20210614134441.497008-3-olteanv@gmail.com |
---|---|
State | New |
Headers | show |
Series | Fixes and improvements to TJA1103 PHY driver | expand |
On Mon, Jun 14, 2021 at 04:44:39PM +0300, Vladimir Oltean wrote: > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > 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) <rmk+kernel@armlinux.org.uk> > Cc: Richard Cochran <richardcochran@gmail.com> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Thanks! Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c index 902fe1aa7782..afdcd6772b1d 100644 --- a/drivers/net/phy/nxp-c45-tja11xx.c +++ b/drivers/net/phy/nxp-c45-tja11xx.c @@ -325,7 +325,7 @@ static void nxp_c45_reconstruct_ts(struct timespec64 *ts, { ts->tv_nsec = hwts->nsec; if ((ts->tv_sec & TS_SEC_MASK) < (hwts->sec & TS_SEC_MASK)) - ts->tv_sec -= BIT(2); + ts->tv_sec -= TS_SEC_MASK + 1; ts->tv_sec &= ~TS_SEC_MASK; ts->tv_sec |= hwts->sec & TS_SEC_MASK; }