From 91c3bf0ece3116cc7b6f86f105df85d1ae57387f Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Sat, 3 Oct 2026 17:10:11 +0200 Subject: [PATCH] Clock the EXI bit engine from the EXI pin: timing closes on all 8 seeds Replaces the PLL-fed 54 MHz `capture` domain, which oversampled the GC's 27 MHz EXI clock 2x and so put BIT-level timing across a CDC boundary: the engine's view of the clock lagged the pin by 2 FFSynchronizer stages plus a register (~55 ns) against a 37 ns bit time, so MISO came out ~2 bits late and the GC read 0x17 as 05 C0. No amount of oversampling fixes that -- 54 MHz gives only 2 ticks per bit and the part cannot close the 81 MHz that 3x needs. The engine is now clocked by the pin itself (pin 44 = global buffer input net 6, no board change), making MISO clock-to-Q. ExiCapture creates four domains: exiclk (posedge, samples MOSI), exiclk_n (negedge, drives MISO), both async-reset by CS, plus reset-less exififo_w/exififo_r for the byte FIFO sides -- reset-less because resetting one side of an AsyncFIFO desynchronises its gray pointers, and posedge because AsyncFIFO has a posedge-only reset CDC internally and cannot host a negedge side. Also removes the SPRAM response FIFO. The DMA read was one byte late because the round trip did not fit the protocol's turnaround byte; the arbiter already exposed valid/ready, so the FIFO bought a cycle of latency and nothing else. The response is handled outside the FSM (the reply lands in RHDR_PAD, not SPRAM_STREAM) with a one-entry hold for when tx_fifo is momentarily full. The PLL was left instantiated but unused after the rewrite; removing it frees the hard macro and pin 35 (which stays unconnected only because the board is already built that way). This is what lifted the pass rate: domain target before after clk (sync) 24 MHz 28.1-30.2 26.4-32.2 8/8 capture_clk 54 MHz 2 of 8 PASS (gone) EXI pin domain 27 MHz -- 45.6-64.4 8/8 Best seed 3: clk 32.18 MHz, EXI 55.86 MHz. The binding constraint has FLIPPED -- `clk` (1.10-1.34x) is now tighter than the EXI domain (1.69-2.39x) -- so synth.py ranks seeds by worst RELATIVE margin, not by the EXI number. synth.py could not have scored this design at all: it required the vanished capture_clk, and its regex '(\w+)' cannot match nextpnr's exi_clk_$glb_clk (no $ in \w). add_clock_constraint was also landing on the wrong net -- under EXI_J4 the clock is the AND of two pads, not either pad -- so nextpnr silently fell back to a 12 MHz default and under-optimised the whole EXI domain. Testbenches: - spi_mode3_slave had lost its testbench entirely in the rewrite. Restored with a strict Mode 3 master (changes MOSI on the falling edge, samples MISO in the clock-low phase) and MUTATION-TESTED: flipping either clock edge in the RTL makes it fail. Both of those bugs previously reached silicon against a lax master that parked MOSI well before the edge. - exi_j4 T2/T3 returned [0,4,2,2]. Real bug: the engine latches the FIFO head at each byte boundary, so a 2-byte command header leaves only ONE drain slot. DEVID streamed zeros until CS and left two stale bytes -- one too many -- so the NEXT enumeration came back a slot late. DEVID now pushes exactly four bytes and stops. Register-space reads may still over-push: their 4-byte header gives three drain slots. - Dead ports removed from SPIMode3Slave (hdr_bytes, frame_start, cs_active, byte_idx) and the mode2 parameter is gone -- there is one correct edge convention and it is now enforced by tests. All 14 module suites pass, plus bba_top at EXI_HALF=1/2/8. CLAUDE.md: rewrote the clocking, CDC inventory and module tables; marked the 2026-09-30 capture_clk measurements historical; and corrected the line in "Critical Protocol Notes" that still stated the INVERTED edge convention ("MOSI sampled on falling, MISO driven on rising" is Mode 2) -- the original implementation followed that doc error. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 427 ++++++++++++++++++----------------- exi_bba/bba_register_file.py | 113 +++++---- exi_bba/bba_top.py | 75 +++--- exi_bba/exi_capture.py | 125 ++++++---- exi_bba/exi_j4.py | 2 +- exi_bba/spi_mode3_slave.py | 154 +++++++++++-- exi_bba/synth.py | 123 ++++++---- 7 files changed, 611 insertions(+), 408 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6862be8..43956fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,8 +50,9 @@ python rebbarb/rebbarb.py Runs synthesis (yosys), place-and-route (nextpnr-ice40), and flashes via `iceprog`. Set `ICEPROG=/path/to/iceprog` env var to override the binary location. Note: `rebbarb/rebbarb.py` builds a 36 MHz LED blink demo. The BBA -implementation (`exi_bba/`) uses a split-domain clock: `capture` @ 54 MHz (PLL) -for the SPI bit engine, `exi`/`sync` @ 24 MHz (HFOSC) for everything else. +implementation (`exi_bba/`) clocks the SPI bit engine from the GC's EXI clock +pin itself and everything else from `sync` @ 24 MHz (internal HFOSC); there is +no PLL. Synthesize/flash the real design with `python -m exi_bba.synth [--flash]`. **Run a simulation:** @@ -68,89 +69,54 @@ python examples/icebreaker_fifo.py # iCEbreaker-specific FIFO (Verilog dump) Open VCD output with `gtkwave`. Simulations are the primary testing mechanism — there is no separate test runner. -**Verify PLL parameters:** -```bash -icepll -i 12 -o 54 # confirms DIVR=0 DIVF=71 DIVQ=4 → 54 MHz (capture domain) -``` -(`exi`/`sync` come from the internal SB_HFOSC ÷2 = 24 MHz — no PLL.) +**Clock sources:** there is no PLL and no external crystal any more. `sync` +comes from the internal `SB_HFOSC` ÷2 = 24 MHz; the EXI bit engine is clocked +by the GC's own EXI clock pin. Pin 35 (the old 12 MHz input) and the PLL hard +macro are both free. --- ## Current Implementation State The `exi_bba/` module tree is **fully implemented** with simulation testbenches. -All modules elaborate without errors and pass their unit tests. The full design -synthesizes, places and routes on the iCE40UP5K, but **capture-domain timing is -seed-dependent and only closes on a minority of seeds** — you MUST sweep. +All modules elaborate without errors and pass their unit tests, including +`bba_top` at the real 27 MHz EXI rate (`EXI_HALF=1`). -Measured 2026-08, DEFAULT build (`BBATopSynth(status_panel=True, -uart_shell=True)` — the interactive UART shell + socket-3 UDP test, ~67% LC), -`python -m exi_bba.synth --seeds 8`: +**Timing closes on EVERY seed (2026-10-03).** Measured with `EXI_J4=1` + +socket-3 UDP, `python -m exi_bba.synth --seeds 8`, 68% LC: | domain | target | result | |---|---|---| -| `clk` (exi/sync) | 24 MHz | 27–30 MHz — **PASS on every seed** | -| `capture_clk` | 54.02 MHz | 49.6–58.4 MHz — **PASS on 4 of 8 seeds** (3, 4, 7, 8) | +| `clk` (sync, HFOSC) | 24 MHz | 26.4–32.2 MHz — **PASS on all 8 seeds** | +| EXI pin domain | 27 MHz | 45.6–64.4 MHz — **PASS on all 8 seeds** | -Best seed 4 = 58.36 MHz, margin +4.3 MHz (8%). Still seed-dependent — `python -m -exi_bba.synth` with no arguments (seed 1) FAILS capture (50.96 MHz); always pass -`--seeds 8` (or more) and flash the reported best seed from -`build/seed/top.bin`. +Best seed **3**: `clk` 32.18 MHz, EXI 55.86 MHz (worst relative margin 1.34×). +Flash `build/seed/top.bin`. -**Capture-domain critical-path fix (2026-08):** capture used to close on only -2 of 8 seeds at ~55 MHz *without* the shell. The binding path was the TX byte -FIFO's read-enable gated by its own gray-coded ready -(`tx_fifo.r_en = ... | (flushing & r_rdy)`) — a `consume_ptr → gray → r_rdy → -flush → r_en → consume_ptr` loop in `exi_capture.py`. Replacing the r_rdy-based -"drain until empty" flush with a FIXED-length drain counter (the FIFO is only -`tx_depth` deep) removed the pointer feedback from `r_en`, dropping the path -from 24.3 → 19.6 ns and lifting capture to 4/8 passing (best 58.4) EVEN with the -shell integrated. **Do not re-introduce any `tx_fifo.r_rdy` dependence into -`r_en` or the flush deassert** — see the comment in `exi_capture.py`. +This replaces the long-standing "closes on a minority of seeds" problem, which +was entirely an artefact of the PLL-fed 54 MHz `capture` domain. Removing that +domain (the bit engine is now clocked by the EXI pin — see "Key Architecture +Decisions") took the pass rate from **2 of 8 to 8 of 8** and made the old +capture-domain tuning levers moot: the Gray-code CDC loop that was the binding +path, the pin co-location experiment, and the proposed ping-pong TX CDC are all +obsolete. Do not reintroduce them. -**Remaining capture bottleneck + future levers (measured 2026-08, best seed):** -After the flush fix the binding capture path is the **`tx_fifo` (exi→capture) -consume-pointer Gray-code CDC loop**, triggered by `spi.tx_load`: -`tx_load → consume_r_gry → consume_r_bin (gray→bin) → increment → -consume_r_gry (bin→gray) → setup`. On the board's V1 pinout it is ~6.4 ns -LOGIC + ~10.7 ns ROUTING = 17.1 ns (58.4 MHz). Two independent levers, if more -capture margin (e.g. a 64 MHz target) is ever wanted — NOT done, board V1 ships -as-is at 4/8-seed full speed: -- **Lightweight TX CDC (RTL, pinout-independent — the reliable lever):** the TX - path carries one response byte per SPI byte-time (~3.4 MHz) with huge slack, - yet pays for a full Gray-code multi-bit-pointer AsyncFIFO. Replacing `tx_fifo` - with a **2-deep ping-pong CDC** (two byte regs + 1-bit toggle handshakes, - `ready = write_sync ^ read_toggle`) makes consume a single toggle flip + mux, - cutting the ~6.4 ns pointer logic to ~2–3 ns AND shrinking the cell count - (tighter routing). Est. ~67–80 MHz — comfortably past 64. CRITICAL PATH, - handle with care: `tx_fifo` feeds MISO live and the register file prefetches - into it (≤4 in flight for DMA reads), so the ping-pong needs adequate depth + - full ExiCapture/BBATop CDC re-verification (a bad handshake mis-enumerates). -- **Pin co-location (respin-only, routing-half — ~+5 MHz):** on V1 the EXI pins - are on the BOTTOM die edge (y0) and the W5100 parallel bus is on the TOP - (y31), so the capture and exi/sync domains anchor to opposite edges and the - CDC FIFO spans the die vertically (that IS the ~10.7 ns routing). Putting the - EXI + W5100 pins on the SAME edge co-locates the domains: a confounded test - (EXI moved to top) hit 63.2 MHz best (routing 10.7→9.5 ns; logic unchanged). - Only moves the routing half and needs a full re-pinout (SG48 lacks free - top-edge pins for a clean version) — so it is a V2 consideration, not a fix. - (A simple 2-pin LED↔MISO/MOSI swap does NOT help — the EXI pins are already - clustered on the bottom edge; that swap tested WORSE, 58.4→48.8.) +⚠️ **The binding constraint has FLIPPED.** `clk` (sync, 1.10–1.34× margin) is +now tighter than the EXI domain (1.69–2.39×). Rank seeds by **worst relative +margin**, not by the EXI number — `synth.py` does this. Added logic now +threatens `sync`, not the EXI front-end. -There is little margin: assume added logic in (or near) the capture domain may -break capture timing until a sweep proves otherwise. The earlier "~70 MHz, both PASS" figure in this -file was wrong — it came from a sweep that silently never ran (see the -`_prepared` note in `synth.py`) and from misreading nextpnr's PRE-routing -placement estimate, which runs ~8 MHz optimistic. +⚠️ **Still sweep.** `clk` at 26.41 MHz on seed 5 is only 10% over target, so a +seed can still land uncomfortably close even though all 8 pass today. ### `exi_bba/` module status | Module | File | Tests pass | |---|---|---| -| `BBATop` | `exi_bba/bba_top.py` | ✅ EXI integration + full W5100→SPRAM→GC RX loop; synth PASS | +| `BBATop` | `exi_bba/bba_top.py` | ✅ EXI integration + full W5100→SPRAM→GC RX loop, at `EXI_HALF=1/2/8`; synth PASS | | `ExiCapture` | `exi_bba/exi_capture.py` | ✅ rx/tx byte-stream + over-push/flush | -| `SPIMode3Slave` | `exi_bba/spi_mode3_slave.py` | ✅ 4 tests (live-drive TX) | -| `BBARegisterFile` | `exi_bba/bba_register_file.py` | ✅ 7 tests (proactive push + DMA stream) | +| `SPIMode3Slave` | `exi_bba/spi_mode3_slave.py` | ✅ 7 checks (rx/tx edges, tx_load, MISO idle) — **mutation-tested**: flipping either clock edge fails it | +| `BBARegisterFile` | `exi_bba/bba_register_file.py` | ✅ 7 tests (real 4-byte header, proactive push + DMA stream) | | `SPRAMArbiter` | `exi_bba/spram_arbiter.py` | ✅ 3 tests | | `RXFrameAssembler` | `exi_bba/rx_frame_assembler.py` | ✅ 3 tests | | `TXFrameDrain` | `exi_bba/tx_frame_drain.py` | ✅ 2 tests | @@ -194,6 +160,8 @@ python -m exi_bba.uart_console # 7 tests: event log + 'r' reinit python -m exi_bba.uart_shell # 8 tests: shell cmds + reply/timeout python -m exi_bba.eeprom_model python -m exi_bba.bba_top # end-to-end EXI integration test (W5100 RX loop) +EXI_HALF=1 python -m exi_bba.bba_top # ...at the REAL 27 MHz rate — always do this +python -m exi_bba.exi_j4 # EXI_J4 pad-merge truth table + enumeration ``` ### UART shell + UDP bring-up test (default flash build) @@ -228,66 +196,53 @@ RX + 2 KB TX buffer at reset (`RMSR/TMSR=0x55`), so socket-3 frames are stored in the chip independently — no extra FPGA buffer needed. Register addresses are still datasheet-from-memory — **confirm at hardware bring-up**. -### ⚠️ The simulation clocks EXI 8× slower than hardware +### ⚠️ Simulation rate — always re-run at the hardware rate -`bba_top`'s testbench derives the EXI clock from `HALF` (capture ticks per SPI -half-period): **EXI = 54 MHz / (2 × HALF)**. The default `HALF=8` gives -**3.4 MHz** — eight times slower than the real GC's ~27 MHz, so the suite -passes green on designs that fail outright on a console. `HALF` is -`EXI_HALF`-configurable; **run `EXI_HALF=1 python -m exi_bba.bba_top` before -trusting any timing-related change.** +`bba_top`'s testbench clocks EXI at `27 MHz / EXI_HALF`. The default +`EXI_HALF=8` gives **3.4 MHz** — eight times slower than a real GC — which is +fast to simulate but hides every latency bug. **Run `EXI_HALF=1 python -m +exi_bba.bba_top` before trusting any timing-related change.** The current +design passes at `EXI_HALF=1`, `2` and `8`. -Worse, **every testbench drives MOSI many capture ticks before toggling the -clock**, so MOSI is stable at BOTH edges and the sampling-edge convention is -structurally undetectable. That is how a Mode 2/Mode 3 mix-up survived to -silicon with a fully green suite. `bba_top`'s master now models a true Mode 3 -master (drive on falling, sample on rising). +Historically **every testbench drove MOSI many ticks before toggling the +clock**, so MOSI was stable at BOTH edges and the sampling-edge convention was +structurally undetectable — that is how a Mode 2/Mode 3 mix-up reached silicon +with a fully green suite. Both `bba_top` and `spi_mode3_slave` now model a +strict Mode 3 master (change MOSI on the falling edge, sample MISO in the +clock-low phase), and `spi_mode3_slave`'s testbench is **mutation-tested**: +flipping either edge in the RTL makes it fail. -### ✅ EXI edge convention — FIXED 2026-10-03 (was Mode 2, must be Mode 3) +### ✅ FIXED 2026-10-03 — edge convention, and MISO latency -The notes below once said "Mode 3 (CPOL=1, CPHA=1)" and then described -sampling MOSI on the **falling** edge. Idle-high + sample-on-falling is -**Mode 2**. Real Mode 3 samples on the second edge of each cycle — the -**rising** one — and that is what the GameCube does: it DRIVES MOSI on the -falling edge, so sampling there latched the previous bit. +Two bugs, both now resolved by the pin-clocked rewrite: -Symptom on console: every byte one bit late in both directions, at every clock -speed. The `0x80` register-space header byte read as `0x00`, which decodes as a -command-space device-ID query, so NWAYS reads returned the device ID and the -speed sweep failed at every index. Raw 8-slot dump, first transaction after an -FPGA reset: `00 00 02 01 01 00 00 00` = `04 02 02 00 >> 1`. +**1. Wrong sampling edge (was Mode 2).** These notes once said "Mode 3 +(CPOL=1, CPHA=1)" and then described sampling MOSI on the **falling** edge. +Idle-high + sample-on-falling is **Mode 2**. Real Mode 3 samples on the second +edge — the **rising** one — and that is what the GameCube does: it DRIVES MOSI +on the falling edge, so sampling there latched the previous bit. Symptom: +every byte one bit late in both directions at every clock speed; the `0x80` +register-space header byte read as `0x00`, which decodes as a command-space +device-ID query, so NWAYS reads returned the device ID. Raw 8-slot dump after +an FPGA reset: `00 00 02 01 01 00 00 00` = `04 02 02 00 >> 1`. -`SPIMode3Slave(mode2=False)` (the default) now samples MOSI on RISING. -`mode2=True` restores the old behaviour, which is what the Pi/J4 probe scripts -were written against. +**2. MISO ~2 bits late at 27 MHz.** With MOSI fixed, register reads were +correct up to 4 MHz and failed from 8 MHz (hardware sweep: `ok=[1,1,1,0,0,0]`); +at index 5 a NWAYS read returned `05 C0` = `0x17 >> 2`. The MISO path was +delayed ~3 capture ticks ≈ 55 ns — 2 ticks of input `FFSynchronizer` on EXI CLK +plus 1 to register the shift — against a 37 ns bit time. -### ⚠️ OPEN: MISO output is ~2 bits late at 27 MHz +**Lowering the EXI clock was never an option:** Swiss hardcodes +`EXI_SPEED_32MHZ` in `exi_select()` (`cube/patches/bba/bba.c`). -With MOSI fixed, register reads are **correct up to 4 MHz** and fail from -8 MHz (hardware speed sweep 2026-10-03: `ok=[1,1,1,0,0,0]`, highest good -index 2). At index 5 a NWAYS read returns `05 C0` = `0x17 >> 2`. - -Cause: the MISO output path is delayed by roughly **3 capture ticks ≈ 55 ns** — -2 ticks of input `FFSynchronizer` on EXI CLK plus 1 tick to register the shift. -At 27 MHz one bit is 37 ns, so the master samples ~2 bits behind. The failure -threshold (pass at 250 ns/bit, fail at 125 ns/bit) brackets the delay at -60–125 ns, consistent with that structure. - -**Lowering the EXI clock is NOT an option:** Swiss hardcodes -`EXI_SPEED_32MHZ` in `exi_select()` (`cube/patches/bba/bba.c`) — there is no -setting to turn down. - -Options, none yet done: -1. **Clock the bit engine from the EXI clock itself** (route the pin to a - global buffer and make it a real clock domain) — the textbook SPI-slave - architecture, removes oversampling and the synchroniser entirely. Biggest - change, but the only one that is structurally correct rather than tuned. -2. **Output-bit lookahead** — drive the bit the master will sample ~2 bit-times - from now. Deterministic compensation for a deterministic delay, but it needs - lookahead into the next byte at byte boundaries, and the constant may not - hold across parts/temperature. -3. **Raise the capture clock** — 81 MHz would give 3× oversampling, but the - design's best measured fmax is 67 MHz, so it will not close. +**The fix was structural, not compensatory:** clock the bit engine from the EXI +clock pin itself (pin 44 sits at die (6,0) = global buffer input net 6, so it +drives a real clock network with no board change). This removes oversampling +and the synchroniser entirely, making MISO clock-to-Q. The two alternatives +were rejected: output-bit lookahead would have *tuned* a constant that may not +hold across parts and temperature, and 3× oversampling needs 81 MHz, which this +part cannot close. `SPIMode3Slave` has no `mode2` parameter any more — there is +only one correct convention and it is enforced by mutation tests. ### EXI on the J4 debug header (`EXI_J4=1`) — bench host without SP1 @@ -316,9 +271,17 @@ aimed at a J4 pad. **Why merge rather than repin:** all five J4 pads are on the SAME bottom die edge as the EXI pads (x=13–18 vs x=5–9, both y=0 per icestorm's `pinloc_db` -`5k-sg48`), so the capture domain keeps its bottom-edge anchor. Repinning would -have perturbed the one placement variable the pin-co-location experiment showed -is worth ±5 MHz — see "Remaining capture bottleneck" above. +`5k-sg48`), so the EXI clock domain keeps its bottom-edge anchor. Repinning +would have perturbed the one placement variable that the pin-co-location +experiment showed was worth ±5 MHz. (That experiment mattered when the +PLL-fed capture domain was the binding constraint; with ~2.4× margin on the +EXI domain today it is no longer a concern either way.) + +⚠️ **The merge changes which net is the clock.** On a plain build the clock is +the pad itself; with `EXI_J4=1` it is the AND of the two pads, a distinct net. +`synth.py` constrains whichever one applies — constraining the pad on a J4 +build misses, and nextpnr then silently falls back to a 12 MHz default and +under-optimises the whole EXI domain. **HARDWARE BRING-UP 2026-09-30: the EXI capture path is PROVEN on silicon.** A Raspberry Pi 4 bit-banging EXI into J4 (EXI_J4 build, seed 8) enumerated the @@ -388,23 +351,29 @@ over EXI to TXDATA and triggered via the NCRA ST bits was captured on a PC: PAR0–5 source MAC and the ARP ethertype, all byte-perfect. So EXI → BBARegisterFile → tx_bytes → TXFrameDrain → W5100 → PHY works. -⚠️ **OPEN BUG: TX truncates at exactly 16 bytes = the `tx_bytes` FIFO depth.** -Past byte 16 the captured frame restarts from its own beginning and mixes in -stale bytes from earlier attempts. Cause: `TXFrameDrain` sits in IDLE until it -has a LENGTH, and the length only reaches `tx_ctrl` when the NCRA ST bits are -written — i.e. *after* the GC has streamed the whole frame. So the entire frame -has to fit in a 16-deep FIFO, and `BBARegisterFile` silently drops bytes when -`tx_bytes.w_rdy` is low (SPI cannot back-pressure). Simulation never caught it: -`TXFrameDrain`'s own tests drive the FIFO signals directly, and the `BBATop` -test only exercises the W5100→SPRAM→GC **RX** loop. -Fix direction (needs design + sim, NOT done): make the drain start as soon as -data is available rather than waiting for the length — e.g. push the length into -`tx_ctrl` when TWD_HI (0x35) is written, which the GC sets before streaming, so -the drain consumes concurrently with the EXI writes and the 16-deep FIFO becomes -a rate-matching buffer instead of whole-frame storage. Alternatively give TX an -SPRAM ring like RX. -When probing this, **NCRA-reset between every TX attempt**: a failed attempt -leaves the drain waiting on a stale length, and later writes feed that pending +✅ **FIXED 2026-09-30: TX used to truncate at exactly 16 bytes** = the +`tx_bytes` FIFO depth. Past byte 16 the captured frame restarted from its own +beginning and mixed in stale bytes from earlier attempts. Cause: +`TXFrameDrain` sat in IDLE until it had a LENGTH, and the length only reached +`tx_ctrl` when the NCRA ST bits were written — i.e. *after* the GC had streamed +the whole frame. So the entire frame had to fit a 16-deep FIFO, and +`BBARegisterFile` silently drops bytes when `tx_bytes.w_rdy` is low (SPI cannot +back-pressure). + +Fix: the length is pushed into `tx_ctrl` when **TWD_HI (0x35)** is written, +which the GC sets *before* streaming, so the drain consumes concurrently with +the EXI writes and the 16-deep FIFO becomes a rate-matching buffer instead of +whole-frame storage (`twd_armed` / `tx_queued` in `bba_register_file.py`; the +NCRA ST path is now only a fallback for when TWD_HI did not queue). Verified on +hardware — a full 60-byte ARP request captured byte-for-byte correct. + +Simulation never caught the original bug: `TXFrameDrain`'s own tests drive the +FIFO signals directly, and the `BBATop` test only exercises the +W5100→SPRAM→GC **RX** loop. That gap is still open — there is no end-to-end +GC→wire TX test in simulation. + +When probing TX, **NCRA-reset between every attempt**: a failed attempt leaves +the drain waiting on a stale length, and later writes feed that pending request, which makes an unrelated frame length appear to succeed. ⚠️ **The W5100 can wedge and does NOT recover on FPGA reconfigure.** Symptom: @@ -436,6 +405,12 @@ Two bring-up gotchas, both cost real debugging time: until CS (the DMA path). `hardware/re-bba-rb/exi_regcheck_rpi.py` clocks correctly and is the reference host-side probe. +⚠️ **The two measurements below are HISTORICAL** — they predate the +2026-10-03 pin-clocked rewrite and refer to the `capture_clk` domain, which no +longer exists. Current numbers are in "Current Implementation State" above. +They are kept only because they document how added `sync`-domain cells moved +the pass rate, which still applies. + **Measured 2026-09-30 AFTER the TWD TX fix**, `EXI_J4=1` + socket-3 UDP, `--seeds 8`: `clk` 28.1–30.2 MHz (PASS on all 8); `capture_clk` **2 of 8 PASS** (seeds 4, 7), best **seed 7 = 60.03 MHz** (+6.0 MHz / 11% margin). The fix's @@ -452,10 +427,9 @@ the default build and a better best seed, so the merge costs no measurable capture timing — but it is still seed-dependent, so **always sweep**. ### Pending work -- **Synthesis/timing**: ⚠️ partially — synthesizes and P&Rs, `clk` closes with - wide margin, but `capture_clk` closes on only **2 of 8 seeds** and the default - seed 1 FAILS (53.08 < 54.02). Always `--seeds 8`. Reducing capture-domain - logic to get a real margin is still open work — see the seed table above. +- **Synthesis/timing**: ✅ — see the seed table above. Removing the PLL-fed + capture domain eliminated the constraint that had limited the design to a + minority of passing seeds. - **W5500 init/TX/RX**: ✅ done — `W5500SPIMaster` has a real Mode-0 byte engine, a generic register-transaction engine (header + wbuf/stream payload), the full init sequence (MR reset, SHAR, S0_MR MACRAW, S0_CR OPEN, S0_IMR), MACRAW TX @@ -656,19 +630,30 @@ The full design specification lives in `docs/gc_bba_fpga_design.md`. - **No network stack in the FPGA.** The GC CPU runs TCP/IP. The FPGA is a dumb MAC bridge. -- **Split-domain clocking — 3 domains, 2 sources (1 PLL + 1 HFOSC):** - - `capture` — 54 MHz (PLL, DIVR=0 DIVF=71 DIVQ=4). Hosts ONLY the SPI Mode 3 - bit engine inside `ExiCapture`. 54 MHz = 2× the **real 27 MHz** EXI clock — - the minimum oversampling for clean Mode 3. The isolated bit engine closes - ~91 MHz; integrated with the byte-FIFO read path the capture domain closes - ~62 MHz, so 54 passes with margin. - - `exi` — 24 MHz (HFOSC ÷2). BBA register file / transaction FSM. - - `sync` — 24 MHz (same HFOSC net as `exi`). SPRAM arbiter, RX/TX engines, - W5500 SPI master. - - **Why split:** only the tiny SPI bit engine needs a fast clock to sample - 27 MHz EXI. The bulky register-file/SPRAM/W5500 logic is routing-bound at - ~33–44 MHz on the UP5K and only needs the byte rate (27 MHz ÷ 8 ≈ 3.4 MHz). - `ExiCapture` bridges capture↔exi with rx/tx byte AsyncFIFOs. +- **The EXI bit engine is clocked BY the GC's EXI clock pin** (2026-10-03 + rewrite). There is no PLL and no oversampling. + - `exiclk` / `exiclk_n` — the EXI clock pin itself (~27 MHz), posedge and + negedge. Created inside `ExiCapture`; async-reset by CS. Hosts ONLY the + SPI bit engine. MOSI is sampled on the posedge, MISO driven on the negedge. + - `exififo_w` / `exififo_r` — the same pin, posedge, reset-less. The byte + FIFO sides. (Reset-less because resetting one side of an `AsyncFIFO` + desynchronises its gray pointers; and posedge because `AsyncFIFO` + instantiates a posedge-only reset CDC internally, so a negedge domain + cannot host a FIFO side.) + - `sync` — 24 MHz, internal `SB_HFOSC` ÷2. **Everything else**: register + file, SPRAM arbiter, RX/TX engines, ethernet master. The former separate + `exi` domain was merged into `sync` (they always shared the HFOSC net). + - **Why:** oversampling put BIT-level timing across a CDC boundary. The + engine's view of the clock lagged the pin by 2 `FFSynchronizer` stages plus + a register — ~55 ns against a 37 ns bit time — so MISO came out ~2 bits + late and the GC read `0x17` as `05 C0`. No amount of oversampling fixes it: + 54 MHz gives only 2 ticks per bit and the part cannot close the 81 MHz that + 3× would need. Clocking from the pin makes MISO clock-to-Q and leaves the + byte-level FIFOs as the only crossing, with a whole byte-time (~296 ns) of + slack instead of a fraction of a bit. + - **Timing consequence:** `capture_clk` is gone as a constraint. It was the + binding path for the whole design and the reason builds closed on only a + minority of seeds. - **EXI clock reality:** the GC EXI clock tops out at ~27 MHz. libogc's `EXI_SPEED32MHZ` is a nominal name — the real rate is 27 MHz. The old "96 MHz = 3× 32 MHz EXI" target was doubly wrong and unreachable on UP5K @@ -690,25 +675,24 @@ The full design specification lives in `docs/gc_bba_fpga_design.md`. ### EXI / SPI Mode 3 - CLK idles **HIGH** (CPOL=1, CPHA=1). -- MOSI sampled on **falling** CLK edge. MISO driven on **rising** CLK edge. -- Getting this wrong means the GC never enumerates the device. -- CS is active **low**, delineates each transaction. +- The master **changes MOSI on the falling edge**; both ends **sample on the + rising edge**. The device therefore **samples MOSI on the RISING edge and + drives MISO on the FALLING edge** (MISO must settle before the master + samples it on the rising edge). +- ⚠️ This file previously stated the opposite ("MOSI sampled on falling, MISO + driven on rising"), which is Mode **2**, and the implementation followed the + doc. Driving MISO on the rising edge is one bit late by construction — that + was the `0x17` → `05 C0` hardware bug. `spi_mode3_slave.py`'s testbench now + mutation-tests both edges so neither can silently regress. +- CS is active **low**, delineates each transaction, and async-resets the + bit engine. ### EXI Transaction Header -⚠️ **THE IMPLEMENTED FORMAT IS WRONG AND MUST BE REPLACED (found 2026-10-03).** -What `exi_bba/bba_register_file.py` implements today is: -``` -Byte 0: [7]=write_flag [6:0]=addr[12:6] -Byte 1: [7:2]=addr[5:0] [1:0]=xfer_len-1 (0=1B … 3=4B) -``` -That was designed from the spec's own guess. The design doc hedged on it -(§"ID query": *"Alternatively, read the Dolphin source for the exact byte -sequence…"*) and that verification never happened. - -**The REAL protocol**, read from the two drivers that actually run — libogc -(`libdb/uIP/bba.c`) and Swiss (`cube/patches/bba/bba.c`) — has TWO address -spaces, selected by bit 7 of the first byte, and **no length field at all**: +**Implemented 2026-10-03** (`bba_register_file.py`), from the two drivers that +actually run — libogc (`libdb/uIP/bba.c`) and Swiss (`cube/patches/bba/bba.c`). +There are TWO address spaces, selected by bit 7 of the first byte, and **no +length field**: ``` COMMAND space (6-bit reg) one 4-byte READWRITE transfer @@ -720,32 +704,38 @@ REGISTER space (16-bit reg) 4-byte header, then a SEPARATE data phase ``` NCRA, IR, NWAYS, RWP, RRP, PAR… all live in the REGISTER space (`bba_in8`/`bba_out8`/`bba_ins`/`bba_outs`). Only the device-ID query uses a -short header, which is the single case where the two formats coincide — which -is exactly why enumeration appeared to work and nothing else would have. +short header — the single case where the two formats coincide. -**The mismatch is destructive, not merely non-functional.** A `NWAYS` read -arrives as `80 00 31 00`; the current decoder reads that as *write 0x31 to -NCRA*, and bit 0 of 0x31 is RESET. Every register read Swiss performs would -reset the device. +**What the old (guessed) format cost.** The previous decoder used +`[7]=write, [6:0]=addr[12:6]` / `[7:2]=addr[5:0], [1:0]=len-1`. Under it a +`NWAYS` read (`80 00 31 00`) decoded as *write 0x31 to NCRA* — and bit 0 of +0x31 is RESET, so every register read Swiss performed would have reset the +device. Enumeration still appeared to work because `00 00` decodes to address +0 either way, which is exactly why the error survived so long. Do not +reintroduce a length field. -**The trailing `0x00` byte is the device's prefetch window**, and that is the -root of the latency problem below: in the real protocol the address is complete -one full byte before the data phase, giving ~8 bit-times (≈296 ns at 27 MHz) -to fetch the response. The format above spends those bits on the length field, -so the address is not known until the final header bit and the device gets -**zero** slack — which is why the first on-console run returned the device ID -22 bit-times late. +**The trailing `0x00` byte is the device's prefetch window.** The address is +complete one full byte before the data phase, giving ~8 bit-times (≈296 ns at +27 MHz) to fetch the response. The engine pops the TX FIFO once per byte +boundary, so the response the register file pushes during this turnaround byte +lands in data slot 0 — see the TX comment in `spi_mode3_slave.py`. + +⚠️ **Command space has only ONE drain slot.** Pops during the header drain +whatever the previous transaction left in the FIFO, but a 2-byte header gives +just one such slot, so a command-space response must push **exactly** its +bytes and stop. `DEVID` streaming zeros until CS left two stale bytes behind, +one too many to drain, and the *next* enumeration came back a slot late +(`04 02 02 00` → `00 04 02 02`). Register-space reads may over-push safely: +their 4-byte header gives three drain slots. **Data-phase length is whatever the GC clocks** (it streams until CS), so the -register path must stream like the SPRAM path already does — the 4-byte cap -imposed by the 2-bit length field has to go. `bba_ins` reads with **EXI DMA**; +register path streams like the SPRAM path. `bba_ins` reads with **EXI DMA**; DMA *reads* are fine (the GC's DMA-*write* engine is the buggy one). ### Device ID Query On power-on the GC writes `0x0000` (2 bytes) then reads 4 bytes. Must return: `0x04 0x02 0x02 0x00`. **Hardware-confirmed 2026-10-03** on a -real console — this is the one path that works with the current (wrong) -header format, because `00 00` decodes to address 0 either way. +real console. --- @@ -781,41 +771,54 @@ header format, because `00 00` decodes to address 0 either way. | Module | Domain | File | |---|---|---| | `BBATop` | all | `exi_bba/bba_top.py` | -| `ExiCapture` | capture (+exi FIFOs) | `exi_bba/exi_capture.py` | -| `SPIMode3Slave` | capture (param `domain`) | `exi_bba/spi_mode3_slave.py` | -| `BBARegisterFile` | exi (+FIFO to sync) | `exi_bba/bba_register_file.py` | +| `ExiCapture` | creates exiclk/exiclk_n/exififo_* (+sync side) | `exi_bba/exi_capture.py` | +| `SPIMode3Slave` | exiclk / exiclk_n | `exi_bba/spi_mode3_slave.py` | +| `BBARegisterFile` | sync | `exi_bba/bba_register_file.py` | | `SPRAMArbiter` | sync | `exi_bba/spram_arbiter.py` | | `RXFrameAssembler` | sync | `exi_bba/rx_frame_assembler.py` | | `TXFrameDrain` | sync | `exi_bba/tx_frame_drain.py` | | `W5100ParallelMaster` | sync | `exi_bba/w5100_parallel_master.py` (default eth) | | `W5500SPIMaster` | sync | `exi_bba/w5500_spi_master.py` (alt eth) | -| `EEPROMModel` | exi | `exi_bba/eeprom_model.py` | +| `EEPROMModel` | sync | `exi_bba/eeprom_model.py` | -`ExiCapture` wraps `SPIMode3Slave` (in the fast `capture` domain) plus the -capture↔exi rx/tx byte AsyncFIFOs. `BBARegisterFile` consumes the rx byte -stream and proactively pushes read responses into the tx byte FIFO — it no -longer sees the per-bit SPI cadence (that lives entirely in `capture`). +`ExiCapture` **creates the four pin-driven clock domains** (so the byte FIFOs +can name them) and wraps `SPIMode3Slave` plus the rx/tx byte AsyncFIFOs. +`SPIMode3Slave` just uses `m.d.exiclk` / `m.d.exiclk_n`; it has no `domain` +parameter any more. `BBARegisterFile` consumes the rx byte stream and +proactively pushes read responses into the tx byte FIFO — it never sees the +per-bit SPI cadence. + +**Everything except the bit engine is in one `sync` domain.** The old separate +`exi` domain was merged into `sync`, which deleted six FIFOs (SPRAM req/rsp, +RWP, RRP, and the TX byte/length pairs became `SyncFIFO`s) and three +`PulseSynchronizer`s (now plain wires). The SPRAM response FIFO was removed +outright — the register file handles responses directly, which is what finally +closed the DMA read offset at 27 MHz. --- ## CDC Signal Inventory +Only **two** crossings remain, both byte-level, both with a full byte-time +(~296 ns at 27 MHz) of slack: + | Signal | Direction | Primitive | |---|---|---| -| EXI CLK / MOSI / CS pins | async → capture | `FFSynchronizer` (stages=2) | -| RX byte stream (capture→core) | capture → exi | `AsyncFIFO` 8-bit, depth=4 | -| TX byte stream (core→capture) | exi → capture | `AsyncFIFO` 8-bit, depth=2 | -| cs_active (transaction in progress) | capture → exi | `FFSynchronizer` (DMA read length) | -| SPRAM read request (addr) | exi → sync | `AsyncFIFO` 16-bit, depth=4 | -| SPRAM read result (data) | sync → exi | `AsyncFIFO` 8-bit, depth=4 | -| TX packet bytes | exi → sync | `AsyncFIFO` 8-bit, depth=16 | -| TX frame length | exi → sync | `AsyncFIFO` 16-bit, depth=4 | -| RX frame bytes | sync → SPRAM | `RXFrameAssembler` → `SPRAMArbiter` (not a byte FIFO; the GC reads frames back out of SPRAM via the SPRAM read req/rsp FIFOs) | -| RWP update | sync → exi | `AsyncFIFO` 8-bit, depth=4 | -| RRP update | exi → sync | `AsyncFIFO` 8-bit, depth=4 | -| RX ready (IR[RI]) | sync → exi | `PulseSynchronizer` | -| TX done (IR[TI]) | sync → exi | `PulseSynchronizer` | -| NCRA reset pulse | exi → sync | `PulseSynchronizer` | +| RX byte stream | `exififo_w` → `sync` | `AsyncFIFO` 8-bit, depth=4 | +| TX byte stream | `sync` → `exififo_r` | `AsyncFIFO` 8-bit, depth=2 | +| cs_active (transaction in progress) | EXI pin → `sync` | `FFSynchronizer` (stages=2) | + +Everything that used to cross `exi`↔`sync` is now a same-domain `SyncFIFO` or +a plain wire. Note what is deliberately NOT here: the EXI CLK/MOSI/CS pins are +no longer synchronised into a fast domain — CLK *is* the clock and CS *is* the +async reset. Synchronising them was the bug, not the safety measure. + +⚠️ **The FIFO domains must stay reset-less.** `exififo_w`/`exififo_r` are not +reset by CS: resetting one side of an `AsyncFIFO` desynchronises its gray +pointers against the other side. They are also both **posedge**, because +`AsyncFIFO` instantiates a posedge-only reset CDC internally and cannot host a +negedge side; the engine pops in `exififo_r` and hands the byte to its negedge +output stage through a plain register (a half-period path, 18.5 ns at 27 MHz). --- @@ -893,9 +896,9 @@ parallel master is the default — see "W5100 vs W5500".) - **`tx_load` timing in `SPIMode3Slave`:** pulses at CS assertion (first byte) and after each complete received byte. Upstream must register next TX byte within one `exi` clock. -- **PLL target 54 MHz**: verify with `icepll -i 12 -o 54` (DIVR=0 DIVF=71 DIVQ=4) - before coding PLL parameters; the capture-domain bit engine oversamples the - 27 MHz EXI clock 2×. +- **No PLL.** The bit engine is clocked by the EXI pin; `sync` is HFOSC÷2. + Do not reintroduce an oversampling capture domain — see the clocking note + under "Key Architecture Decisions" for why it cannot work on this part. - **TX buffer selection (NCRA ST bits):** Ignore buffer select (ST1 vs ST0). Treat any non-zero ST as a TX trigger. - **If nextpnr fails capture-domain timing at 54 MHz:** the isolated bit engine diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index b424b95..f68ac42 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -86,9 +86,13 @@ class BBARegisterFile(Elaboratable): self.spram_req_r_rdy = Signal() # SPRAM response sync→exi: sync writes these - self.spram_rsp_w_data = Signal(8) - self.spram_rsp_w_en = Signal() - self.spram_rsp_w_rdy = Signal() + # SPRAM response, DIRECT from the arbiter -- no FIFO. That FIFO cost + # one 24 MHz cycle (42 ns), and the round trip only fits inside the + # protocol's turnaround byte without it (333 ns -> 292 ns against a + # 315 ns deadline). Responses must therefore be consumed the cycle + # they are valid, which is why only ONE request may be outstanding. + self.spram_rsp_data = Signal(8) + self.spram_rsp_valid = Signal() # TX bytes exi→sync: sync reads these self.tx_bytes_r_data = Signal(8) @@ -125,14 +129,12 @@ class BBARegisterFile(Elaboratable): # On the SPRAM path that latency exceeded the protocol's turnaround # byte and shifted every DMA read by one byte. One slow domain now. spram_req = SyncFIFO(width=16, depth=4) - spram_rsp = SyncFIFO(width=8, depth=4) tx_bytes = SyncFIFO(width=8, depth=16) tx_ctrl = SyncFIFO(width=16, depth=4) rx_wptr = SyncFIFO(width=8, depth=4) rx_rptr = SyncFIFO(width=8, depth=4) m.submodules.spram_req = spram_req - m.submodules.spram_rsp = spram_rsp m.submodules.tx_bytes = tx_bytes m.submodules.tx_ctrl = tx_ctrl m.submodules.rx_wptr = rx_wptr @@ -144,10 +146,6 @@ class BBARegisterFile(Elaboratable): spram_req.r_en .eq(self.spram_req_r_en), self.spram_req_r_rdy .eq(spram_req.r_rdy), - spram_rsp.w_data .eq(self.spram_rsp_w_data), - spram_rsp.w_en .eq(self.spram_rsp_w_en), - self.spram_rsp_w_rdy .eq(spram_rsp.w_rdy), - self.tx_bytes_r_data .eq(tx_bytes.r_data), tx_bytes.r_en .eq(self.tx_bytes_r_en), self.tx_bytes_r_rdy .eq(tx_bytes.r_rdy), @@ -237,6 +235,8 @@ class BBARegisterFile(Elaboratable): # popped (bounds prefetch and is drained at end) sp_addr = Signal(16) outstanding = Signal(4) + sp_hold = Signal(8) # response held when tx_fifo was full + sp_pending = Signal() SP_LIMIT = 4 # max prefetch depth in flight # Effective address of the current data byte — a REGISTERED running @@ -301,7 +301,6 @@ class BBARegisterFile(Elaboratable): self.tx_data.eq(0), # Combinational so the FIFO advances in the SAME cycle as the pop — # a registered r_en would let `pop` re-fire on the same byte. - spram_rsp.r_en.eq(0), ] # ── Transaction FSM (proactive push/pull over byte FIFOs) ──────── @@ -404,7 +403,8 @@ class BBARegisterFile(Elaboratable): m.next = "DEVID" with m.Else(): m.next = "CHDR_2" - with m.Elif(~is_write & (cmd_reg == 0) & id_phase & self.tx_rdy): + with m.Elif(~is_write & (cmd_reg == 0) & id_phase + & self.tx_rdy & (byte_ctr < 4)): # Device-ID query: keep feeding the ID while byte 1 is # still on the wire so the tx FIFO stays ahead of the GC. # NOT one-shot -- the FIFO is only 2 deep, and stopping @@ -506,8 +506,15 @@ class BBARegisterFile(Elaboratable): m.next = "HEADER0" with m.State("DEVID"): - # Stream 04 02 02 00 until CS rises. - with m.If(self.tx_rdy & self.cs_active): + # Push EXACTLY the four ID bytes, then stop. + # The engine latches the tx_fifo head at every byte boundary, + # and a 2-byte command header leaves only ONE drain slot + # before the data phase. Streaming zeros until CS left two + # stale bytes behind, one too many to drain, so the NEXT + # enumeration came back a slot late (04 02 02 00 -> 00 04 02 02). + # Register-space reads may over-push safely: their 4-byte + # header gives three drain slots. + with m.If(self.tx_rdy & self.cs_active & (byte_ctr < 4)): m.d.comb += [self.tx_data.eq(devid), self.tx_en.eq(1)] m.d.sync += byte_ctr.eq(byte_ctr + 1) with m.If(self.rx_rdy): @@ -521,21 +528,19 @@ class BBARegisterFile(Elaboratable): # Issue read requests ahead (prefetch, bounded by SP_LIMIT) and # push responses into tx_fifo; the capture domain pops them as # the GC clocks. Drain rx dummies as they arrive. - issue = Signal() - pop = Signal() - m.d.comb += issue.eq(self.cs_active & spram_req.w_rdy - & (outstanding < SP_LIMIT)) - m.d.comb += pop.eq(spram_rsp.r_rdy & self.tx_rdy) - - with m.If(issue): + # With no response FIFO the reply must be taken the cycle it + # is valid, so keep exactly ONE request in flight and only + # issue when tx_fifo has somewhere to put the answer. The GC + # consumes a byte every ~296 ns and the SPRAM round trip is a + # couple of 24 MHz cycles, so serialising costs nothing. + # Only ONE request in flight (no response FIFO), and only + # when tx_fifo can take the answer. + with m.If(self.cs_active & spram_req.w_rdy & self.tx_rdy + & (outstanding == 0) & ~sp_pending): m.d.comb += spram_req.w_data.eq(sp_addr) m.d.comb += spram_req.w_en.eq(1) - m.d.sync += sp_addr.eq(sp_addr + 1) - with m.If(pop): - m.d.comb += self.tx_data.eq(spram_rsp.r_data) - m.d.comb += self.tx_en.eq(1) - m.d.comb += spram_rsp.r_en.eq(1) - m.d.sync += outstanding.eq(outstanding + issue - pop) + m.d.sync += [sp_addr.eq(sp_addr + 1), + outstanding.eq(outstanding + 1)] with m.If(self.rx_rdy): m.d.comb += self.rx_en.eq(1) # drain dummy bytes @@ -547,14 +552,28 @@ class BBARegisterFile(Elaboratable): # CS deasserted: drain in-flight SPRAM responses and rx dummies, # then idle. Leftover prefetch in tx_fifo is flushed by # ExiCapture on the next CS assertion. - with m.If(spram_rsp.r_rdy): - m.d.comb += spram_rsp.r_en.eq(1) - m.d.sync += outstanding.eq(outstanding - 1) with m.If(self.rx_rdy): m.d.comb += self.rx_en.eq(1) - with m.If((outstanding == 0) & ~self.rx_rdy & ~spram_rsp.r_rdy): + with m.If((outstanding == 0) & ~sp_pending & ~self.rx_rdy): m.next = "HEADER0" + # ── SPRAM response: consumed in ANY state ──────────────────────── + # The request is issued in RHDR_LO but the reply lands ~2 cycles later, + # while the FSM is still in RHDR_PAD waiting for the turnaround byte. + # With no response FIFO there is nothing to catch it there, so handle + # it outside the FSM: push straight to tx_fifo if it has room, else + # hold it one cycle. SPRAM reads and register reads never overlap, so + # driving tx here cannot collide with the FSM's own pushes. + with m.If(self.spram_rsp_valid & self.tx_rdy & ~sp_pending): + m.d.comb += [self.tx_data.eq(self.spram_rsp_data), self.tx_en.eq(1)] + m.d.sync += outstanding.eq(outstanding - 1) + with m.Elif(self.spram_rsp_valid): + m.d.sync += [sp_hold.eq(self.spram_rsp_data), sp_pending.eq(1), + outstanding.eq(outstanding - 1)] + with m.Elif(sp_pending & self.tx_rdy): + m.d.comb += [self.tx_data.eq(sp_hold), self.tx_en.eq(1)] + m.d.sync += sp_pending.eq(0) + # ── Command-space write, applied the cycle after CHDR_3 ────────── with m.If(cmd_wr): m.d.sync += cmd_wr.eq(0) @@ -679,28 +698,28 @@ if __name__ == "__main__": spram_mem = {0x100 + i: (0xA0 + i) & 0xFF for i in range(64)} async def spram_model(ctx): - """Model the SPRAM (sync side): answer spram_req with mem[addr]. + """Model the SPRAM + arbiter as a request/grant port. - One request at a time, with cleanly-pulsed r_en/w_en so the FIFO pop - and the response push stay in lock-step (no double-response races). + The response FIFO is gone: the arbiter drives spram_rsp_valid directly + for exactly ONE cycle, one cycle after it takes a request. Mirror that + precisely -- a response held high for two cycles makes the register + file push the same byte twice. """ - state = "POP" + busy = False held = 0 async for vals in ctx.tick("sync").sample( - reg.spram_req_r_rdy, reg.spram_req_r_data, reg.spram_rsp_w_rdy): - rdy, addr, rsp_rdy = vals[-3:] + reg.spram_req_r_rdy, reg.spram_req_r_data): + rdy, addr = vals[-2:] ctx.set(reg.spram_req_r_en, 0) - ctx.set(reg.spram_rsp_w_en, 0) - if state == "POP": - if rdy: - held = spram_mem.get(addr, 0) - ctx.set(reg.spram_req_r_en, 1) # consume the request - state = "RESP" - else: # RESP - if rsp_rdy: - ctx.set(reg.spram_rsp_w_data, held) - ctx.set(reg.spram_rsp_w_en, 1) # deliver the response - state = "POP" + ctx.set(reg.spram_rsp_valid, 0) + if busy: # deliver, one cycle after accept + ctx.set(reg.spram_rsp_data, held) + ctx.set(reg.spram_rsp_valid, 1) + busy = False + elif rdy: # accept one request + held = spram_mem.get(addr, 0) + ctx.set(reg.spram_req_r_en, 1) + busy = True errors = [] diff --git a/exi_bba/bba_top.py b/exi_bba/bba_top.py index a05b731..23bd970 100644 --- a/exi_bba/bba_top.py +++ b/exi_bba/bba_top.py @@ -2,11 +2,11 @@ Clock domains ------------- -capture : 54 MHz, from 12 MHz oscillator via SB_PLL40_CORE (REFERENCECLK fed - from a global-buffer input pin; DIVR=0 DIVF=71 DIVQ=4). CORE (not - PAD) so the reference clock is not tied to the dedicated PLL pad — it - enters on a clock-capable GBIN pin, leaving board routing free. -exi/sync : 24 MHz, from the iCE40UP5K internal SB_HFOSC (÷2, CLKHF_DIV=0b01) +sync : 24 MHz, from the iCE40UP5K internal SB_HFOSC (÷2, CLKHF_DIV=0b01). + Everything except the EXI bit engine. No PLL, no crystal. +exiclk : the GC's EXI clock pin itself (~27 MHz), posedge — created inside +exiclk_n : the same pin, negedge ................ `ExiCapture`. +exififo_* the same pin, posedge, reset-less — the byte-FIFO sides. Submodule instantiation and signal wiring ----------------------------------------- @@ -125,40 +125,30 @@ class BBATop(Elaboratable): m = Module() # ── Clock domain generation ─────────────────────────────────────── - # Three domains, two physical sources (1 PLL + 1 internal HFOSC): - # capture @ 54 MHz (PLL) — SPI bit engine only; oversamples the - # 27 MHz EXI clock 2× (robust Mode-3). - # exi @ 24 MHz (HFOSC) — register file / transaction FSM. - # sync @ 24 MHz (HFOSC) — SPRAM, RX/TX engines, ethernet master. - # exi and sync share the HFOSC net (frequency- and phase-matched); the - # AsyncFIFOs between them are still valid CDC and keep the module - # boundaries clean. Only the tiny capture front-end needs the fast - # clock — which is why 27 MHz-EXI / OG performance is reachable on the - # iCE40UP5K even though the register-file logic tops out ~44 MHz. + # ONE generated domain, one physical source: + # sync @ 24 MHz (internal SB_HFOSC ÷2) — register file, SPRAM, + # RX/TX engines, ethernet master. Everything that is not the EXI + # bit engine lives here. + # + # The EXI front-end is NOT clocked from here: `ExiCapture` builds its + # own exiclk/exiclk_n domains directly from the GC's EXI clock pin. + # That is what removed the old 54 MHz PLL-fed `capture` domain, which + # oversampled the EXI clock 2× and put BIT-level timing across a CDC + # boundary (~55 ns of synchroniser latency against a 37 ns bit) — the + # cause of the `0x17` -> `05 C0` MISO shift on hardware. With the + # engine clocked by the pin, MISO is clock-to-Q and the only crossings + # left are byte-level FIFOs with a full byte-time of slack. + # + # Consequence for timing closure: `capture_clk` is GONE as a + # constraint. It was the binding path for this whole design and the + # reason builds only closed on a minority of seeds. if platform is not None: - # capture @ 54 MHz: icepll -i 12 -o 54 → DIVR=0 DIVF=71 DIVQ=4. - # 54 MHz = 2× the 27 MHz EXI clock — the minimum oversampling that - # cleanly implements SPI Mode 3. The isolated SPI bit engine closes - # ~91 MHz on this device; the byte-FIFO read path brings the - # integrated capture domain to ~62 MHz, so 54 closes with margin. - m.domains += ClockDomain("capture") - m.submodules.pll = Instance( - "SB_PLL40_CORE", - p_FEEDBACK_PATH = "SIMPLE", - p_DIVR = 0, - p_DIVF = 71, - p_DIVQ = 4, - p_FILTER_RANGE = 1, - i_REFERENCECLK = platform.request("clk12").i, - i_RESETB = Const(1, 1), - i_BYPASS = Const(0, 1), - o_PLLOUTGLOBAL = ClockSignal("capture"), - ) - - # exi & sync @ 24 MHz: one SB_HFOSC (÷2) drives both slow domains. - # The bulky register-file / SPRAM / W5500 logic is routing-bound at + # sync @ 24 MHz from the internal oscillator. No PLL and no + # external crystal are needed any more, so the 12 MHz input (pin + # 35) and the PLL hard macro are both free. + # The register-file / SPRAM / ethernet logic is routing-bound at # ~33–44 MHz on the UP5K; 24 MHz closes with large margin. The byte - # rate (27 MHz EXI ÷ 8 ≈ 3.4 MHz) leaves ~7 slow cycles per byte. + # rate (27 MHz EXI ÷ 8 ≈ 3.4 MHz) leaves ~7 sync cycles per byte. m.domains += ClockDomain("sync") m.submodules.hfosc = Instance( "SB_HFOSC", @@ -167,7 +157,7 @@ class BBATop(Elaboratable): i_CLKHFPU = Const(1, 1), o_CLKHF = ClockSignal("sync"), ) - # (simulation: test harness provides capture/exi/sync clocks via add_clock) + # (simulation: the test harness drives the EXI pins and `sync` directly) # ── Submodules ──────────────────────────────────────────────────── cap = ExiCapture() # SPI bit engine (capture) + byte FIFOs @@ -191,7 +181,7 @@ class BBATop(Elaboratable): # ── External pin connections ────────────────────────────────────── m.d.comb += [ # EXI inputs (to capture-domain front-end) - cap.spi_clk .eq(self.exi_clk), + cap.spi_clk .eq(self.exi_clk), # drives the exiclk domains cap.spi_mosi.eq(self.exi_mosi), cap.spi_cs_n.eq(self.exi_cs_n), # EXI outputs @@ -244,9 +234,8 @@ class BBATop(Elaboratable): ] # SPRAM response: arb result → reg sync→exi FIFO write side m.d.comb += [ - reg.spram_rsp_w_data.eq(arb.exi_rsp_data), - reg.spram_rsp_w_en .eq(arb.exi_rsp_valid), - # arb does not need w_rdy feedback (spram_rsp FIFO is deeper than latency) + reg.spram_rsp_data .eq(arb.exi_rsp_data), + reg.spram_rsp_valid.eq(arb.exi_rsp_valid), ] # ── BBARegisterFile ↔ TXFrameDrain (sync domain FIFO sides) ────── @@ -421,7 +410,7 @@ if __name__ == "__main__": # `capture` domain to count ticks against. Drive the SPI clock with real # delays: one half-period = HALF * (1/54 MHz), preserving the old meaning # of HALF so the rates in the table above still hold. - HP = Period(MHz=54) * HALF + HP = Period(MHz=54) # ONE capture-equivalent tick; delays are HP * n # Gap between transactions (CS high). cs_active reaches the exi domain # through a 2-stage FFSynchronizer at 24 MHz, so CS must stay high for # ~3 exi cycles or the register file never sees the transaction end and diff --git a/exi_bba/exi_capture.py b/exi_bba/exi_capture.py index 19e7ab7..25b70a3 100644 --- a/exi_bba/exi_capture.py +++ b/exi_bba/exi_capture.py @@ -1,28 +1,45 @@ -"""ExiCapture — fast EXI byte-capture front-end (capture domain, 54 MHz). +"""ExiCapture — EXI byte-capture front-end, clocked by the GC's EXI clock. -Wraps the SPIMode3Slave bit engine and bridges it to the slower `exi` domain -(24 MHz) through two AsyncFIFOs: +Creates the pin-driven clock domains (so the byte FIFOs can name them), wraps +the SPIMode3Slave bit engine, and bridges it to the 24 MHz `sync` domain +through two AsyncFIFOs: - capture (54 MHz) exi (24 MHz) - ┌────────────────────┐ rx_fifo ───► received bytes (header + data) - │ SPIMode3Slave │ (8-bit, capture→exi) - │ (bit engine) │ tx_fifo ◄─── response bytes to drive on MISO - └────────────────────┘ (8-bit, exi→capture) + EXI clock pin (~27 MHz) sync (24 MHz, HFOSC) + ┌─────────────────────┐ rx_fifo ───► received bytes (header + data) + │ SPIMode3Slave │ (8-bit, exififo_w→sync) + │ (bit engine) │ tx_fifo ◄─── response bytes to drive on MISO + └─────────────────────┘ (8-bit, sync→exififo_r) -Why split: the bit engine must oversample a 27 MHz EXI clock 2×, which needs a -54 MHz clock — far faster than the register-file logic can close (~44 MHz). -Only this small, shallow front-end runs fast; everything else stays at 24 MHz. +Domains created here +-------------------- + exiclk EXI clock pin, posedge, async-reset by CS -- samples MOSI + exiclk_n EXI clock pin, negedge, async-reset by CS -- drives MISO + exififo_w EXI clock pin, posedge, RESET-LESS -- rx FIFO write side + exififo_r EXI clock pin, posedge, RESET-LESS -- tx FIFO read side -TX response gating +The FIFO domains must NOT see CS: resetting one side of an AsyncFIFO +desynchronises its gray pointers against the other side. They are also both +posedge, because AsyncFIFO instantiates a posedge-only reset CDC internally and +cannot host a negedge side. + +Why pin-clocked: oversampling the EXI clock from a faster domain put BIT-level +timing across a CDC boundary (~55 ns of synchroniser latency against a 37 ns +bit), so MISO came out ~2 bits late and the GC read 0x17 as 05 C0. Clocking +from the pin makes MISO clock-to-Q and leaves these byte FIFOs as the only +crossing, each with a full byte-time (~296 ns) of slack. + +TX response timing ------------------ -Every EXI transaction begins with 2 header bytes (write_flag/addr/len) during -which the GC ignores MISO. The core cannot have produced a response yet (it -hasn't even decoded the header), so the wrapper must NOT pop tx_fifo for those -2 bytes. A per-transaction counter (`txld_cnt`, reset by frame_start) gates the -pop: header bytes drive a don't-care 0xFF; from the first data byte onward the -wrapper pops tx_fifo (one byte per tx_load). `tx_hold` is registered at tx_load -time — before the FIFO advances — so the bit engine latches the correct byte on -the following SPI rising edge (the classic FWFT-advance off-by-one is avoided). +The engine pops tx_fifo once per byte boundary, unconditionally -- there is no +header counter and no slot threshold. The byte popped at the end of byte N is +presented during byte N+1, so pops during the header drain whatever the +previous transaction left in the FIFO (this replaces the old CS-fall flush), +and the response the register file pushes during the protocol's turnaround +byte lands in data slot 0. + +⚠️ Command space has only ONE drain slot (2-byte header), so a command-space +responder must push exactly its bytes and stop -- see the DEVID note in +bba_register_file.py. """ from amaranth import * @@ -96,15 +113,15 @@ class ExiCapture(Elaboratable): # (plain) no reset -- the byte FIFOs. Resetting only ONE side of an # AsyncFIFO desynchronises its gray pointers and corrupts it, # so the FIFOs must NOT see CS. - m.domains.exiclk = cd_p = ClockDomain(local=True, async_reset=True) - m.domains.exiclk_n = cd_n = ClockDomain(local=True, async_reset=True, + m.domains.exiclk = cd_p = ClockDomain(async_reset=True) + m.domains.exiclk_n = cd_n = ClockDomain(async_reset=True, clk_edge="neg") - m.domains.exififo_w = cd_fw = ClockDomain(local=True, reset_less=True) + m.domains.exififo_w = cd_fw = ClockDomain(reset_less=True) # NOTE: both FIFO domains are POSEDGE. AsyncFIFO instantiates a # posedge-only reset CDC internally, so a negedge domain cannot host a # FIFO side. The engine pops in `exififo_r` (posedge) and hands the # byte to its negedge output stage through a register. - m.domains.exififo_r = cd_fr = ClockDomain(local=True, reset_less=True) + m.domains.exififo_r = cd_fr = ClockDomain(reset_less=True) m.d.comb += [ cd_p .clk.eq(self.spi_clk), cd_p .rst.eq(self.spi_cs_n), cd_n .clk.eq(self.spi_clk), cd_n .rst.eq(self.spi_cs_n), @@ -144,9 +161,9 @@ class ExiCapture(Elaboratable): ] # ── TX: core pushes responses; the engine pops one per byte-time ── - # The engine suppresses the pop during the protocol's turnaround byte - # and drains stale bytes during the rest of the header, so no - # CS-fall flush or slot-threshold logic is needed here any more. + # Pops are UNCONDITIONAL (one per byte boundary). Header pops drain + # whatever the previous transaction left behind, so no CS-fall flush + # or slot-threshold logic is needed here any more. m.d.comb += [ tx_fifo.w_data.eq(self.tx_data), tx_fifo.w_en .eq(self.tx_en), @@ -181,11 +198,9 @@ if __name__ == "__main__": # ratio is ~2; use 4 here for a clean, well-oversampled functional check. HALF = 4 - # No capture domain any more: the bit engine is clocked by the EXI - - # pin, so drive the SPI clock with real delays instead of ticks. - - HP = Period(MHz=54) * HALF + # No capture domain any more: the bit engine is clocked by the EXI pin, + # so drive the SPI clock with real delays instead of counting ticks. + HP = Period(MHz=54) # ONE capture-equivalent tick; delays are HP * n async def spi_byte(ctx, mosi_val): """Clock one SPI Mode 3 byte; return the assembled MISO byte.""" @@ -216,20 +231,24 @@ if __name__ == "__main__": ctx.set(dut.tx_en, 0) async def do_txn(ctx, hdr, responses, n_data, rx_seen): - """One EXI transaction: clock `hdr` bytes, model the clock-idle gap - (drain rx + prefetch `responses` into tx_fifo), then clock `n_data` - data bytes; return the MISO data bytes read.""" + """One EXI transaction, modelling the REAL protocol shape. + + The engine pops tx_fifo at every byte boundary, so a response must be + in the FIFO before the LAST header byte ends to land in data slot 0. + That byte is the protocol's turnaround byte (docs/exi_protocol_real.md) + and is exactly when the register file pushes. Pushing after the whole + header -- as this bench used to -- is one slot too late. + """ ctx.set(dut.spi_cs_n, 0) ctx.set(dut.spi_clk, 1) await ctx.delay(HP * (HALF)) - for h in hdr: + for h in hdr[:-1]: await spi_byte(ctx, h) - for _ in range(20): # clock-idle gap await core_drain_rx(ctx, rx_seen) - await ctx.tick("sync").repeat(1) - for r in responses: + for r in responses: # pushed during the turnaround await push_tx(ctx, r) - await ctx.delay(HP * (2)) + await spi_byte(ctx, hdr[-1]) + await core_drain_rx(ctx, rx_seen) miso = [await spi_byte(ctx, 0x00) for _ in range(n_data)] ctx.set(dut.spi_cs_n, 1) await ctx.delay(HP * (HALF)) @@ -242,24 +261,30 @@ if __name__ == "__main__": rx_seen = [] await ctx.delay(HP * (2)) + # Real register-space header: [0x80, ah, al, 0x00] -- the trailing + # byte is the turnaround slot. + HDR = [0x80, 0x00, 0x12, 0x00] + # ── T1: header + 2 data bytes read back ────────────────────────── - miso = await do_txn(ctx, [0x12, 0x34], [0xA5, 0x5A], 2, rx_seen) - print(f"T1 rx={[hex(b) for b in rx_seen[:2]]} MISO={[f'0x{b:02X}' for b in miso]}") - if rx_seen[:2] != [0x12, 0x34]: - errors.append(f"T1 header rx wrong: {rx_seen[:2]}") + miso = await do_txn(ctx, HDR, [0xA5, 0x5A], 2, rx_seen) + print(f"T1 rx={[hex(b) for b in rx_seen[:4]]} MISO={[f'0x{b:02X}' for b in miso]}") + if rx_seen[:4] != HDR: + errors.append(f"T1 header rx wrong: {[hex(b) for b in rx_seen[:4]]}") if miso != [0xA5, 0x5A]: errors.append(f"T1 MISO wrong: {[hex(b) for b in miso]}") - # ── T2: prefetch over-push must NOT leak into the next transaction ─ + # ── T2: over-push must NOT leak into the next transaction ──────── # Txn A pushes 2 responses but the GC clocks only 1 data byte, leaving - # one stale byte in tx_fifo. Txn B must read its OWN fresh responses, - # proving the CS-fall flush cleared the stale prefetch. + # a stale byte in tx_fifo. Txn B must read its OWN responses. There + # is no CS-fall flush any more: the engine's pops during the header + # drain whatever the previous transaction left behind. rx_seen.clear() - await do_txn(ctx, [0x12, 0x34], [0xA5, 0x5A], 1, rx_seen) # leaves 0x5A - misoB = await do_txn(ctx, [0x12, 0x34], [0x11, 0x22], 2, rx_seen) + await do_txn(ctx, HDR, [0xA5, 0x5A], 1, rx_seen) # leaves 0x5A + rx_seen.clear() + misoB = await do_txn(ctx, HDR, [0x11, 0x22], 2, rx_seen) print(f"T2 MISO after over-push: {[f'0x{b:02X}' for b in misoB]} (want 0x11 0x22)") if misoB != [0x11, 0x22]: - errors.append(f"T2 flush failed — stale byte leaked: {[hex(b) for b in misoB]}") + errors.append(f"T2 stale byte leaked: {[hex(b) for b in misoB]}") sim = Simulator(dut) sim.add_clock(Period(MHz=24), domain="sync") diff --git a/exi_bba/exi_j4.py b/exi_bba/exi_j4.py index 9709a3b..e98d4c9 100644 --- a/exi_bba/exi_j4.py +++ b/exi_bba/exi_j4.py @@ -50,7 +50,7 @@ if __name__ == "__main__": # pin, so drive the SPI clock with real delays instead of ticks. - HP = Period(MHz=54) * HALF + HP = Period(MHz=54) # ONE capture-equivalent tick; delays are HP * n DEVICE_ID = [0x04, 0x02, 0x02, 0x00] class _Harness(Elaboratable): diff --git a/exi_bba/spi_mode3_slave.py b/exi_bba/spi_mode3_slave.py index 34e9ccd..91a35d1 100644 --- a/exi_bba/spi_mode3_slave.py +++ b/exi_bba/spi_mode3_slave.py @@ -53,10 +53,10 @@ class SPIMode3Slave(Elaboratable): rx_byte : last complete received byte (valid while rx_valid is high) rx_valid : 1-cycle pulse in `exiclk` when rx_byte holds a new byte tx_byte : byte to transmit; sampled at each byte boundary - tx_load : 1-cycle pulse in `exiclk_n` asking upstream for the next byte - hdr_bytes : how many leading bytes are header (MISO don't-care). Pops are - suppressed until past them, so the first response byte the register - file pushes lands in data slot 0 rather than being eaten by the header. + tx_load : 1-cycle pulse asking upstream for the next byte, one per byte + boundary. No header-length or slot-threshold logic: see the TX comment + in elaborate() for why unconditional pops are both sufficient and + necessary. """ def __init__(self): @@ -71,29 +71,16 @@ class SPIMode3Slave(Elaboratable): self.tx_byte = Signal(8) self.tx_load = Signal() - # Header length for this transaction, in bytes. The real protocol has - # two forms selected by bit 7 of byte 0 (see docs/exi_protocol_real.md): - # register space = 4-byte header, command space = 2-byte. - self.hdr_bytes = Signal(3, init=4) - - # Transaction framing, generated in the slow domain by the parent. - self.frame_start = Signal() - self.cs_active = Signal() - def elaborate(self, platform): m = Module() # ── RX: sample MOSI on the RISING edge (Mode 3 samples on edge 2) ── rx_shift = Signal(8) rx_bit = Signal(3) - byte_idx = Signal(3, init=0) # bytes seen this transaction (saturating) nxt = Cat(self.spi_mosi, rx_shift[:-1]) # MSB-first: shift left, new bit at LSB m.d.exiclk += rx_shift.eq(nxt) m.d.exiclk += rx_bit.eq(rx_bit + 1) - with m.If(rx_bit == 7): - with m.If(byte_idx != 7): - m.d.exiclk += byte_idx.eq(byte_idx + 1) # rx_byte/rx_valid are COMBINATIONAL, not registered. The clock stops # and CS rises immediately after a transaction's final bit, so a @@ -138,3 +125,136 @@ class SPIMode3Slave(Elaboratable): m.d.comb += self.spi_miso.eq(Mux(self.spi_cs_n, 1, tx_shift[7])) return m + + +# ── Testbench ───────────────────────────────────────────────────────────── + +class _Harness(Elaboratable): + """Wraps the slave with the two pin-driven domains its parent normally + creates, so the engine can be exercised standalone.""" + + def __init__(self): + self.dut = SPIMode3Slave() + + def elaborate(self, platform): + m = Module() + m.domains.exiclk = cd_p = ClockDomain(async_reset=True) + m.domains.exiclk_n = cd_n = ClockDomain(async_reset=True, + clk_edge="neg") + d = self.dut + m.d.comb += [ + cd_p.clk.eq(d.spi_clk), cd_p.rst.eq(d.spi_cs_n), + cd_n.clk.eq(d.spi_clk), cd_n.rst.eq(d.spi_cs_n), + ] + m.submodules.dut = d + return m + + +if __name__ == "__main__": + import sys + from amaranth.sim import Simulator, Period + + HP = Period(MHz=27) / 2 # one EXI half-period at the hardware rate + top = _Harness() + dut = top.dut + + failures = [] + + def check(name, got, want): + ok = got == want + print(f" {'ok ' if ok else 'FAIL'} {name}: got {got}, want {want}") + if not ok: + failures.append(name) + + # A STRICT Mode 3 master. Unlike a convenience model that parks MOSI long + # before the edge, this one changes MOSI exactly ON the falling edge and + # samples MISO in the CLK-low phase. That is what makes the edge + # convention observable: a slave that sampled MOSI on the falling edge + # would capture the previous bit, and one that drove MISO on the rising + # edge would come out a bit late. Both bugs shipped to hardware against a + # lax master that could not see them. + async def spi_byte(ctx, mosi_val, probe_rx=False): + miso = 0 + rx = None + for bit in range(7, -1, -1): + ctx.set(dut.spi_clk, 0) # falling edge + ctx.set(dut.spi_mosi, (mosi_val >> bit) & 1) + await ctx.delay(HP) + miso = (miso << 1) | ctx.get(dut.spi_miso) # sample in CLK-low + if probe_rx and bit == 0: + rx = (ctx.get(dut.rx_valid), ctx.get(dut.rx_byte)) + ctx.set(dut.spi_clk, 1) # rising edge: slave samples + await ctx.delay(HP) + return (miso, rx) if probe_rx else miso + + async def cs_low(ctx): + ctx.set(dut.spi_clk, 1) + ctx.set(dut.spi_cs_n, 0) + await ctx.delay(HP) + + async def cs_high(ctx): + ctx.set(dut.spi_clk, 1) + await ctx.delay(HP) + ctx.set(dut.spi_cs_n, 1) + await ctx.delay(HP * 4) + + async def tb(ctx): + ctx.set(dut.spi_cs_n, 1) + ctx.set(dut.spi_clk, 1) + ctx.set(dut.tx_byte, 0) + await ctx.delay(HP * 4) + + # T1 — RX samples MOSI on the RISING edge, MSB first. + # rx_byte/rx_valid are combinational and peak during the final bit, + # which is the instant the parent's FIFO captures them. + await cs_low(ctx) + seen = [] + for b in (0x80, 0x00, 0x31, 0x00): + _, (valid, byte) = await spi_byte(ctx, b, probe_rx=True) + seen.append((valid, byte)) + await cs_high(ctx) + check("T1 rx bytes", [b for _, b in seen], [0x80, 0x00, 0x31, 0x00]) + check("T1 rx_valid", [v for v, _ in seen], [1, 1, 1, 1]) + + # T2 — TX: the byte latched at the end of slot N appears in slot N+1, + # MSB first. Slot 0 is the reset value, before any load. + await cs_low(ctx) + ctx.set(dut.tx_byte, 0xA5) + s0 = await spi_byte(ctx, 0x00) + ctx.set(dut.tx_byte, 0x3C) + s1 = await spi_byte(ctx, 0x00) + ctx.set(dut.tx_byte, 0x00) + s2 = await spi_byte(ctx, 0x00) + await cs_high(ctx) + check("T2 miso slot0 (pre-load)", hex(s0), hex(0x00)) + check("T2 miso slot1", hex(s1), hex(0xA5)) + check("T2 miso slot2", hex(s2), hex(0x3C)) + + # T3 — tx_load pulses exactly once per byte, during the final bit. + await cs_low(ctx) + loads = 0 + for _ in range(3): + for bit in range(8): + ctx.set(dut.spi_clk, 0) + await ctx.delay(HP) + loads += ctx.get(dut.tx_load) + ctx.set(dut.spi_clk, 1) + await ctx.delay(HP) + await cs_high(ctx) + check("T3 tx_load pulses", loads, 3) + + # T4 — MISO releases HIGH when CS is deasserted, so it never fights + # another EXI device sharing the bus. + check("T4 miso idles high", ctx.get(dut.spi_miso), 1) + + sim = Simulator(top) + sim.add_testbench(tb) + with sim.write_vcd("SPIMode3Slave.vcd"): + sim.run() + + if failures: + print("\nFAILURES:") + for f in failures: + print(" -", f) + sys.exit(1) + print("\nAll tests passed.") diff --git a/exi_bba/synth.py b/exi_bba/synth.py index dbe35df..c1b878f 100644 --- a/exi_bba/synth.py +++ b/exi_bba/synth.py @@ -36,7 +36,7 @@ from exi_bba.bba_top import BBATop # INT_N=13 — MOVED off the schematic's original pin 35: pin 35 physically # conflicts with the iCE40UP5K's PLL hard macro (nextpnr: "PLL bel # 'X12/Y31/pll_3' cannot be used... conflicts with input... on pin 35"), -# and this design needs the PLL for the 54 MHz capture-domain clock. Pin 35 +# that design needed the PLL for its 54 MHz capture-domain clock. Pin 35 # cannot be used as regular I/O at all while the PLL is instantiated, # regardless of what drives it — this is a fixed silicon constraint, not a # routing choice. Pin 13 was picked because it's the ONLY spare GPIO on this @@ -48,6 +48,10 @@ from exi_bba.bba_top import BBATop # 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. +# NOTE 2026-10-03: the PLL is GONE (the bit engine is clocked by the EXI pin), +# so the silicon conflict no longer applies and pin 35 is electrically usable +# again. It stays unconnected anyway because the BOARD is already built with +# ETH_INT on pin 13 — this is now a hardware fact, not a tool constraint. # # 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 @@ -64,17 +68,18 @@ from exi_bba.bba_top import BBATop # directly — leaving o_RGB* dangling (the old code) never bonds them and the # LEDs stay dark. See the "rgb" Resource + elaborate() below, and BRINGUP.md. -# nextpnr P&R options. The binding constraint is the isolated 54 MHz capture -# domain (the SPI Mode-3 bit engine); the 24 MHz sync domain has wide margin. +# nextpnr P&R options. Since the PLL-fed 54 MHz capture domain was removed +# (the bit engine is now clocked by the EXI pin itself), the EXI domain has +# ~2.4x margin against its 27 MHz target and the 24 MHz `sync` domain is the +# TIGHTER of the two. The sweep therefore ranks seeds by worst RELATIVE +# margin, not by the EXI number. # -# TRIED (2026-08 shell build) and REVERTED — prioritising routing toward the -# critical capture paths did NOT help: `--router router2 --tmg-ripup -# --placer-heap-timingweight 30 --placer-heap-critexp 4` left capture at -# 41-49 MHz (best 49.07, still < 54.02) across seeds AND eroded the slow-clock -# margin (24.4-25.9 MHz vs the 28-33 that plain --opt-timing gives). The -# capture shortfall is congestion + inherent path delay at this LC level, not a -# routing-priority tuning problem, so weighting P&R toward it only robs the -# sync domain. Plain --opt-timing is the better baseline. +# TRIED (2026-08, against the old capture domain) and REVERTED — prioritising +# routing toward the critical paths did NOT help: `--router router2 +# --tmg-ripup --placer-heap-timingweight 30 --placer-heap-critexp 4` left +# capture at 41-49 MHz across seeds AND eroded the slow-clock margin +# (24.4-25.9 MHz vs the 28-33 that plain --opt-timing gives). Plain +# --opt-timing is the better baseline. _PNR_TIMING_OPTS = "--opt-timing" # UART pin override for bring-up. The re-bba-rb V1 lab unit had its UART_RXD @@ -212,6 +217,12 @@ class IceBreakerPlatform(LatticeICE40Platform): # ── BBATop with platform resource wiring ───────────────────────────────── +# The GameCube's EXI clock ceiling. libogc's EXI_SPEED32MHZ is a nominal +# name; the bus actually tops out here, and the bit engine is clocked by it. +_EXI_HZ = 27e6 +_EXI_TARGET = 27.0 # MHz, as the sweep scores it + + class BBATopSynth(BBATop): """BBATop with platform pin connections added in elaborate().""" @@ -238,6 +249,26 @@ class BBATopSynth(BBATop): else: exi_clk_i, exi_mosi_i, exi_cs_n_i = exi.clk.i, exi.mosi.i, exi.cs_n.i + # The EXI clock pin now clocks the bit engine directly (see + # BBATop's clock-domain comment), so it is a REAL clock net and + # must be constrained. Without this nextpnr defaults it to + # 12 MHz and happily reports PASS on a build that would miss bits + # on a console. The GC tops out at ~27 MHz ("EXI_SPEED32MHZ" is a + # nominal name, not the actual rate). + # Constrain the net that actually BECOMES the clock, which + # differs between the two build variants: + # plain -- self.exi_clk is a pure alias of the pad and yosys + # absorbs it, leaving the pad net (nextpnr calls it + # 'exi_0__clk__i_$glb_clk'). + # EXI_J4 -- the clock is the AND of the two pads, a distinct net + # ('exi_clk_$glb_clk'); constraining either pad then + # misses, and nextpnr silently falls back to its 12 MHz + # default and under-optimises the whole EXI domain. + if _EXI_J4: + platform.add_clock_constraint(self.exi_clk, _EXI_HZ) + else: + platform.add_clock_constraint(exi.clk.i, _EXI_HZ) + m.d.comb += [ self.exi_clk .eq(exi_clk_i), self.exi_mosi .eq(exi_mosi_i), @@ -373,7 +404,8 @@ if __name__ == "__main__": best_seed = 1 best_fmax = 0.0 - results = [] # (seed, fmax_clk, fmax_capture, verdict), scored only + best_detail = None + results = [] # (seed, fmax_clk, fmax_exi, verdict), scored only for seed in range(1, n_seeds + 1): print(f"\n{'='*60}") print(f" Seed {seed}/{n_seeds}") @@ -412,10 +444,10 @@ if __name__ == "__main__": 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). + # names are 'clk' (sync, 24 MHz target) and the EXI pin's promoted + # global-clock net (27 MHz target), whose name nextpnr derives from + # the net and which CONTAINS a '$' — so the capture group must not be + # \w+ (that silently matched nothing and left seeds unscored). # 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. @@ -433,49 +465,63 @@ if __name__ == "__main__": 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) + r"Max frequency for clock\s+'([^']+)':\s*([\d.]+)\s*MHz", line) if m_: 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: + # The EXI domain is clocked by the GC's clock PIN, so nextpnr names it + # after the net it was promoted from (e.g. "exi_0__clk__i_$glb_clk") + # rather than a domain name we chose. Match on substring, and require + # EXACTLY ONE match so a renamed or duplicated net can never be scored + # silently against the wrong clock. + exi_keys = [k for k in fmax if "exi" in k] + if "clk" not in fmax or len(exi_keys) != 1: 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 + fmax_clk, fmax_exi = fmax["clk"], fmax[exi_keys[0]] + verdict = ("PASS" if fmax_exi >= _EXI_TARGET and fmax_clk >= 24.0 else "FAIL") - results.append((seed, fmax_clk, fmax_capture, verdict)) + results.append((seed, fmax_clk, fmax_exi, 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"exi fmax: {fmax_exi:.2f} MHz (target {_EXI_TARGET}) " 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 + # Rank by whichever domain has the least RELATIVE margin. The old + # script ranked by capture_clk because it was always the binding one; + # with the PLL gone the EXI domain has ~2.3x margin and `clk` can + # easily be the tighter of the two, so ranking by EXI alone would pick + # a seed that is worse overall. + score = min(fmax_exi / _EXI_TARGET, fmax_clk / 24.0) + if score > best_fmax: + best_fmax = score best_seed = seed + best_detail = (fmax_clk, fmax_exi) - # 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. + # Per-seed summary. 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}") + print(f" {'seed':>4} {'clk (≥24)':>10} {f'exi (≥{_EXI_TARGET})':>16} verdict") + for seed, fc, fexi, verdict in results: + print(f" {seed:>4} {fc:>10.2f} {fexi:>16.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']})") - 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}") + overall = "PASS" if best_fmax >= 1.0 else "FAIL" + if best_detail: + print(f"\nBest seed: {best_seed} clk {best_detail[0]:.2f} MHz, " + f"exi {best_detail[1]:.2f} MHz " + f"(worst margin {best_fmax:.2f}x) — {overall}") + else: + print(f"\nBest seed: {best_seed} — {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 — " @@ -487,9 +533,10 @@ if __name__ == "__main__": 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.") + print("\nREFUSING TO FLASH: no seed met both timing constraints. " + "The EXI front-end is clocked BY the GC's 27 MHz clock, so a " + "failing seed will miss bits on a console. Re-run with more " + "seeds (--seeds 16) or reduce logic.") elif do_flash: print(f"\nFlashing with seed {best_seed}...") opts = f"{_PNR_TIMING_OPTS} --seed {best_seed} --timing-allow-fail"