All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Radu Pirea <radu-nicolae.pirea@oss.nxp.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Richard Cochran <richardcochran@gmail.com>
Subject: [PATCH v3 net-next 2/4] net: phy: nxp-c45-tja11xx: express timestamp wraparound interval in terms of TS_SEC_MASK
Date: Mon, 14 Jun 2021 16:44:39 +0300	[thread overview]
Message-ID: <20210614134441.497008-3-olteanv@gmail.com> (raw)
In-Reply-To: <20210614134441.497008-1-olteanv@gmail.com>

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>
---
v2->v3: Patch is new

 drivers/net/phy/nxp-c45-tja11xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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;
 }
-- 
2.25.1


  parent reply	other threads:[~2021-06-14 13:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 13:44 [PATCH v3 net-next 0/4] Fixes and improvements to TJA1103 PHY driver Vladimir Oltean
2021-06-14 13:44 ` [PATCH v3 net-next 1/4] net: phy: nxp-c45-tja11xx: demote the "no PTP support" message to debug Vladimir Oltean
2021-06-14 13:44 ` Vladimir Oltean [this message]
2021-06-14 14:14   ` [PATCH v3 net-next 2/4] net: phy: nxp-c45-tja11xx: express timestamp wraparound interval in terms of TS_SEC_MASK Russell King (Oracle)
2021-06-14 13:44 ` [PATCH v3 net-next 3/4] net: phy: nxp-c45-tja11xx: fix potential RX timestamp wraparound Vladimir Oltean
2021-06-14 13:44 ` [PATCH v3 net-next 4/4] net: phy: nxp-c45-tja11xx: enable MDIO write access to the master/slave registers Vladimir Oltean
2021-06-14 20:20 ` [PATCH v3 net-next 0/4] Fixes and improvements to TJA1103 PHY driver patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210614134441.497008-3-olteanv@gmail.com \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=radu-nicolae.pirea@oss.nxp.com \
    --cc=richardcochran@gmail.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.