Skip to content

Drive invalid in AXI VCs#994

Merged
LarsAsplund merged 3 commits intoVUnit:masterfrom
LukasVik:drive_invalid
Mar 11, 2024
Merged

Drive invalid in AXI VCs#994
LarsAsplund merged 3 commits intoVUnit:masterfrom
LukasVik:drive_invalid

Conversation

@LukasVik
Copy link
Contributor

@LukasVik LukasVik commented Mar 4, 2024

When the 'valid' signal is low for an AXI (-Lite) channel, the signals of the channel should be driven with 'X'.
They should not keep their value from the last time the channel was valid.
This can hide errors in the DUT if the DUT samples values in the wrong clock cycle.

This change is largely inspired by the AXI-Stream VCs.

Note that this changes the default behavior of the three VCs, so in some sense it is a breaking change.
This could be avoided by setting the default value drive_invalid generic to false.
Though having it set to true is symmetric with the AXI-Stream VCs.
And in my personal opinion it should be enabled in pretty much all cases, to get a stronger test.

LukasVik added 2 commits March 4, 2024 10:48
When the 'valid' signal is low for an AXI (-Lite) channel, the signals of the channel should be driven with 'X'.
They should not keep their value from the last time the channel was valid.
This can hide errors in the DUT if the DUT samples values in the wrong clock cycle.

This change is largely inspired by the AXI-Stream VCs.

Note that this changes the default behavior of the three VCs, so in some sense it is a breaking change.
This could be avoided by setting the default value 'drive_invalid' generic to 'false'.
Though having it set to 'true' is symmetric with the AXI-Stream VCs.
And in my personal opinion it should be enabled in pretty much all cases, to get a stronger test.
@LukasVik LukasVik changed the title Drive invalid Drive invalid in AXI VCs Mar 4, 2024
@LukasVik
Copy link
Contributor Author

LukasVik commented Mar 4, 2024

Note that this is a reopen of #802, which I accidentally closed by deleting my fork, not realizing that this would close the PR. Could not find a way to open that again, so I create this new PR instead.

The diff is exactly the same, which has been reviewed by two people in #802. I have added two commits above, however, that fix some minor formatting and comments.

@LarsAsplund
Copy link
Collaborator

I'm ok with these changes but still some work on the test cases to have them pass. At this point, breaking changes are ok since next release is v5.0.

@LukasVik
Copy link
Contributor Author

Ah sorry, I didn't check CI status. Should be fixed now @LarsAsplund. Thank you!

@LarsAsplund LarsAsplund merged commit 1749a9b into VUnit:master Mar 11, 2024
@eine eine added this to the v5.0.0 milestone Mar 11, 2024
@umarcor
Copy link
Member

umarcor commented Mar 11, 2024

See #997.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants