diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index 544999a..d99b43c 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -150,7 +150,25 @@ class BBARegisterFile(Elaboratable): # On the SPRAM path that latency exceeded the protocol's turnaround # byte and shifted every DMA read by one byte. One slow domain now. spram_req = SyncFIFO(width=16, depth=4) - tx_bytes = SyncFIFO(width=8, depth=16) + # TX byte FIFO: deep enough for a WHOLE maximum-size frame. + # + # SPI cannot back-pressure, so BBARegisterFile silently drops bytes + # when w_rdy is low -- there is no mechanism to tell the GC to wait. + # The depth is therefore not a buffering choice, it is the only thing + # standing between a drain stall and a corrupted frame. + # + # At 16 deep it held 16 bytes = ~4.7 us at the EXI byte rate (296 + # ns/byte). TXFrameDrain shares the W5100 bus arbiter with RX and the + # socket-3 UDP test, so any stall longer than that dropped bytes + # mid-frame. Swiss writes an ENTIRE frame in one register-space + # transaction (bba_outs to BBA_WRTXFIFOD), so 16 could never work for + # real traffic; the on-console test app only got away with it by + # writing one byte per transaction, which paced the drain by accident. + # + # 2048 covers the 1518-byte maximum frame with margin, so a frame + # cannot be truncated even if the drain makes no progress at all for + # the whole transaction. Costs ~4 of 27 free BRAMs. + 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) @@ -527,7 +545,28 @@ class BBARegisterFile(Elaboratable): # No length field in the real protocol: consume data # bytes until CS rises. - m.d.sync += eff_addr.eq(eff_addr + 1) + # + # ⚠️ TXDATA IS A FIFO PORT, NOT A REGISTER. A whole frame + # is written to that ONE address, so the address must not + # advance across its data phase. Swiss does exactly this + # (bba_outs to BBA_WRTXFIFOD streams the entire frame in a + # single register-space write), so incrementing here + # delivered only the FIRST byte of every frame and silently + # dropped the rest into 0x49+ -- which the switch above + # ignores. + # + # It went unnoticed because the on-console test app wrote + # TXDATA one byte per EXI transaction, so every byte got a + # fresh header at 0x48. The moment it batched 8 bytes per + # transaction, 7 of every 8 vanished: the frame was an + # eighth of its declared length, TXFrameDrain blocked + # waiting for the rest and TI never set (hardware + # 2026-10-09: txto=16, "no ACK after 20 tries"). + # + # Every other register IS a register and must advance, so + # that bba_outs can write PAR0-5 or MAR0-7 as a block. + with m.If(rd_sel != _TXDATA): + m.d.sync += eff_addr.eq(eff_addr + 1) with m.If(~self.cs_active): m.next = "HEADER0" @@ -737,6 +776,19 @@ if __name__ == "__main__": ctx.set(reg.cs_active, 0) await ctx.tick("sync").repeat(6) + async def drain_tx_bytes(ctx, limit=80): + """Pop everything the register file pushed into tx_bytes.""" + out = [] + for _ in range(limit): + if not ctx.get(reg.tx_bytes_r_rdy): + break + out.append(ctx.get(reg.tx_bytes_r_data)) + ctx.set(reg.tx_bytes_r_en, 1) + await ctx.tick("sync").repeat(1) + ctx.set(reg.tx_bytes_r_en, 0) + await ctx.tick("sync").repeat(1) + return out + async def exi_devid(ctx): """EXI_GetID: write 0x0000 (2 bytes), then clock 4 read bytes.""" tx_log.clear() @@ -920,7 +972,31 @@ if __name__ == "__main__": errors.append(f"T7 NWAYS after DMA: got {result}") print(f"T7 NWAYS after DMA read: 0x{result[0]:02X}") - # NO T8 FOR THE LATE-RESPONSE LEAK -- and that is deliberate. + # T8: a MULTI-BYTE write to TXDATA must push EVERY byte. + # + # TXDATA (0x48) is a FIFO port, not a register: a whole frame goes to + # that one address. The write address used to advance across the data + # phase unconditionally, so byte 0 landed in TXDATA and bytes 1..N-1 + # went to 0x49+ and were silently ignored. + # + # Swiss writes an entire frame this way (bba_outs to BBA_WRTXFIFOD), so + # its TX would have delivered ONE BYTE per frame. It went unnoticed + # because the on-console test app wrote TXDATA one byte per EXI + # transaction; the moment it batched 8 bytes per transaction, 7 of + # every 8 vanished and TXFrameDrain blocked waiting for a frame that + # never completed (hardware: txto=16, no ACK). + # 64 bytes, not 8: this also exercises the deepened FIFO and matches + # the largest chunk bba_write() can send in one transaction. + payload = [(0x11 + i * 3) & 0xFF for i in range(64)] + await exi_write(ctx, _TXDATA, payload) + got = await drain_tx_bytes(ctx) + if got != payload: + errors.append(f"T8 multi-byte TXDATA write: got {got}, " + f"want {payload}") + print(f"T8 multi-byte TXDATA write: {len(got)}/{len(payload)} bytes " + f"{'ok' if got == payload else 'MISMATCH'}") + + # NO T9 FOR THE LATE-RESPONSE LEAK -- and that is deliberate. # # The guard added to the response handler (outstanding != 0) and the # sp_pending clear in HEADER0 fix a real latent bug: a response that diff --git a/gc_bba_test/source/bba.c b/gc_bba_test/source/bba.c index bd21a70..b12c02e 100644 --- a/gc_bba_test/source/bba.c +++ b/gc_bba_test/source/bba.c @@ -125,7 +125,7 @@ int bba_tx_bytes(const u8 *buf, u32 len) { u32 off = 0; - /* Stream in 8-byte chunks, ONE EXI transaction each. + /* Stream in 64-byte chunks, ONE EXI transaction each. * * Writing byte by byte with bba_w8() costs a full EXI_Lock / EXI_Select / * header / EXI_Deselect / EXI_Unlock PER BYTE -- about sixty transactions @@ -142,14 +142,14 @@ int bba_tx_bytes(const u8 *buf, u32 len) * tries"). Byte-by-byte worked only because the per-byte EXI overhead * happened to give the drain time. * - * 8 is half the FIFO depth: it cannot overflow even if the drain consumes - * nothing for the whole transaction, while still cutting a 60-byte ACK - * from ~60 transactions to 8. Do not raise it without deepening - * tx_bytes or giving the TX path real back-pressure. */ + * tx_bytes is now 2048 deep -- a whole maximum-size frame -- so the old + * 8-byte cap is gone and 64 (bba_write()'s buffer limit) sends a 60-byte + * ACK in a SINGLE transaction. The depth is what makes this safe: SPI + * cannot back-pressure, so the FIFO must absorb the entire burst. */ while (off < len) { u32 n = len - off; int rc; - if (n > 8) n = 8; + if (n > 64) n = 64; rc = bba_write(REG_TXDATA, buf + off, n); if (rc < 0) return rc; off += n;