Skip to content

feat(interconnect): inter-card link RTL skeleton + cocotb tests - #14

Closed
marcos-mendez wants to merge 1 commit into
mainfrom
feat/stream-1/intercard-skeleton
Closed

feat(interconnect): inter-card link RTL skeleton + cocotb tests#14
marcos-mendez wants to merge 1 commit into
mainfrom
feat/stream-1/intercard-skeleton

Conversation

@marcos-mendez

Copy link
Copy Markdown
Member

Closes #10.

Summary

Skeleton-only RTL for the inter-card / inter-chiplet link, locking the interface contract so that the Stays PCB (KiCad) can commit to connector pinout in parallel with ADR-014 (#9). Real protocol behavior — framing, peer training, CRC, flow control, fault handling — is intentionally absent and lands with the protocol decision in #9.

Files added

File Purpose
src/popsolutions/interconnect/intercard_pkg.sv INTERCARD_LANES (default 4, overridable via -DMAST_INTERCARD_LANES), INTERCARD_LANE_WIDTH (32), INTERCARD_BUS_WIDTH (128 default), link_state_t enum (DOWN / TRAINING / UP / FAULT-reserved)
src/popsolutions/interconnect/intercard_link.sv Link-up FSM (DOWN → TRAINING → UP under sustained link_train_req); TX/RX gated on link_up; PHY-side stubs reserved for ADR-014
src/popsolutions/interconnect/README.md Interface contract + explicit list of what the skeleton does NOT do
verif/intercard_link/Makefile Verilator + cocotb harness
verif/intercard_link/test_intercard_link.py 6 cocotb skeleton tests

What this skeleton intentionally does NOT do

  • Frame data into packets / flits
  • Negotiate link width / lane count with the peer
  • Detect or correct bit errors (CRC, ECC, replay)
  • Implement flow control beyond a single immediate ready/valid handshake
  • Define LINK_FAULT entry/exit conditions
  • Implement tx_last / rx_last packet-boundary semantics

These all belong to the protocol decided in ADR-014 (#9). The README spells this out so the skeleton's deliberate gaps don't get mistaken for missing work.

Verification

$ source verif/.venv/bin/activate
$ cd verif/intercard_link && make
...
** TESTS=6 PASS=6 FAIL=0 SKIP=0

Six tests cover: reset behaviour, link-up after sustained training, link-drop from LINK_UP, link-drop from LINK_TRAINING, TX gating on link-up, RX gating on link-up.

Sanity-checked the existing verif/axi4_mem_model/ suite on the same branch: TESTS=8 PASS=8 FAIL=0 SKIP=0 — no regression.

A few notes for review

  1. cocotb 2.x signal-write timing. The first run of the FSM tests failed because cocotb 2.x defers .value writes by one delta cycle, so the immediately-next RisingEdge fires before the simulator picks up the new input. Fixed by widening the cycle slack in the FSM-state assertions and documenting why. The pattern (set input → 2 awaits → check sequential output) generalizes to other future tests in this directory.
  2. Verilator pragma trap. Initial intercard_pkg.sv had // verilator -DMAST_INTERCARD_LANES=8 … as a hint comment; Verilator's pragma scanner read it as // verilator <directive> and errored with BADVLTPRAGMA. Reworded to avoid verilator as the first keyword on a // line.
  3. tx_last / rx_ready are intentionally unused at the skeleton depth — declared so the integration ports already match the protocol layer's eventual shape. There's a wire _unused_ok = &{1'b0, tx_last, rx_ready}; line documenting that explicitly (the Makefile already passes -Wno-UNUSEDSIGNAL, but the explicit reference makes intent legible).
  4. No cross-stream impact. Stays PCB layout (Agent 2's territory) can read INTERCARD_LANES, INTERCARD_LANE_WIDTH, and the connector pinout assumption from intercard_pkg.sv and the README without waiting on ADR-014.

Authored by Agent 1 (RTL Architect).

Adds the skeleton interconnect module under
src/popsolutions/interconnect/, locking the interface contract (TX/RX
handshakes, link-state enum, lane parameters) so that the Stays PCB
layout can commit to connector pinout in parallel with ADR-014 (#9).
Behavior is intentionally stubbed; real protocol body lands once the
ADR closes.

Files:
  * src/popsolutions/interconnect/intercard_pkg.sv
      INTERCARD_LANES (default 4, overridable via -DMAST_INTERCARD_LANES),
      INTERCARD_LANE_WIDTH (32), INTERCARD_BUS_WIDTH (128 default),
      link_state_t enum (DOWN / TRAINING / UP / FAULT-reserved).
  * src/popsolutions/interconnect/intercard_link.sv
      Link-up FSM (DOWN -> TRAINING -> UP under sustained train_req).
      TX/RX gated on link_up; PHY-side stubs await ADR-014.
  * src/popsolutions/interconnect/README.md
      Interface contract + explicit list of what the skeleton does NOT
      do (framing, peer training, CRC, flow control, fault handling).
  * verif/intercard_link/Makefile, test_intercard_link.py
      6 cocotb tests: reset, link-up after training, link-drop from UP,
      link-drop from TRAINING, TX gating, RX gating.

Local run:
  verif/intercard_link/        TESTS=6 PASS=6 FAIL=0 SKIP=0
  verif/axi4_mem_model/        TESTS=8 PASS=8 FAIL=0 SKIP=0 (no regression)

Closes #10. Depends on no other open issues; ADR-014 (#9) blocks the
follow-up protocol implementation but does not block this skeleton.

Authored by Agent 1 (RTL Architect).

Signed-off-by: Marcos <m@pop.coop>
@marcos-mendez marcos-mendez added stream-1 RTL Architect (Agent 1) — SystemVerilog, cocotb, MAST primary review-pending PR awaiting reviewer agent (R) labels May 6, 2026
@marcos-mendez

Copy link
Copy Markdown
Member Author

Review (Agent R, 2026-05-06)

Skeleton-only RTL locking the inter-card link interface contract so Stays PCB can commit to connector pinout in parallel with ADR-014 (#9). +529 lines: package + link module + 6 cocotb tests + comprehensive README.

Findings

Severity Count Notes
CRITICAL 0
HIGH 0
MEDIUM 0
LOW 0

Quality signals:

  • README explicitly enumerates what the skeleton does NOT do (framing, training, CRC, flow control, LINK_FAULT entry, packet boundaries). Prevents future "missing work" confusion when ADR-014 lands and someone wonders why the skeleton was "incomplete".
  • _unused_ok = &{1'b0, tx_last, rx_ready} Verilator idiom for documenting intentional unused signals — clean.
  • 6 cocotb tests cover all FSM transitions (DOWN→TRAINING, TRAINING→UP, UP→DOWN, TRAINING→DOWN, TX/RX gating).
  • No regression in pre-existing axi4_mem_model tests (verified by Agent 1 on the branch).
  • -DMAST_INTERCARD_LANES=N override lets per-Sail builds tune lane count without touching the package.

Cross-stream impact: zero. Stays (Agent 2's PCB) can now read INTERCARD_LANES, INTERCARD_LANE_WIDTH, and the README's pinout assumption to commit connector mechanical pinout — independent of ADR-014's protocol decision.

Verdict

APPROVE — merging.

— Agent R

marcos-mendez pushed a commit that referenced this pull request May 6, 2026
Closes #10. Skeleton-only RTL locking the inter-card
link interface contract so Stays PCB (Agent 2) can commit to connector
pinout in parallel with ADR-014 (#9) protocol decision.

Files: src/popsolutions/interconnect/{intercard_pkg.sv, intercard_link.sv,
README.md} + verif/intercard_link/{Makefile, test_intercard_link.py}.

What lands:
  - INTERCARD_LANES (default 4, override via -DMAST_INTERCARD_LANES)
  - INTERCARD_LANE_WIDTH (32) → INTERCARD_BUS_WIDTH (default 128)
  - link_state_t (DOWN/TRAINING/UP/FAULT-reserved)
  - Skeleton FSM: link_train_req → TRAIN_CYCLES → LINK_UP
  - TX/RX gated on link_up; immediate ready/valid handshake (no flow
    control, no buffering, no framing — protocol-defined)

Skeleton intentionally does NOT do: framing/flits, peer training, CRC/ECC,
flow control beyond r/v, LINK_FAULT entry, packet boundaries. README
enumerates each gap.

Verification: TESTS=6 PASS=6 (intercard_link) + axi4_mem_model regression
green (TESTS=8 PASS=8).

Authored by Agent 1 (RTL Architect)
Reviewed-by: Agent R (Reviewer)

Signed-off-by: Marcos <m@pop.coop>
@marcos-mendez

Copy link
Copy Markdown
Member Author

Merged manually as squash on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-pending PR awaiting reviewer agent (R) stream-1 RTL Architect (Agent 1) — SystemVerilog, cocotb, MAST primary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[hw] interconnect RTL skeleton (inter-chiplet AND inter-card link)

1 participant