Roflin
86cd5d1abd
Both from one mistake: eth_wr_valid is scheduled with m.d.sync and so is LOW
for the whole first cycle in a state, while eth_wr_ready was tested
immediately -- and `ready` is usually already high on entry. Both
valid.eq(1) and valid.eq(0) then fired in the same cycle, the later won, the
write was never issued, and it was dropped SILENTLY.
1. WRITE_LEN_HI dropped the descriptor's HIGH length byte, leaving it at its
placeholder 0x00. Hardware read a descriptor of 00 00 00 01 -- a 1-byte
frame length, impossible for ethernet -- and rejected the ring.
2. WRITE_PLACEHOLDER counted a phantom first write, advancing wr_addr without
issuing one. Only THREE placeholder bytes were written and wr_addr ended
at desc_base+3, so the frame body started at descriptor byte 3 and EVERY
RECEIVED FRAME WAS SHIFTED ONE BYTE EARLY. The GC's ethertype test then
read the wrong offset and silently dropped every frame: the host saw "no
ACK" while the ring filled and RWP advanced normally.
Both now advance on `valid & ready`, and the body offset is set explicitly
(desc_base + 4) rather than relying on an incremented pointer, so it cannot
drift with the handshake again.
WHY NO TEST CAUGHT THIS, which is the more important half:
- The testbench drove the RX stream OPEN-LOOP -- one byte per tick, never
checking rx_ready, which is low through IDLE and the whole placeholder
write. It silently lost bytes, including the SOF byte. A real master
must hold each byte until accepted.
- It only ever asserted on rwp and rx_irq. It declared a spram_writes list
and never filled it, so NOTHING had ever checked what was written.
- Every frame it sent was short, and a short frame's high length byte is
legitimately zero -- so a dropped high byte was indistinguishable from a
correct one.
The testbench now respects back-pressure, records every write, and checks the
descriptor bytes and the body offset. T2 sends a 300-byte frame specifically
so the high length byte is non-zero. Mutation-tested: restoring either bug
fails T2 with got [0,0,0,48] want [0,0,1,48] -- the same shape as the
hardware's 00 00 00 01 -- while T1 (short frame) still passes, which is
exactly the blind spot that let this ship.
Bring-up's 342-byte DHCP frame read a correct 00 00 01 5A only because the
arbiter happened to hold `ready` low on that entry cycle; the domain merge
and the SPRAM response-FIFO removal changed that timing.
--seeds 8: clk 27.8-32.2 MHz, EXI 54.9-64.5 MHz, 8/8 PASS. Best seed 8
(clk 32.22, EXI 62.25), flashed. Gateware only -- the .dol is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
387 lines
16 KiB
Python
387 lines
16 KiB
Python
"""RX frame assembler — sync domain (24 MHz).
|
||
|
||
Receives raw ethernet frames from W5500SPIMaster and writes them into the SPRAM
|
||
ring buffer in MX98730EC format.
|
||
|
||
Ring buffer layout (SPRAM byte addresses)
|
||
------------------------------------------
|
||
0x0100–0x0FFF 15 pages × 256 bytes = 3840 bytes
|
||
Pages 0x01–0x0F; page 0x00 is reserved.
|
||
Page wrap: after 0x0F → 0x01 (skip 0x00).
|
||
|
||
Frame descriptor (4 bytes at page start)
|
||
-----------------------------------------------
|
||
Byte 0: LRPS (last received packet status) — 0x00
|
||
Byte 1: 0x00
|
||
Byte 2: total_length[15:8] (big-endian; includes 4 descriptor bytes)
|
||
Byte 3: total_length[7:0]
|
||
Bytes 4+: raw ethernet frame
|
||
|
||
Write sequence
|
||
--------------
|
||
1. Issue 4 SPRAM writes of 0x00 (placeholder descriptor).
|
||
2. For each byte received from W5500, issue one SPRAM write.
|
||
3. After EOF: rewrite descriptor bytes 2 and 3 with actual length.
|
||
4. Advance RWP, push to rx_wptr FIFO, pulse rx_irq.
|
||
"""
|
||
|
||
from amaranth import *
|
||
|
||
__all__ = ["RXFrameAssembler"]
|
||
|
||
_RX_PAGE_FIRST = 0x01
|
||
_RX_PAGE_LAST = 0x0F
|
||
_PAGES_TOTAL = _RX_PAGE_LAST - _RX_PAGE_FIRST + 1 # 15
|
||
|
||
|
||
class RXFrameAssembler(Elaboratable):
|
||
"""Writes incoming ethernet frames into the SPRAM ring buffer.
|
||
|
||
W5500 streaming interface (sync domain)
|
||
----------------------------------------
|
||
rx_data / rx_valid / rx_ready : byte stream
|
||
rx_sof / rx_eof : frame delimiters (same cycle as rx_valid)
|
||
|
||
SPRAM write interface (to SPRAMArbiter, sync domain)
|
||
-----------------------------------------------------
|
||
eth_wr_addr / eth_wr_data / eth_wr_valid / eth_wr_ready
|
||
|
||
CDC outputs (wired by BBATop)
|
||
-----------------------------
|
||
rx_wptr_w_data / rx_wptr_w_en / rx_wptr_w_rdy
|
||
rx_irq : 1-cycle pulse → PulseSynchronizer input
|
||
rx_enabled : controlled by NCRA SR bit (from BBARegisterFile)
|
||
"""
|
||
|
||
def __init__(self):
|
||
# W5500 stream in
|
||
self.rx_data = Signal(8)
|
||
self.rx_valid = Signal()
|
||
self.rx_ready = Signal()
|
||
self.rx_sof = Signal()
|
||
self.rx_eof = Signal()
|
||
|
||
# SPRAM write out
|
||
self.eth_wr_addr = Signal(16)
|
||
self.eth_wr_data = Signal(8)
|
||
self.eth_wr_valid = Signal()
|
||
self.eth_wr_ready = Signal()
|
||
|
||
# RWP FIFO write-side (sync→exi)
|
||
self.rx_wptr_w_data = Signal(8)
|
||
self.rx_wptr_w_en = Signal()
|
||
self.rx_wptr_w_rdy = Signal()
|
||
|
||
# rx_irq pulse (→ PulseSynchronizer)
|
||
self.rx_irq = Signal()
|
||
|
||
# RX gate from NCRA SR bit
|
||
self.rx_enabled = Signal()
|
||
|
||
def elaborate(self, platform):
|
||
m = Module()
|
||
|
||
# ── Ring-buffer state ─────────────────────────────────────────────
|
||
rwp = Signal(8, init=_RX_PAGE_FIRST) # current RX write page (1–15)
|
||
|
||
# Write address within current frame
|
||
wr_addr = Signal(16)
|
||
# Number of frame data bytes received
|
||
data_ctr = Signal(12)
|
||
# Total length = data_ctr + 4
|
||
total_len = Signal(12)
|
||
|
||
# Descriptor base (rwp*256) — saved when frame starts
|
||
desc_base = Signal(16)
|
||
|
||
# Placeholder descriptor byte counter (0..3)
|
||
desc_ctr = Signal(2)
|
||
|
||
# Number of pages consumed by this frame (rounded up)
|
||
pages_used = Signal(5)
|
||
|
||
# Default: no pulses
|
||
m.d.sync += self.rx_irq.eq(0)
|
||
m.d.sync += self.rx_wptr_w_en.eq(0)
|
||
|
||
# Combinatorial outputs
|
||
m.d.comb += total_len.eq(data_ctr + 4)
|
||
|
||
with m.FSM(domain="sync", name="rx_fsm"):
|
||
|
||
with m.State("IDLE"):
|
||
m.d.comb += self.rx_ready.eq(0)
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
with m.If(self.rx_valid & self.rx_sof & self.rx_enabled):
|
||
frame_base = Signal(16)
|
||
m.d.comb += frame_base.eq(Cat(Const(0, 8), rwp))
|
||
m.d.sync += desc_base.eq(frame_base)
|
||
m.d.sync += wr_addr.eq(frame_base)
|
||
m.d.sync += data_ctr.eq(0)
|
||
m.d.sync += desc_ctr.eq(0)
|
||
m.next = "WRITE_PLACEHOLDER"
|
||
|
||
with m.State("WRITE_PLACEHOLDER"):
|
||
# Write 4 bytes of 0x00 as placeholder descriptor.
|
||
#
|
||
# ⚠️ Advance on `valid & ready`, NOT on `ready` alone.
|
||
# `eth_wr_valid` is scheduled with m.d.sync and so is LOW for
|
||
# the whole first cycle in this state, while `ready` is tested
|
||
# immediately -- and `ready` is usually already high here.
|
||
# Testing `ready` alone therefore counted a write that was
|
||
# never issued: only THREE placeholder bytes went out and
|
||
# wr_addr ended at desc_base+3, so the frame body started at
|
||
# descriptor byte 3 and EVERY RECEIVED FRAME WAS SHIFTED ONE
|
||
# BYTE EARLY.
|
||
#
|
||
# Hardware symptom (2026-10-09): the ring filled normally and
|
||
# RWP advanced, but the GC's ethertype test read the wrong
|
||
# offset and silently dropped every frame -- "no ACK" from the
|
||
# host with no other sign of trouble. The matching
|
||
# descriptor bug (same cause, in WRITE_LEN_HI) showed up as a
|
||
# descriptor of 00 00 00 01.
|
||
#
|
||
# The first byte is written twice as a consequence of
|
||
# eth_wr_addr trailing wr_addr by a cycle, and desc_base+3 is
|
||
# left unwritten -- both harmless, because every descriptor
|
||
# byte is either zero anyway or overwritten by WRITE_LEN_*.
|
||
# What matters is that wr_addr ends at desc_base+4.
|
||
m.d.sync += self.eth_wr_addr .eq(wr_addr)
|
||
m.d.sync += self.eth_wr_data .eq(0x00)
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += wr_addr.eq(wr_addr + 1)
|
||
with m.If(desc_ctr == 3):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "RECV_AND_WRITE"
|
||
with m.Else():
|
||
m.d.sync += desc_ctr.eq(desc_ctr + 1)
|
||
|
||
with m.State("RECV_AND_WRITE"):
|
||
# Accept bytes from W5500 and write each to SPRAM immediately
|
||
m.d.comb += self.rx_ready.eq(~self.eth_wr_valid | self.eth_wr_ready)
|
||
with m.If(self.rx_valid & (~self.eth_wr_valid | self.eth_wr_ready)):
|
||
m.d.sync += self.eth_wr_addr.eq(wr_addr)
|
||
m.d.sync += self.eth_wr_data.eq(self.rx_data)
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
m.d.sync += wr_addr.eq(wr_addr + 1)
|
||
m.d.sync += data_ctr.eq(data_ctr + 1)
|
||
with m.If(self.rx_eof):
|
||
m.next = "WAIT_LAST_WRITE"
|
||
with m.Elif(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
|
||
with m.State("WAIT_LAST_WRITE"):
|
||
# Wait for the last data byte write to be accepted
|
||
with m.If(~self.eth_wr_valid | self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
# Compute pages used: ceil((data_ctr + 4) / 256)
|
||
# = (total_len + 255) >> 8 = total_len[11:8] + (total_len[7:0] != 0)
|
||
m.d.sync += pages_used.eq(total_len[8:12] + (total_len[:8] != 0))
|
||
m.next = "WRITE_LEN_HI"
|
||
|
||
# ⚠️ Both length writes must advance on `valid & ready`, NOT on
|
||
# `ready` alone. `valid` is scheduled with m.d.sync and so is low
|
||
# for the whole first cycle in the state, while `ready` is tested
|
||
# immediately -- and coming out of WAIT_LAST_WRITE `ready` is
|
||
# usually already high. Testing `ready` alone therefore fired both
|
||
# valid.eq(1) and valid.eq(0) in the same cycle, the later won, the
|
||
# write was never issued, and the length byte was SILENTLY DROPPED.
|
||
#
|
||
# Hardware symptom (2026-10-09): a descriptor of 00 00 00 01 --
|
||
# byte 2 still at its placeholder 0x00 because the HI write was
|
||
# dropped -- so the GC read a 1-byte frame length and rejected the
|
||
# ring. Intermittent by nature: it depends on whether the arbiter
|
||
# happens to hold `ready` high on the entry cycle, which is why
|
||
# bring-up's 342-byte DHCP frame read a correct 00 00 01 5A and
|
||
# this went unnoticed.
|
||
with m.State("WRITE_LEN_HI"):
|
||
# Overwrite descriptor byte 2 with total_len[15:8]
|
||
m.d.sync += self.eth_wr_addr.eq(desc_base + 2)
|
||
m.d.sync += self.eth_wr_data.eq(total_len[8:12])
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "WRITE_LEN_LO"
|
||
|
||
with m.State("WRITE_LEN_LO"):
|
||
# Overwrite descriptor byte 3 with total_len[7:0]
|
||
m.d.sync += self.eth_wr_addr.eq(desc_base + 3)
|
||
m.d.sync += self.eth_wr_data.eq(total_len[:8])
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "ADVANCE_RWP"
|
||
|
||
with m.State("ADVANCE_RWP"):
|
||
# next_rwp = ((rwp - 1 + pages_used) % 15) + 1
|
||
next_rwp_raw = Signal(8)
|
||
m.d.comb += next_rwp_raw.eq(rwp + pages_used)
|
||
with m.If(next_rwp_raw > _RX_PAGE_LAST):
|
||
m.d.sync += rwp.eq(next_rwp_raw - _PAGES_TOTAL)
|
||
with m.Else():
|
||
m.d.sync += rwp.eq(next_rwp_raw)
|
||
m.next = "PUSH_WPT"
|
||
|
||
with m.State("PUSH_WPT"):
|
||
with m.If(self.rx_wptr_w_rdy):
|
||
m.d.sync += self.rx_wptr_w_data.eq(rwp)
|
||
m.d.sync += self.rx_wptr_w_en.eq(1)
|
||
m.d.sync += self.rx_irq.eq(1)
|
||
m.next = "IDLE"
|
||
|
||
return m
|
||
|
||
|
||
# ── Testbench ─────────────────────────────────────────────────────────────
|
||
|
||
if __name__ == "__main__":
|
||
import sys
|
||
from amaranth.sim import Simulator, Period
|
||
|
||
dut = RXFrameAssembler()
|
||
errors = []
|
||
|
||
def brief(v):
|
||
"""Long byte lists are unreadable in a pass/fail line; show the ends
|
||
and the length, and keep the full value only for a failure."""
|
||
if isinstance(v, list) and len(v) > 8:
|
||
head = ", ".join(str(x) for x in v[:4])
|
||
tail = ", ".join(str(x) for x in v[-2:])
|
||
return f"[{head}, ... {tail}] ({len(v)} bytes)"
|
||
return str(v)
|
||
|
||
def check(name, got, want):
|
||
ok = got == want
|
||
print(f" {'ok ' if ok else 'FAIL'} {name}: "
|
||
f"got {brief(got)}, want {brief(want)}")
|
||
if not ok:
|
||
if isinstance(got, list) and isinstance(want, list):
|
||
first = next((i for i, (g, w) in enumerate(zip(got, want))
|
||
if g != w), None)
|
||
if first is not None:
|
||
print(f" first difference at index {first}: "
|
||
f"got {got[first]}, want {want[first]}")
|
||
errors.append(name)
|
||
|
||
async def testbench(ctx):
|
||
# Final contents of every SPRAM address the DUT wrote.
|
||
#
|
||
# The old testbench declared a writes list and never filled it, and
|
||
# drove the RX stream OPEN-LOOP -- one byte per tick, never checking
|
||
# rx_ready. Both of the 2026-10-09 handshake bugs were therefore
|
||
# invisible to it: it only ever asserted on rwp and rx_irq, so a
|
||
# dropped descriptor byte or a frame body written one byte early
|
||
# changed nothing it looked at.
|
||
mem = {}
|
||
|
||
async def tick(n=1):
|
||
for _ in range(n):
|
||
# A write is issued on a cycle where valid AND ready are both
|
||
# high, so sample before the edge that completes it.
|
||
if ctx.get(dut.eth_wr_valid) and ctx.get(dut.eth_wr_ready):
|
||
mem[ctx.get(dut.eth_wr_addr)] = ctx.get(dut.eth_wr_data)
|
||
await ctx.tick("sync")
|
||
|
||
async def send_frame(frame):
|
||
"""Push a frame RESPECTING BACK-PRESSURE, the way the real
|
||
ethernet master does. rx_ready is LOW through IDLE and the whole
|
||
placeholder-descriptor write, so a master that advances anyway
|
||
simply loses those bytes -- which is what the old testbench did,
|
||
and why it could not see the frame body land at the wrong
|
||
offset."""
|
||
for i, b in enumerate(frame):
|
||
ctx.set(dut.rx_data, b)
|
||
ctx.set(dut.rx_valid, 1)
|
||
ctx.set(dut.rx_sof, 1 if i == 0 else 0)
|
||
ctx.set(dut.rx_eof, 1 if i == len(frame) - 1 else 0)
|
||
guard = 0
|
||
while not ctx.get(dut.rx_ready):
|
||
await tick()
|
||
guard += 1
|
||
if guard > 400:
|
||
errors.append("send_frame: rx_ready never asserted")
|
||
return
|
||
await tick()
|
||
ctx.set(dut.rx_valid, 0)
|
||
ctx.set(dut.rx_sof, 0)
|
||
ctx.set(dut.rx_eof, 0)
|
||
|
||
async def wait_irq(limit=200):
|
||
for _ in range(limit):
|
||
await tick()
|
||
if ctx.get(dut.rx_irq):
|
||
return ctx.get(dut.rx_wptr_w_data)
|
||
return None
|
||
|
||
def descriptor(page):
|
||
base = page << 8
|
||
return [mem.get(base + i) for i in range(4)]
|
||
|
||
def body(page, n):
|
||
base = (page << 8) + 4
|
||
return [mem.get(base + i) for i in range(n)]
|
||
|
||
ctx.set(dut.eth_wr_ready, 1)
|
||
ctx.set(dut.rx_wptr_w_rdy, 1)
|
||
ctx.set(dut.rx_enabled, 1)
|
||
await tick(2)
|
||
|
||
# ── T1: short frame at page 1 ────────────────────────────────────
|
||
f1 = [0xAA, 0xBB, 0xCC, 0xDD, 0x08, 0x00, 0x45, 0x00, 0x00, 0x01]
|
||
await send_frame(f1)
|
||
check("T1 rwp advance", await wait_irq(), 2)
|
||
total1 = len(f1) + 4
|
||
check("T1 descriptor", descriptor(1),
|
||
[0x00, 0x00, (total1 >> 8) & 0xFF, total1 & 0xFF])
|
||
# The body MUST start at base+4. This is the check that catches a
|
||
# placeholder write being dropped: with only three placeholder bytes
|
||
# written the body landed at base+3 and every frame was shifted one
|
||
# byte early, which on hardware made the GC drop every frame.
|
||
check("T1 body at base+4", body(1, len(f1)), f1)
|
||
|
||
await tick(4)
|
||
|
||
# ── T2: frame LONGER THAN 255 BYTES, so the descriptor's HIGH length
|
||
# byte is non-zero. This is the case that catches a dropped
|
||
# WRITE_LEN_HI: with the high byte left at its placeholder 0x00 the GC
|
||
# read an impossible length (hardware saw 00 00 00 01) and rejected
|
||
# the ring. Every earlier test used a short frame, where a dropped
|
||
# high byte is indistinguishable from a correct zero. ───────────────
|
||
f2 = [(i * 7 + 3) & 0xFF for i in range(300)]
|
||
await send_frame(f2)
|
||
rwp2 = await wait_irq()
|
||
total2 = len(f2) + 4
|
||
check("T2 descriptor (len > 255)", descriptor(2),
|
||
[0x00, 0x00, (total2 >> 8) & 0xFF, total2 & 0xFF])
|
||
check("T2 body at base+4", body(2, len(f2)), f2)
|
||
# 304 bytes spans two pages, so page 2 -> page 4.
|
||
check("T2 rwp advance (2 pages)", rwp2, 4)
|
||
|
||
await tick(4)
|
||
|
||
# ── T3: RX disabled — SOF must be ignored ────────────────────────
|
||
before = len(mem)
|
||
ctx.set(dut.rx_enabled, 0)
|
||
ctx.set(dut.rx_data, 0xDE)
|
||
ctx.set(dut.rx_valid, 1)
|
||
ctx.set(dut.rx_sof, 1)
|
||
await tick(8)
|
||
ctx.set(dut.rx_valid, 0)
|
||
ctx.set(dut.rx_sof, 0)
|
||
check("T3 no writes while rx disabled", len(mem), before)
|
||
|
||
sim = Simulator(dut)
|
||
sim.add_clock(Period(MHz=24), domain="sync")
|
||
sim.add_testbench(testbench)
|
||
|
||
with sim.write_vcd("RXFrameAssembler.vcd"):
|
||
sim.run()
|
||
|
||
if errors:
|
||
print("\nFAILURES:")
|
||
for e in errors:
|
||
print(" -", e)
|
||
sys.exit(1)
|
||
print("\nAll tests passed.")
|