From dac9c0c0650b6db69479ffc86dfe6aae5e349904 Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Sat, 10 Oct 2026 19:25:51 +0200 Subject: [PATCH] bba_register_file: delete the dead rx_rptr FIFO and unused constants An audit for signals that are written but never read, ports nothing wires, and constants only the tests touch turned up something bigger than stray signals: the whole rx_rptr SyncFIFO was dead. asm.rrp is wired from reg.rrp -- the combinational tap -- and the FIFO's far side (rx_rptr_r_data / _r_en / _r_rdy) is wired NOWHERE in BBATop. It is a leftover from when the register file and the frame assembler sat in different clock domains; when those merged the direct tap replaced it and the FIFO was never removed. Gone: a 4x8 SyncFIFO, its write logic in the RRP case, its default strobe and three ports. Also removed _RWP_HI and _RRP_HI, which were never referenced. The register table now says why instead: the ring is 15 pages, so the high bytes of both pointers are always zero and the read mux's default already returns that. NCRB_PR went the other way -- it was used only by a test while BBATop steered the ethernet address filter from a bare `reg.ncrb[0]`. It is now public and BBATop imports it, so the constant is live and the magic index is gone. Kept deliberately: _NCRB_PM and _NCRB_AB are referenced only by T15, but they document the filtering bits of a real register and make that assertion readable, so they stay. Area went DOWN, which is the evidence the FIFO was real logic rather than something yosys had already pruned: ICESTORM_LC 4350 original -> 4380 after the docs pass -> 4328 now Full suite passes, bba_top passes at EXI_HALF=1/2/8. Co-Authored-By: Claude Opus 5 --- exi_bba/bba_register_file.py | 36 ++++++++++++++---------------------- exi_bba/bba_top.py | 4 ++-- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index aef860b..80fa6f7 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -45,7 +45,7 @@ from types import SimpleNamespace from amaranth import * from amaranth.lib.fifo import SyncFIFO -__all__ = ["BBARegisterFile"] +__all__ = ["BBARegisterFile", "NCRB_PR"] # ─────────────────────────── register space ────────────────────────────── @@ -66,10 +66,12 @@ _TLBP_LO = 0x0C # TX Low Boundary Page: start of the TX buffer region _TLBP_HI = 0x0D _RXINTT_LO = 0x14 # RX Interrupt Timer: interrupt coalescing delay _RXINTT_HI = 0x15 -_RWP_LO = 0x16 # RX Write Pointer (page) — device-owned, follows the -_RWP_HI = 0x17 # frame assembler -_RRP_LO = 0x18 # RX Read Pointer (page) — driver-owned, written once the -_RRP_HI = 0x19 # host has consumed a frame +_RWP_LO = 0x16 # RX Write Pointer (page) — device-owned, follows the frame + # assembler. The high byte (0x17) is not decoded: the + # ring is 15 pages, so it is always zero and the read + # mux's default already returns that. +_RRP_LO = 0x18 # RX Read Pointer (page) — host-owned, written once it has + # consumed a frame. High byte (0x19) as above. _RHBP_LO = 0x1A # RX High Boundary Page: last page of the RX ring _RHBP_HI = 0x1B _PAR0 = 0x20 # Physical Address Registers 0-5: the MAC, PAR0 first on @@ -107,8 +109,9 @@ _MISC_VAL = 0x3C _BP_VAL = _RX_PAGE_FIRST _RHBP_VAL = 0x0F -# NCRB bit positions. -_NCRB_PR = 0 # promiscuous: accept every frame +# NCRB bit positions. PR is public because BBATop steers the ethernet chip's +# address filter from it. +NCRB_PR = 0 # promiscuous: accept every frame _NCRB_PM = 2 # pass multicast _NCRB_AB = 4 # accept broadcast @@ -198,13 +201,11 @@ class BBARegisterFile(Elaboratable): self.tx_ctrl_r_data = Signal(16) self.tx_ctrl_r_en = Signal() self.tx_ctrl_r_rdy = Signal() - # RX ring write pointer in from the assembler, read pointer out to it. + # RX ring write pointer in from the assembler. The read pointer goes + # back out through the `rrp` tap above, not a FIFO. self.rx_wptr_w_data = Signal(8) self.rx_wptr_w_en = Signal() self.rx_wptr_w_rdy = Signal() - self.rx_rptr_r_data = Signal(8) - self.rx_rptr_r_en = Signal() - self.rx_rptr_r_rdy = Signal() # ──────────────────────────── structure ───────────────────────────── @@ -219,13 +220,11 @@ class BBARegisterFile(Elaboratable): tx_bytes=SyncFIFO(width=8, depth=2048), 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 = f.spram_req m.submodules.tx_bytes = f.tx_bytes m.submodules.tx_ctrl = f.tx_ctrl m.submodules.rx_wptr = f.rx_wptr - m.submodules.rx_rptr = f.rx_rptr m.d.comb += [ self.spram_req_r_data.eq(f.spram_req.r_data), @@ -243,10 +242,6 @@ class BBARegisterFile(Elaboratable): f.rx_wptr.w_data .eq(self.rx_wptr_w_data), f.rx_wptr.w_en .eq(self.rx_wptr_w_en), self.rx_wptr_w_rdy .eq(f.rx_wptr.w_rdy), - - self.rx_rptr_r_data.eq(f.rx_rptr.r_data), - f.rx_rptr.r_en .eq(self.rx_rptr_r_en), - self.rx_rptr_r_rdy .eq(f.rx_rptr.r_rdy), ] return f @@ -389,10 +384,8 @@ class BBARegisterFile(Elaboratable): with m.Case(_IR): m.d.sync += r.ir.eq(r.ir & ~self.rx_data) # write-1-to-clear with m.Case(_RRP_LO): + # The assembler sees this through the `rrp` tap. m.d.sync += r.rrp_lo.eq(self.rx_data) - with m.If(f.rx_rptr.w_rdy): # forward to the assembler - m.d.sync += [f.rx_rptr.w_data.eq(self.rx_data), - f.rx_rptr.w_en.eq(1)] with m.Case(_PAR0, _PAR1, _PAR2, _PAR3, _PAR4, _PAR5): m.d.sync += r.par[t.eff_addr[0:3]].eq(self.rx_data) with m.Case(_TWD_LO): @@ -736,7 +729,6 @@ class BBARegisterFile(Elaboratable): m.d.sync += [ f.tx_bytes.w_en.eq(0), f.tx_ctrl.w_en .eq(0), - f.rx_rptr.w_en .eq(0), f.rx_wptr.r_en .eq(0), t.ncra_rst .eq(0), ] @@ -1142,7 +1134,7 @@ if __name__ == "__main__": # survive: 0x52 = AB | CA | 2_PACKETS_PER_INT, with PR and PM CLEAR # (so the real BBA delivers own-MAC + broadcast only). ncrb = ctx.get(reg.ncrb) - pr = (ncrb >> _NCRB_PR) & 1 + pr = (ncrb >> NCRB_PR) & 1 pm = (ncrb >> _NCRB_PM) & 1 ab = (ncrb >> _NCRB_AB) & 1 print(f"T15 ncrb export = 0x{ncrb:02X} (PR={pr} PM={pm} AB={ab})") diff --git a/exi_bba/bba_top.py b/exi_bba/bba_top.py index 26ad8c2..51a72c4 100644 --- a/exi_bba/bba_top.py +++ b/exi_bba/bba_top.py @@ -16,7 +16,7 @@ See CLAUDE.md "Module Breakdown" and "CDC Signal Inventory" for the full list. from amaranth import * from exi_bba.exi_capture import ExiCapture -from exi_bba.bba_register_file import BBARegisterFile +from exi_bba.bba_register_file import BBARegisterFile, NCRB_PR from exi_bba.spram_arbiter import SPRAMArbiter from exi_bba.rx_frame_assembler import RXFrameAssembler from exi_bba.tx_frame_drain import TXFrameDrain @@ -318,7 +318,7 @@ class BBATop(Elaboratable): # ask for promiscuous by setting PR. See the S0_MODE comment in # w5100_parallel_master.py -- running unfiltered hung Swiss. if self._eth == "w5100": - m.d.comb += eth.promisc.eq(reg.ncrb[0]) + m.d.comb += eth.promisc.eq(reg.ncrb[NCRB_PR]) # ── Optional bring-up peripherals (sync domain) ─────────────────── # Build init_req as an OR of all reinit sources (NCRA pulse plus any