From 358c287891847b1abb9a506c6ab5439d9d012bd6 Mon Sep 17 00:00:00 2001 From: ScottW514 Date: Wed, 2 Sep 2026 20:30:41 -0400 Subject: [PATCH] kernel.pic-pacing drill; BRINGUP item 10 and the record kernel.pic-pacing reads a coolant thermistor twice back to back, 300 pairs, with the module's pacing off (the control, reported) and on (the claim: the second read agrees with the first). The bench proof for the module's pic_gap_us pacing; the catalog counts 56 tests, 0 uncovered. BRINGUP item 10 and the facts bullet describe the pacing as it is; the CAMPAIGN-LOG entry records the change and its proof. --- docs/BRINGUP.md | 41 +++++++-------- docs/CAMPAIGN-LOG.md | 17 +++++++ forgetest/forgetest/suite/kernel.py | 79 +++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 20 deletions(-) diff --git a/docs/BRINGUP.md b/docs/BRINGUP.md index 2ddd323..a49a59f 100644 --- a/docs/BRINGUP.md +++ b/docs/BRINGUP.md @@ -1005,18 +1005,17 @@ is committed. within seconds and inflates the instant reading by a degree; the warm-up release therefore judges a one-minute rolling minimum of that reading. - **Coolant-ADC readings depend on the read pattern.** What the PIC returns - for a thermistor depends on how soon the read follows the previous PIC read - (measured 2026-09-02 on the two coolant channels): the second of a pair - issued within 0.1 ms comes back 6 to 8 counts high with a wide spread, - either sensor, either order; a pair 0.5 to 10 ms apart reads tight and a - steady 3 counts (about 0.2 C) above sparse reads; and concurrent readers - land such pairs at random, so a fast reader sees excursions of 10 to 15 - counts in a share of its samples that grows with the read rate (a third at - 100 Hz). The `aa-offset-calibrate` diagnostic reads its two sensors 31 ms - apart and reduces each window to an interquartile mean; the cooling engine - and `/status` still read the PIC back to back (the bias is inside the - gates' margins). A pacing of PIC reads in the kernel would give every reader - the same value (Next work). + for a thermistor depends on how soon the read follows the previous PIC + transaction (measured 2026-09-02 on the two coolant channels): the second + of a pair issued within 0.1 ms comes back 6 to 8 counts high with a wide + spread, either sensor, either order; a pair 0.5 to 10 ms apart reads tight + and a steady 3 counts (about 0.2 C) above sparse reads. The module paces + every PIC transaction (`pic_gap_us`, 1000 by default, a runtime-writable + parameter), so no reader lands a disturbed pair whatever the others do; + the `aa-offset-calibrate` diagnostic still reads its two sensors 31 ms + apart and reduces each window to an interquartile mean, which takes the + steady bias out of its edges. `kernel.pic-pacing` measures the pairs with + the pacing off and on. - **Coolant-ADC offsets around a lit tube.** The air-assist fan's return current rides a ground path the thermistor reference shares, so both coolant sensors read about 1.2 C low at the run duty (proportional to the fan's @@ -1388,14 +1387,16 @@ feature requests, enhancements) will eventually be tracked as GitHub issues. live-fire drill is gone; a coolant sensor unreadable for two ticks is the SENSOR verdict. -10. **PIC read pacing.** A PIC read within a fraction of a millisecond of - the previous one returns a disturbed value (facts bank, "Coolant-ADC - readings depend on the read pattern"). Serialize the PIC transactions in - the kernel module with a minimum spacing (a millisecond is enough by the - measurement), so every reader, the engine, `/status`, the diagnostics and - a bench sampler, sees the same value whatever the others do. A module - change: it rides the next image flash, with the coolant-reading tests - (`cooling.aa-offset-calibrate`, `cooling.flow-verify`) as its proof. +10. **PIC read pacing, done and waiting for the image.** The module paces + every PIC transaction at least `pic_gap_us` (1000, a runtime-writable + parameter) after the last one ended, so every reader, the engine, + `/status`, the diagnostics and a bench sampler, sees the same value + whatever the others do (facts bank, "Coolant-ADC readings depend on the + read pattern"). Host-proven by the module's -Werror cross-build; on the + bench, the new `kernel.pic-pacing` drill (300 back-to-back pairs with the + pacing off, then on) and the coolant-reading tests + (`cooling.aa-offset-calibrate`, `cooling.flow-verify`). Rides item 9's + image; the item closes with the campaign. 11. **The audit's deferred findings, done and waiting for the image.** All six are fixed and host-proven, and ride the local image item 9's campaign diff --git a/docs/CAMPAIGN-LOG.md b/docs/CAMPAIGN-LOG.md index 01db28d..21c5045 100644 --- a/docs/CAMPAIGN-LOG.md +++ b/docs/CAMPAIGN-LOG.md @@ -7428,6 +7428,23 @@ warnings. Owed: the operator flashes the dev image, takes a fresh-boot baseline, and runs the full campaign; then the push in CI order and the pin bumps. +## 2026-09-02: PIC transaction pacing in the module, host-proven + +The operator put item 10 on the campaign's image. The module now paces every +transaction with the sensor PIC: under the driver's lock, a transaction +waits until `pic_gap_us` (a new module parameter, 1000 microseconds by +default, writable at runtime) has passed since the last one ended, whoever +the reader is, so the cooling engine, `/status`, a diagnostic and a bench +sampler read the same value whatever the others do. The pacing wraps all +five transaction paths (single and range reads and writes, and the raw +write), the LED work and the dead-man safing included. Host proof: the +-Werror cross-build against the pinned kernel. Bench proof: the new +`kernel.pic-pacing` drill reads a coolant thermistor twice back to back, 300 +pairs, with the pacing off (the control, reported) and on (the claim: the +second read agrees with the first, mean within 2 counts, interquartile +within 3), and the coolant-reading tests. The diagnostic's spaced reads and +interquartile means stay: they take the steady bias out of its edges. + ## Reference notes ### Head-IRQ source validation — the beam-emission hypothesis diff --git a/forgetest/forgetest/suite/kernel.py b/forgetest/forgetest/suite/kernel.py index 64ae9f1..5b96cb9 100644 --- a/forgetest/forgetest/suite/kernel.py +++ b/forgetest/forgetest/suite/kernel.py @@ -806,3 +806,82 @@ def resume_lead(ctx): except OSError: pass ctx.log("safe state restored: state=%s latch=LOCKED", rd("cnc/state")) + + +# ---------------------------------------------------------------- PIC pacing + +PIC_GAP_PARAM = "/sys/module/glowforge/parameters/pic_gap_us" + + +def _param_read(path): + try: + with open(path) as f: + return f.read().strip() + except OSError: + return None + + +def _param_write(path, value): + with open(path, "w") as f: + f.write("%s\n" % value) + + +def _pair_stats(diffs): + s = sorted(diffs) + n = len(s) + return {"n": n, "mean": round(sum(s) / float(n), 2), "iqr": s[(3 * n) // 4] - s[n // 4], + "min": s[0], "max": s[-1]} + + +@test("kernel.pic-pacing", title="PIC transactions are paced a millisecond apart", + subsystem="kernel", kind="auto", est_min=1, + covers=_KERNEL_COVERS + [("forgectrl", "src/diag.c")], + description="What the PIC returns for a thermistor depends on how soon the read follows " + "the previous transaction: the second of a pair issued within a fraction of a " + "millisecond comes back high and wide. The module paces every transaction at " + "least pic_gap_us after the last one ended, so every reader sees the same value " + "whatever the others do. The drill reads a coolant thermistor twice back to back, " + "300 pairs, with the pacing off (the control, reported) and on (the claim): " + "paced, the second read agrees with the first.") +def pic_pacing(ctx): + ev = ctx.evidence + saved = _param_read(PIC_GAP_PARAM) + ctx.check(saved is not None, "the module has no pic_gap_us parameter (%s)", PIC_GAP_PARAM) + ev["pic_gap_us_before"] = saved + + def pairs(n): + diffs = [] + for i in range(n): + a = int(rd("pic/water_temp_1")) + b = int(rd("pic/water_temp_1")) + diffs.append(b - a) + if i % 100 == 99: + ctx.checkpoint() + return diffs + + try: + _param_write(PIC_GAP_PARAM, 0) + ctx.sleep(0.05) + control = _pair_stats(pairs(300)) + _param_write(PIC_GAP_PARAM, 1000) + ctx.sleep(0.05) + paced = _pair_stats(pairs(300)) + finally: + _param_write(PIC_GAP_PARAM, saved if saved not in (None, "0") else 1000) + ev["control"] = control + ev["paced"] = paced + ev["pic_gap_us_after"] = _param_read(PIC_GAP_PARAM) + ctx.log("control (no pacing): second minus first over %d pairs: mean %+.2f, interquartile %d, " + "range %+d..%+d", control["n"], control["mean"], control["iqr"], control["min"], control["max"]) + ctx.log("paced (1000 us): second minus first over %d pairs: mean %+.2f, interquartile %d, " + "range %+d..%+d", paced["n"], paced["mean"], paced["iqr"], paced["min"], paced["max"]) + ctx.check(abs(paced["mean"]) <= 2.0, + "paced pairs still differ by %+.2f counts on average", paced["mean"]) + ctx.check(paced["iqr"] <= 3, + "paced pairs still spread %d counts (interquartile)", paced["iqr"]) + ctx.check(paced["iqr"] <= control["iqr"] + 1 and abs(paced["mean"]) <= abs(control["mean"]) + 0.5, + "pacing did not tighten the pairs: control mean %+.2f iqr %d, paced mean %+.2f iqr %d", + control["mean"], control["iqr"], paced["mean"], paced["iqr"]) + ctx.check(ev["pic_gap_us_after"] == "1000", "pic_gap_us left at %s", ev["pic_gap_us_after"]) + ctx.log("PASS: paced pairs agree (mean %+.2f, interquartile %d); control mean %+.2f, interquartile %d", + paced["mean"], paced["iqr"], control["mean"], control["iqr"])