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 <noreply@anthropic.com>
This commit is contained in:
2026-10-10 19:25:51 +02:00
parent b175cb0e77
commit dac9c0c065
2 changed files with 16 additions and 24 deletions
+14 -22
View File
@@ -45,7 +45,7 @@ from types import SimpleNamespace
from amaranth import * from amaranth import *
from amaranth.lib.fifo import SyncFIFO from amaranth.lib.fifo import SyncFIFO
__all__ = ["BBARegisterFile"] __all__ = ["BBARegisterFile", "NCRB_PR"]
# ─────────────────────────── register space ────────────────────────────── # ─────────────────────────── register space ──────────────────────────────
@@ -66,10 +66,12 @@ _TLBP_LO = 0x0C # TX Low Boundary Page: start of the TX buffer region
_TLBP_HI = 0x0D _TLBP_HI = 0x0D
_RXINTT_LO = 0x14 # RX Interrupt Timer: interrupt coalescing delay _RXINTT_LO = 0x14 # RX Interrupt Timer: interrupt coalescing delay
_RXINTT_HI = 0x15 _RXINTT_HI = 0x15
_RWP_LO = 0x16 # RX Write Pointer (page) — device-owned, follows the _RWP_LO = 0x16 # RX Write Pointer (page) — device-owned, follows the frame
_RWP_HI = 0x17 # frame assembler # assembler. The high byte (0x17) is not decoded: the
_RRP_LO = 0x18 # RX Read Pointer (page) — driver-owned, written once the # ring is 15 pages, so it is always zero and the read
_RRP_HI = 0x19 # host has consumed a frame # 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_LO = 0x1A # RX High Boundary Page: last page of the RX ring
_RHBP_HI = 0x1B _RHBP_HI = 0x1B
_PAR0 = 0x20 # Physical Address Registers 0-5: the MAC, PAR0 first on _PAR0 = 0x20 # Physical Address Registers 0-5: the MAC, PAR0 first on
@@ -107,8 +109,9 @@ _MISC_VAL = 0x3C
_BP_VAL = _RX_PAGE_FIRST _BP_VAL = _RX_PAGE_FIRST
_RHBP_VAL = 0x0F _RHBP_VAL = 0x0F
# NCRB bit positions. # NCRB bit positions. PR is public because BBATop steers the ethernet chip's
_NCRB_PR = 0 # promiscuous: accept every frame # address filter from it.
NCRB_PR = 0 # promiscuous: accept every frame
_NCRB_PM = 2 # pass multicast _NCRB_PM = 2 # pass multicast
_NCRB_AB = 4 # accept broadcast _NCRB_AB = 4 # accept broadcast
@@ -198,13 +201,11 @@ class BBARegisterFile(Elaboratable):
self.tx_ctrl_r_data = Signal(16) self.tx_ctrl_r_data = Signal(16)
self.tx_ctrl_r_en = Signal() self.tx_ctrl_r_en = Signal()
self.tx_ctrl_r_rdy = 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_data = Signal(8)
self.rx_wptr_w_en = Signal() self.rx_wptr_w_en = Signal()
self.rx_wptr_w_rdy = 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 ───────────────────────────── # ──────────────────────────── structure ─────────────────────────────
@@ -219,13 +220,11 @@ class BBARegisterFile(Elaboratable):
tx_bytes=SyncFIFO(width=8, depth=2048), tx_bytes=SyncFIFO(width=8, depth=2048),
tx_ctrl=SyncFIFO(width=16, depth=4), tx_ctrl=SyncFIFO(width=16, depth=4),
rx_wptr=SyncFIFO(width=8, 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.spram_req = f.spram_req
m.submodules.tx_bytes = f.tx_bytes m.submodules.tx_bytes = f.tx_bytes
m.submodules.tx_ctrl = f.tx_ctrl m.submodules.tx_ctrl = f.tx_ctrl
m.submodules.rx_wptr = f.rx_wptr m.submodules.rx_wptr = f.rx_wptr
m.submodules.rx_rptr = f.rx_rptr
m.d.comb += [ m.d.comb += [
self.spram_req_r_data.eq(f.spram_req.r_data), 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_data .eq(self.rx_wptr_w_data),
f.rx_wptr.w_en .eq(self.rx_wptr_w_en), f.rx_wptr.w_en .eq(self.rx_wptr_w_en),
self.rx_wptr_w_rdy .eq(f.rx_wptr.w_rdy), 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 return f
@@ -389,10 +384,8 @@ class BBARegisterFile(Elaboratable):
with m.Case(_IR): with m.Case(_IR):
m.d.sync += r.ir.eq(r.ir & ~self.rx_data) # write-1-to-clear m.d.sync += r.ir.eq(r.ir & ~self.rx_data) # write-1-to-clear
with m.Case(_RRP_LO): with m.Case(_RRP_LO):
# The assembler sees this through the `rrp` tap.
m.d.sync += r.rrp_lo.eq(self.rx_data) 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): with m.Case(_PAR0, _PAR1, _PAR2, _PAR3, _PAR4, _PAR5):
m.d.sync += r.par[t.eff_addr[0:3]].eq(self.rx_data) m.d.sync += r.par[t.eff_addr[0:3]].eq(self.rx_data)
with m.Case(_TWD_LO): with m.Case(_TWD_LO):
@@ -736,7 +729,6 @@ class BBARegisterFile(Elaboratable):
m.d.sync += [ m.d.sync += [
f.tx_bytes.w_en.eq(0), f.tx_bytes.w_en.eq(0),
f.tx_ctrl.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), f.rx_wptr.r_en .eq(0),
t.ncra_rst .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 # survive: 0x52 = AB | CA | 2_PACKETS_PER_INT, with PR and PM CLEAR
# (so the real BBA delivers own-MAC + broadcast only). # (so the real BBA delivers own-MAC + broadcast only).
ncrb = ctx.get(reg.ncrb) ncrb = ctx.get(reg.ncrb)
pr = (ncrb >> _NCRB_PR) & 1 pr = (ncrb >> NCRB_PR) & 1
pm = (ncrb >> _NCRB_PM) & 1 pm = (ncrb >> _NCRB_PM) & 1
ab = (ncrb >> _NCRB_AB) & 1 ab = (ncrb >> _NCRB_AB) & 1
print(f"T15 ncrb export = 0x{ncrb:02X} (PR={pr} PM={pm} AB={ab})") print(f"T15 ncrb export = 0x{ncrb:02X} (PR={pr} PM={pm} AB={ab})")
+2 -2
View File
@@ -16,7 +16,7 @@ See CLAUDE.md "Module Breakdown" and "CDC Signal Inventory" for the full list.
from amaranth import * from amaranth import *
from exi_bba.exi_capture import ExiCapture 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.spram_arbiter import SPRAMArbiter
from exi_bba.rx_frame_assembler import RXFrameAssembler from exi_bba.rx_frame_assembler import RXFrameAssembler
from exi_bba.tx_frame_drain import TXFrameDrain 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 # ask for promiscuous by setting PR. See the S0_MODE comment in
# w5100_parallel_master.py -- running unfiltered hung Swiss. # w5100_parallel_master.py -- running unfiltered hung Swiss.
if self._eth == "w5100": 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) ─────────────────── # ── Optional bring-up peripherals (sync domain) ───────────────────
# Build init_req as an OR of all reinit sources (NCRA pulse plus any # Build init_req as an OR of all reinit sources (NCRA pulse plus any