From 9d1f15045f5b91d5f1c51d3cece864a6e207b8d8 Mon Sep 17 00:00:00 2001 From: flamingcow Date: Thu, 13 Aug 2026 10:19:32 -0700 Subject: [PATCH] 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 --- docs/modules/fs/README.md | 15 +- docs/state.md | 2 +- main.go | 10 +- phy.go | 367 +++++++++++++++++++------------------- 4 files changed, 204 insertions(+), 190 deletions(-) diff --git a/docs/modules/fs/README.md b/docs/modules/fs/README.md index 1ee01b0..dd1d269 100644 --- a/docs/modules/fs/README.md +++ b/docs/modules/fs/README.md @@ -14,14 +14,18 @@ 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 Protocol and full verified catalog: [bcm84891l-mdio-commands.md](bcm84891l-mdio-commands.md). Traps: -- **SET commands execute stale DATA1** — the handler never clears DATA registers; write every parameter register explicitly before any SET. +- **SET commands execute stale DATA** — the handler never clears DATA registers; write every parameter register explicitly before any SET. +- **GETs must be invoked bare** — pre-writing *any* DATA register, not just the documented DATA1 display flag, leaves the handler executing as a no-op with results never written (sentinel pre-fills survived GET_SNR untouched, proven on hardware). +- **STATUS must never be written** — any user value (0x0000 and 0x0008 both tried) closes the mailbox: the next command is silently ignored until firmware restores it. +- **PASS cannot prove completion.** The previous command's PASS stays latched while a slow firmware still executes, and until then the DATA registers are firmware scratch — the die temperature turns up in them (0x004E–0x0051 observed as "SNR" and "EEE mode"; the campaign's "anomalous 0x0047" was this same race). cabletest reads results twice and requires agreement (DATA1 excluded — temperature lands there autonomously), and discards SNR samples below 15 dB absolute, far under anything a trained link produces and exactly where every garbage signature sits. +- **Handler writes during firmware-busy windows wedge the µC permanently** — DATA/CMD writes landing while firmware does post-AN provisioning or training work (a window stretching ~10 s past relink) killed the SMI service four times; reads alone never once. Every handler write — SETs, GET commands, the ECD trigger — runs only in a quiet window: carrier up and stable ≥ 10 s (`phy.go`). +- **Bridge reads carry no fetch identity** — a fetch that outruns the delay leaves the previous transaction's data at 0xAD with no error. cabletest brackets read batches with a known-answer canary (PHY ID 1.2 = 0x3590) and discards the batch on mismatch (`phy.go`). - Poll STATUS ~100 ms; frozen up to 2 s during 10GBASE-T training — only run after link-up. ## Proven diagnostics @@ -38,19 +42,19 @@ Protocol and full verified catalog: [bcm84891l-mdio-commands.md](bcm84891l-mdio- | Item | Command | State | |---|---|---| -| EEE / AutogrEEEn | 0x8008/0x8009 | Once read 0x0047 (10G AutogrEEEn variable latency + 5G/1G native — local-only, invisible in IEEE 7.60/7.61, which read 0); later reads 0, treated as the truth. **Forced all-off** via SET with explicit params `(0, 0, 0x7A12, 0x480, 0)` + AN restart, verified. cabletest checks 0x8008 and forces only on mismatch, verifying 7.60 reads 0 either way (`phy.go`) | +| EEE / AutogrEEEn | 0x8008/0x8009 | The famous 0x0047 GET reading was the die temperature in DATA1 (firmware scratch — see reliability notes), not an AutogrEEEn mode; GET 0x8008 is unusable. cabletest forces all-off every boot (SET with explicit params `(0, 0, 0x7A12, 0x480, 0)` + AN restart, proven) and verifies 7.60 reads 0 after relink (`phy.go`) | | EEE wire-truth | arm 0x801A after link-up, read 0x801B | Zero LPI events/duration on idle link; repeat under traffic | | Fast retrain | 0x800A (datasheet titles it EMI_MODE; description is fast retrain) | Enabled 10G/5G/2.5G; IEEE 1.147 = 0x0019, count bits zero. Keep enabled; read the 1.147 count per run — a marginal cable that fast-retrains still gets counted | | Pair map | 0x8000 | DATA2 = 0x00E4 = identity (A/B/C/D straight through) — MDI wiring verification works | | Fault pass-through | 0x800D | 0 = LF/RF pass to the line (0x8011/0x8010 could mask them as IDLE — leave off) | | Host interface | 0x802B/0x802D/0x8027 | XFI polarity normal; TX FIR = main tap 0x3C only; MAC/PHY frequency locked; KR/SyncE/USXGMII/WOL disabled; temp warnings off | -| Jumbo | 0x801C/0x801D | cabletest checks 0x801D and forces enable on mismatch (`phy.go`). The ODM firmware reads back the 9K size option regardless of the requested 10K — and 9018-byte wire frames pass at line rate with zero loss/corrupt, so its "9K" covers ≥ 9018 in practice | +| Jumbo | 0x801C/0x801D | GET 0x801D answers in DATA1 — scratch-prone, unusable — and bringup cannot assume a cable to probe through, so cabletest forces enable every boot (SET 0x801C + AN restart); the running mix's 9018-byte frames are the standing wire truth (`phy.go`). The FS at its 9K option passes them at line rate with zero loss/corrupt | | 1588 | 0x8004 | Disabled (engine registers undocumented — see asks below) | | Limited reach | 0x8006 | Disabled — its DATA3 "linked in LR mode" bit, the only handler-visible trace of the DSP length estimate, is unavailable without a config change | ## Firmware-reliability notes -GET returns are trusted: early exploration saw one anomalous DATA1 (0x0047 from the EEE GET, never reproduced) and die-temperature-like values in other commands' DATA1, treated as one-off noise. cabletest reads config before forcing it and corroborates EEE through 7.60 after any relink. The hard rule that stands: write every DATA register explicitly before any SET — the handler executes stale DATA. +**The firmware is its own mailbox client — the handler is a shared, unarbitrated resource.** Watched read-only on an idle module: CMD sits at 0x0031 (`CMD_GET_CURRENT_TEMP`, bit 15 consumed) and DATA1 tracks the live die temperature (0x4F–0x52 ≈ 79–82 °C under load), refreshed every ~3.5 s — the firmware issues its own temperature command through the same CMD/STATUS/DATA registers the host uses, with no arbitration. Every mystery this explains: temperature appearing in DATA1 (it is that command's output — the campaign's "anomalous 0x0047" included), PASS satisfying a host poll when it belongs to the internal command, and the µC wedges (two writers colliding on CMD/DATA, likeliest when internal management activity spikes after AN events). Host defenses in `phy.go`: quiet-window writes, double-read results (DATA1 excluded), a CMD readback proving the command was not interleaved, the PHY-ID canary, and the SNR floor. Any GET whose answer lives in DATA1 is unusable; corroborate through IEEE registers or the wire. The hard rule that stands: write every DATA register explicitly before any SET — the handler executes stale DATA. ## ECD — recovered from the OpenBCM SDK, proven on hardware @@ -65,6 +69,7 @@ The ECD register mechanism is absent from the handler catalog and the datasheet Observed on a plugged, linked, healthy cable: verdicts OK×4 and per-pair lengths of [45, 45, 41, 46] on a ~45 m cable — **meter-accurate with no calibration**, and this ECD reports length for healthy pairs, not just faults, resolving the terminated-far-end concern in [../README.md](../README.md). Caveats: - The run blips the link (PMA 1.1 latch-low catches a drop even with the break-link bit clear) — do not run mid-measurement until the disturbance is characterized. +- **An interrupted run can wedge the µC's SMI service** (observed after a process died mid-diag): the bridge ACKs I2C but serves 0x0000 for every register at any delay and silently drops MDIO writes — no in-band recovery, not even PMA reset — while the EEPROM path and the trained link keep working. Recovery: a driver unbind/rebind of the port (the SFP re-initialization resets the µC); a reseat would do the same. - Fault verdicts (open/short/inter-pair) are unexercised — deliberately: the product is a closed-loop tester, both ends always plugged. - Family constraints from the SDK: port must be enabled; unsupported at forced 100M. - `bcm_ecd_probe.py` in phydiag-work implements the recipe. diff --git a/docs/state.md b/docs/state.md index e28f70d..87ab7e4 100644 --- a/docs/state.md +++ b/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, 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. +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; handler writes confined to quiet windows — the µC wedges otherwise — with results double-read, canary-validated, and floor-checked — modules/fs/ for the full trap list): bringup identifies both modules and forces EEE off and jumbo on every boot — no trustworthy readback exists and no cable is guaranteed to probe through; 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 diff --git a/main.go b/main.go index 5205922..0fad486 100644 --- a/main.go +++ b/main.go @@ -750,9 +750,13 @@ func run(aName, bName string) (err error) { if diag.kick(&done) { fmt.Println(stats.rule("measuring cable")) } - case <-diag.completed: - info, _ := diag.snapshot() - fmt.Println(stats.rule("cable diag: " + cableLine(info))) + case err := <-diag.completed: + if err != nil { + fmt.Println(stats.rule("cable diag failed: " + err.Error())) + } else { + info, _ := diag.snapshot() + fmt.Println(stats.rule("cable diag: " + cableLine(info))) + } start = resetAll(dirs, modules, stats) case <-disp.fb.flips: now := time.Now() diff --git a/phy.go b/phy.go index 1948cd8..c12eb8c 100644 --- a/phy.go +++ b/phy.go @@ -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) }