Skip to content

Update TB for BNW s2 bug + remove recv_aborted_by_reset protocol - #318

Merged
ekiwi merged 9 commits into
mainfrom
update_s2_tb
Aug 27, 2026
Merged

Update TB for BNW s2 bug + remove recv_aborted_by_reset protocol#318
ekiwi merged 9 commits into
mainfrom
update_s2_tb

Conversation

@ngernest

Copy link
Copy Markdown
Contributor

This PR updates the test-bench for the BNW s2 bug (AXI-Stream implementation not holding tlast stable) so that reset does not become 0 (active) at the end of the waveform. This was a one line change in the BNW test-bench, changing PI_M_AXIS_ARESETN <= 1'b1 to PI_M_AXIS_ARESETN <= 1'b0 in the last cycle of the waveform. (I've included the Verilog code for the BNW test bench in this PR as well.) I then regenerated the buggy + fixed waveforms using this updated version of the BNW test-bench

The updated version of the fixed waveform looks like this (note that M_AXIS_ARESETN remains 1 towards the end of the waveform and no longer suddenly becomes 0):

Screenshot 2026-08-27 at 12 46 44 PM

Since the waveform has been updated, this PR also removes the recv_aborted_by_reset protocol from s2.prot. On the fixed waveform, the BI now infers:

trace {
    reset();  // [time: 0ns -> 12.5ns]
    recv(1, 15, 0, 0);  // [time: 12.5ns -> 887.5ns]
    recv(2, 15, 0, 0);  // [time: 887.5ns -> 912.5ns]
    recv(3, 15, 0, 0);  // [time: 912.5ns -> 937.5ns]
    recv(4, 15, 0, 0);  // [time: 937.5ns -> 962.5ns]
    recv(5, 15, 0, 0);  // [time: 962.5ns -> 987.5ns]
    recv(6, 15, 0, 0);  // [time: 987.5ns -> 1012.5ns]
}

If we pass the --include-in-progress flag to the BI, we also get:

...
recv(6, 15, 0, 0);  // [time: 987.5ns -> 1012.5ns]
recv(7, 15, 0, X);  // [time: 1012.5ns -> ]

This indicates that the final recv(7, ...) transaction did not complete in the waveform, which is what we expect since we see that ready = 0 during the last 4 cycles of the waveform above (so the data transfer for 7 never happens).

The BI continues to report an error on the (updated) buggy waveform, but we now only have an error message for the recv transaction (we no longer have an error for recv_aborted_by_reset, since this transaction has been deleted):

// trace 0
trace {
    reset();  // [time: 0ns -> 12.5ns]
    recv(1, 15, 0, 0);  // [time: 12.5ns -> 887.5ns]
    recv(2, 15, 0, 0);  // [time: 887.5ns -> 912.5ns]
    recv(3, 15, 0, 0);  // [time: 912.5ns -> 937.5ns]
    recv(4, 15, 0, 0);  // [time: 937.5ns -> 962.5ns]
    recv(5, 15, 0, 0);  // [time: 962.5ns -> 987.5ns]
    recv(6, 15, 0, 0);  // [time: 987.5ns -> 1012.5ns]
}
error: [recv@411?] executing step 1 of the transaction: 1 != 0
   ┌─ tests/fpga-debugging/axi-stream-s2/s2.prot:50:7
   │
50 │       assert_eq(DUT.M_AXIS_TLAST, is_last);
   │       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [recv@411?] executing step 1 of the transaction: 1 != 0

@ngernest
ngernest requested a review from ekiwi August 27, 2026 16:50
@ekiwi
ekiwi merged commit 7383151 into main Aug 27, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants