Bridged MDIO reads verify freshness: a late async fetch serves the previous response as the wrong register (observed STATUS 0x0004 as 0.4dB SNR); unchanged-or-zero results re-read bare without re-arming until stable, first read always verified, identify retries against the constant PHY ID

docs/modules/fs/README.md

@@ -14,6 +14,7 @@ PHY at I2C 8-bit **0xAC write / 0xAD read**.
 | Clause-45 read | I2C write to 0xAC: `[001+DevAD, RegH, RegL]` → **delay >1 ms** (3 ms proven) → I2C read 2 B from 0xAD |
 
 - Single-byte reads see the bridge as inert (returns 0) — the DevAD-prefixed frame + delay is mandatory. This is why early probing wrongly declared 0x56 dead. Treat 0x0000 reads as retry-with-longer-delay: 0 is also the bridge's not-ready signature.
+- **The fetch is asynchronous and a late one serves the wrong register silently**: the read frame arms a fetch, and if it outruns the delay, reading 0xAD returns the *previous* transaction's response with no error — observed under load even at 3 ms (STATUS 0x0004 served as an SNR DATA read → 0.4 dB). Since the buffer only changes when a fetch lands, a result differing from the previous buffer value is provably fresh; an unchanged (or zero) result must be re-read **bare, without re-arming** — preserving clear-on-read registers — until it stabilizes. cabletest implements this freshness check on every bridged read (`phy.go`).
 - Recorded for a future multi-byte firmware transport (mlx5 MCIA shape): read = write `[001+devad,RegH,RegL]` offset_size=1, STOP, >1 ms, read 2 B offset_size=0; write = offset_size=2, addr = devad<<8|RegH, data = `[RegL,DataH,DataL]`.
 
 ## MDIO command handler

docs/state.md

@@ -2,7 +2,7 @@
 
 ## Committed tree
 
-AF_PACKET raw sockets everywhere (`sock.go`); flow-director steering; per-packet-MAC-rx-stamped rate buckets (`SO_TIMESTAMPING` cmsg, `rx_filter=ALL` as a hard host check — nics/README.md for what that demands of the NIC; **temporarily bypassed** in `ts.go` so BCM work can run on the X520, which cannot stamp — the check reports yellow and the panel rates read zero there; restore to fatal for the product NIC); read-time-stamped NIC-counter rates; test interfaces pinned to MTU 9000 with a 9018-byte jumbo in the size mix (the modules' jumbo path is exercised, not assumed); BCM module diagnostics (`phy.go`, over the patched-ixgbe `sff_i2c` debugfs, compound-op framing): bringup identifies both modules and checks EEE off and jumbo on (forcing with an AN restart only on mismatch); the ECD — per-pair verdicts, lengths and pair maps are the length/wiring path — runs through one async path at startup and on every reset, never blocking the UI, with counters re-baselining only after the diag's own link blip so it is never charged to the run; a 1 Hz poller feeds per-pair SNR margin (vs the ≈26.5 dB operating point; green ≥ 3 dB, amber ≥ 1 dB — provisional until the graded-noise run) and the corrected-error set (PCS 3.33 errored blocks/BER, PMA 1.147 fast-retrain count) to the panel and console; framebuffer UI; harness.
+AF_PACKET raw sockets everywhere (`sock.go`); flow-director steering; per-packet-MAC-rx-stamped rate buckets (`SO_TIMESTAMPING` cmsg, `rx_filter=ALL` as a hard host check — nics/README.md for what that demands of the NIC; **temporarily bypassed** in `ts.go` so BCM work can run on the X520, which cannot stamp — the check reports yellow and the panel rates read zero there; restore to fatal for the product NIC); read-time-stamped NIC-counter rates; test interfaces pinned to MTU 9000 with a 9018-byte jumbo in the size mix (the modules' jumbo path is exercised, not assumed); BCM module diagnostics (`phy.go`, over the patched-ixgbe `sff_i2c` debugfs, compound-op framing, every bridged read staleness-checked — modules/fs/ for the late-fetch hazard): bringup identifies both modules and checks EEE off and jumbo on (forcing with an AN restart only on mismatch); the ECD — per-pair verdicts, lengths and pair maps are the length/wiring path — runs through one async path at startup and on every reset, never blocking the UI, with counters re-baselining only after the diag's own link blip so it is never charged to the run; a 1 Hz poller feeds per-pair SNR margin (vs the ≈26.5 dB operating point; green ≥ 3 dB, amber ≥ 1 dB — provisional until the graded-noise run) and the corrected-error set (PCS 3.33 errored blocks/BER, PMA 1.147 fast-retrain count) to the panel and console; framebuffer UI; harness.
 
 ## Stashes
 

phy.go

@@ -40,12 +40,14 @@ const (
 	bcmPHYIDHi = 0x3590
 	bcmPHYIDLo = 0x5081
 
-	bcmReadDelayUs  = 3000
-	bcmRetryDelayUs = 10000
+	bcmReadDelayUs = 3000
 
 	bcmStatusPoll  = 100 * time.Millisecond
 	bcmStatusTries = 30
 
+	bcmVerifyGap   = 2 * time.Millisecond
+	bcmVerifyTries = 8
+
 	ecdPoll     = 200 * time.Millisecond
 	ecdDeadline = 50 * time.Second
 
@@ -70,6 +72,10 @@ type bcm struct {
 	// Guards multi-op sequences only; single reads are already atomic on the
 	// wire through the compound op.
 	mu sync.Mutex
+
+	rmu     sync.Mutex
+	lastBuf uint16
+	haveBuf bool
 }
 
 func openBCM(ifname string) (*bcm, error) {
@@ -155,14 +161,50 @@ func (b *bcm) mdioReadDelay(devad, reg uint16, delayUs int) (uint16, error) {
 	return uint16(d[0])<<8 | uint16(d[1]), nil
 }
 
-// 0x0000 is also the bridge's not-ready signature, so a zero is read again at
-// a longer delay before being believed.
-func (b *bcm) mdioRead(devad, reg uint16) (uint16, error) {
-	v, err := b.mdioReadDelay(devad, reg, bcmReadDelayUs)
-	if err != nil || v != 0 {
-		return v, err
+func (b *bcm) bufRead() (uint16, error) {
+	resp, err := b.op(fmt.Sprintf("r %02x 2", bcmI2CRead))
+	if err != nil {
+		return 0, err
 	}
-	return b.mdioReadDelay(devad, reg, bcmRetryDelayUs)
+	d, err := parseHexBytes(resp, 2)
+	if err != nil {
+		return 0, err
+	}
+	return uint16(d[0])<<8 | uint16(d[1]), nil
+}
+
+// The bridge fetch is asynchronous: one that outruns the delay leaves the
+// previous response in the buffer, served silently as the wrong register's
+// data. A changed buffer value proves a fresh fetch; an unchanged or zero one
+// is re-read bare — never re-armed, so clear-on-read registers keep their
+// data — until it stabilizes.
+func (b *bcm) mdioRead(devad, reg uint16) (uint16, error) {
+	b.rmu.Lock()
+	defer b.rmu.Unlock()
+
+	v, err := b.mdioReadDelay(devad, reg, bcmReadDelayUs)
+	if err != nil {
+		return 0, err
+	}
+	if b.haveBuf && v != 0 && v != b.lastBuf {
+		b.lastBuf = v
+		return v, nil
+	}
+	stable := 0
+	for i := 0; i < bcmVerifyTries && stable < 2; i++ {
+		time.Sleep(bcmVerifyGap)
+		r, err := b.bufRead()
+		if err != nil {
+			return 0, err
+		}
+		if r == v {
+			stable++
+		} else {
+			v, stable = r, 0
+		}
+	}
+	b.haveBuf, b.lastBuf = true, v
+	return v, nil
 }
 
 func (b *bcm) mdioWrite(devad, reg, val uint16) error {
@@ -229,24 +271,29 @@ func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
 	return data, nil
 }
 
+// Retried against the known constant: the first reads after a process start
+// can land while the bridge still holds a dead process's pending fetch.
 func (b *bcm) identify() (string, error) {
-	hi, err := b.mdioRead(1, 2)
-	if err != nil {
-		return "", err
+	var hi, lo uint16
+	var err error
+	for i := 0; i < 5; i++ {
+		if hi, err = b.mdioRead(1, 2); err != nil {
+			return "", err
+		}
+		if lo, err = b.mdioRead(1, 3); err != nil {
+			return "", err
+		}
+		if hi == bcmPHYIDHi && lo == bcmPHYIDLo {
+			sn, err := b.eeprom(68, 16)
+			if err != nil {
+				return "", err
+			}
+			return "BCM84891L sn " + strings.TrimSpace(string(sn)), nil
+		}
+		time.Sleep(200 * time.Millisecond)
 	}
-	lo, err := b.mdioRead(1, 3)
-	if err != nil {
-		return "", err
-	}
-	if hi != bcmPHYIDHi || lo != bcmPHYIDLo {
-		return "", fmt.Errorf("%s: PHY ID %#04x:%#04x, want %#04x:%#04x",
-			b.ifname, hi, lo, bcmPHYIDHi, bcmPHYIDLo)
-	}
-	sn, err := b.eeprom(68, 16)
-	if err != nil {
-		return "", err
-	}
-	return "BCM84891L sn " + strings.TrimSpace(string(sn)), nil
+	return "", fmt.Errorf("%s: PHY ID %#04x:%#04x, want %#04x:%#04x",
+		b.ifname, hi, lo, bcmPHYIDHi, bcmPHYIDLo)
 }
 
 // PMA 1.1 latches low, so the first read reports any drop since it was last