diff mbox series

net: macb: ptp: Switch to gettimex64() interface

Message ID 20210929120739.22168-1-lars@metafoo.de (mailing list archive)
State Accepted
Commit e51bb5c2784c30959535ed20bda9754bbf67416a
Delegated to: Netdev Maintainers
Headers show
Series net: macb: ptp: Switch to gettimex64() interface | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 51 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Lars-Peter Clausen Sept. 29, 2021, 12:07 p.m. UTC
The macb PTP support currently implements the `gettime64` callback to allow
to retrieve the hardware clock time. Update the implementation to provide
the `gettimex64` callback instead.

The difference between the two is that with `gettime64` a snapshot of the
system clock is taken before and after invoking the callback. Whereas
`gettimex64` expects the callback itself to take the snapshots.

To get the time from the macb Ethernet core multiple register accesses have
to be done. Only one of which will happen at the time reported by the
function. This leads to a non-symmetric delay and adds a slight offset
between the hardware and system clock time when using the `gettime64`
method. This offset can be a few 100 nanoseconds. Switching to the
`gettimex64` method allows for a more precise correlation of the hardware
and system clocks and results in a lower offset between the two.

On a Xilinx ZynqMP system `phc2sys` reports a delay of 1120 ns before and
300 ns after the patch. With the latter being mostly symmetric.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/net/ethernet/cadence/macb_ptp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Richard Cochran Sept. 29, 2021, 2:55 p.m. UTC | #1
On Wed, Sep 29, 2021 at 02:07:39PM +0200, Lars-Peter Clausen wrote:
> The macb PTP support currently implements the `gettime64` callback to allow
> to retrieve the hardware clock time. Update the implementation to provide
> the `gettimex64` callback instead.
> 
> The difference between the two is that with `gettime64` a snapshot of the
> system clock is taken before and after invoking the callback. Whereas
> `gettimex64` expects the callback itself to take the snapshots.
> 
> To get the time from the macb Ethernet core multiple register accesses have
> to be done. Only one of which will happen at the time reported by the
> function. This leads to a non-symmetric delay and adds a slight offset
> between the hardware and system clock time when using the `gettime64`
> method. This offset can be a few 100 nanoseconds. Switching to the
> `gettimex64` method allows for a more precise correlation of the hardware
> and system clocks and results in a lower offset between the two.
> 
> On a Xilinx ZynqMP system `phc2sys` reports a delay of 1120 ns before and
> 300 ns after the patch. With the latter being mostly symmetric.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Please put me on CC for future PTP patches, thanks.

Acked-by: Richard Cochran <richardcochran@gmail.com>
patchwork-bot+netdevbpf@kernel.org Sept. 30, 2021, 12:21 p.m. UTC | #2
Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Wed, 29 Sep 2021 14:07:39 +0200 you wrote:
> The macb PTP support currently implements the `gettime64` callback to allow
> to retrieve the hardware clock time. Update the implementation to provide
> the `gettimex64` callback instead.
> 
> The difference between the two is that with `gettime64` a snapshot of the
> system clock is taken before and after invoking the callback. Whereas
> `gettimex64` expects the callback itself to take the snapshots.
> 
> [...]

Here is the summary with links:
  - net: macb: ptp: Switch to gettimex64() interface
    https://git.kernel.org/netdev/net-next/c/e51bb5c2784c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index c2e1f163bb14..095c5a2144a7 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -38,7 +38,8 @@  static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp,
 	return NULL;
 }
 
-static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts)
+static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts,
+			    struct ptp_system_timestamp *sts)
 {
 	struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
 	unsigned long flags;
@@ -46,7 +47,9 @@  static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts)
 	u32 secl, sech;
 
 	spin_lock_irqsave(&bp->tsu_clk_lock, flags);
+	ptp_read_system_prets(sts);
 	first = gem_readl(bp, TN);
+	ptp_read_system_postts(sts);
 	secl = gem_readl(bp, TSL);
 	sech = gem_readl(bp, TSH);
 	second = gem_readl(bp, TN);
@@ -56,7 +59,9 @@  static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts)
 		/* if so, use later read & re-read seconds
 		 * (assume all done within 1s)
 		 */
+		ptp_read_system_prets(sts);
 		ts->tv_nsec = gem_readl(bp, TN);
+		ptp_read_system_postts(sts);
 		secl = gem_readl(bp, TSL);
 		sech = gem_readl(bp, TSH);
 	} else {
@@ -161,7 +166,7 @@  static int gem_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	}
 
 	if (delta > TSU_NSEC_MAX_VAL) {
-		gem_tsu_get_time(&bp->ptp_clock_info, &now);
+		gem_tsu_get_time(&bp->ptp_clock_info, &now, NULL);
 		now = timespec64_add(now, then);
 
 		gem_tsu_set_time(&bp->ptp_clock_info,
@@ -192,7 +197,7 @@  static const struct ptp_clock_info gem_ptp_caps_template = {
 	.pps		= 1,
 	.adjfine	= gem_ptp_adjfine,
 	.adjtime	= gem_ptp_adjtime,
-	.gettime64	= gem_tsu_get_time,
+	.gettimex64	= gem_tsu_get_time,
 	.settime64	= gem_tsu_set_time,
 	.enable		= gem_ptp_enable,
 };
@@ -251,7 +256,7 @@  static int gem_hw_timestamp(struct macb *bp, u32 dma_desc_ts_1,
 	 * The timestamp only contains lower few bits of seconds,
 	 * so add value from 1588 timer
 	 */
-	gem_tsu_get_time(&bp->ptp_clock_info, &tsu);
+	gem_tsu_get_time(&bp->ptp_clock_info, &tsu, NULL);
 
 	/* If the top bit is set in the timestamp,
 	 * but not in 1588 timer, it has rolled over,