TX: count the TXDATA bytes, because libogc never writes TWD
Third contract checked against libogc rather than our notes. __bba_link_tx is four transactions -- read NWAYS, ONE register-space write of the whole frame to 0x0048, read NCRA, write NCRA = 0x04 -- with no TWD_LO/TWD_HI anywhere. So the device has to count the bytes it was handed. The NCRA ST fallback queued tx_frame_len, which only TWD sets, so any driver that skips TWD would have transmitted a stale or zero length. BBARegisterFile now keeps txdata_cnt, incremented per byte accepted at 0x0048 and cleared whenever a frame is queued from either path, and the ST path queues that. T11 drives TX in libogc's shape with no TWD write and checks both the queued length and the drained bytes. Mutation-tested: switching the ST path back to tx_frame_len queues length 0, which is exactly the bug. gc_bba_test gains the hardware counterpart, "TX without TWD", which deliberately omits the TWD writes that t_tx makes. 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 rather than whole-frame storage. Swiss writes TWD, libogc does not, so both are supported. Also recorded: libogc streams the payload with EXI_Dma (32-byte-aligned bulk after PPCDCacheStoreAsync, then EXI_ImmEx for the remainder). CLAUDE.md said EXI DMA writes are avoided for the GC's 1-bit-shift bug; that is true of Swiss, not libogc. Nothing in the gateware changes -- a DMA write is just bytes clocked in -- but the path will be exercised, and if that bug is real it appears as corrupted TX frames that are not fixable here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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]);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user