-
Notifications
You must be signed in to change notification settings - Fork 803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[rom_ctrl,dv] Stress sequence tidyup #25864
Merged
rswarbrick
merged 6 commits into
lowRISC:master
from
rswarbrick:rom-ctrl-stress-seq-tidyup
Jan 21, 2025
Merged
[rom_ctrl,dv] Stress sequence tidyup #25864
rswarbrick
merged 6 commits into
lowRISC:master
from
rswarbrick:rom-ctrl-stress-seq-tidyup
Jan 21, 2025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This sequence sends an error message from KMAC and checks that it does indeed get reflected as an alert from rom_ctrl. This works fine, but the sequence followed up with a dut_init call, which is a bit strange! I suspect this was to avoid getting to cip_base_vseq::post_start with an alert in progress. This was a problem because rom_ctrl_kmac_err_chk_vseq didn't set the expect_fatal_alerts flag. Do so, and drop the extra reset. Signed-off-by: Rupert Swarbrick <[email protected]>
rom_ctrl is a little unusual and it doesn't really make sense to run a vseq without a reset. Change things so that we don't do so. Signed-off-by: Rupert Swarbrick <[email protected]>
This is a problem if the last child vseq causes a fatal alert, which won't have been cleared by the end of the stress_all sequence. Signed-off-by: Rupert Swarbrick <[email protected]>
We don't actually have an intr_test register, so this code (copied from other blocks, it seems) didn't do anything. Drop it. Signed-off-by: Rupert Swarbrick <[email protected]>
This is just changing the vseq slightly so that the body task exits quickly if it sees a reset. Signed-off-by: Rupert Swarbrick <[email protected]>
The motivation is a bit silly: it covers a toggle coverage point, where we'd like to see the KMAC response signal go between 0 and 1. Running this sequence in the middle of some smoke sequences covers it appropriately. Signed-off-by: Rupert Swarbrick <[email protected]>
alees24
approved these changes
Jan 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, I think it'd be safer to separate the final commit to avoid potentially having a negative impact on other blocks. The rest looks fine, thanks.
rswarbrick
force-pushed
the
rom-ctrl-stress-seq-tidyup
branch
from
January 21, 2025 13:55
60a38f2
to
dcefc49
Compare
I've just pushed a version with all but the last commit and will merge that :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are several commits in this PR, with detailed individual commit messages. The original motivation was actually a hole in toggle coverage, but this has ended up tidying up a couple of other items