Take the TX bit phase from the rising-edge counter, not a falling-edge one

The bit engine had TWO independent phase references: rx_bit counting rising
edges and tx_bit counting falling edges.  Those only stay 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, the right register is selected, and ONLY MISO
is wrong -- which is what made this look like an output-edge or output-delay
problem for two rounds of debugging.

Measured on a GameCube: every MISO byte shifted right by 2 bits, identically
at every EXI speed index.  NWAYS read 05 C0 (= 0x17 with two leading zero
bits), device ID 01 00 80 80, speed sweep ok=[0,0,0,0,0,0].

Two diagnoses were wrong before this one, and both were ruled out by evidence
rather than argument:
  - NOT propagation delay.  Index 0 is 1 MHz, a 1000 ns bit time; no output
    path fails there.  Speed-independence is the tell.
  - NOT the output edge.  The old rising-edge build and the new falling-edge
    build returned BYTE-IDENTICAL 05 C0.  Flipping the edge would have moved
    it; it did not, so the slip is upstream of the output stage.

What settled it was a measurement.  PHASE_METER=1 builds a diagnostic,
non-functional bitstream whose engine transmits a constant 0x80 per byte
instead of the FIFO head, taking the register file and both byte FIFOs out of
the MISO path so the host reads the engine's byte PHASE directly: 0x80
aligned, 0x40 one bit late, 0x20 two bits late.  Hardware read 0x20 in all
eight slots, localising the fault to the bit engine in a single run.

Fix: the shift register loads on rx_bit == 0.  One phase reference, on the
edge the host counts, so the two cannot drift.

Regression test (spi_mode3_slave T5) asserts CS while CLK is LOW so the
transaction begins on a rising edge.  The old counter fails it by exactly one
bit: A5 3C comes back as 52 9E.  EVERY earlier testbench asserted CS with CLK
high, so falls and rises always matched and the slip was structurally
invisible -- the same blind spot as the Mode 2/3 mix-up, which was invisible
to a master that parked MOSI early.  Vary the starting edge in SPI tests.

Timing after the fix, EXI_J4=1, --seeds 8: clk 26.9-31.1 MHz, EXI 45.8-61.7
MHz, 8/8 PASS.  Best seed 4 (clk 31.07, EXI 56.70), flashed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-10-03 19:04:52 +02:00
parent 91c3bf0ece
commit d35688390e
5 changed files with 122 additions and 12 deletions
+41
View File
@@ -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 part cannot close. `SPIMode3Slave` has no `mode2` parameter any more — there is
only one correct convention and it is enforced by mutation tests. 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 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 `EXI_J4=1` lets a bench host (the Pi) drive EXI through the unpopulated J4
+4 -2
View File
@@ -53,11 +53,13 @@ class BBATop(Elaboratable):
udp_socket=3, udp_src_ip="192.168.1.123", udp_socket=3, udp_src_ip="192.168.1.123",
udp_subnet="255.255.255.0", udp_gateway="192.168.1.1", udp_subnet="255.255.255.0", udp_gateway="192.168.1.1",
udp_dst_ip="192.168.1.100", 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 # Ethernet back-end: "w5100" (indirect parallel bus, reaches the EXI
# ceiling) or "w5500" (SPI, ~12 Mbit/s). Both expose the identical # ceiling) or "w5500" (SPI, ~12 Mbit/s). Both expose the identical
# tx/rx/init/par interface, so only the physical pins differ. # tx/rx/init/par interface, so only the physical pins differ.
self._eth = eth self._eth = eth
self._phase_meter = phase_meter # diagnostic, see SPIMode3Slave
# MR-reset settle wait passed to the ethernet master (~1 ms on hardware; # MR-reset settle wait passed to the ethernet master (~1 ms on hardware;
# the testbench overrides with a small value for fast simulation). # the testbench overrides with a small value for fast simulation).
self._reset_cycles = reset_cycles self._reset_cycles = reset_cycles
@@ -160,7 +162,7 @@ class BBATop(Elaboratable):
# (simulation: the test harness drives the EXI pins and `sync` directly) # (simulation: the test harness drives the EXI pins and `sync` directly)
# ── Submodules ──────────────────────────────────────────────────── # ── Submodules ────────────────────────────────────────────────────
cap = ExiCapture() # SPI bit engine (capture) + byte FIFOs cap = ExiCapture(phase_meter=self._phase_meter)
reg = BBARegisterFile() reg = BBARegisterFile()
arb = SPRAMArbiter() arb = SPRAMArbiter()
asm = RXFrameAssembler() asm = RXFrameAssembler()
+3 -2
View File
@@ -72,9 +72,10 @@ class ExiCapture(Elaboratable):
tx_rdy : tx_fifo has room 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._rx_depth = rx_depth
self._tx_depth = tx_depth self._tx_depth = tx_depth
self._phase_meter = phase_meter # diagnostic, see SPIMode3Slave
# Physical SPI (capture domain, wired to pins by BBATop) # Physical SPI (capture domain, wired to pins by BBATop)
self.spi_clk = Signal(init=1) self.spi_clk = Signal(init=1)
@@ -129,7 +130,7 @@ class ExiCapture(Elaboratable):
cd_fr.clk.eq(self.spi_clk), cd_fr.clk.eq(self.spi_clk),
] ]
spi = SPIMode3Slave() spi = SPIMode3Slave(phase_meter=self._phase_meter)
m.submodules.spi = spi m.submodules.spi = spi
# RX bytes cross exiclk -> exi; TX bytes cross exi -> exiclk_n. # RX bytes cross exiclk -> exi; TX bytes cross exi -> exiclk_n.
+58 -6
View File
@@ -59,7 +59,15 @@ class SPIMode3Slave(Elaboratable):
necessary. 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_clk = Signal(init=1) # idles HIGH
self.spi_mosi = Signal() self.spi_mosi = Signal()
self.spi_cs_n = Signal(init=1) # active LOW self.spi_cs_n = Signal(init=1) # active LOW
@@ -109,16 +117,36 @@ class SPIMode3Slave(Elaboratable):
tx_hold = Signal(8) tx_hold = Signal(8)
m.d.comb += self.tx_load.eq(rx_bit == 7) m.d.comb += self.tx_load.eq(rx_bit == 7)
with m.If(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 # 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 # on the rising one. Driving it on the rising edge is one bit late by
# construction -- that was the 05 C0 bug. # 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_shift = Signal(8)
tx_bit = Signal(3) with m.If(rx_bit == 0):
m.d.exiclk_n += tx_bit.eq(tx_bit + 1) m.d.exiclk_n += tx_shift.eq(tx_hold) # byte boundary: load MSB
with m.If(tx_bit == 0):
m.d.exiclk_n += tx_shift.eq(tx_hold) # first falling edge: MSB
with m.Else(): with m.Else():
m.d.exiclk_n += tx_shift.eq(Cat(0, tx_shift[:-1])) 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. # another EXI device sharing the bus.
check("T4 miso idles high", ctx.get(dut.spi_miso), 1) 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 = Simulator(top)
sim.add_testbench(tb) sim.add_testbench(tb)
with sim.write_vcd("SPIMode3Slave.vcd"): with sim.write_vcd("SPIMode3Slave.vcd"):
+16 -2
View File
@@ -387,10 +387,24 @@ if __name__ == "__main__":
udp_dst_port = int(os.environ.get("UDP_DST_PORT", "6464")), 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(): def make_dut():
if use_shell: if use_shell:
return BBATopSynth(status_panel=True, uart_shell=True, **udp_kw) return BBATopSynth(status_panel=True, uart_shell=True,
return BBATopSynth(status_panel=True, uart_console=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}-" print(f"Synthesizing BBATop for {IceBreakerPlatform.device}-"
f"{IceBreakerPlatform.package} " f"{IceBreakerPlatform.package} "