Shared-mailbox reality: module firmware is its own handler client (internal GET_CURRENT_TEMP ~3.5s, proven idle-watch), handler writes wedge the uC in busy windows; quiet-window writes, bare GETs double-read with CMD-readback tripwire, SNR floor, SETs verified externally; sentinel handshake and wire-probe jumbo check removed

2026-08-13 10:19:32 -07:00
parent e99c7121e7
commit 9d1f15045f
4 changed files with 204 additions and 190 deletions
@@ -27,10 +27,8 @@ const (
 	bcmStBusy       uint16 = 0xBBBB

 	bcmCmdGetPairSwap uint16 = 0x8000
-	bcmCmdGetEEEMode  uint16 = 0x8008
 	bcmCmdSetEEEMode  uint16 = 0x8009
 	bcmCmdSetJumbo    uint16 = 0x801C
-	bcmCmdGetJumbo    uint16 = 0x801D
 	bcmCmdGetSNR      uint16 = 0x8030

 	bcmRegECDCtrl   uint16 = 0x4006
@@ -40,13 +38,13 @@ const (
 	bcmPHYIDHi = 0x3590
 	bcmPHYIDLo = 0x5081

-	bcmReadDelayUs = 3000
+	bcmReadDelayUs  = 3000
+	bcmRetryDelayUs = 10000

 	bcmStatusPoll  = 100 * time.Millisecond
 	bcmStatusTries = 30
-
-	bcmVerifyGap   = 2 * time.Millisecond
-	bcmVerifyTries = 8
+	bcmSetSettle   = time.Second
+	bcmQuiet       = 10 * time.Second

 	ecdPoll     = 200 * time.Millisecond
 	ecdDeadline = 50 * time.Second
@@ -72,10 +70,6 @@ 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) {
@@ -161,50 +155,14 @@ func (b *bcm) mdioReadDelay(devad, reg uint16, delayUs int) (uint16, error) {
 	return uint16(d[0])<<8 | uint16(d[1]), nil
 }

-func (b *bcm) bufRead() (uint16, error) {
-	resp, err := b.op(fmt.Sprintf("r %02x 2", bcmI2CRead))
-	if err != nil {
-		return 0, err
-	}
-	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.
+// 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) {
-	b.rmu.Lock()
-	defer b.rmu.Unlock()
-
 	v, err := b.mdioReadDelay(devad, reg, bcmReadDelayUs)
-	if err != nil {
-		return 0, err
+	if err != nil || v != 0 {
+		return v, 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
+	return b.mdioReadDelay(devad, reg, bcmRetryDelayUs)
 }

 func (b *bcm) mdioWrite(devad, reg, val uint16) error {
@@ -233,8 +191,27 @@ func (b *bcm) waitStatus(want func(uint16) bool) (uint16, error) {
 	return 0, fmt.Errorf("%s: command handler stuck, status %#04x", b.ifname, st)
 }

-// The handler never clears DATA registers it does not use, so every SET must
-// pass its full parameter set and every GET must pass none.
+// The bridge gives no signal tying a response to its fetch, so a known-answer
+// read validates each batch: 0x3590 back from PHY ID 1.2 proves the bridge
+// was serving timely through the window; anything else discards the batch.
+func (b *bcm) canary() error {
+	v, err := b.mdioRead(1, 2)
+	if err != nil {
+		return err
+	}
+	if v != bcmPHYIDHi {
+		return fmt.Errorf("%s: bridge served %#04x for the PHY ID canary", b.ifname, v)
+	}
+	return nil
+}
+
+// GETs must be invoked bare: pre-writing any DATA register — not just the
+// documented DATA1 flag — leaves the handler executing as a no-op. SETs must
+// pass their full parameter set (the handler executes stale DATA) and get
+// settle time in place of unprovable completion. Results are read twice and
+// must agree — a stale PASS otherwise serves the handler's in-flight scratch
+// — with DATA1 excluded from the comparison (firmware writes temperature
+// there autonomously).
 func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
 	b.mu.Lock()
 	defer b.mu.Unlock()
@@ -253,6 +230,9 @@ func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
 	if err := b.mdioWrite(bcmMMDVendor, bcmRegCmd, code); err != nil {
 		return data, err
 	}
+	if len(params) > 0 {
+		time.Sleep(bcmSetSettle)
+	}
 	st, err := b.waitStatus(func(st uint16) bool {
 		return st == bcmStPass || st == bcmStError
 	})
@@ -262,21 +242,47 @@ func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
 	if st == bcmStError {
 		return data, fmt.Errorf("%s: command %#04x returned ERROR", b.ifname, code)
 	}
+	// A SET's DATA registers are its parameters, scribbled over by firmware
+	// afterwards; there is nothing to read back.
+	if len(params) > 0 {
+		return data, b.canary()
+	}
 	for i := range data {
-		data[i], err = b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i))
+		if data[i], err = b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i)); err != nil {
+			return data, err
+		}
+	}
+	for i := 1; i < len(data); i++ {
+		again, err := b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i))
 		if err != nil {
 			return data, err
 		}
+		if again != data[i] {
+			return data, fmt.Errorf("%s: command %#04x results unstable", b.ifname, code)
+		}
+	}
+	// The firmware is its own mailbox client (an internal GET_CURRENT_TEMP
+	// every few seconds); CMD still holding our code proves a GET's results
+	// are ours. Bit 15 is consumed on acceptance.
+	cmdv, err := b.mdioRead(bcmMMDVendor, bcmRegCmd)
+	if err != nil {
+		return data, err
+	}
+	if cmdv != code&^0x8000 {
+		return data, fmt.Errorf("%s: command %#04x preempted, CMD reads %#04x", b.ifname, code, cmdv)
+	}
+	if err := b.canary(); err != nil {
+		return data, err
 	}
 	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.
+// Retried against the known constant, long enough to outlast a µC left busy
+// by a dead process's in-flight diag or a link mid-training.
 func (b *bcm) identify() (string, error) {
 	var hi, lo uint16
 	var err error
-	for i := 0; i < 5; i++ {
+	for i := 0; i < 30; i++ {
 		if hi, err = b.mdioRead(1, 2); err != nil {
 			return "", err
 		}
@@ -290,7 +296,7 @@ func (b *bcm) identify() (string, error) {
 			}
 			return "BCM84891L sn " + strings.TrimSpace(string(sn)), nil
 		}
-		time.Sleep(200 * time.Millisecond)
+		time.Sleep(500 * time.Millisecond)
 	}
 	return "", fmt.Errorf("%s: PHY ID %#04x:%#04x, want %#04x:%#04x",
 		b.ifname, hi, lo, bcmPHYIDHi, bcmPHYIDLo)
@@ -309,28 +315,11 @@ func (b *bcm) linkUp() (bool, error) {
 	return v&0x0004 != 0, nil
 }

-func (b *bcm) eeeMode() (uint16, error) {
-	d, err := b.command(bcmCmdGetEEEMode)
-	return d[0], err
-}
-
 func (b *bcm) forceEEEOff() error {
 	_, err := b.command(bcmCmdSetEEEMode, 0x0000, 0x0000, 0x7A12, 0x0480, 0x0000)
 	return err
 }

-func (b *bcm) jumboState() (bool, string, error) {
-	d, err := b.command(bcmCmdGetJumbo)
-	if err != nil {
-		return false, "", err
-	}
-	size := map[uint16]string{0: "10K", 1: "18K", 2: "9K"}[d[1]]
-	if size == "" {
-		size = fmt.Sprintf("size %d", d[1])
-	}
-	return d[0] == 1, size, nil
-}
-
 func (b *bcm) forceJumbo() error {
 	_, err := b.command(bcmCmdSetJumbo, 1, 0, 0, 0, 0)
 	return err
@@ -415,19 +404,30 @@ func (b *bcm) cableDiag() (ecdResult, error) {
 		}
 		time.Sleep(ecdPoll)
 	}
-	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
-		m, err := b.mdioRead(1, bcmRegECDLen+uint16(i))
+	// The run's own blip leaves the µC busy training, so a failed canary here
+	// means try the latched results again shortly, not give up.
+	var canaryErr error
+	for try := 0; try < 20; try++ {
+		if try > 0 {
+			time.Sleep(500 * time.Millisecond)
+		}
+		v, err := b.mdioRead(1, bcmRegECDResult)
 		if err != nil {
 			return res, err
 		}
-		res.metres[i] = int(m)
+		for i := range res.verdicts {
+			res.verdicts[i] = int(v>>(4*i)) & 0xF
+			m, err := b.mdioRead(1, bcmRegECDLen+uint16(i))
+			if err != nil {
+				return res, err
+			}
+			res.metres[i] = int(m)
+		}
+		if canaryErr = b.canary(); canaryErr == nil {
+			return res, nil
+		}
 	}
-	return res, nil
+	return res, canaryErr
 }

 const (
@@ -443,7 +443,9 @@ const (
 )

 type phyModule struct {
-	bcm *bcm
+	bcm     *bcm
+	busy    atomic.Bool
+	upSince time.Time

 	mu           sync.Mutex
 	sampled      bool
@@ -459,16 +461,42 @@ type phyModule struct {
 	retrainCount uint16
 }

+// Silent while a measure owns the module, and no handler command outside a
+// quiet window (carrier up and stable): writes landing on a µC busy with
+// post-AN work have wedged its SMI service permanently. Reads are always safe.
 func (m *phyModule) poll() error {
+	if m.busy.Load() {
+		return nil
+	}
+	if carrierUp(m.bcm.ifname) {
+		if m.upSince.IsZero() {
+			m.upSince = time.Now()
+		}
+	} else {
+		m.upSince = time.Time{}
+	}
+	quiet := !m.upSince.IsZero() && time.Since(m.upSince) >= bcmQuiet
+
+	if err := m.bcm.canary(); err != nil {
+		return err
+	}
 	link, err := m.bcm.linkUp()
 	if err != nil {
 		return err
 	}
+	link = link && quiet
 	var snr [4]float64
 	if link {
 		if snr, err = m.bcm.snr(); err != nil {
 			return err
 		}
+		// The proven garbage signatures — die temperature, handler status —
+		// all sit far below any SNR a trained link can have.
+		for _, s := range snr {
+			if s < 15 {
+				return fmt.Errorf("%s: implausible SNR %.1f discarded", m.bcm.ifname, s)
+			}
+		}
 	}
 	blocks, ber, err := m.bcm.pcsLatch()
 	if err != nil {
@@ -478,6 +506,9 @@ func (m *phyModule) poll() error {
 	if err != nil {
 		return err
 	}
+	if err := m.bcm.canary(); err != nil {
+		return err
+	}

 	m.mu.Lock()
 	m.sampled = true
@@ -657,30 +688,24 @@ func phyDisplayFrom(cable cableInfo, measuring bool, a, b phyModView) phyDisplay
 	return d
 }

-func waitLink(mods []*phyModule, done *atomic.Bool) (time.Duration, bool, error) {
-	start := time.Now()
-	deadline := start.Add(linkWaitSpan)
-	for {
-		up := true
-		for _, m := range mods {
-			v, err := m.bcm.linkUp()
-			if err != nil {
-				return 0, false, err
-			}
-			up = up && v
-		}
-		if up {
-			return time.Since(start), true, nil
-		}
-		if time.Now().After(deadline) || (done != nil && done.Load()) {
-			return time.Since(start), false, nil
-		}
-		time.Sleep(linkWaitPoll)
-	}
-}
-
-// Pair maps are read after the relink, so the MDI resolution is the fresh one.
+// Every handler write inside runs in a quiet window: the ECD trigger only
+// after the carrier has settled, the pair-map commands only after the run's
+// own blip has settled too. The pollers are held silent throughout.
 func measureCable(mods []*phyModule, waitRelink bool, done *atomic.Bool) (cableInfo, bool, error) {
+	for _, m := range mods {
+		m.busy.Store(true)
+	}
+	defer func() {
+		for _, m := range mods {
+			m.busy.Store(false)
+		}
+	}()
+	names := [2]string{mods[0].bcm.ifname, mods[1].bcm.ifname}
+	time.Sleep(2 * phyInterval)
+	if _, up := waitCarrier(names, done); up {
+		time.Sleep(bcmQuiet)
+	}
+
 	var c cableInfo
 	var err error
 	c.ecd, err = mods[0].bcm.cableDiag()
@@ -689,10 +714,9 @@ func measureCable(mods []*phyModule, waitRelink bool, done *atomic.Bool) (cableI
 	}
 	relinked := false
 	if waitRelink {
-		if _, relinked, err = waitLink(mods, done); err != nil {
-			return c, false, err
-		}
+		_, relinked = waitCarrier(names, done)
 	}
+	time.Sleep(bcmQuiet)
 	for i, m := range mods {
 		if c.maps[i], err = m.bcm.pairMap(); err != nil {
 			return c, false, err
@@ -703,7 +727,7 @@ func measureCable(mods []*phyModule, waitRelink bool, done *atomic.Bool) (cableI

 type cableDiag struct {
 	mods      []*phyModule
-	completed chan struct{}
+	completed chan error

 	mu      sync.Mutex
 	info    cableInfo
@@ -711,7 +735,7 @@ type cableDiag struct {
 }

 func newCableDiag(mods []*phyModule, info cableInfo) *cableDiag {
-	return &cableDiag{mods: mods, completed: make(chan struct{}, 1), info: info}
+	return &cableDiag{mods: mods, completed: make(chan error, 1), info: info}
 }

 func (c *cableDiag) snapshot() (cableInfo, bool) {
@@ -733,14 +757,14 @@ func (c *cableDiag) kick(done *atomic.Bool) bool {
 		defer holdPanic()
 		info, _, err := measureCable(c.mods, true, done)
 		if err != nil {
-			panic(err)
+			info = cableInfo{}
 		}
 		c.mu.Lock()
 		c.info = info
 		c.running = false
 		c.mu.Unlock()
 		select {
-		case c.completed <- struct{}{}:
+		case c.completed <- err:
 		default:
 		}
 	}()
@@ -788,102 +812,83 @@ func openModules(names [2]string) ([]*phyModule, [2]string, error) {
 		if err != nil {
 			return nil, idents, err
 		}
-		mods = append(mods, &phyModule{bcm: b})
+		m := &phyModule{bcm: b}
+		// Born busy: the pollers stay silent through bringup's SETs and
+		// retrains until the first measure completes and lifts the gate.
+		m.busy.Store(true)
+		mods = append(mods, m)
 	}
 	return mods, idents, nil
 }

+func waitCarrier(names [2]string, done *atomic.Bool) (time.Duration, bool) {
+	start := time.Now()
+	deadline := start.Add(linkWaitSpan)
+	for {
+		if carrierUp(names[0]) && carrierUp(names[1]) {
+			return time.Since(start), true
+		}
+		if time.Now().After(deadline) || (done != nil && done.Load()) {
+			return time.Since(start), false
+		}
+		time.Sleep(linkWaitPoll)
+	}
+}
+
+// No trustworthy config readback exists (DATA1 is firmware scratch) and no
+// cable is guaranteed at bringup, so both settings are forced every boot: the
+// one deterministic assurance. The handler freezes during training, so the
+// carrier settles — the host checks just reset the links — before any command.
 func moduleChecks(mods []*phyModule, names [2]string) []checkResult {
 	var out []checkResult
 	fail := func(item string, err error) []checkResult {
 		return append(out, checkResult{item: item, err: err})
 	}

-	retrained := false
+	_, hadLink := waitCarrier(names, nil)
+	if hadLink {
+		time.Sleep(bcmQuiet)
+	}
+
 	for i, m := range mods {
-		changed := false
-
-		res := checkResult{item: names[i] + " eee"}
-		mode, err := m.bcm.eeeMode()
-		if err != nil {
+		res := checkResult{item: names[i] + " eee", state: "forced off"}
+		if err := m.bcm.forceEEEOff(); err != nil {
 			return fail(res.item, err)
 		}
-		if mode == 0 {
-			res.state = "off"
-		} else {
-			if err := m.bcm.forceEEEOff(); err != nil {
-				return fail(res.item, err)
-			}
-			res.fixed = true
-			res.state = fmt.Sprintf("was %#04x, forced off", mode)
-			changed = true
-		}
 		out = append(out, res)

-		res = checkResult{item: names[i] + " jumbo"}
-		on, size, err := m.bcm.jumboState()
-		if err != nil {
+		res = checkResult{item: names[i] + " jumbo", state: "forced on"}
+		if err := m.bcm.forceJumbo(); err != nil {
 			return fail(res.item, err)
 		}
-		if on {
-			res.state = "on, " + size
-		} else {
-			if err := m.bcm.forceJumbo(); err != nil {
-				return fail(res.item, err)
-			}
-			res.fixed = true
-			res.state = "was off, forced on"
-			changed = true
-		}
 		out = append(out, res)

-		if changed {
-			if err := m.bcm.restartAN(); err != nil {
-				return fail(names[i]+" retrain", err)
-			}
-			retrained = true
+		if err := m.bcm.restartAN(); err != nil {
+			return fail(names[i]+" retrain", err)
 		}
 	}

-	res := checkResult{item: "link"}
+	res := checkResult{item: "link retrain"}
 	var took time.Duration
-	var up bool
-	var err error
-	if retrained {
-		res.item = "link retrain"
-		took, up, err = waitLink(mods, nil)
+	up := false
+	if hadLink {
+		took, up = waitCarrier(names, nil)
+	}
+	var adv [2]string
+	for i, m := range mods {
+		v, err := m.bcm.eeeAdvert()
 		if err != nil {
 			return fail(res.item, err)
 		}
-	} else {
-		up = true
-		for _, m := range mods {
-			v, err := m.bcm.linkUp()
-			if err != nil {
-				return fail(res.item, err)
-			}
-			up = up && v
+		adv[i] = fmt.Sprintf("%#04x", v)
+		if up && v != 0 {
+			res.err = fmt.Errorf("%s still advertises EEE %#04x", names[i], v)
 		}
 	}
 	if up {
-		var adv [2]string
-		for i, m := range mods {
-			v, err := m.bcm.eeeAdvert()
-			if err != nil {
-				return fail(res.item, err)
-			}
-			adv[i] = fmt.Sprintf("%#04x", v)
-			if v != 0 {
-				res.err = fmt.Errorf("%s still advertises EEE %#04x", names[i], v)
-			}
-		}
-		if retrained {
-			res.state = fmt.Sprintf("up in %.1fs, eee advert %s/%s", took.Seconds(), adv[0], adv[1])
-		} else {
-			res.state = fmt.Sprintf("up, eee advert %s/%s", adv[0], adv[1])
-		}
+		res.state = fmt.Sprintf("up in %.1fs, eee advert %s/%s", took.Seconds(), adv[0], adv[1])
 	} else {
-		res.state = "no link (cable unplugged?)"
+		res.state = fmt.Sprintf("no link (cable unplugged?), eee advert %s/%s", adv[0], adv[1])
 	}
 	return append(out, res)
 }