Answer the device-ID query from the bit engine, not the register file

bba_devid fails on a console with 00 04 02 02 -- the ID correct but one byte
late -- and it fails in its REAL shape, with the 16 us gap and exactly four
bytes clocked, so this is not the t_raw_id probe artefact.  The register suite
passes, so register-space reads are fine; only command space is affected.

Cause is latency, not phase.  A command-space header is two bytes, so the
reply must be in the tx FIFO by the LAST RISING EDGE of the header transfer:
one byte time, 296 ns at 27 MHz.  The round trip is rx AsyncFIFO (~125 ns) +
a sync cycle + tx AsyncFIFO (~110 ns) = 280-320 ns, marginal by construction.
HEADER0 already pushed in the same cycle it consumed byte 0, so the FSM side
was already as early as it can be.

The inter-transfer gap cannot rescue it: the load happens on that last rising
edge and the clock is PAUSED afterwards, so no edge exists during the gap for
anything to update.  That is why bba_devid's 16 us gap makes no difference.

Pre-pushing the ID at CS-fall does not work either -- the engine's
unconditional per-byte pops would eat it, and the header length is not known
until byte 0 has been decoded, which is the latency in question.

So take the round trip out of the path.  The device ID is a hardwired constant
and belongs to the physical interface rather than to the register file, and
byte 0 == 0x00 identifies the query exactly (bit 7 clear = command space, bit
6 clear = read, reg 0).  SPIMode3Slave now recognises it and answers
04 02 02 00 from the EXI clock domain, with no CDC at all.  The only thing it
needs from sync is id_phase, a single bit that changes once, crossed with an
FFSynchronizer.

The register file's ID path is REMOVED rather than kept as a fallback: left
in, it would push bytes that a 2-byte header has too few pops to drain, which
is exactly how an earlier version made the NEXT enumeration come back a slot
late.

Tests follow the responsibility:
  - spi_mode3_slave T7: the query returns 04 02 02 00 with the tx FIFO
    deliberately feeding 0x00, so only the fast path can be answering.
  - spi_mode3_slave T8: and it is GATED -- with id_arm low, 0x5A comes
    through from the FIFO.  Without this the fast path would hijack address 0
    forever and the GC could never read NCRA back after its own reset.
  - bba_register_file T1: retargeted to assert the FSM pushes NOTHING for an
    ID query, which is the property that stops stale bytes corrupting the
    following transaction.

Note what these tests do NOT establish: bba_top already passed the device ID
at EXI_HALF=1 before this change, because simulation cannot reproduce a
296 ns window miss -- the sync domain's latency relative to the EXI clock in
sim does not match silicon closely enough.  T7/T8 prove the fast path works;
the evidence that it is NECESSARY is the hardware result plus the paused-clock
argument above.

--seeds 8: clk 28.4-31.0 MHz, EXI 56.7-64.4 MHz, 8/8 PASS.  Best seed 2
(clk 30.98, EXI 59.47), flashed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 10:51:37 +02:00
parent 5db3b7751a
commit 7587636b48
4 changed files with 116 additions and 51 deletions
+34 -46
View File
@@ -70,6 +70,13 @@ class BBARegisterFile(Elaboratable):
# (DMA) bulk reads, not just ≤4-byte immediate transfers.
self.cs_active = Signal()
# One-shot: set at reset, cleared by the GC's first NCRA reset. While
# set, a command-space read of register 0 returns the device ID -- but
# the REPLY is produced by SPIMode3Slave's fast path, not here, because
# a 2-byte command header leaves too little time for a round trip
# through both byte FIFOs. This output is what arms that path.
self.id_phase = Signal(init=1)
# ── Interrupt (exi domain) ────────────────────────────────────────
self.exi_int_n = Signal(init=1)
@@ -222,7 +229,7 @@ class BBARegisterFile(Elaboratable):
tx_queued = Signal()
# True until first NCRA reset write: return device ID on addr=0 reads
id_phase = Signal(init=1)
id_phase = self.id_phase
# Per-byte SPRAM read handshake (register-read path): sp_req marks a
# request in flight; drain_ctr counts the read-phase dummy bytes.
@@ -262,13 +269,8 @@ class BBARegisterFile(Elaboratable):
with m.Case(_HIPR): m.d.comb += reg_rdval.eq(_HIPR_VAL)
with m.Default(): m.d.comb += reg_rdval.eq(0)
# Device-ID bytes (addr 0 read while id_phase): 0x04 0x02 0x02 0x00
devid = Signal(8)
with m.Switch(byte_ctr):
with m.Case(0): m.d.comb += devid.eq(0x04)
with m.Case(1): m.d.comb += devid.eq(0x02)
with m.Case(2): m.d.comb += devid.eq(0x02)
with m.Default(): m.d.comb += devid.eq(0x00)
# The device-ID constant lives in SPIMode3Slave now (it must be
# answered with zero CDC latency), so there is no copy here.
cmd_rdval = Signal(8)
with m.Switch(cmd_reg):
@@ -325,14 +327,12 @@ class BBARegisterFile(Elaboratable):
cmd_reg.eq(self.rx_data[0:6]),
addr.eq(self.rx_data[0:6]),
eff_addr.eq(self.rx_data[0:6])]
# EXI_GetID (byte 0 == 0x00) gives only ONE header
# byte of warning before the GC starts clocking the
# reply, so push the first ID byte the instant byte 0
# identifies the query -- the earliest point possible.
with m.If((self.rx_data == 0x00) & id_phase
& self.tx_rdy):
m.d.comb += [self.tx_data.eq(0x04), self.tx_en.eq(1)]
m.d.sync += byte_ctr.eq(1)
# NOTE: the device-ID reply is NOT produced here.
# A 2-byte command header leaves one byte time for a
# round trip through both AsyncFIFOs, which measured
# too slow on a console (the ID came back one byte
# late). SPIMode3Slave answers it from the EXI clock
# domain instead -- see its device-ID fast path.
m.next = "CHDR_1"
# ── register space: [0x80|w<<6, ah, al, 0x00] ──────────────────
@@ -392,25 +392,14 @@ class BBARegisterFile(Elaboratable):
with m.State("CHDR_1"):
with m.If(self.rx_rdy):
m.d.comb += self.rx_en.eq(1)
# EXI_GetID is its own shape: the GC writes 0x0000 (two
# bytes, so it lands here as command reg 0) and then clocks
# FOUR read bytes. Hand it the ID stream rather than the
# single-byte command response.
# The device-ID query (command reg 0 while id_phase) is
# answered by SPIMode3Slave, not here: pushing anything
# for it would leave stale bytes in the tx FIFO that a
# 2-byte header has too few pops to drain.
with m.If(~is_write & (cmd_reg == 0) & id_phase):
# byte_ctr is NOT reset here: HEADER0 already pushed
# ID byte 0 and advanced it. Resetting would restart
# the sequence and duplicate the first byte.
m.next = "DEVID"
with m.Else():
m.next = "CHDR_2"
with m.Elif(~is_write & (cmd_reg == 0) & id_phase
& self.tx_rdy & (byte_ctr < 4)):
# Device-ID query: keep feeding the ID while byte 1 is
# still on the wire so the tx FIFO stays ahead of the GC.
# NOT one-shot -- the FIFO is only 2 deep, and stopping
# after one byte leaves a hole in the next slot.
m.d.comb += [self.tx_data.eq(devid), self.tx_en.eq(1)]
m.d.sync += byte_ctr.eq(byte_ctr + 1)
with m.Elif(~is_write & self.tx_rdy & ~prefetched):
# Ordinary command read: response lands in byte slot 3.
m.d.comb += [self.tx_data.eq(rd_val), self.tx_en.eq(1)]
@@ -506,17 +495,10 @@ class BBARegisterFile(Elaboratable):
m.next = "HEADER0"
with m.State("DEVID"):
# Push EXACTLY the four ID bytes, then stop.
# The engine latches the tx_fifo head at every byte boundary,
# and a 2-byte command header leaves only ONE drain slot
# before the data phase. Streaming zeros until CS left two
# stale bytes behind, one too many to drain, so the NEXT
# enumeration came back a slot late (04 02 02 00 -> 00 04 02 02).
# Register-space reads may over-push safely: their 4-byte
# header gives three drain slots.
with m.If(self.tx_rdy & self.cs_active & (byte_ctr < 4)):
m.d.comb += [self.tx_data.eq(devid), self.tx_en.eq(1)]
m.d.sync += byte_ctr.eq(byte_ctr + 1)
# The reply comes from SPIMode3Slave's device-ID fast path;
# this state only drains the GC's dummy bytes and waits for
# CS, so NOTHING is pushed into the tx FIFO here. Pushing
# would leave stale bytes a 2-byte header cannot drain.
with m.If(self.rx_rdy):
m.d.comb += self.rx_en.eq(1)
with m.If(~self.cs_active):
@@ -727,11 +709,17 @@ if __name__ == "__main__":
ctx.set(reg.tx_rdy, 1) # tx_fifo always has room in this model
await ctx.tick("sync").repeat(8)
# T1: Device ID (addr=0, 4-byte read)
# T1: a device-ID query must push NOTHING.
# The reply comes from SPIMode3Slave's fast path (a 2-byte command
# header leaves too little time for a round trip through both byte
# FIFOs), so anything pushed here is stale data the header has too few
# pops to drain -- which is exactly how an earlier version made the
# NEXT enumeration come back a slot late. The ID value itself is
# tested in spi_mode3_slave (T7) and end-to-end in bba_top.
result = await exi_devid(ctx)
if result != _DEVICE_ID:
errors.append(f"T1 device ID: expected {_DEVICE_ID}, got {result}")
print(f"T1 device ID: {[f'0x{b:02X}' for b in result]}")
if result != []:
errors.append(f"T1 device-ID query must push nothing, got {result}")
print(f"T1 device-ID query pushes nothing: {result == []}")
await ctx.tick("sync").repeat(4)
# T2: Write and read back PAR0-PAR3