Skip to content
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

Backport 25065 from master to earlgrey_1.0.0 #25111

Merged
merged 4 commits into from
Nov 24, 2024

Conversation

engdoreis
Copy link
Contributor

@engdoreis engdoreis commented Nov 13, 2024

Update testplans and SiVal test suites

This Backport depends on #25091 to be merged first.

Copy link
Contributor

@matutem matutem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I flagged some issues with clkmgr and pwrmgr tests, and filed an issue for the clkmgr testpoints mentioned. Also, we need to resolve if redundant tests in pwrmgr are really worth running and taking CI time.

Obviously these issues need to be resolved in master, and if some change is made they should be cherry-picked.

@engdoreis
Copy link
Contributor Author

Thanks @matutem for the review.
Do we never want to run these tests that you commented on in Silicon?
If yes, I should change the PR and keep them as NA, otherwise we merge it as it is, and then we open issues to adjust the tests as you suggested.

@matutem
Copy link
Contributor

matutem commented Nov 20, 2024

@engdoreis I will be working on #25177 now and expect to have "//sw/device/tests:clkmgr_jitter_frequency_test" ready and testing jitter frequency more thoroughly. I will change "//sw/device/tests:clkmgr_jitter_test" to be a test of jitter_regwen. Incidentally, the various DV tests in chip_sw_clkmgr_jitter enable jitter, but they are obviously not SiVal tests.

As for the other changes, my concern about pwrmgr is weak, since it may be useful to have simpler tests than the more comprehensive one and the tests are quick, so they are good to go.

I am concerned about the chip_sw_flash_init testpoint: I think we need a more robust test in master before cherry picking it.

So we need a new issue to address the shortcomings of chip_sw_flash_init in master before cherry picking it.

Bottom line, all but the clkmgr and flash_ctrl testplan changes are okay. WDYT? @moidx?

@engdoreis
Copy link
Contributor Author

@matutem I made the suggested changes.

@engdoreis engdoreis force-pushed the update_testplans branch 4 times, most recently from 48d33b3 to ffd7762 Compare November 21, 2024 14:22
Signed-off-by: Douglas Reis <[email protected]>
(cherry picked from commit b038317)
Signed-off-by: Douglas Reis <[email protected]>
(cherry picked from commit 1dc2424)
Signed-off-by: Douglas Reis <[email protected]>
(cherry picked from commit 51edb16)
Signed-off-by: Douglas Reis <[email protected]>
(cherry picked from commit a61b9e3)
Copy link
Contributor

@matutem matutem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Douglas. LGTM

@engdoreis engdoreis merged commit 17928c0 into lowRISC:earlgrey_1.0.0 Nov 24, 2024
34 checks passed
@engdoreis engdoreis deleted the update_testplans branch November 24, 2024 13:16
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