Support BOTH command-space forms: Swiss's 4-byte and libogc's 2-byte

Reading Swiss's own driver (cube/patches/bba/bba.c) shows the previous commit
fixed libogc and BROKE Swiss.  The two disagree, and both are real:

  Swiss   bba_cmd_out8: exi_imm_read_write(0x40<<24 | reg<<24 | val, 4)
          -> ONE 4-byte transfer [0x40|reg, 00, 00, val], and bba_cmd_in8
             returns the LOW byte, so a read response must be in BYTE 3.
  libogc2 bba_cmd_outsnosel: req = (reg<<8)|0x4000;
          EXI_ImmEx(&req, 2, WRITE); EXI_ImmEx(val, len, WRITE)
          -> THREE bytes, value in byte 2, and a read takes byte 2.
  stock libogc: identical to libogc2 (read out of libbba.a).

They share bytes 0 and 1 and differ only in how many bytes follow, so one
decoder serves both: take the LAST data byte before CS rise as the write value
(Swiss sends [00, val], libogc sends [val] -- last is val either way), and push
a read response TWICE so bytes 2 and 3 both carry it.  Swiss clocks 4 bytes and
pops both; libogc clocks 3 and leaves one, which the next transaction's header
pops drain -- a 2-byte header gives exactly one drain slot, so the leftover is
bounded at one and cannot accumulate.

Critically this means NO auto-increment of cmd_reg across the data phase: that
applied Swiss's padding 00 to the register and val to reg+1, which is precisely
how the libogc-only version broke Swiss.

T12 covers the Swiss form and asserts bytes 2 AND 3; T10 keeps the libogc form.
Both mutation-tested: re-adding the auto-increment fails T12 with 0x00 in both
slots, and pushing the response once fails T12's byte 3.  gc_bba_test gains
"cmd-space Swiss" alongside "cmd-space write", with bba_cmd_write4/read4.

Also corrects two things in CLAUDE.md now checkable against three drivers:
- NO driver writes TWD -- not Swiss, not libogc, not libogc2.  The "GC sets
  TWD before streaming" note was self-confirming: t_tx writes TWD because we
  wrote t_tx.  The counted TXDATA length is the primary path, not a fallback.
- Swiss's exi_dma_write rounds the length UP to 32 bytes (OSRoundUp32B), so
  TXDATA gets up to 31 trailing bytes past the frame and a counted length sends
  that much extra.  Real hardware must do the same since Swiss works on it;
  trailing padding past the IP total length is ignored.  A 1514-byte frame
  rounds to 1536 though, over the 1518 maximum.

Both drivers trigger TX with NCRA ST1, which the existing any-ST test accepts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-10-10 11:01:33 +02:00
parent 71ec6a2b37
commit b3685237b2
5 changed files with 223 additions and 52 deletions
+58 -19
View File
@@ -1065,25 +1065,51 @@ REGISTER space (16-bit reg) 4-byte header, then a SEPARATE data phase
write: [0xC0, reg>>8, reg&0xFF, 0x00] then N bytes written
```
⚠️ **Command space is a 2-byte header + data phase — corrected 2026-10-10.**
This file previously said "one 4-byte READWRITE transfer,
`[0x40|reg, 00, 00, data]`", and the decoder followed it: it waited for a
fourth byte, consumed the DATA byte as header byte 2, and **silently dropped
every command-space write.** libogc's `__bba_init` writes command register 2
(its interrupt mask) at both the start and the end of init, so none of that
would have taken effect. The real shape, from the disassembly:
⚠️ **COMMAND SPACE HAS TWO FORMS AND BOTH ARE REAL — settled 2026-10-10 by
reading all three drivers.** Register space is the same everywhere; command
space is not:
```
EXI_Imm (len=2, write) -> 42 00 the WHOLE header
EXI_Sync
EXI_ImmEx(len=1, write) -> 00 the data
```
| contract | Swiss patch | libogc (stock) | libogc2 |
|---|---|---|---|
| RX descriptor | packed LE bitfield | packed LE bitfield | packed LE bitfield |
| register space | 4-byte header + data phase | same | same |
| **command space** | **ONE 4-byte transfer**, value/response in **byte 3** | 2-byte header + data phase, **byte 2** | 2-byte header + data phase, **byte 2** |
| TWD on TX | **never written** | never written | never written |
| TX trigger | NCRA **ST1** | NCRA ST1 (`0x04`) | NCRA ST1 |
Three bytes between CS fall and CS rise, not four. `0x4200 = 0x4000 |
(reg << 8)`. The device-ID read survived the wrong decode only because
`SPIMode3Slave` answers it on a fast path independent of the FSM — which is
exactly why no test caught it. `bba_register_file` **T10** now writes command
register 2 and reads it back, and is mutation-tested against the 4-byte decode.
Sources, so this can be re-checked rather than re-guessed:
- Swiss — `cube/patches/bba/bba.c`, `bba_cmd_out8`:
`exi_imm_read_write(0x40<<24 | (reg&0x3F)<<24 | val, 4)`, and `bba_cmd_in8`
returns the **low byte** of that 32-bit result.
- libogc2 — `lwip/arch/gc/netif/gcif.c`, `bba_cmd_outsnosel`:
`req = (reg<<8)|0x4000; EXI_ImmEx(&req, 2, WRITE); EXI_ImmEx(val, len, WRITE)`.
- stock libogc — the same, read out of `libbba.a` (`gcif.o`, `__bba_init`):
`EXI_Imm(len=2)` of `42 00`, then `EXI_ImmEx(len=1)` of the data.
**How one decoder serves both:** they share bytes 0 and 1, and differ only in
how many bytes follow. So the device takes the **LAST data byte before CS
rise** as the write value (Swiss sends `[00, val]`, libogc sends `[val]` —
last is `val` either way), and **pushes a read response TWICE** so byte 2 and
byte 3 both carry it. Swiss clocks 4 bytes and pops both copies; libogc clocks
3 and leaves one, which the next transaction's header pops drain — a 2-byte
header gives exactly one drain slot, so the leftover is bounded at one and
cannot accumulate.
⚠️ **There must be NO auto-increment of `cmd_reg` across the data phase.** It
would apply Swiss's padding `0x00` to the register and `val` to `reg+1`. This
is how the libogc-only version broke Swiss.
History worth keeping, because the failure mode repeated: this file first had
Swiss's 4-byte form only, so libogc's writes were dropped (the decoder waited
for a fourth byte and ate the data byte as header byte 2). It was then
"fixed" to libogc's form only, which would have dropped Swiss's writes. Only
reading all three drivers showed that neither is "the" driver. The device-ID
read masked the original bug throughout, because `SPIMode3Slave` answers it on
a fast path independent of this FSM.
Tests: `bba_register_file` **T10** (libogc form) and **T12** (Swiss form,
asserting bytes 2 AND 3), both mutation-tested — re-adding the auto-increment
fails T12, and pushing the response once fails T12's byte 3.
NCRA, IR, NWAYS, RWP, RRP, PAR… all live in the REGISTER space
(`bba_in8`/`bba_out8`/`bba_ins`/`bba_outs`). Only the device-ID query uses a
short header — the single case where the two formats coincide.
@@ -1250,8 +1276,13 @@ parallel master is the default — see "W5100 vs W5500".)
| 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`,
⚠️ **No driver writes `TWD_LO`/`TWD_HI` — not Swiss, not libogc, not
libogc2** (Swiss: `bba_output` is `bba_outs(BBA_WRTXFIFOD, data, size)` then
NCRA ST1; libogc2: `bba_outsregister(BBA_WRTXFIFOD)` then NCRA ST1). So the
device has to COUNT the bytes it was handed. The TWD path is OUR test app's
convention, not a driver's — which is why the "GC sets TWD before streaming"
note in the TWD fix was self-confirming: `t_tx` writes TWD because we wrote
`t_tx`. `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
@@ -1265,6 +1296,14 @@ 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.
⚠️ **Swiss rounds the DMA length UP to 32 bytes** (`exi_dma_write` sets
`EXI[..][2] = OSRoundUp32B(len)`), so TXDATA receives up to 31 trailing bytes
beyond the frame. With a counted length the frame goes out that much longer.
Real hardware must behave the same way, since Swiss works on it — trailing
padding past the IP total length is ignored by receivers. Worth knowing before
treating a slightly-too-long TX frame as a bug. Note a 1514-byte frame rounds
to 1536, over the 1518 maximum.
⚠️ **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
+102 -30
View File
@@ -257,6 +257,8 @@ class BBARegisterFile(Elaboratable):
cmd_space = Signal() # this transaction is a command access
cmd_reg = Signal(6)
cmd_wreg = Signal(6) # target of a pending command-space write
cmd_have = Signal() # at least one data byte seen this txn
cmd_pushes = Signal(2) # command-read responses pushed
byte_ctr = Signal(3) # response-byte index (device-ID sequence)
cmd_wr = Signal() # command-space write pending
prefetched = Signal() # first response byte already pushed
@@ -378,7 +380,8 @@ class BBARegisterFile(Elaboratable):
# PREVIOUS transaction would be pushed as this
# one's first response and shift the whole
# read by one.
sp_pending.eq(0)]
sp_pending.eq(0),
cmd_have.eq(0), cmd_pushes.eq(0)]
with m.If(self.rx_data[7]): # register space
m.d.sync += cmd_space.eq(0)
m.next = "RHDR_HI"
@@ -448,25 +451,39 @@ class BBARegisterFile(Elaboratable):
m.d.comb += [self.tx_data.eq(rd_val), self.tx_en.eq(1)]
m.d.sync += [eff_addr.eq(eff_addr + 1), prefetched.eq(1)]
# ── command space: a 2-BYTE header, then a data phase ─────────
# ── command space: 2-byte header, then a data phase ───────────
#
# [0x40|reg, 0x00] + N data bytes written, or
# [reg, 0x00] + N data bytes read.
# ⚠️ THE TWO REAL DRIVERS DISAGREE ON THE LENGTH, and both must
# work. Checked against the source, not guessed:
#
# ⚠️ NOT a single 4-byte transfer. This file used to decode
# [reg, 00, 00, data] and wait for a fourth byte, taking the DATA
# byte as header byte 2 -- so every command-space WRITE was
# silently dropped. libogc's own driver (see the init map in
# CLAUDE.md) does, for a write of 0x00 to command register 2:
# Swiss (cube/patches/bba/bba.c, bba_cmd_out8 / bba_cmd_in8):
# exi_imm_read_write(0x40<<24 | reg<<24 | val, 4)
# -> ONE 4-byte transfer [0x40|reg, 00, 00, val], and a read
# takes the LOW byte of the 32-bit result, so the response
# must land in BYTE 3.
#
# libogc (libbba.a, gcif.o, __bba_init):
# EXI_Imm (len=2, write) -> 42 00 the whole header
# EXI_Sync
# EXI_ImmEx(len=1, write) -> 00 the data
# -> THREE bytes, data in byte 2, and a read takes byte 2.
#
# i.e. three bytes between CS fall and CS rise, not four. The
# device-ID read survived the old decode only because
# SPIMode3Slave answers it on the fast path, independently of
# this FSM, which is why nothing caught it.
# Both share bytes 0 and 1, so one decoder serves both:
#
# WRITE: latch every data byte and apply the LAST one at CS
# rise. Swiss sends [00, val] so the last is val;
# libogc sends [val] so the last is val. Hence NO
# auto-increment of cmd_reg -- that would apply Swiss's
# padding 00 to reg and val to reg+1.
# READ : push the response TWICE so byte 2 AND byte 3 carry it.
# Swiss clocks 4 bytes and pops both; libogc clocks 3 and
# leaves one, which the next transaction's header pops
# drain. A 2-byte header gives exactly one drain slot,
# so the leftover is bounded at one and cannot build up.
#
# This file previously had only the 4-byte form (Swiss's), and the
# decoder waited for a fourth byte -- so libogc's writes were
# dropped. Changing it to libogc's form alone then broke Swiss.
# Neither driver is "the" driver; support both.
with m.State("CHDR_1"):
with m.If(self.rx_rdy):
m.d.comb += self.rx_en.eq(1)
@@ -479,29 +496,37 @@ class BBARegisterFile(Elaboratable):
with m.Elif(is_write):
m.next = "CMD_WRITE"
with m.Else():
# Read: the response was prefetched below and the pop
# at the end of this byte lands it in data slot 0.
m.next = "TRAILER"
with m.Elif(~is_write & self.tx_rdy & ~prefetched):
# Ordinary command read: push during header byte 1 so the
# byte-boundary pop presents it in data slot 0.
m.next = "CMD_READ"
with m.Elif(~is_write & self.tx_rdy & (cmd_pushes == 0)):
# Push #1, during header byte 1, so the byte-boundary pop
# presents it in byte 2 -- what libogc reads.
m.d.comb += [self.tx_data.eq(rd_val), self.tx_en.eq(1)]
m.d.sync += prefetched.eq(1)
m.d.sync += cmd_pushes.eq(1)
with m.State("CMD_READ"):
# Push #2 lands in byte 3 -- what Swiss reads. It has a whole
# byte-time to get there, unlike push #1.
with m.If(self.tx_rdy & (cmd_pushes < 2)):
m.d.comb += [self.tx_data.eq(rd_val), self.tx_en.eq(1)]
m.d.sync += cmd_pushes.eq(cmd_pushes + 1)
with m.If(self.rx_rdy):
m.d.comb += self.rx_en.eq(1) # Swiss's padding bytes
with m.If(~self.cs_active):
m.next = "HEADER0"
with m.State("CMD_WRITE"):
# Data phase. Each byte writes the running command register,
# which auto-increments so a multi-byte command write behaves
# like the register-space path. cmd_wreg latches the target
# separately because the handler below applies the write a
# cycle later, by which time cmd_reg has already moved on.
# Data phase. Latch each byte; the LAST one before CS rise is
# the value, which is what lets Swiss's 4-byte form and
# libogc's 3-byte form both decode correctly.
with m.If(self.rx_rdy):
m.d.comb += self.rx_en.eq(1)
m.d.sync += [cmd_wr.eq(1),
cmd_wdata.eq(self.rx_data),
m.d.sync += [cmd_wdata.eq(self.rx_data),
cmd_wreg.eq(cmd_reg),
cmd_reg.eq(cmd_reg + 1)]
cmd_have.eq(1)]
with m.If(~self.cs_active):
m.next = "TRAILER"
with m.If(cmd_have):
m.d.sync += cmd_wr.eq(1)
m.next = "HEADER0"
with m.State("TRAILER"):
# Soak up anything else the GC clocks, then wait for CS to rise.
@@ -882,6 +907,32 @@ if __name__ == "__main__":
await ctx.tick("sync").repeat(6)
return result
async def cmd_write_swiss(ctx, creg, val):
"""Swiss's command write: ONE 4-byte transfer
[0x40|reg, 0x00, 0x00, val] -- exi_imm_read_write(..., 4)."""
ctx.set(reg.cs_active, 1)
await push_rx(ctx, 0x40 | (creg & 0x3F))
await push_rx(ctx, 0x00)
await push_rx(ctx, 0x00) # padding, NOT the value
await push_rx(ctx, val) # the value is the LAST byte
ctx.set(reg.cs_active, 0)
await ctx.tick("sync").repeat(6)
async def cmd_read_swiss(ctx, creg):
"""Swiss's command read: a 4-byte read/write whose LOW byte (byte 3)
is the response. Returns (byte2, byte3)."""
tx_log.clear()
ctx.set(reg.cs_active, 1)
await push_rx(ctx, creg & 0x3F)
for _ in range(4):
sample_tx(ctx)
await ctx.tick("sync")
await push_rx(ctx, 0x00)
got = await collect_tx(ctx, 2)
ctx.set(reg.cs_active, 0)
await ctx.tick("sync").repeat(6)
return got
# SPRAM contents the streaming-read test reads back (byte i = 0xA0+i).
spram_mem = {0x100 + i: (0xA0 + i) & 0xFF for i in range(64)}
@@ -1146,6 +1197,27 @@ if __name__ == "__main__":
ctx.set(reg.tx_irq_i, 0)
await ctx.tick("sync").repeat(2)
# ── T12: Swiss's command-space form, which DIFFERS from libogc's ──
# Swiss sends ONE 4-byte transfer with the value as byte 3 and reads
# the response from byte 3; libogc sends three bytes with the value as
# byte 2 and reads byte 2. Both are real drivers, so both must work:
# taking the LAST data byte as the value serves both, and pushing the
# read response twice puts it in byte 2 AND byte 3.
#
# Supporting only one of them is not a theoretical risk -- this file
# had Swiss's form only (libogc's writes were dropped), then briefly
# had libogc's form only (Swiss's writes would have been dropped).
await cmd_write_swiss(ctx, 0x02, 0x5A)
both = await cmd_read_swiss(ctx, 0x02)
print(f"T12 Swiss 4-byte cmd write 0x5A -> bytes 2,3 = "
f"{[hex(x) for x in both]}")
if len(both) < 2 or both[1] != 0x5A:
errors.append(f"T12 Swiss cmd form: byte 3 = "
f"{[hex(x) for x in both]}, want 0x5A")
if len(both) >= 1 and both[0] != 0x5A:
errors.append(f"T12 libogc byte-2 slot also wrong: {hex(both[0])}")
await cmd_write_swiss(ctx, 0x02, 0x00) # restore
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
+29
View File
@@ -139,6 +139,35 @@ int bba_cmd_read(u8 creg)
return (int)v;
}
/* SWISS's command-space form: ONE 4-byte transfer, value in byte 3, and a
* read takes byte 3 (the low byte of the 32-bit result). Swiss's
* bba_cmd_out8/in8 use exi_imm_read_write(..., 4).
*
* This DIFFERS from bba_cmd_write/bba_cmd_read above, which use libogc's
* 2-byte-header form. Both are real drivers and the device must serve both,
* so both shapes are exercised. */
int bba_cmd_write4(u8 creg, u8 val)
{
int rc;
u32 w = ((u32)(0x40 | (creg & 0x3F)) << 24) | val;
if ((rc = begin_raw()) < 0) return rc;
if (EXI_Imm(BBA_EXI_CHAN, &w, 4, EXI_WRITE, NULL) <= 0) { end_raw(); return -3; }
if (EXI_Sync(BBA_EXI_CHAN) <= 0) { end_raw(); return -4; }
end_raw();
return 0;
}
int bba_cmd_read4(u8 creg)
{
int rc;
u32 w = ((u32)(creg & 0x3F) << 24);
if ((rc = begin_raw()) < 0) return rc;
if (EXI_Imm(BBA_EXI_CHAN, &w, 4, EXI_READWRITE, NULL) <= 0) { end_raw(); return -3; }
if (EXI_Sync(BBA_EXI_CHAN) <= 0) { end_raw(); return -4; }
end_raw();
return (int)(w & 0xFF); /* Swiss reads the LOW byte = byte 3 */
}
int bba_r8(u16 addr)
{
u8 v;
+6
View File
@@ -82,6 +82,12 @@ void end_raw(void);
int bba_cmd_write(u8 creg, u8 val);
int bba_cmd_read(u8 creg);
/* Swiss's command-space form: ONE 4-byte transfer, value/response in byte 3.
* Differs from bba_cmd_write/read above (libogc's 2-byte-header form); the
* device must serve both. */
int bba_cmd_write4(u8 creg, u8 val);
int bba_cmd_read4(u8 creg);
/* Convenience single-byte helpers. bba_r8 returns the byte, or <0 on error. */
int bba_r8(u16 addr);
int bba_w8(u16 addr, u8 val);
+26 -1
View File
@@ -421,6 +421,30 @@ static int t_cmd_write(char *m, size_t n)
return rb == 0x00 ? 0 : 1;
}
static int t_cmd_write_swiss(char *m, size_t n)
{
int rc, rb;
/* Swiss sends ONE 4-byte transfer with the value as byte 3 and reads the
* response from byte 3; libogc (and libogc2) send a 2-byte header plus a
* data phase, value in byte 2. Both are real drivers. The device takes
* the LAST data byte as the value and pushes a read response twice, so
* bytes 2 and 3 both carry it -- this test is the Swiss half. */
if ((rc = bba_cmd_write4(0x02, 0x5A)) < 0) {
snprintf(m, n, "Swiss cmd write failed: EXI error %d", rc); return 1;
}
if ((rb = bba_cmd_read4(0x02)) < 0) {
snprintf(m, n, "Swiss cmd read failed: EXI error %d", rb); return 1;
}
if (rb != 0x5A) {
snprintf(m, n, "Swiss 4-byte form: wrote 0x5A, byte 3 = 0x%02X", rb);
bba_cmd_write4(0x02, 0x00);
return 1;
}
bba_cmd_write4(0x02, 0x00);
snprintf(m, n, "4-byte form ok (byte 3 = 0x%02X)", rb);
return 0;
}
static int t_mac_is_readable(char *m, size_t n)
{
u8 mac[6];
@@ -654,6 +678,7 @@ static test_t s_spram[] = {
static test_t s_init[] = {
{"cmd-space write","2-byte header + data phase, as libogc sends it", t_cmd_write, T_PENDING, {0}},
{"cmd-space Swiss","4-byte transfer, value in byte 3, as Swiss sends", t_cmd_write_swiss, 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}},
@@ -666,7 +691,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, 4},
{"driver shapes", "Swiss + libogc init sequences and transfer shapes", s_init, 5},
};
const int g_nsuites = sizeof(g_suites) / sizeof(g_suites[0]);