Clock the EXI bit engine from the EXI pin: timing closes on all 8 seeds
Replaces the PLL-fed 54 MHz `capture` domain, which oversampled the GC's
27 MHz EXI clock 2x and so put BIT-level timing across a CDC boundary: the
engine's view of the clock lagged the pin by 2 FFSynchronizer stages plus a
register (~55 ns) against a 37 ns bit time, so MISO came out ~2 bits late and
the GC read 0x17 as 05 C0. No amount of oversampling fixes that -- 54 MHz
gives only 2 ticks per bit and the part cannot close the 81 MHz that 3x needs.
The engine is now clocked by the pin itself (pin 44 = global buffer input net
6, no board change), making MISO clock-to-Q. ExiCapture creates four domains:
exiclk (posedge, samples MOSI), exiclk_n (negedge, drives MISO), both
async-reset by CS, plus reset-less exififo_w/exififo_r for the byte FIFO sides
-- reset-less because resetting one side of an AsyncFIFO desynchronises its
gray pointers, and posedge because AsyncFIFO has a posedge-only reset CDC
internally and cannot host a negedge side.
Also removes the SPRAM response FIFO. The DMA read was one byte late because
the round trip did not fit the protocol's turnaround byte; the arbiter already
exposed valid/ready, so the FIFO bought a cycle of latency and nothing else.
The response is handled outside the FSM (the reply lands in RHDR_PAD, not
SPRAM_STREAM) with a one-entry hold for when tx_fifo is momentarily full.
The PLL was left instantiated but unused after the rewrite; removing it frees
the hard macro and pin 35 (which stays unconnected only because the board is
already built that way). This is what lifted the pass rate:
domain target before after
clk (sync) 24 MHz 28.1-30.2 26.4-32.2 8/8
capture_clk 54 MHz 2 of 8 PASS (gone)
EXI pin domain 27 MHz -- 45.6-64.4 8/8
Best seed 3: clk 32.18 MHz, EXI 55.86 MHz. The binding constraint has FLIPPED
-- `clk` (1.10-1.34x) is now tighter than the EXI domain (1.69-2.39x) -- so
synth.py ranks seeds by worst RELATIVE margin, not by the EXI number.
synth.py could not have scored this design at all: it required the vanished
capture_clk, and its regex '(\w+)' cannot match nextpnr's exi_clk_$glb_clk
(no $ in \w). add_clock_constraint was also landing on the wrong net -- under
EXI_J4 the clock is the AND of two pads, not either pad -- so nextpnr silently
fell back to a 12 MHz default and under-optimised the whole EXI domain.
Testbenches:
- spi_mode3_slave had lost its testbench entirely in the rewrite. Restored
with a strict Mode 3 master (changes MOSI on the falling edge, samples MISO
in the clock-low phase) and MUTATION-TESTED: flipping either clock edge in
the RTL makes it fail. Both of those bugs previously reached silicon
against a lax master that parked MOSI well before the edge.
- exi_j4 T2/T3 returned [0,4,2,2]. Real bug: the engine latches the FIFO head
at each byte boundary, so a 2-byte command header leaves only ONE drain
slot. DEVID streamed zeros until CS and left two stale bytes -- one too
many -- so the NEXT enumeration came back a slot late. DEVID now pushes
exactly four bytes and stops. Register-space reads may still over-push:
their 4-byte header gives three drain slots.
- Dead ports removed from SPIMode3Slave (hdr_bytes, frame_start, cs_active,
byte_idx) and the mode2 parameter is gone -- there is one correct edge
convention and it is now enforced by tests.
All 14 module suites pass, plus bba_top at EXI_HALF=1/2/8.
CLAUDE.md: rewrote the clocking, CDC inventory and module tables; marked the
2026-09-30 capture_clk measurements historical; and corrected the line in
"Critical Protocol Notes" that still stated the INVERTED edge convention
("MOSI sampled on falling, MISO driven on rising" is Mode 2) -- the original
implementation followed that doc error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+75
-50
@@ -1,28 +1,45 @@
|
||||
"""ExiCapture — fast EXI byte-capture front-end (capture domain, 54 MHz).
|
||||
"""ExiCapture — EXI byte-capture front-end, clocked by the GC's EXI clock.
|
||||
|
||||
Wraps the SPIMode3Slave bit engine and bridges it to the slower `exi` domain
|
||||
(24 MHz) through two AsyncFIFOs:
|
||||
Creates the pin-driven clock domains (so the byte FIFOs can name them), wraps
|
||||
the SPIMode3Slave bit engine, and bridges it to the 24 MHz `sync` domain
|
||||
through two AsyncFIFOs:
|
||||
|
||||
capture (54 MHz) exi (24 MHz)
|
||||
┌────────────────────┐ rx_fifo ───► received bytes (header + data)
|
||||
│ SPIMode3Slave │ (8-bit, capture→exi)
|
||||
│ (bit engine) │ tx_fifo ◄─── response bytes to drive on MISO
|
||||
└────────────────────┘ (8-bit, exi→capture)
|
||||
EXI clock pin (~27 MHz) sync (24 MHz, HFOSC)
|
||||
┌─────────────────────┐ rx_fifo ───► received bytes (header + data)
|
||||
│ SPIMode3Slave │ (8-bit, exififo_w→sync)
|
||||
│ (bit engine) │ tx_fifo ◄─── response bytes to drive on MISO
|
||||
└─────────────────────┘ (8-bit, sync→exififo_r)
|
||||
|
||||
Why split: the bit engine must oversample a 27 MHz EXI clock 2×, which needs a
|
||||
54 MHz clock — far faster than the register-file logic can close (~44 MHz).
|
||||
Only this small, shallow front-end runs fast; everything else stays at 24 MHz.
|
||||
Domains created here
|
||||
--------------------
|
||||
exiclk EXI clock pin, posedge, async-reset by CS -- samples MOSI
|
||||
exiclk_n EXI clock pin, negedge, async-reset by CS -- drives MISO
|
||||
exififo_w EXI clock pin, posedge, RESET-LESS -- rx FIFO write side
|
||||
exififo_r EXI clock pin, posedge, RESET-LESS -- tx FIFO read side
|
||||
|
||||
TX response gating
|
||||
The FIFO domains must NOT see CS: resetting one side of an AsyncFIFO
|
||||
desynchronises its gray pointers against the other side. They are also both
|
||||
posedge, because AsyncFIFO instantiates a posedge-only reset CDC internally and
|
||||
cannot host a negedge side.
|
||||
|
||||
Why pin-clocked: oversampling the EXI clock from a faster domain put BIT-level
|
||||
timing across a CDC boundary (~55 ns of synchroniser latency against a 37 ns
|
||||
bit), so MISO came out ~2 bits late and the GC read 0x17 as 05 C0. Clocking
|
||||
from the pin makes MISO clock-to-Q and leaves these byte FIFOs as the only
|
||||
crossing, each with a full byte-time (~296 ns) of slack.
|
||||
|
||||
TX response timing
|
||||
------------------
|
||||
Every EXI transaction begins with 2 header bytes (write_flag/addr/len) during
|
||||
which the GC ignores MISO. The core cannot have produced a response yet (it
|
||||
hasn't even decoded the header), so the wrapper must NOT pop tx_fifo for those
|
||||
2 bytes. A per-transaction counter (`txld_cnt`, reset by frame_start) gates the
|
||||
pop: header bytes drive a don't-care 0xFF; from the first data byte onward the
|
||||
wrapper pops tx_fifo (one byte per tx_load). `tx_hold` is registered at tx_load
|
||||
time — before the FIFO advances — so the bit engine latches the correct byte on
|
||||
the following SPI rising edge (the classic FWFT-advance off-by-one is avoided).
|
||||
The engine pops tx_fifo once per byte boundary, unconditionally -- there is no
|
||||
header counter and no slot threshold. The byte popped at the end of byte N is
|
||||
presented during byte N+1, so pops during the header drain whatever the
|
||||
previous transaction left in the FIFO (this replaces the old CS-fall flush),
|
||||
and the response the register file pushes during the protocol's turnaround
|
||||
byte lands in data slot 0.
|
||||
|
||||
⚠️ Command space has only ONE drain slot (2-byte header), so a command-space
|
||||
responder must push exactly its bytes and stop -- see the DEVID note in
|
||||
bba_register_file.py.
|
||||
"""
|
||||
|
||||
from amaranth import *
|
||||
@@ -96,15 +113,15 @@ class ExiCapture(Elaboratable):
|
||||
# (plain) no reset -- the byte FIFOs. Resetting only ONE side of an
|
||||
# AsyncFIFO desynchronises its gray pointers and corrupts it,
|
||||
# so the FIFOs must NOT see CS.
|
||||
m.domains.exiclk = cd_p = ClockDomain(local=True, async_reset=True)
|
||||
m.domains.exiclk_n = cd_n = ClockDomain(local=True, async_reset=True,
|
||||
m.domains.exiclk = cd_p = ClockDomain(async_reset=True)
|
||||
m.domains.exiclk_n = cd_n = ClockDomain(async_reset=True,
|
||||
clk_edge="neg")
|
||||
m.domains.exififo_w = cd_fw = ClockDomain(local=True, reset_less=True)
|
||||
m.domains.exififo_w = cd_fw = ClockDomain(reset_less=True)
|
||||
# NOTE: both FIFO domains are POSEDGE. AsyncFIFO instantiates a
|
||||
# posedge-only reset CDC internally, so a negedge domain cannot host a
|
||||
# FIFO side. The engine pops in `exififo_r` (posedge) and hands the
|
||||
# byte to its negedge output stage through a register.
|
||||
m.domains.exififo_r = cd_fr = ClockDomain(local=True, reset_less=True)
|
||||
m.domains.exififo_r = cd_fr = ClockDomain(reset_less=True)
|
||||
m.d.comb += [
|
||||
cd_p .clk.eq(self.spi_clk), cd_p .rst.eq(self.spi_cs_n),
|
||||
cd_n .clk.eq(self.spi_clk), cd_n .rst.eq(self.spi_cs_n),
|
||||
@@ -144,9 +161,9 @@ class ExiCapture(Elaboratable):
|
||||
]
|
||||
|
||||
# ── TX: core pushes responses; the engine pops one per byte-time ──
|
||||
# The engine suppresses the pop during the protocol's turnaround byte
|
||||
# and drains stale bytes during the rest of the header, so no
|
||||
# CS-fall flush or slot-threshold logic is needed here any more.
|
||||
# Pops are UNCONDITIONAL (one per byte boundary). Header pops drain
|
||||
# whatever the previous transaction left behind, so no CS-fall flush
|
||||
# or slot-threshold logic is needed here any more.
|
||||
m.d.comb += [
|
||||
tx_fifo.w_data.eq(self.tx_data),
|
||||
tx_fifo.w_en .eq(self.tx_en),
|
||||
@@ -181,11 +198,9 @@ if __name__ == "__main__":
|
||||
# ratio is ~2; use 4 here for a clean, well-oversampled functional check.
|
||||
HALF = 4
|
||||
|
||||
# No capture domain any more: the bit engine is clocked by the EXI
|
||||
|
||||
# pin, so drive the SPI clock with real delays instead of ticks.
|
||||
|
||||
HP = Period(MHz=54) * HALF
|
||||
# No capture domain any more: the bit engine is clocked by the EXI pin,
|
||||
# so drive the SPI clock with real delays instead of counting ticks.
|
||||
HP = Period(MHz=54) # ONE capture-equivalent tick; delays are HP * n
|
||||
|
||||
async def spi_byte(ctx, mosi_val):
|
||||
"""Clock one SPI Mode 3 byte; return the assembled MISO byte."""
|
||||
@@ -216,20 +231,24 @@ if __name__ == "__main__":
|
||||
ctx.set(dut.tx_en, 0)
|
||||
|
||||
async def do_txn(ctx, hdr, responses, n_data, rx_seen):
|
||||
"""One EXI transaction: clock `hdr` bytes, model the clock-idle gap
|
||||
(drain rx + prefetch `responses` into tx_fifo), then clock `n_data`
|
||||
data bytes; return the MISO data bytes read."""
|
||||
"""One EXI transaction, modelling the REAL protocol shape.
|
||||
|
||||
The engine pops tx_fifo at every byte boundary, so a response must be
|
||||
in the FIFO before the LAST header byte ends to land in data slot 0.
|
||||
That byte is the protocol's turnaround byte (docs/exi_protocol_real.md)
|
||||
and is exactly when the register file pushes. Pushing after the whole
|
||||
header -- as this bench used to -- is one slot too late.
|
||||
"""
|
||||
ctx.set(dut.spi_cs_n, 0)
|
||||
ctx.set(dut.spi_clk, 1)
|
||||
await ctx.delay(HP * (HALF))
|
||||
for h in hdr:
|
||||
for h in hdr[:-1]:
|
||||
await spi_byte(ctx, h)
|
||||
for _ in range(20): # clock-idle gap
|
||||
await core_drain_rx(ctx, rx_seen)
|
||||
await ctx.tick("sync").repeat(1)
|
||||
for r in responses:
|
||||
for r in responses: # pushed during the turnaround
|
||||
await push_tx(ctx, r)
|
||||
await ctx.delay(HP * (2))
|
||||
await spi_byte(ctx, hdr[-1])
|
||||
await core_drain_rx(ctx, rx_seen)
|
||||
miso = [await spi_byte(ctx, 0x00) for _ in range(n_data)]
|
||||
ctx.set(dut.spi_cs_n, 1)
|
||||
await ctx.delay(HP * (HALF))
|
||||
@@ -242,24 +261,30 @@ if __name__ == "__main__":
|
||||
rx_seen = []
|
||||
await ctx.delay(HP * (2))
|
||||
|
||||
# Real register-space header: [0x80, ah, al, 0x00] -- the trailing
|
||||
# byte is the turnaround slot.
|
||||
HDR = [0x80, 0x00, 0x12, 0x00]
|
||||
|
||||
# ── T1: header + 2 data bytes read back ──────────────────────────
|
||||
miso = await do_txn(ctx, [0x12, 0x34], [0xA5, 0x5A], 2, rx_seen)
|
||||
print(f"T1 rx={[hex(b) for b in rx_seen[:2]]} MISO={[f'0x{b:02X}' for b in miso]}")
|
||||
if rx_seen[:2] != [0x12, 0x34]:
|
||||
errors.append(f"T1 header rx wrong: {rx_seen[:2]}")
|
||||
miso = await do_txn(ctx, HDR, [0xA5, 0x5A], 2, rx_seen)
|
||||
print(f"T1 rx={[hex(b) for b in rx_seen[:4]]} MISO={[f'0x{b:02X}' for b in miso]}")
|
||||
if rx_seen[:4] != HDR:
|
||||
errors.append(f"T1 header rx wrong: {[hex(b) for b in rx_seen[:4]]}")
|
||||
if miso != [0xA5, 0x5A]:
|
||||
errors.append(f"T1 MISO wrong: {[hex(b) for b in miso]}")
|
||||
|
||||
# ── T2: prefetch over-push must NOT leak into the next transaction ─
|
||||
# ── T2: over-push must NOT leak into the next transaction ────────
|
||||
# Txn A pushes 2 responses but the GC clocks only 1 data byte, leaving
|
||||
# one stale byte in tx_fifo. Txn B must read its OWN fresh responses,
|
||||
# proving the CS-fall flush cleared the stale prefetch.
|
||||
# a stale byte in tx_fifo. Txn B must read its OWN responses. There
|
||||
# is no CS-fall flush any more: the engine's pops during the header
|
||||
# drain whatever the previous transaction left behind.
|
||||
rx_seen.clear()
|
||||
await do_txn(ctx, [0x12, 0x34], [0xA5, 0x5A], 1, rx_seen) # leaves 0x5A
|
||||
misoB = await do_txn(ctx, [0x12, 0x34], [0x11, 0x22], 2, rx_seen)
|
||||
await do_txn(ctx, HDR, [0xA5, 0x5A], 1, rx_seen) # leaves 0x5A
|
||||
rx_seen.clear()
|
||||
misoB = await do_txn(ctx, HDR, [0x11, 0x22], 2, rx_seen)
|
||||
print(f"T2 MISO after over-push: {[f'0x{b:02X}' for b in misoB]} (want 0x11 0x22)")
|
||||
if misoB != [0x11, 0x22]:
|
||||
errors.append(f"T2 flush failed — stale byte leaked: {[hex(b) for b in misoB]}")
|
||||
errors.append(f"T2 stale byte leaked: {[hex(b) for b in misoB]}")
|
||||
|
||||
sim = Simulator(dut)
|
||||
sim.add_clock(Period(MHz=24), domain="sync")
|
||||
|
||||
Reference in New Issue
Block a user