[RFC PATCH net] net: macb: fix ordering around PTP timestamp read

[James Clark <jjc@jclark.com>]

PTP_SYS_OFFSET_EXTENDED returns system timestamps that do not correctly
bracket the PHC register read on MACB/GEM. On a Raspberry Pi 5, the
returned interval can be as short as 37 ns, while an ordered register
read takes approximately 1 us. This biases the midpoint used by phc2sys,
causing CLOCK_REALTIME to run approximately 0.5 us ahead when synchronized
to the PHC.

gem_tsu_get_time() reads the nanoseconds register using the driver's
relaxed MMIO accessor. On weakly ordered systems, the subsequent system
timestamp can be taken before the register read completes.

Add rmb() after the bracketed nanoseconds read in both the normal and
seconds rollover paths, ensuring that the read completes before the post
timestamp is taken. With the fix, the minimum interval on the same
Raspberry Pi 5 increases to approximately 1 us.

Fixes: e51bb5c2784c ("net: macb: ptp: Switch to gettimex64() interface")
Signed-off-by: James Clark <jjc@jclark.com>
---
This uses rmb() to preserve the existing accessor and endianness handling.
Would an ordered MMIO accessor be preferable for these two reads?

Reproducer:

#include <fcntl.h>
#include <linux/ptp_clock.h>
#include <stdio.h>
#include <sys/ioctl.h>

#define DEVICE "/dev/ptp0"

int main(void)
{
	struct ptp_sys_offset_extended ex = { .n_samples = 25 };
	long long min = -1;
	int fd = open(DEVICE, O_RDONLY);

	if (fd < 0) {
		perror(DEVICE);
		return 1;
	}
	for (int batch = 0; batch < 40; batch++) {
		if (ioctl(fd, PTP_SYS_OFFSET_EXTENDED, &ex) < 0) {
			perror("PTP_SYS_OFFSET_EXTENDED");
			return 1;
		}
		for (unsigned int i = 0; i < ex.n_samples; i++) {
			long long bracket = (ex.ts[i][2].sec - ex.ts[i][0].sec) * 1000000000LL
				+ (long long)ex.ts[i][2].nsec - ex.ts[i][0].nsec;
			if (min < 0 || bracket < min)
				min = bracket;
		}
	}
	printf("min bracket: %lld ns\n", min);
	return 0;
}

 drivers/net/ethernet/cadence/macb_ptp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index e5195d7da..8209ec190 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -51,6 +51,8 @@ static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts,
 	spin_lock_irqsave(&bp->tsu_clk_lock, flags);
 	ptp_read_system_prets(sts);
 	first = gem_readl(bp, TN);
+	/* Ensure the PHC read completes before taking the post timestamp. */
+	rmb();
 	ptp_read_system_postts(sts);
 	secl = gem_readl(bp, TSL);
 	sech = gem_readl(bp, TSH);
@@ -63,6 +65,8 @@ static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts,
 		 */
 		ptp_read_system_prets(sts);
 		ts->tv_nsec = gem_readl(bp, TN);
+		/* Ensure the PHC read completes before taking the post timestamp. */
+		rmb();
 		ptp_read_system_postts(sts);
 		secl = gem_readl(bp, TSL);
 		sech = gem_readl(bp, TSH);
-- 
2.47.3