Fixes review comments.

exi_bba/synth.py

@@ -8,7 +8,9 @@ This file re-declares IceBreakerPlatform inline so that importing
 rebbarb/rebbarb.py (which has a module-level platform.build() call) is avoided.
 """
 
+import glob
 import os
+import re
 import subprocess
 import sys
 
@@ -42,8 +44,10 @@ from exi_bba.bba_top import BBATop
 # total (per icestorm's icebox.py pin database), and every other one is
 # already assigned to a real signal on this board — pin 13 shows up in the
 # schematic netlist as an unrouted U9 pad, nothing else was available.
-# Board is still in design, so PIN 13 ON THE SCHEMATIC/PCB MUST BE REWORKED
-# to carry ETH_INT instead of pin 35 before layout is final.
+# DONE on the board: the schematic + PCB now carry /ethernet/ETH_INT on U9
+# pin 13, and pin 35 is left unconnected (net
+# "unconnected-(U9A-IOT_46b_G0-Pad35)") so the PLL hard macro can claim it.
+# Verified against the schematic netlist 2026-07-31 — do not "restore" pin 35.
 #
 # Debug (J4 header, unpopulated): DBG0=9 DBG1=10 DBG2=11 DBG3=12 DBG4=6
 # UART (FT2232H channel B): net UART_TXD is the FTDI's OUTPUT, so it is the
@@ -205,63 +209,137 @@ if __name__ == "__main__":
     n_seeds   = next((int(sys.argv[i+1]) for i, a in enumerate(sys.argv)
                       if a == "--seeds"), 1)
 
-    platform = IceBreakerPlatform()
-    print(f"Synthesizing BBATop for {platform.device}-{platform.package}  "
+    print(f"Synthesizing BBATop for {IceBreakerPlatform.device}-"
+          f"{IceBreakerPlatform.package}  "
           f"(do_program={do_flash}, seeds=1..{n_seeds})")
 
     best_seed = 1
     best_fmax = 0.0
+    results = []          # (seed, fmax_clk, fmax_capture, verdict), scored only
     for seed in range(1, n_seeds + 1):
         print(f"\n{'='*60}")
         print(f"  Seed {seed}/{n_seeds}")
         print(f"{'='*60}")
         opts = (f"--opt-timing --seed {seed} --timing-allow-fail")
+
+        # A Platform instance can only be built ONCE — amaranth's
+        # TemplatedPlatform.prepare() does `assert not self._prepared`. Reusing
+        # one platform across the sweep made seeds 2..N raise a bare
+        # AssertionError (empty message, so the handler below printed nothing),
+        # after which the fmax parser re-read the PREVIOUS seed's build/top.tim
+        # and reported identical numbers for every seed — a sweep that looked
+        # like it ran but never did. Build a fresh platform per seed.
+        platform = IceBreakerPlatform()
+
+        # Each seed gets its OWN build directory. Sharing one `build/top.tim`
+        # across the sweep is not safe on this workspace: /workspace is a WSL2
+        # drvfs (Windows drive) mount, and a re-read of a file just rewritten
+        # by a child process can return stale or partially-flushed content.
+        # That silently mis-scored the sweep — seeds were credited with other
+        # seeds' numbers, including nextpnr's PRE-routing placement estimates
+        # (which run ~8 MHz optimistic), so a 53.08 MHz FAIL got reported as a
+        # 61.94 MHz PASS. Deleting the stale file first did NOT fix it; only
+        # not sharing the path does. Do not "simplify" this back to build/.
+        build_dir = f"build/seed{seed}"
+        build_ok = True
+
         try:
             platform.build(BBATopSynth(status_panel=True, uart_console=True), do_program=False,
-                           verbose=True, nextpnr_opts=opts)
+                           verbose=True, nextpnr_opts=opts, build_dir=build_dir)
         except Exception as exc:
             # nextpnr exits non-zero even with --timing-allow-fail on some
             # versions; treat as non-fatal timing failure.
-            print(f"  [seed {seed}] build exception (timing?): {exc}")
+            build_ok = False
+            print(f"  [seed {seed}] build exception (timing?): "
+                  f"{type(exc).__name__}: {exc}")
 
         # Parse fmax from nextpnr log in build/top.tim (if present). Domain
         # names are 'clk' (exi/sync, 24 MHz target) and 'capture_clk'
         # (54 MHz target, the tighter constraint) — NOT 'exi', which never
         # matched any real log line (this regex silently reported 0.0 MHz
         # for every seed until fixed).
-        import glob, re
-        tim_files = glob.glob("build/top.tim") + glob.glob("build/*.tim")
-        fmax_clk = 0.0
-        fmax_capture = 0.0
-        for tf in tim_files:
+        # nextpnr reports each domain TWICE: once post-placement (an estimate)
+        # and once post-routing. Only the post-route number is real, so take
+        # the LAST occurrence of each domain — never the first.
+        #
+        # A seed is only scored if BOTH domains were actually found. Defaulting
+        # a missing value to 0.0 and scoring it anyway is how this script once
+        # declared "Best seed: 8 ... PASS" while seed 8 in fact failed at
+        # 49.66 MHz: unparsed seeds silently mixed with real ones. An
+        # unparseable seed must be reported and skipped, never ranked.
+        fmax = {}
+        for tf in sorted(set(glob.glob(f"{build_dir}/*.tim"))):
             try:
-                with open(tf) as f:
+                # errors="replace": a decode hiccup must not abort the parse
+                # and leave the seed looking like a legitimate 0.0 MHz result.
+                with open(tf, errors="replace") as f:
                     for line in f:
                         m_ = re.search(
                             r"Max frequency for clock\s+'(\w+)':\s*([\d.]+)\s*MHz", line)
                         if m_:
-                            domain, freq = m_.group(1), float(m_.group(2))
-                            if domain == "clk":
-                                fmax_clk = freq
-                            elif domain == "capture_clk":
-                                fmax_capture = freq
-            except OSError:
-                pass
+                            fmax[m_.group(1)] = float(m_.group(2))
+            except OSError as exc:
+                print(f"  [seed {seed}] could not read {tf}: {exc}")
+
+        if "clk" not in fmax or "capture_clk" not in fmax:
+            print(f"  [seed {seed}] NO USABLE TIMING REPORT "
+                  f"(found {sorted(fmax) or 'nothing'}) — NOT SCORED"
+                  f"{'' if build_ok else '; build also reported an error'}")
+            continue
+
+        fmax_clk, fmax_capture = fmax["clk"], fmax["capture_clk"]
+        verdict = ("PASS" if fmax_capture >= 54.02 and fmax_clk >= 24.0
+                   else "FAIL")
+        results.append((seed, fmax_clk, fmax_capture, verdict))
         print(f"  [seed {seed}] clk fmax: {fmax_clk:.2f} MHz (target 24)  "
-              f"capture_clk fmax: {fmax_capture:.2f} MHz (target 54.02)")
+              f"capture_clk fmax: {fmax_capture:.2f} MHz (target 54.02)  "
+              f"{verdict}"
+              f"{'' if build_ok else '  [build reported an error]'}")
         # capture_clk is the binding constraint (tighter target, historically
         # the one that swings ±20% with seed) — rank seeds by it.
         if fmax_capture > best_fmax:
             best_fmax = fmax_capture
             best_seed = seed
 
-    print(f"\nBest seed: {best_seed}  capture_clk fmax: {best_fmax:.2f} MHz "
-          f"(target 54.02) — {'PASS' if best_fmax >= 54.02 else 'FAIL'}")
+    # Per-seed summary. capture_clk swings hard with seed on this design, so
+    # the pass RATE matters as much as the best number — a design that only
+    # closes on a minority of seeds has no real margin.
+    print(f"\n{'='*60}")
+    print(f"  Seed sweep summary ({len(results)}/{n_seeds} seeds scored)")
+    print(f"{'='*60}")
+    print(f"  {'seed':>4}  {'clk (≥24)':>10}  {'capture_clk (≥54.02)':>21}  verdict")
+    for seed, fc, fcap, verdict in results:
+        print(f"  {seed:>4}  {fc:>10.2f}  {fcap:>21.2f}  {verdict}")
+    n_pass = sum(1 for *_, v in results if v == "PASS")
+    if results:
+        print(f"\n  passing seeds: {n_pass}/{len(results)} "
+              f"({[s for s, *_, v in results if v == 'PASS']})")
 
-    if do_flash:
+    overall = "PASS" if best_fmax >= 54.02 else "FAIL"
+    print(f"\nBest seed: {best_seed}  capture_clk fmax: {best_fmax:.2f} MHz "
+          f"(target 54.02) — {overall}")
+    if overall == "PASS" and n_pass * 2 < len(results):
+        print("  NOTE: a MINORITY of seeds close timing. The bitstream from "
+              "the best seed is usable, but this design has little margin — "
+              "treat any logic addition as likely to break timing.")
+
+    # build/ now holds one subdirectory per seed; the flashable bitstream for
+    # the best seed is build/seed<N>/top.bin. There is no top-level build/top.bin.
+    if results:
+        print(f"\nBitstream for best seed: build/seed{best_seed}/top.bin")
+
+    if do_flash and overall == "FAIL":
+        print("\nREFUSING TO FLASH: no seed met the capture-domain timing "
+              "constraint. The EXI front-end samples a 27 MHz clock and will "
+              "miss bits. Re-run with more seeds (--seeds 16) or reduce logic.")
+    elif do_flash:
         print(f"\nFlashing with seed {best_seed}...")
         opts = f"--opt-timing --seed {best_seed} --timing-allow-fail"
-        platform.build(BBATopSynth(status_panel=True, uart_console=True), do_program=True,
-                       verbose=True, nextpnr_opts=opts)
+        # Fresh platform again — the sweep above already consumed one per seed.
+        # Reuse the best seed's own build dir so the flashed bitstream is the
+        # one that was actually measured.
+        IceBreakerPlatform().build(
+            BBATopSynth(status_panel=True, uart_console=True), do_program=True,
+            verbose=True, nextpnr_opts=opts, build_dir=f"build/seed{best_seed}")
 
     print("Done.")