diff --git a/CLAUDE.md b/CLAUDE.md index a20d09a..e64c376 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1239,6 +1239,44 @@ parallel master is the default — see "W5100 vs W5500".) --- +## TX path — libogc writes NO TWD (found 2026-10-10) + +`__bba_link_tx` is **four transactions**: + +| # | transaction | +|---|---| +| 0 | read reg `0x0031` (NWAYS) — check the link first | +| 1 | **one** register-space write to `0x0048`, the whole frame | +| 2 | read reg `0x0000` (NCRA) | +| 3 | write reg `0x0000` = `0x04` (NCRA ST) | + +⚠️ **There is no `TWD_LO`/`TWD_HI` write anywhere**, so the device has to +COUNT the bytes it was handed. `BBARegisterFile` now keeps `txdata_cnt`, +incremented per byte accepted at `0x0048`, and the NCRA ST fallback queues +**that** rather than `tx_frame_len`. Previously it queued `tx_frame_len`, +which a driver that skips TWD never sets — so the frame would have gone out +with a stale or zero length. `bba_register_file` **T11** drives TX in libogc's +shape with no TWD and is mutation-tested: switching the ST path back to +`tx_frame_len` queues length 0. `gc_bba_test` has the hardware counterpart +(**TX without TWD**). + +The TWD path stays, and is still preferred when present: writing the length +before streaming is what lets `TXFrameDrain` consume concurrently and keeps +`tx_bytes` a rate-matching buffer instead of whole-frame storage (see the TWD +fix note). Swiss writes TWD; libogc does not. Support both. + +⚠️ **libogc streams the payload with `EXI_Dma`** — the 32-byte-aligned bulk via +`EXI_Dma` after a `PPCDCacheStoreAsync`, then `EXI_ImmEx` for the remainder. +This file says elsewhere that EXI DMA *writes* are avoided because the GC's +DMA-write engine has a 1-bit-shift bug; that is true of **Swiss**, not of +libogc, which uses them routinely for TX. From the FPGA's side a DMA write is +just bytes clocked in, so nothing in the gateware changes — but it means the +DMA-write path WILL be exercised by libogc-based software, and if the shift bug +is real it will show up as corrupted TX frames that are not our fault and not +fixable here. Worth knowing before chasing it in the gateware. + +--- + ## RX Ring Descriptor — the format GC software actually expects ✅ **Corrected 2026-10-10 from libogc's own driver.** The 4 bytes at each ring diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index ecb7578..e693c67 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -271,6 +271,17 @@ class BBARegisterFile(Elaboratable): # that the drain already has a length for this frame. twd_armed = Signal() tx_queued = Signal() + # Bytes actually streamed into TXDATA since the last trigger. + # + # ⚠️ Needed because libogc DOES NOT WRITE TWD AT ALL. Its + # __bba_link_tx is four transactions -- read NWAYS, one register-space + # write of the whole frame to 0x0048 (EXI_Dma for the 32-byte-aligned + # bulk plus EXI_ImmEx for the remainder), read NCRA, write NCRA = 0x04 + # -- with no TWD_LO/TWD_HI anywhere. The real device must therefore + # count what it was given, which is what this does. Using + # tx_frame_len on the ST fallback path would transmit a stale or zero + # length for any driver that skips TWD. + txdata_cnt = Signal(16) # True until first NCRA reset write: return device ID on addr=0 reads id_phase = self.id_phase @@ -534,9 +545,13 @@ class BBARegisterFile(Elaboratable): # 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): - m.d.sync += tx_ctrl.w_data.eq(tx_frame_len) + # The COUNTED length, not tx_frame_len: + # a driver reaching this path never wrote + # TWD, so tx_frame_len is stale or zero. + m.d.sync += tx_ctrl.w_data.eq(txdata_cnt) m.d.sync += tx_ctrl.w_en.eq(1) m.d.sync += tx_queued.eq(1) + m.d.sync += txdata_cnt.eq(0) with m.Case(_IMR): m.d.sync += r_imr.eq(self.rx_data) with m.Case(_IR): @@ -567,6 +582,7 @@ class BBARegisterFile(Elaboratable): with m.If(tx_bytes.w_rdy): m.d.sync += tx_bytes.w_data.eq(self.rx_data) m.d.sync += tx_bytes.w_en.eq(1) + m.d.sync += txdata_cnt.eq(txdata_cnt + 1) # All other addresses silently ignored # No length field in the real protocol: consume data @@ -709,6 +725,7 @@ class BBARegisterFile(Elaboratable): # tx_ctrl has room so a full control FIFO cannot drop the trigger. with m.If(twd_armed & tx_ctrl.w_rdy): m.d.sync += tx_ctrl.w_data.eq(tx_frame_len) + m.d.sync += txdata_cnt.eq(0) m.d.sync += tx_ctrl.w_en.eq(1) m.d.sync += twd_armed.eq(0) m.d.sync += tx_queued.eq(1) @@ -1111,6 +1128,38 @@ if __name__ == "__main__": if got2 != [0x00]: errors.append(f"T10b command-space rewrite failed: {got2}") + # ── T11: TX in libogc's shape -- NO TWD WRITE AT ALL ───────────── + # __bba_link_tx is four transactions: read NWAYS, ONE register-space + # write of the whole frame to 0x0048, read NCRA, write NCRA = 0x04. + # There is no TWD_LO/TWD_HI anywhere, so the device has to count what + # it was handed. The ST fallback used to queue tx_frame_len, which a + # driver that skips TWD never sets -- so the frame would go out with a + # stale or zero length. + await drain_tx_bytes(ctx) # clear anything pending + while ctx.get(reg.tx_ctrl_r_rdy): # and any queued length + ctx.set(reg.tx_ctrl_r_en, 1) + await ctx.tick("sync") + ctx.set(reg.tx_ctrl_r_en, 0) + await ctx.tick("sync") + ctx.set(reg.tx_irq_i, 1) # TX done -> clears tx_queued + await ctx.tick("sync") + ctx.set(reg.tx_irq_i, 0) + await ctx.tick("sync").repeat(2) + + frame11 = [0x40 + i for i in range(24)] + await exi_write(ctx, _TXDATA, frame11) # the frame, in ONE write + await exi_write(ctx, _NCRA, [0x04]) # ST trigger, no TWD + qlen = ctx.get(reg.tx_ctrl_r_data) if ctx.get(reg.tx_ctrl_r_rdy) else None + body = await drain_tx_bytes(ctx) + print(f"T11 libogc-shape TX (no TWD): queued len={qlen} " + f"bytes={len(body)}") + if qlen != len(frame11): + errors.append(f"T11 queued length {qlen}, want {len(frame11)} " + f"(ST path must use the COUNTED length)") + if body != frame11: + errors.append(f"T11 frame bytes wrong: {len(body)} of " + f"{len(frame11)}") + # NO T10 FOR THE LATE-RESPONSE LEAK -- and that is deliberate. # # The guard added to the response handler (outstanding != 0) and the diff --git a/gc_bba_test/source/tests.c b/gc_bba_test/source/tests.c index 1ef0991..81ac4b0 100644 --- a/gc_bba_test/source/tests.c +++ b/gc_bba_test/source/tests.c @@ -439,6 +439,37 @@ static int t_mac_is_readable(char *m, size_t n) return (mac[0] == 0x00 && mac[1] == 0x09 && mac[2] == 0xBF) ? 0 : 1; } +static int t_tx_no_twd(char *m, size_t n) +{ + u8 f[60]; + u32 len = build_arp(f); + int i, ir = 0; + + /* libogc's __bba_link_tx writes NO TWD at all: read NWAYS, one + * register-space write of the whole frame to 0x0048, read NCRA, write + * NCRA = 0x04. So the device must count the bytes it was handed. The + * NCRA ST fallback used to queue the TWD value, which a driver that + * skips TWD never sets -- the frame went out with a stale or zero + * length. Deliberately does NOT write TWD_LO/TWD_HI. */ + bba_w8(REG_IR, 0xFF); + if (bba_tx_bytes(f, len) < 0) { + snprintf(m, n, "TXDATA stream failed"); return 1; + } + bba_w8(REG_NCRA, NCRA_SR | NCRA_ST0); + + for (i = 0; i < 60; i++) { + ir = bba_r8(REG_IR); + if (ir >= 0 && (ir & IR_TI)) { + snprintf(m, n, "%u B sent with NO TWD, TI after %d poll(s)", + (unsigned)len, i); + return 0; + } + msleep(10); + } + snprintf(m, n, "TI never set (IR=0x%02X) without TWD", ir); + return 1; +} + static int t_init_sequence(char *m, size_t n) { /* The register writes libogc issues, in order, with the values it uses. @@ -625,6 +656,7 @@ static test_t s_init[] = { {"cmd-space write","2-byte header + data phase, as libogc sends it", t_cmd_write, T_PENDING, {0}}, {"MAC readable", "libogc READS the MAC from PAR0-5; it must be sane", t_mac_is_readable, T_PENDING, {0}}, {"init replay", "libogc's 30 init writes, in order, do no harm", t_init_sequence, T_PENDING, {0}}, + {"TX without TWD","libogc never writes TWD; length must be counted", t_tx_no_twd, T_PENDING, {0}}, }; suite_t g_suites[] = { @@ -634,7 +666,7 @@ suite_t g_suites[] = { {"EXI integrity", "the EXI link is error-free, and how fast", s_exi, 6}, {"Ethernet", "frames get to and from the W5100", s_eth, 3}, {"SPRAM/DMA", "bulk reads stream correctly from the ring", s_spram, 2}, - {"libogc init", "the real driver's init sequence and shapes", s_init, 3}, + {"libogc init", "the real driver's init sequence and shapes", s_init, 4}, }; const int g_nsuites = sizeof(g_suites) / sizeof(g_suites[0]);