[PATCH] net: macb: fix stale data returned by MDIO reads

macb_mdio_wait_for_idle() samples NSR.IDLE with no delay after MAN has been
written. readx_poll_timeout() expands to read_poll_timeout() with
sleep_before_read = false, so the very first NSR read is issued immediately
after the register write.

The controller does not deassert NSR.IDLE until its MDIO state machine
actually begins the frame, which takes up to one MDC period. Neither
macb_mdc_clk_div() nor gem_mdc_clk_div() ever selects an MDC above 2.5 MHz,
so one period is up to ~400 ns - on the order of a hundred pclk cycles, and
far longer than a back-to-back register access. The poll therefore observes
IDLE still set from the *previous* operation and returns success before the
operation just issued has started.

The caller then reads a MAN DATA field that still holds the result of
whatever completed before, so every read returns the data of the previously
accessed register. For clause 45 the race occurs three times per access: the
address operation is still in flight when the read operation is written, and
the final MACB_BFEXT(DATA, macb_readl(bp, MAN)) picks up a stale value.

Observed on a Versal board with a Marvell 88Q1111 on GEM1, where reading each
MMD register twice showed the first read returning the preceding register's
value:

  31.8002: 0x0149 then 0x002b   (0x0149 is 31.8001)
  31.8004: 0x0b21 then 0x1401   (0x0b21 is 31.8003)
  31.8011: 0x400e then 0x6000   (0x400e is 31.8010)
  31.8010: 0x6000 then 0x400e   (0x6000 is 31.8011)

The lag persists across callers and across driver entry points, so it is a
property of the controller path rather than of any PHY driver. It silently
corrupts every MMD read. Latch-high, clear-on-read registers are worse than
corrupted: their content cannot be recovered by retrying, because the first
read still reaches the PHY and clears the latch.

Sleep before the first NSR sample so that an operation issued immediately
beforehand is guaranteed to be underway. The pre-operation bus-free check
pays the same delay, which is immaterial - a single MDIO frame takes over
25 us at 2.5 MHz MDC, so this is well under the cost of the transfer itself.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Polak, Leszek <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c

@@ -340,10 +340,26 @@ static void macb_get_hwaddr(struct macb *bp)
 
 static int macb_mdio_wait_for_idle(struct macb *bp)
 {
+	/*
+	 * NSR.IDLE is not deasserted until the MDIO state machine begins the
+	 * frame, which takes up to one MDC period after MAN is written. MDC is
+	 * never clocked above 2.5 MHz, so one period is at most ~400 ns; allow
+	 * an order of magnitude of margin. read_poll_timeout() sleeps at least
+	 * (sleep_us >> 2) + 1 us, so the guaranteed minimum here is 6 us.
+	 */
+	const unsigned int start_delay_us = 20;
 	u32 val;
 
-	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
-				  1, MACB_MDIO_TIMEOUT);
+	/*
+	 * Do not sample NSR.IDLE immediately. When this is called straight
+	 * after a MAN write, IDLE is still set from the previous operation,
+	 * so the poll would return at once and the caller would go on to read
+	 * a MAN DATA field that still holds the previous transaction's
+	 * result. Sleeping first guarantees the new operation has started and
+	 * IDLE has gone low before it is sampled.
+	 */
+	return read_poll_timeout(MACB_READ_NSR, val, val & MACB_BIT(IDLE),
+				 start_delay_us, MACB_MDIO_TIMEOUT, true, bp);
 }
 
 static int macb_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)

-- 
2.43.0