[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.
| "i2c_host_tx_rx_cheri"] | ||
| } | ||
| { | ||
| name: chip_i2c_device_tx_rx |
There was a problem hiding this comment.
What about i2c host?
There was a problem hiding this comment.
What do you mean? i2c_host is just above.
| 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?
Signed-off-by: martin-velay <mvelay@lowrisc.org>
Signed-off-by: martin-velay <mvelay@lowrisc.org>
Signed-off-by: martin-velay <mvelay@lowrisc.org>
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.
| every byte read back matches the byte written, confirming an agent-driven | ||
| controller-mode transaction over the chip I2C pins in both directions. | ||
|
|
||
| Both vanilla and CHERI firmware images are exercised. |
There was a problem hiding this comment.
I don't think it's worth repeating this for each test.
|
|
||
| { | ||
| name: chip_clkmgr_smoke | ||
| desc: '''Verify software control of gateable and hintable clocks via the clock manager. |
There was a problem hiding this comment.
The software and hint clocks are unused in Mocha and are likely to be removed in the future.
| "clkmgr_smoke_cheri"] | ||
| } | ||
| { | ||
| name: chip_clkmgr_clock_gating |
There was a problem hiding this comment.
Clock gating is currently not used in the Mocha design and not sure if this will be included in the future.
|
|
||
| Both vanilla and CHERI firmware images are exercised. | ||
| ''' | ||
| stage: V1 |
There was a problem hiding this comment.
Is this necessary for V1 or can this be V2?
| once the chip returns to active power. | ||
| ''' | ||
| stage: V2 | ||
| tests: ["rv_dm_access_after_wakeup"] |
There was a problem hiding this comment.
Why does this not have CHERI version?
| /////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| { | ||
| name: chip_alert_handler_smoke |
There was a problem hiding this comment.
There is no alert handler in Mocha.
2 participants
Create the full testplan for existing and future tests for top-level DV.