From 41d487e1a79f40aafe23e5a28e7ce63e845bb186 Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Sat, 10 Oct 2026 19:42:02 +0200 Subject: [PATCH] Cut dead signals and the unused W5100 debug peek An audit for signals that are written but never read turned up three things no longer connected to anything. All verified unreferenced across every module, testbench and script before removal. exi_capture cs_n_prev registered on sync, read by nothing. w5100 udp_rx_sof driven in the UDP rx stream mux; no consumer uses it. uart_shell needs only data/valid/eof, because the request frames the transfer. The MACRAW rx interface keeps its sof -- the frame assembler does need it. w5100 the debug peek 4 ports, 2 FSM states (DBG_RD/DBG_RD_W) and a branch in the IDLE arbiter's priority chain. dbg_rd_req was driven by nothing -- not even its own testbench -- so this was untested logic sitting in the path that decides what the bus master does next. Kept deliberately, against the audit: the unused register-address constants in both ethernet masters (_IR, _TMSR, _S0_SR, _S0_TX_FSR, _S0_TX_RD and the three W5500 buffer ones). They cost no logic and they document the chip's register map in one place; that map was the source of three hardware bugs, and a partial map invites re-guessing the missing half. Same for status_panel's eth_rst_n, which is a documented panel feature synth.py has not wired yet -- a gap, not dead weight. And rx_frame_assembler's geom_honoured, which is one comb line asserting a property the tests otherwise could not observe. Also fixed the register file's opening docstring, which claimed the module did "three jobs" and invited splitting it into three. It does not: the interrupt level is IR & IMR, a frame arriving is a register side effect, and the read mux feeds the TX stream combinationally in the cycle the FSM pushes it. Those are facets of one job that share the register bank, not separable parts. _read_mux now records why that path must stay unregistered -- one pipeline register there is 42 ns against a 296 ns byte budget, which is the tx FIFO underrun that the +1 descriptor hunt ended in. Full suite passes; bba_top T9 6/6 at EXI_HALF=1, 2 and 8; W5100 elaborates with enable_udp_test both ways. Sweep: 8/8 seeds PASS, best seed 3 at clk 29.08 MHz (1.21x the 24 MHz target), and area down again: ICESTORM_LC 4350 original -> 4328 after the rx_rptr cut -> 4305 now Co-Authored-By: Claude Opus 5 --- exi_bba/bba_register_file.py | 25 ++++++++++++++++++------- exi_bba/exi_capture.py | 2 -- exi_bba/w5100_parallel_master.py | 25 ------------------------- 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index 80fa6f7..cbbbf66 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -1,12 +1,15 @@ -"""BBA register file — the GameCube-facing side of the adapter. +"""BBA register file — presents the BBA register interface to the GameCube. -Sits between the EXI byte stream (`ExiCapture`) and the ethernet side, and does -three jobs: +Sits between the EXI byte stream (`ExiCapture`) and the ethernet side. It +decodes EXI transactions into register reads and writes, and carries the +results across to the ethernet engines: TX frame bytes and the transmit length +out, the RX write pointer and interrupt events back in. Interrupt state is a +function of the register bank, so it lives here too. - * decode EXI transactions and drive the register space; - * buffer TX frame bytes and RX ring pointers across to the ethernet engines - through `SyncFIFO`s; - * raise the two levels of BBA interrupt. +Those are facets of one job, not separable parts: the interrupt level is +`IR & IMR`, a frame arriving is a register side effect, and the read mux feeds +the TX stream combinationally in the same cycle the FSM pushes it. See the +note on `_read_mux` for why that path must stay unregistered. Everything here runs in the `sync` domain (24 MHz). The per-bit SPI cadence is `SPIMode3Slave`'s problem; this module only ever sees whole bytes. @@ -302,6 +305,14 @@ class BBARegisterFile(Elaboratable): Returns `rd_val`, valid for both address spaces: `t.cmd_space` selects which decode applies. SPRAM reads do not come through here. + + This path must stay unregistered. The FSM pushes `rd_val` into the TX + byte FIFO in the same cycle it evaluates it, and the host gives the + device one byte time to answer -- 296 ns at 27 MHz -- for a round trip + that already spends ~240 ns in the two byte FIFOs. A single pipeline + register here is 42 ns and overruns that budget, which shows up as a + TX FIFO underrun: the bit engine resends the previous byte and every + byte after it shifts by one. """ reg_rd = Signal(8, name="reg_rdval") with m.Switch(t.rd_sel): diff --git a/exi_bba/exi_capture.py b/exi_bba/exi_capture.py index 7403976..6ad21f1 100644 --- a/exi_bba/exi_capture.py +++ b/exi_bba/exi_capture.py @@ -186,8 +186,6 @@ class ExiCapture(Elaboratable): cs_n_s = Signal(init=1) m.submodules.cs_sync = FFSynchronizer(self.spi_cs_n, cs_n_s, o_domain="sync", init=1) - cs_n_prev = Signal(init=1) - m.d.sync += cs_n_prev.eq(cs_n_s) m.d.comb += self.cs_active.eq(~cs_n_s) return m diff --git a/exi_bba/w5100_parallel_master.py b/exi_bba/w5100_parallel_master.py index d29bc66..ccddb02 100644 --- a/exi_bba/w5100_parallel_master.py +++ b/exi_bba/w5100_parallel_master.py @@ -203,7 +203,6 @@ class W5100ParallelMaster(Elaboratable): self.udp_rx_none = Signal() self.udp_rx_data = Signal(8) self.udp_rx_valid = Signal() - self.udp_rx_sof = Signal() self.udp_rx_eof = Signal() self.udp_rx_ready = Signal() @@ -222,13 +221,6 @@ class W5100ParallelMaster(Elaboratable): self.init_req = Signal() self.init_done = Signal() - # Debug register peek (bring-up diagnostics): pulse dbg_rd_req with - # dbg_rd_addr set; dbg_rd_data is valid when dbg_rd_done pulses. Reads - # one byte over the indirect bus, reusing the fixed-length read path. - self.dbg_rd_req = Signal() - self.dbg_rd_addr = Signal(16) - self.dbg_rd_data = Signal(8) - self.dbg_rd_done = Signal() self.par = Signal(48) # MAC address (PAR0..5 packed) # NCRB.PR from the GC. Low (the normal case) means MACRAW filters to # own-MAC + broadcast, which is what the real BBA does -- see the @@ -353,7 +345,6 @@ class W5100ParallelMaster(Elaboratable): with m.If(udp_rx_streaming): m.d.comb += [self.udp_rx_data.eq(r_data), self.udp_rx_valid.eq(r_valid), - self.udp_rx_sof.eq(r_first), self.udp_rx_eof.eq(r_last), r_ready.eq(self.udp_rx_ready)] with m.Else(): @@ -563,7 +554,6 @@ class W5100ParallelMaster(Elaboratable): # ── Main control FSM (Phase 1: init only) ──────────────────────────── if self._enable_udp: m.d.sync += self.udp_rx_none.eq(0) # pulse: default low - m.d.sync += self.dbg_rd_done.eq(0) # pulse: default low with m.FSM(domain="sync", name="main_fsm"): with m.State("IDLE"): m.d.sync += self.init_done.eq(0) @@ -581,21 +571,6 @@ class W5100ParallelMaster(Elaboratable): m.next = "UDP_DIPR" with m.Elif(self.udp_rx_req): m.next = "UDP_RX_RSR" - with m.Elif(self.dbg_rd_req): - m.next = "DBG_RD" - - # Debug peek: read one byte from dbg_rd_addr over the indirect bus. - with m.State("DBG_RD"): - m.d.sync += [xfer_addr.eq(self.dbg_rd_addr), xfer_rw.eq(0), - xfer_stream.eq(0), xfer_sread.eq(0), xfer_wrap.eq(0), - xfer_direct.eq(0), xfer_len.eq(1), xfer_start.eq(1)] - m.next = "DBG_RD_W" - with m.State("DBG_RD_W"): - m.d.sync += xfer_start.eq(0) - with m.If(xfer_done): - m.d.sync += [self.dbg_rd_data.eq(rbuf[0]), - self.dbg_rd_done.eq(1)] - m.next = "IDLE" # Pulse the W5100's RSTn pin, then let it boot, BEFORE touching the # bus. This used to be missing entirely: w5100_rst_n was declared