diff --git a/CLAUDE.md b/CLAUDE.md index 43956fd..be82a67 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -244,6 +244,47 @@ 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. +### ⚠️ TX bit phase MUST come from the rising-edge counter (fixed 2026-10-03) + +**Never give the TX shift register its own free-running falling-edge counter.** +`SPIMode3Slave` takes its byte phase from `rx_bit`, the RISING-edge counter — +the same edge the host counts bits on. A separate `tx_bit` counting falling +edges only stays in step while the host emits exactly as many falls as rises, +and it does not: a transfer that begins on a RISING edge (CS asserted while +CLK is already low, or the clock gating between two back-to-back host +transfers) delivers a rise with no matching fall. `tx_bit` then slips one bit +and never recovers. RX never notices, because the rising count always equals +the host's bit count — so the header decodes perfectly and only MISO is wrong. + +**Symptom on console:** every MISO byte shifted right by 2 bits, *identically +at every EXI speed index*. A NWAYS read returned `05 C0` (= `0x17` with two +leading zero bits) and the device ID `01 00 80 80`. The speed sweep read +`ok=[0,0,0,0,0,0]`. + +**How it was found, after two wrong diagnoses.** It is not propagation delay: +index 0 is 1 MHz, a 1000 ns bit time, and no output path fails there — +speed-independence is the tell. It is not the output edge either: the old +rising-edge build and the new falling-edge build returned *byte-identical* +`05 C0`, and an edge flip would have moved it. What settled it was a +measurement, not more theory — see PHASE_METER below. + +**`PHASE_METER=1 python -m exi_bba.synth`** builds a DIAGNOSTIC, +NON-FUNCTIONAL bitstream in which the bit engine transmits a constant `0x80` +for every byte instead of the FIFO head. The register file and both byte FIFOs +drop out of the MISO path, so whatever the host reads back is purely the +engine's byte phase: `0x80` aligned, `0x40` one bit late, `0x20` two bits late. +On hardware it read `0x20` in all 8 slots, which localised the fault to the bit +engine and ruled out everything upstream in one run. Re-flash a normal build +afterwards. + +**Regression test:** `spi_mode3_slave` T5 asserts CS while CLK is LOW, so the +transaction begins on a rising edge and carries one fewer falling edge. The +old counter fails it by exactly one bit (`A5 3C` → `52 9E`). Every earlier +testbench asserted CS with CLK high, so falls and rises always matched and the +slip was *structurally invisible* — the same failure of imagination as the +Mode 2/3 mix-up, which was invisible to a master that parked MOSI early. When +adding SPI tests, vary the edge the transaction starts on. + ### EXI on the J4 debug header (`EXI_J4=1`) — bench host without SP1 `EXI_J4=1` lets a bench host (the Pi) drive EXI through the unpopulated J4 diff --git a/exi_bba/bba_top.py b/exi_bba/bba_top.py index 23bd970..73a29d5 100644 --- a/exi_bba/bba_top.py +++ b/exi_bba/bba_top.py @@ -53,11 +53,13 @@ class BBATop(Elaboratable): udp_socket=3, udp_src_ip="192.168.1.123", udp_subnet="255.255.255.0", udp_gateway="192.168.1.1", udp_dst_ip="192.168.1.100", - udp_src_port=40000, udp_dst_port=6464): + udp_src_port=40000, udp_dst_port=6464, + phase_meter=False): # Ethernet back-end: "w5100" (indirect parallel bus, reaches the EXI # ceiling) or "w5500" (SPI, ~12 Mbit/s). Both expose the identical # tx/rx/init/par interface, so only the physical pins differ. self._eth = eth + self._phase_meter = phase_meter # diagnostic, see SPIMode3Slave # MR-reset settle wait passed to the ethernet master (~1 ms on hardware; # the testbench overrides with a small value for fast simulation). self._reset_cycles = reset_cycles @@ -160,7 +162,7 @@ class BBATop(Elaboratable): # (simulation: the test harness drives the EXI pins and `sync` directly) # ── Submodules ──────────────────────────────────────────────────── - cap = ExiCapture() # SPI bit engine (capture) + byte FIFOs + cap = ExiCapture(phase_meter=self._phase_meter) reg = BBARegisterFile() arb = SPRAMArbiter() asm = RXFrameAssembler() diff --git a/exi_bba/exi_capture.py b/exi_bba/exi_capture.py index 25b70a3..5a9f978 100644 --- a/exi_bba/exi_capture.py +++ b/exi_bba/exi_capture.py @@ -72,9 +72,10 @@ class ExiCapture(Elaboratable): tx_rdy : tx_fifo has room """ - def __init__(self, rx_depth=4, tx_depth=2): + def __init__(self, rx_depth=4, tx_depth=2, phase_meter=False): self._rx_depth = rx_depth self._tx_depth = tx_depth + self._phase_meter = phase_meter # diagnostic, see SPIMode3Slave # Physical SPI (capture domain, wired to pins by BBATop) self.spi_clk = Signal(init=1) @@ -129,7 +130,7 @@ class ExiCapture(Elaboratable): cd_fr.clk.eq(self.spi_clk), ] - spi = SPIMode3Slave() + spi = SPIMode3Slave(phase_meter=self._phase_meter) m.submodules.spi = spi # RX bytes cross exiclk -> exi; TX bytes cross exi -> exiclk_n. diff --git a/exi_bba/spi_mode3_slave.py b/exi_bba/spi_mode3_slave.py index 91a35d1..100a5e4 100644 --- a/exi_bba/spi_mode3_slave.py +++ b/exi_bba/spi_mode3_slave.py @@ -59,7 +59,15 @@ class SPIMode3Slave(Elaboratable): necessary. """ - def __init__(self): + def __init__(self, phase_meter=False): + # DIAGNOSTIC: when set, every byte the engine transmits is the constant + # 0x80 instead of the FIFO head. The load/shift/mux/output path is + # otherwise untouched, so the register file and both byte FIFOs drop + # out of the picture and what the host reads back is purely the + # engine's byte PHASE: 0x80 = aligned, 0x40 = 1 bit late, 0x20 = 2 bits + # late, and so on. Not a functional build. + self._phase_meter = phase_meter + self.spi_clk = Signal(init=1) # idles HIGH self.spi_mosi = Signal() self.spi_cs_n = Signal(init=1) # active LOW @@ -109,16 +117,36 @@ class SPIMode3Slave(Elaboratable): tx_hold = Signal(8) m.d.comb += self.tx_load.eq(rx_bit == 7) with m.If(rx_bit == 7): - m.d.exiclk += tx_hold.eq(self.tx_byte) + m.d.exiclk += tx_hold.eq(0x80 if self._phase_meter else self.tx_byte) # MISO must settle on the falling edge, before the master samples it # on the rising one. Driving it on the rising edge is one bit late by # construction -- that was the 05 C0 bug. + # The byte phase comes from `rx_bit` -- the RISING-edge counter -- NOT + # from a free-running falling-edge counter of its own. + # + # A separate tx_bit counting falling edges only stays in step with the + # host while the host emits exactly as many falls as rises. It does + # not: a transfer that begins on a rising edge (CS asserted while CLK + # is low, or the clock gating between two of the host's back-to-back + # transfers) delivers a rise with no matching fall, and tx_bit silently + # slips one bit behind for the rest of the transaction. RX never + # notices, because the rising count always equals the host's bit count. + # + # Measured on a GameCube 2026-10-03 with a diagnostic build that drives + # a constant 0x80 per byte (PHASE_METER=1 in synth.py): every slot read + # back 0x20 = 0x80 >> 2, i.e. the engine was TWO falling edges behind, + # identically at 1 MHz and at 27 MHz. Speed-independence is the tell — + # a propagation problem cannot survive a 1000 ns bit time, and an + # output-edge error would have moved when the edge was flipped (it did + # not: the rising-edge and falling-edge builds returned byte-identical + # 05 C0 for a NWAYS read). + # + # Slaving the shift register to rx_bit gives the engine ONE phase + # reference, the same edge the host counts, so the two cannot drift. tx_shift = Signal(8) - tx_bit = Signal(3) - m.d.exiclk_n += tx_bit.eq(tx_bit + 1) - with m.If(tx_bit == 0): - m.d.exiclk_n += tx_shift.eq(tx_hold) # first falling edge: MSB + with m.If(rx_bit == 0): + m.d.exiclk_n += tx_shift.eq(tx_hold) # byte boundary: load MSB with m.Else(): m.d.exiclk_n += tx_shift.eq(Cat(0, tx_shift[:-1])) @@ -247,6 +275,30 @@ if __name__ == "__main__": # another EXI device sharing the bus. check("T4 miso idles high", ctx.get(dut.spi_miso), 1) + # T5 — a transaction that BEGINS ON A RISING EDGE. The host asserts + # CS while CLK is already low, so the first edge the device sees is a + # rise and the transaction carries one fewer FALLING edge than rising. + # A TX phase counted on falling edges slips one bit here and never + # recovers; a phase taken from rx_bit cannot. + # + # THIS IS THE HARDWARE BUG. On a GameCube the phase-meter build read + # 0x20 (= 0x80 >> 2) in every slot, identically at 1 MHz and 27 MHz. + # Nothing in the old testbench started a transaction this way -- every + # case asserted CS with CLK high -- so the falling and rising counts + # always matched and the slip was structurally invisible, exactly as + # the Mode 2/3 mix-up was invisible to a master that parked MOSI early. + ctx.set(dut.spi_clk, 0) # CLK already LOW ... + ctx.set(dut.spi_cs_n, 0) # ... when CS asserts + await ctx.delay(HP) + ctx.set(dut.tx_byte, 0xA5) + await spi_byte(ctx, 0x00) # slot 0: header, MISO don't-care + ctx.set(dut.tx_byte, 0x3C) + s1 = await spi_byte(ctx, 0x00) + s2 = await spi_byte(ctx, 0x00) + await cs_high(ctx) + check("T5 rising-edge start, slot1", hex(s1), hex(0xA5)) + check("T5 rising-edge start, slot2", hex(s2), hex(0x3C)) + sim = Simulator(top) sim.add_testbench(tb) with sim.write_vcd("SPIMode3Slave.vcd"): diff --git a/exi_bba/synth.py b/exi_bba/synth.py index c1b878f..fa31953 100644 --- a/exi_bba/synth.py +++ b/exi_bba/synth.py @@ -387,10 +387,24 @@ if __name__ == "__main__": udp_dst_port = int(os.environ.get("UDP_DST_PORT", "6464")), ) + # PHASE_METER=1 builds a DIAGNOSTIC, NON-FUNCTIONAL bitstream: the bit + # engine transmits the constant 0x80 for every byte instead of the FIFO + # head, taking the register file and both byte FIFOs out of the MISO path. + # Whatever the host reads back is then purely the engine's byte PHASE + # relative to the host's -- 0x80 aligned, 0x40 one bit late, 0x20 two bits + # late. Used to measure the 2-bit MISO offset seen on console 2026-10-03, + # which simulation does not reproduce. Re-flash a normal build after. + _PHASE_METER = os.environ.get("PHASE_METER") == "1" + if _PHASE_METER: + print("*** PHASE_METER build: MISO is a constant 0x80 pattern. " + "This bitstream is a measuring instrument, NOT a working BBA. ***") + def make_dut(): if use_shell: - return BBATopSynth(status_panel=True, uart_shell=True, **udp_kw) - return BBATopSynth(status_panel=True, uart_console=True) + return BBATopSynth(status_panel=True, uart_shell=True, + phase_meter=_PHASE_METER, **udp_kw) + return BBATopSynth(status_panel=True, uart_console=True, + phase_meter=_PHASE_METER) print(f"Synthesizing BBATop for {IceBreakerPlatform.device}-" f"{IceBreakerPlatform.package} "