[top,dv] Add testplan for top-level - #668
Conversation
0407f19 to
86054da
Compare
b26d306 to
78f957d
Compare
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
My initial review.
| SW flushes both FIFOs, acknowledges any pending interrupts, sets threshold | ||
| registers to `MAILBOX_FIFO_DEPTH`, enables all three interrupts, then: | ||
| - Confirms the read FIFO is empty and write FIFO is not full. | ||
| - Writes `MAILBOX_FIFO_DEPTH` elements (0xBEEF0000..N) and confirms the write |
There was a problem hiding this comment.
Why did you chose the value BEEF0000?
There was a problem hiding this comment.
It's just an arbitrary value easy to find in ca log or waves. Do you have another suggestion?
There was a problem hiding this comment.
It's a bit sad that you are adding the image files before they are available. Is there a cleaner way to add a test that is expected to fail?
There was a problem hiding this comment.
I haven't found any other way, and I took this path as it has been mentioned in a meeting that it's good to see some red tests becoming green eventually. I agree with that, then we know directly from a quick glance at the dashboard where we stand. Do you have another suggestion?
78f957d to
87f9041
Compare
|
@marnovandermaas, I have implemented most of your feedback and answer to your questions. The comment I've marked as resolved are because I am certain it's addressed. I also did some simplification about the IRQ tests and added a forgotten link to the testplan from the sim_cfg file. |
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
I've had another go at reviewing this PR, but I think it still has too many mistakes to approve it. I haven't yet gone through the body text since I already found some mistakes in the categorisation and the test titles, but I assume there will be many more mistakes in the body. My opinion is that we need to cut the pull request down significantly if we are going to merge this in a reasonable time. To make it more manageable, what do you think about deleting the description body text (not the initial sentence) for all the V2 items and that you have a go at checking/rewriting the body text for the V1 points to gain confidence that there are no mistakes there?
There was a problem hiding this comment.
Is there a way to prove that these two are equivalent? I don't really want to spend the time manually checking that everything is still there after the re-order.
There was a problem hiding this comment.
I have a script to check it, I think you could ignore this commit
99d7e5c to
367ca7e
Compare
Signed-off-by: martin-velay <mvelay@lowrisc.org>
Signed-off-by: martin-velay <mvelay@lowrisc.org>
367ca7e to
9141345
Compare
|
@marnovandermaas I think it should be better now |
2 participants
Create the full testplan for existing and future tests for top-level DV.