fix(tx): queue the frame length at TWD_HI so the drain runs concurrently
TXFrameDrain sat in IDLE until it had a length, and the length only reached tx_ctrl when the NCRA ST bits were written -- after the GC had streamed the whole frame. So the entire frame had to fit in the 16-deep tx_bytes FIFO, and BBARegisterFile silently drops bytes when w_rdy is low (SPI cannot back-pressure). On hardware this truncated frames at exactly 16 bytes: a captured ARP had 16 correct bytes followed by the frame restarting over itself. The length is now handed to the drain one cycle after the TWD_HI (0x35) write, which the GC issues BEFORE streaming data. The drain starts early and blocks on tx_bytes_r_rdy, consuming bytes as they arrive, so tx_bytes becomes a rate-matching buffer instead of whole-frame storage. The ST write remains a fallback trigger for software that skips TWD, guarded by tx_queued so one frame is never queued twice. Adds BBATop T8: streams a 34-byte frame (> the 16-deep FIFO) over EXI and checks the W5100 model received it byte-for-byte. Verified to FAIL without the fix with exactly the hardware symptom -- 16/34 bytes, first bad byte at index 16. The gap that let this ship: TXFrameDrain's own tests drive the FIFO signals directly, and the BBATop test only covered the RX loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -208,6 +208,11 @@ class BBARegisterFile(Elaboratable):
|
|||||||
xfer_len = Signal(2) # 0=1B … 3=4B
|
xfer_len = Signal(2) # 0=1B … 3=4B
|
||||||
byte_ctr = Signal(2)
|
byte_ctr = Signal(2)
|
||||||
tx_frame_len = Signal(16)
|
tx_frame_len = Signal(16)
|
||||||
|
# TX trigger bookkeeping. `twd_armed` fires one cycle AFTER TWD_HI is
|
||||||
|
# written, so tx_frame_len has both halves by then; `tx_queued` records
|
||||||
|
# that the drain already has a length for this frame.
|
||||||
|
twd_armed = Signal()
|
||||||
|
tx_queued = Signal()
|
||||||
|
|
||||||
# True until first NCRA reset write: return device ID on addr=0 reads
|
# True until first NCRA reset write: return device ID on addr=0 reads
|
||||||
id_phase = Signal(init=1)
|
id_phase = Signal(init=1)
|
||||||
@@ -328,10 +333,15 @@ class BBARegisterFile(Elaboratable):
|
|||||||
m.d.exi += r_ncra[0].eq(0) # RESET self-clears
|
m.d.exi += r_ncra[0].eq(0) # RESET self-clears
|
||||||
m.d.exi += ncra_rst_ps.i.eq(1)
|
m.d.exi += ncra_rst_ps.i.eq(1)
|
||||||
m.d.exi += id_phase.eq(0)
|
m.d.exi += id_phase.eq(0)
|
||||||
with m.If(self.rx_data[1:3].any()):
|
# Fallback trigger: TWD_HI normally queues the
|
||||||
|
# frame (see _TWD_HI). Only fire here if it did
|
||||||
|
# not, so software that skips TWD still transmits
|
||||||
|
# and we never double-queue one frame.
|
||||||
|
with m.If(self.rx_data[1:3].any() & ~tx_queued):
|
||||||
with m.If(tx_ctrl.w_rdy):
|
with m.If(tx_ctrl.w_rdy):
|
||||||
m.d.exi += tx_ctrl.w_data.eq(tx_frame_len)
|
m.d.exi += tx_ctrl.w_data.eq(tx_frame_len)
|
||||||
m.d.exi += tx_ctrl.w_en.eq(1)
|
m.d.exi += tx_ctrl.w_en.eq(1)
|
||||||
|
m.d.exi += tx_queued.eq(1)
|
||||||
with m.Case(_IMR):
|
with m.Case(_IMR):
|
||||||
m.d.exi += r_imr.eq(self.rx_data)
|
m.d.exi += r_imr.eq(self.rx_data)
|
||||||
with m.Case(_IR):
|
with m.Case(_IR):
|
||||||
@@ -349,6 +359,15 @@ class BBARegisterFile(Elaboratable):
|
|||||||
with m.Case(_TWD_HI):
|
with m.Case(_TWD_HI):
|
||||||
m.d.exi += r_twd_hi.eq(self.rx_data)
|
m.d.exi += r_twd_hi.eq(self.rx_data)
|
||||||
m.d.exi += tx_frame_len[8:16].eq(self.rx_data)
|
m.d.exi += tx_frame_len[8:16].eq(self.rx_data)
|
||||||
|
# Hand the length to TXFrameDrain NOW, not at the ST
|
||||||
|
# trigger. The drain blocks on tx_bytes_r_rdy, so
|
||||||
|
# starting it here lets it consume bytes AS the GC
|
||||||
|
# streams them and keeps tx_bytes a rate-matching
|
||||||
|
# buffer. Waiting for ST meant the whole frame had
|
||||||
|
# to fit in the 16-deep FIFO, and anything past 16
|
||||||
|
# bytes was silently dropped (hardware 2026-09-30:
|
||||||
|
# frames truncated at exactly 16 bytes on the wire).
|
||||||
|
m.d.exi += twd_armed.eq(1)
|
||||||
with m.Case(_TXDATA):
|
with m.Case(_TXDATA):
|
||||||
with m.If(tx_bytes.w_rdy):
|
with m.If(tx_bytes.w_rdy):
|
||||||
m.d.exi += tx_bytes.w_data.eq(self.rx_data)
|
m.d.exi += tx_bytes.w_data.eq(self.rx_data)
|
||||||
@@ -433,9 +452,19 @@ class BBARegisterFile(Elaboratable):
|
|||||||
# ── PulseSynchronizer arrivals ────────────────────────────────────
|
# ── PulseSynchronizer arrivals ────────────────────────────────────
|
||||||
with m.If(rx_irq_ps.o):
|
with m.If(rx_irq_ps.o):
|
||||||
m.d.exi += r_ir[1].eq(1) # RI bit
|
m.d.exi += r_ir[1].eq(1) # RI bit
|
||||||
|
# ── Deferred TX queue (armed by a TWD_HI write) ──────────────────
|
||||||
|
# One cycle after TWD_HI so tx_frame_len holds both bytes. Held until
|
||||||
|
# tx_ctrl has room so a full control FIFO cannot drop the trigger.
|
||||||
|
with m.If(twd_armed & tx_ctrl.w_rdy):
|
||||||
|
m.d.exi += tx_ctrl.w_data.eq(tx_frame_len)
|
||||||
|
m.d.exi += tx_ctrl.w_en.eq(1)
|
||||||
|
m.d.exi += twd_armed.eq(0)
|
||||||
|
m.d.exi += tx_queued.eq(1)
|
||||||
|
|
||||||
with m.If(tx_irq_ps.o):
|
with m.If(tx_irq_ps.o):
|
||||||
m.d.exi += r_ir[2].eq(1) # TI bit
|
m.d.exi += r_ir[2].eq(1) # TI bit
|
||||||
m.d.exi += r_ncra[1:3].eq(0) # clear ST bits
|
m.d.exi += r_ncra[1:3].eq(0) # clear ST bits
|
||||||
|
m.d.exi += tx_queued.eq(0) # ready for the next frame
|
||||||
|
|
||||||
return m
|
return m
|
||||||
|
|
||||||
|
|||||||
@@ -474,6 +474,7 @@ if __name__ == "__main__":
|
|||||||
return mem
|
return mem
|
||||||
|
|
||||||
w5100_mem = w5100_preload()
|
w5100_mem = w5100_preload()
|
||||||
|
tx_seen = [] # bytes the model saw written into the TX buffer
|
||||||
|
|
||||||
async def w5100_model(ctx):
|
async def w5100_model(ctx):
|
||||||
idm_ar = 0
|
idm_ar = 0
|
||||||
@@ -503,6 +504,8 @@ if __name__ == "__main__":
|
|||||||
idm_ar = (idm_ar & 0xFF00) | do
|
idm_ar = (idm_ar & 0xFF00) | do
|
||||||
else:
|
else:
|
||||||
w5100_mem[idm_ar] = do
|
w5100_mem[idm_ar] = do
|
||||||
|
if 0x4000 <= idm_ar < 0x6000: # socket TX buffer region
|
||||||
|
tx_seen.append(do)
|
||||||
if idm_ar == _W_S0_CR and do == _W_CR_RECV:
|
if idm_ar == _W_S0_CR and do == _W_CR_RECV:
|
||||||
w5100_mem[_W_S0_RX_RSR] = 0
|
w5100_mem[_W_S0_RX_RSR] = 0
|
||||||
w5100_mem[_W_S0_RX_RSR + 1] = 0
|
w5100_mem[_W_S0_RX_RSR + 1] = 0
|
||||||
@@ -597,6 +600,40 @@ if __name__ == "__main__":
|
|||||||
errors.append(f"T7 panel: EXI-activity LED not lit (led=0b{leds:05b})")
|
errors.append(f"T7 panel: EXI-activity LED not lit (led=0b{leds:05b})")
|
||||||
print(f"T7 panel led=0b{leds:05b} (bit1=EXI activity, expect 1)")
|
print(f"T7 panel led=0b{leds:05b} (bit1=EXI activity, expect 1)")
|
||||||
|
|
||||||
|
# T8: GC → wire TX of a frame LONGER than the 16-deep tx_bytes FIFO.
|
||||||
|
# This is the case that shipped broken: TXFrameDrain used to wait for
|
||||||
|
# the length until the NCRA ST write, i.e. until after the GC had
|
||||||
|
# streamed the whole frame, so the frame had to fit in tx_bytes and
|
||||||
|
# everything past byte 16 was silently dropped (hardware 2026-09-30:
|
||||||
|
# frames truncated at exactly 16 bytes on the wire). The length is now
|
||||||
|
# queued on the TWD_HI write so the drain runs concurrently.
|
||||||
|
TX_FRAME = [0xFF] * 6 + [0x00, 0x09, 0xBF, 0x00, 0x00, 0x01] + \
|
||||||
|
[0x88, 0xB5] + [0x40 + i for i in range(20)] # 34 bytes
|
||||||
|
assert len(TX_FRAME) > 16, "test must exceed the tx_bytes depth to be meaningful"
|
||||||
|
tx_seen.clear()
|
||||||
|
await exi_write(ctx, 0x34, [len(TX_FRAME) & 0xFF])
|
||||||
|
await exi_write(ctx, 0x35, [(len(TX_FRAME) >> 8) & 0xFF])
|
||||||
|
for b in TX_FRAME:
|
||||||
|
# One byte per transaction: eff_addr increments per data byte, so a
|
||||||
|
# multi-byte write to 0x48 would spill into 0x49+.
|
||||||
|
await exi_write(ctx, 0x48, [b])
|
||||||
|
await exi_write(ctx, 0x00, [0x08 | 0x02]) # SR + ST0 → transmit
|
||||||
|
for _ in range(4000):
|
||||||
|
if len(tx_seen) >= len(TX_FRAME):
|
||||||
|
break
|
||||||
|
await ctx.tick("sync")
|
||||||
|
got = tx_seen[:len(TX_FRAME)]
|
||||||
|
print(f"T8 TX {len(TX_FRAME)}B frame: W5100 got {len(tx_seen)} bytes")
|
||||||
|
if got != TX_FRAME:
|
||||||
|
n = min(len(got), len(TX_FRAME))
|
||||||
|
first = next((i for i in range(n) if got[i] != TX_FRAME[i]), n)
|
||||||
|
errors.append(f"T8 TX frame mismatch: {len(got)}/{len(TX_FRAME)} bytes, "
|
||||||
|
f"first bad byte at index {first}")
|
||||||
|
print(f" got {[f'0x{b:02X}' for b in got[:20]]}")
|
||||||
|
print(f" want {[f'0x{b:02X}' for b in TX_FRAME[:20]]}")
|
||||||
|
else:
|
||||||
|
print(f"T8 TX frame matches byte-for-byte ({len(TX_FRAME)} bytes)")
|
||||||
|
|
||||||
sim = Simulator(dut)
|
sim = Simulator(dut)
|
||||||
sim.add_clock(Period(MHz=54), domain="capture")
|
sim.add_clock(Period(MHz=54), domain="capture")
|
||||||
sim.add_clock(Period(MHz=24), domain="exi")
|
sim.add_clock(Period(MHz=24), domain="exi")
|
||||||
|
|||||||
@@ -6,7 +6,14 @@ to notify the GC that the transmit is complete.
|
|||||||
|
|
||||||
Flow
|
Flow
|
||||||
----
|
----
|
||||||
1. Wait for tx_len FIFO to have a length word (signals a complete frame queued).
|
1. Wait for tx_len FIFO to have a length word.
|
||||||
|
|
||||||
|
NOTE the length arrives when the GC writes TWD_HI (0x35), i.e. BEFORE it
|
||||||
|
streams the frame bytes -- not at the NCRA ST trigger. So this FSM starts
|
||||||
|
early and blocks in DRAIN on tx_bytes_r_rdy, consuming bytes as the GC
|
||||||
|
writes them. That is deliberate: tx_bytes is only 16 deep, so waiting for
|
||||||
|
ST (after the whole frame had been written) silently dropped everything past
|
||||||
|
byte 16 -- observed on hardware as frames truncating at exactly 16 bytes.
|
||||||
2. Pop the length from tx_len FIFO.
|
2. Pop the length from tx_len FIFO.
|
||||||
3. Assert tx_sof on first byte, tx_eof on last byte, consuming tx_bytes FIFO.
|
3. Assert tx_sof on first byte, tx_eof on last byte, consuming tx_bytes FIFO.
|
||||||
4. When W5500SPIMaster accepts the final byte: pulse tx_irq.
|
4. When W5500SPIMaster accepts the final byte: pulse tx_irq.
|
||||||
|
|||||||
Reference in New Issue
Block a user