Re-lock the mailbox window on lost phase instead of anchoring blind: one loop goroutine per module owns all transport, windows edge-locked from first touch, bringup verifies both modules before either AN restart; corrected charges and link-down reads print raw registers; harness -fuzz repeats lifecycles judging by exit status (30/30 module-clean)

phy.go

@@ -73,26 +73,52 @@ type bcm struct {
 	ifname string
 	path   string
 
-	// Every method holds it for its whole logical operation: exactly one
-	// host-side conversation with the module at a time, by construction.
-	mu        sync.Mutex
+	// Every transport touch happens on the loop goroutine: requests execute
+	// one at a time, each admitted into the quiet window first.
+	reqs      chan func()
 	windowEnd time.Time
-	windowed  atomic.Bool
 }
 
-// The firmware's internal temp poll (every 3.5–4.2 s, never under 3.49) serves
-// stale bridge reads for ~50 ms around it; work stays inside 3.4 s post-poll.
+func (b *bcm) loop() {
+	defer holdPanic()
+	for fn := range b.reqs {
+		b.window()
+		fn()
+	}
+}
+
+func (b *bcm) exec(fn func()) {
+	done := make(chan struct{})
+	b.reqs <- func() { fn(); close(done) }
+	<-done
+}
+
+// The firmware's internal temp poll serves stale bridge reads for ~50 ms
+// around it; work stays inside 3.4 s of an observed poll. A resident 0x0031 at
+// expiry means the phase is unknown, so re-lock: arm, then take the true edge.
 func (b *bcm) window() {
-	if !b.windowed.Load() || time.Now().Add(bcmWindowFit).Before(b.windowEnd) {
+	if time.Now().Add(bcmWindowFit).Before(b.windowEnd) {
 		return
 	}
+	armed := false
 	deadline := time.Now().Add(bcmFlipWait)
 	for {
 		v, err := b.mdioRead(bcmMMDVendor, bcmRegCmd)
 		if err != nil {
 			panic(fmt.Sprintf("%s: heartbeat poll: %v", b.ifname, err))
 		}
-		if v == bcmCmdResidentTemp || time.Now().After(deadline) {
+		if v == bcmCmdResidentTemp {
+			if armed {
+				b.windowEnd = time.Now().Add(bcmWindow)
+				return
+			}
+			b.rearm()
+			armed = true
+			deadline = time.Now().Add(bcmFlipWait)
+			continue
+		}
+		armed = true
+		if time.Now().After(deadline) {
 			b.windowEnd = time.Now().Add(bcmWindow)
 			return
 		}
@@ -100,10 +126,20 @@ func (b *bcm) window() {
 	}
 }
 
-func (b *bcm) acquire() func() {
-	b.mu.Lock()
-	b.window()
-	return b.mu.Unlock
+func (b *bcm) rearm() {
+	if _, err := b.waitStatus(func(st uint16) bool {
+		return st != bcmStInProgress && st != bcmStBusy
+	}); err != nil {
+		panic(fmt.Sprintf("%s: rearm: %v", b.ifname, err))
+	}
+	if err := b.mdioWrite(bcmMMDVendor, bcmRegCmd, bcmCmdGetPairSwap); err != nil {
+		panic(fmt.Sprintf("%s: rearm: %v", b.ifname, err))
+	}
+	if _, err := b.waitStatus(func(st uint16) bool {
+		return st == bcmStPass || st == bcmStError
+	}); err != nil {
+		panic(fmt.Sprintf("%s: rearm: %v", b.ifname, err))
+	}
 }
 
 func openBCM(ifname string) (*bcm, error) {
@@ -121,10 +157,12 @@ func openBCM(ifname string) (*bcm, error) {
 	b := &bcm{
 		ifname: ifname,
 		path:   "/sys/kernel/debug/ixgbe/" + filepath.Base(devLink) + "/sff_i2c",
+		reqs:   make(chan func()),
 	}
 	if _, err := os.Stat(b.path); err != nil {
 		return nil, fmt.Errorf("%s: %w (patched ixgbe?)", ifname, err)
 	}
+	go b.loop()
 	return b, nil
 }
 
@@ -231,76 +269,79 @@ func (b *bcm) waitStatus(want func(uint16) bool) (uint16, error) {
 // GETs must be invoked bare (pre-writing any DATA register leaves the handler
 // executing as a no-op); SETs must pass their full parameter set (the handler
 // executes stale DATA).
-func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
-	defer b.acquire()()
-
-	var data [5]uint16
-	if _, err := b.waitStatus(func(st uint16) bool {
-		return st != bcmStInProgress && st != bcmStBusy
-	}); err != nil {
-		return data, err
-	}
-	for i, p := range params {
-		if err := b.mdioWrite(bcmMMDVendor, bcmRegData1+uint16(i), p); err != nil {
-			return data, err
+func (b *bcm) command(code uint16, params ...uint16) (data [5]uint16, err error) {
+	b.exec(func() {
+		if _, err = b.waitStatus(func(st uint16) bool {
+			return st != bcmStInProgress && st != bcmStBusy
+		}); err != nil {
+			return
+		}
+		for i, p := range params {
+			if err = b.mdioWrite(bcmMMDVendor, bcmRegData1+uint16(i), p); err != nil {
+				return
+			}
+		}
+		if err = b.mdioWrite(bcmMMDVendor, bcmRegCmd, code); err != nil {
+			return
+		}
+		var st uint16
+		if st, err = b.waitStatus(func(st uint16) bool {
+			return st == bcmStPass || st == bcmStError
+		}); err != nil {
+			return
+		}
+		if st == bcmStError {
+			err = fmt.Errorf("%s: command %#04x returned ERROR", b.ifname, code)
+			return
+		}
+		if len(params) > 0 {
+			return
+		}
+		for i := range data {
+			if data[i], err = b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i)); err != nil {
+				return
+			}
 		}
-	}
-	if err := b.mdioWrite(bcmMMDVendor, bcmRegCmd, code); err != nil {
-		return data, err
-	}
-	st, err := b.waitStatus(func(st uint16) bool {
-		return st == bcmStPass || st == bcmStError
 	})
-	if err != nil {
-		return data, err
-	}
-	if st == bcmStError {
-		return data, fmt.Errorf("%s: command %#04x returned ERROR", b.ifname, code)
-	}
-	if len(params) > 0 {
-		return data, nil
-	}
-	for i := range data {
-		if data[i], err = b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i)); err != nil {
-			return data, err
-		}
-	}
-	return data, nil
+	return
 }
 
-func (b *bcm) identify() (string, error) {
-	defer b.acquire()()
-	hi, err := b.mdioRead(1, 2)
-	if err != nil {
-		return "", err
-	}
-	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
+func (b *bcm) identify() (ident string, err error) {
+	b.exec(func() {
+		var hi, lo uint16
+		if hi, err = b.mdioRead(1, 2); err != nil {
+			return
+		}
+		if lo, err = b.mdioRead(1, 3); err != nil {
+			return
+		}
+		if hi != bcmPHYIDHi || lo != bcmPHYIDLo {
+			err = fmt.Errorf("%s: PHY ID %#04x:%#04x, want %#04x:%#04x",
+				b.ifname, hi, lo, bcmPHYIDHi, bcmPHYIDLo)
+			return
+		}
+		var sn []byte
+		if sn, err = b.eeprom(68, 16); err != nil {
+			return
+		}
+		ident = "BCM84891L sn " + strings.TrimSpace(string(sn))
+	})
+	return
 }
 
 // PMA 1.1 latches low, so the first read reports any drop since it was last
 // read and the second reports the wire as it is now.
-func (b *bcm) linkUp() (bool, error) {
-	defer b.acquire()()
-	if _, err := b.mdioRead(1, 1); err != nil {
-		return false, err
-	}
-	v, err := b.mdioRead(1, 1)
-	if err != nil {
-		return false, err
-	}
-	return v&0x0004 != 0, nil
+func (b *bcm) linkUp() (up bool, raw uint16, err error) {
+	b.exec(func() {
+		if _, err = b.mdioRead(1, 1); err != nil {
+			return
+		}
+		if raw, err = b.mdioRead(1, 1); err != nil {
+			return
+		}
+		up = raw&0x0004 != 0
+	})
+	return
 }
 
 func (b *bcm) forceEEEOff() error {
@@ -316,33 +357,37 @@ func (b *bcm) forceJumbo() error {
 // Left to AN, master/slave is a per-training lottery and each training's DSP
 // convergence moves per-pair SNR by up to ~3.6 dB; pinned roles at least keep
 // every session measured under identical conditions.
-func (b *bcm) forceRole(master bool) error {
-	defer b.acquire()()
-	v, err := b.mdioRead(7, 32)
-	if err != nil {
-		return err
-	}
-	v |= 0x8000
-	if master {
-		v |= 0x4000
-	} else {
-		v &^= 0x4000
-	}
-	return b.mdioWrite(7, 32, v)
+func (b *bcm) forceRole(master bool) (err error) {
+	b.exec(func() {
+		var v uint16
+		if v, err = b.mdioRead(7, 32); err != nil {
+			return
+		}
+		v |= 0x8000
+		if master {
+			v |= 0x4000
+		} else {
+			v &^= 0x4000
+		}
+		err = b.mdioWrite(7, 32, v)
+	})
+	return
 }
 
-func (b *bcm) restartAN() error {
-	defer b.acquire()()
-	v, err := b.mdioRead(7, 0)
-	if err != nil {
-		return err
-	}
-	return b.mdioWrite(7, 0, v|0x0200)
+func (b *bcm) restartAN() (err error) {
+	b.exec(func() {
+		var v uint16
+		if v, err = b.mdioRead(7, 0); err != nil {
+			return
+		}
+		err = b.mdioWrite(7, 0, v|0x0200)
+	})
+	return
 }
 
-func (b *bcm) eeeAdvert() (uint16, error) {
-	defer b.acquire()()
-	return b.mdioRead(7, 60)
+func (b *bcm) eeeAdvert() (v uint16, err error) {
+	b.exec(func() { v, err = b.mdioRead(7, 60) })
+	return
 }
 
 func (b *bcm) pairMap() (byte, error) {
@@ -365,22 +410,24 @@ func (b *bcm) snr() ([4]float64, error) {
 	return out, nil
 }
 
-func (b *bcm) pcsLatch() (blocks, ber uint64, err error) {
-	defer b.acquire()()
-	v, err := b.mdioRead(3, 33)
-	if err != nil {
-		return 0, 0, err
-	}
-	return uint64(v & 0xFF), uint64((v >> 8) & 0x3F), nil
+func (b *bcm) pcsLatch() (blocks, ber uint64, raw uint16, err error) {
+	b.exec(func() {
+		if raw, err = b.mdioRead(3, 33); err != nil {
+			return
+		}
+		blocks, ber = uint64(raw&0xFF), uint64((raw>>8)&0x3F)
+	})
+	return
 }
 
-func (b *bcm) fastRetrainCount() (uint16, error) {
-	defer b.acquire()()
-	v, err := b.mdioRead(1, 147)
-	if err != nil {
-		return 0, err
-	}
-	return v >> 11, nil
+func (b *bcm) fastRetrainCount() (count, raw uint16, err error) {
+	b.exec(func() {
+		if raw, err = b.mdioRead(1, 147); err != nil {
+			return
+		}
+		count = raw >> 11
+	})
+	return
 }
 
 type ecdResult struct {
@@ -388,48 +435,47 @@ type ecdResult struct {
 	metres   [4]int
 }
 
-func (b *bcm) cableDiag() (ecdResult, error) {
-	defer b.acquire()()
-
-	var res ecdResult
-	ctrl, err := b.mdioRead(bcmMMDVendor, bcmRegECDCtrl)
-	if err != nil {
-		return res, err
-	}
-	if err := b.mdioWrite(bcmMMDVendor, bcmRegECDCtrl, ctrl&^0xF400|0x8400); err != nil {
-		return res, err
-	}
-	deadline := time.Now().Add(ecdDeadline)
-	for {
-		ctrl, err = b.mdioRead(bcmMMDVendor, bcmRegECDCtrl)
-		if err != nil {
-			return res, err
+func (b *bcm) cableDiag() (res ecdResult, err error) {
+	b.exec(func() {
+		var ctrl uint16
+		if ctrl, err = b.mdioRead(bcmMMDVendor, bcmRegECDCtrl); err != nil {
+			return
 		}
-		if ctrl&0x0800 == 0 {
-			break
+		if err = b.mdioWrite(bcmMMDVendor, bcmRegECDCtrl, ctrl&^0xF400|0x8400); err != nil {
+			return
 		}
-		if time.Now().After(deadline) {
-			return res, fmt.Errorf("%s: cable diag still busy after %s", b.ifname, ecdDeadline)
+		deadline := time.Now().Add(ecdDeadline)
+		for {
+			if ctrl, err = b.mdioRead(bcmMMDVendor, bcmRegECDCtrl); err != nil {
+				return
+			}
+			if ctrl&0x0800 == 0 {
+				break
+			}
+			if time.Now().After(deadline) {
+				err = fmt.Errorf("%s: cable diag still busy after %s", b.ifname, ecdDeadline)
+				return
+			}
+			time.Sleep(ecdPoll)
 		}
-		time.Sleep(ecdPoll)
-	}
-	b.window()
-	v, err := b.mdioRead(1, bcmRegECDResult)
-	if err != nil {
-		return res, err
-	}
-	for i := range res.verdicts {
-		res.verdicts[i] = int(v>>(4*i)) & 0xF
-		if res.verdicts[i] > pairXtalk {
-			panic(fmt.Sprintf("%s: ghost ECD verdict %#04x", b.ifname, v))
+		b.window()
+		var v uint16
+		if v, err = b.mdioRead(1, bcmRegECDResult); err != nil {
+			return
 		}
-		m, err := b.mdioRead(1, bcmRegECDLen+uint16(i))
-		if err != nil {
-			return res, err
+		for i := range res.verdicts {
+			res.verdicts[i] = int(v>>(4*i)) & 0xF
+			if res.verdicts[i] > pairXtalk {
+				panic(fmt.Sprintf("%s: ghost ECD verdict %#04x", b.ifname, v))
+			}
+			var m uint16
+			if m, err = b.mdioRead(1, bcmRegECDLen+uint16(i)); err != nil {
+				return
+			}
+			res.metres[i] = int(m)
 		}
-		res.metres[i] = int(m)
-	}
-	return res, nil
+	})
+	return
 }
 
 const (
@@ -465,6 +511,7 @@ type phyModule struct {
 	recentDelta  uint64
 	primed       bool
 	retrainCount uint16
+	notes        []string
 }
 
 // Silent while a measure owns the module.
@@ -472,7 +519,7 @@ func (m *phyModule) poll() error {
 	if m.busy.Load() {
 		return nil
 	}
-	link, err := m.bcm.linkUp()
+	link, linkRaw, err := m.bcm.linkUp()
 	if err != nil {
 		return err
 	}
@@ -487,16 +534,20 @@ func (m *phyModule) poll() error {
 			}
 		}
 	}
-	blocks, ber, err := m.bcm.pcsLatch()
+	blocks, ber, pcsRaw, err := m.bcm.pcsLatch()
 	if err != nil {
 		return err
 	}
-	count, err := m.bcm.fastRetrainCount()
+	count, frRaw, err := m.bcm.fastRetrainCount()
 	if err != nil {
 		return err
 	}
 
 	m.mu.Lock()
+	if m.link && !link {
+		m.notes = append(m.notes,
+			fmt.Sprintf("%s link read down: 1.1=0x%04x", m.bcm.ifname, linkRaw))
+	}
 	m.sampled = true
 	m.lastOK = time.Now()
 	m.link = link
@@ -505,10 +556,16 @@ func (m *phyModule) poll() error {
 	// The first poll after a baseline drains latches from the bringup/diag
 	// retrain era, so it only sets the origin; the retrain counter is 5 bits.
 	if m.primed {
-		delta := blocks + ber + uint64((count-m.retrainCount)&0x1F)
+		rt := uint64((count - m.retrainCount) & 0x1F)
+		delta := blocks + ber + rt
+		if delta > 0 {
+			m.notes = append(m.notes, fmt.Sprintf(
+				"%s corrected +%d raw: 3.33=0x%04x (blocks %d ber %d) 1.147=0x%04x (retrain +%d) 1.1=0x%04x",
+				m.bcm.ifname, delta, pcsRaw, blocks, ber, frRaw, rt, linkRaw))
+		}
 		m.blocks += blocks
 		m.ber += ber
-		m.retrains += uint64((count - m.retrainCount) & 0x1F)
+		m.retrains += rt
 		m.recentDelta = delta
 	} else {
 		m.recentDelta = 0
@@ -519,6 +576,14 @@ func (m *phyModule) poll() error {
 	return nil
 }
 
+func (m *phyModule) takeNotes() []string {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	n := m.notes
+	m.notes = nil
+	return n
+}
+
 func (m *phyModule) run(done *atomic.Bool) {
 	tick := time.NewTicker(phyInterval)
 	defer tick.Stop()
@@ -848,12 +913,11 @@ func moduleChecks(mods []*phyModule, names [2]string) []checkResult {
 			return fail(res.item, err)
 		}
 		out = append(out, res)
-
-		if err := m.bcm.restartAN(); err != nil {
-			return fail(names[i]+" retrain", err)
-		}
 	}
 
+	// Both modules configured and verified before either AN restart: the
+	// modules link to each other, so one restart puts both µCs into training,
+	// and no read should race that. The restarts fire last, nothing after.
 	res := checkResult{item: "eee advert"}
 	var adv [2]string
 	for i, m := range mods {
@@ -867,10 +931,14 @@ func moduleChecks(mods []*phyModule, names [2]string) []checkResult {
 		}
 	}
 	res.state = adv[0] + "/" + adv[1]
-	for _, m := range mods {
-		m.bcm.windowed.Store(true)
+	out = append(out, res)
+
+	for i, m := range mods {
+		if err := m.bcm.restartAN(); err != nil {
+			return fail(names[i]+" retrain", err)
+		}
 	}
-	return append(out, res)
+	return out
 }
 
 func cableLine(c cableInfo) string {