-
Notifications
You must be signed in to change notification settings - Fork 11
Fix regressions in Earlgrey 1.0.0 tests #159
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
Merged
loiclefort
merged 9 commits into
lowRISC:ot-earlgrey-9.2.0
from
AlexJones0:fix-earlgrey-regressions
Aug 26, 2025
Merged
Fix regressions in Earlgrey 1.0.0 tests #159
loiclefort
merged 9 commits into
lowRISC:ot-earlgrey-9.2.0
from
AlexJones0:fix-earlgrey-regressions
Aug 26, 2025
Conversation
This file contains hidden or 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
Format the error string to include the HJSON error message. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The stable `earlgrey_1.0.0` branch of OpenTitan stores its OTP constant file in a different location from the current `master` branch, which has changed with multitop to allow different OTP constant files for different tops. Introduce a very basic mechanism for checking many possible locations to allow the `cfggen.py` script to work with both branches, and to easily support potential future changes in location in a way that remains backwards-compatible. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
When running `cfggen.py` on OpenTitan, we now generate additional LC diversification values: `test_unlocked`, `dev` and `rma`, which were previously combined under the shared `test_dev_rma` value. This commit introduces these three separate values as properties of the lc_ctrl, and replaces the original `test_dev_rma` value as appropriate. This should allow the QEMU model to work with newer generated configurations. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Much more work needs to be done for Earlgrey's OTP functionality to catch up to Darjeeling's OTP; this commit simply stubs out the `inv_default_part_X` properties which are now being generated by `cfggen.py`, despite only being present in Darjeeling's OTP and not in Earlgrey's. This allows newer config generations to be used with Earlgrey for QEMU emulation. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These were accidentally shifted in commit 95aa0a8 whilst removing static clock definitions. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These were broken in commit 7622b7e adding clock configuration and distribution to the clock manager - these registers reads all previously fell through to the readable `R_FATAL_ERR_CODE` register, but when that was moved to be placed after the dynamic register range, it was not replaced and so all these register reads instead fell through to reading the clock hints. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
2584aa7
to
fbcb27d
Compare
See the added documentation on `OtClkMgrHintContext`: we necessarily initialise singular named GPIO input signals per hintable clock, but the handler is the same and thus the current clock hint handler cannot distinguish between the different incoming clock hints, instead incorrectly determining them all to be from the first hintable clock (AES in Earlgrey). Add some additional context to the opaque data passed to the clock hint handler so that the hintable clock index of the incoming hint can be determined. A preferable approach might be to use a GPIO array and use the hint as it seems was previously desired, but this then loses the ability to name the individual clock signals, which is needed for easily retrieving clock GPIO without giving each object access to the clock configuration property string. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
On Earlgrey, currently only 3/8 resettable devices are implemented because the USB Device and I2C are not yet available. The changes in commit 5e3834c made it so that the relevant RST_CTRL and RST_REGWEN registers are only reset for configured resettable devices (i.e. devices with some typename), but there is still value in being able to use these regwens for basic tests that use the rstmgr registers without checking the actual device reset behaviour. This helps to fix a regression seen in an Earlgrey test. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Fixes an error introduced in commit 7d42424. As part of this commit the traces were improved to include the rstmgr's `ot-id` property, but this was also added to the `ot_rstmgr_sw_rst_walker` function, which does not walk over just rstmgrs but instead all devices on the system bus. This meant that upon triggering a software reset, the cast would cause a QEMU error. Instead, we pass the relevant property through the opaque handle with the rest of the software reset description to retain the improved tracing. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
fbcb27d
to
553219f
Compare
loiclefort
approved these changes
Aug 26, 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.
Thanks, LGTM
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.
This PR introduces commits to allow us to continue building QEMU OpenTitan configurations from the latest OpenTitan
master
/earlgrey_1.0.0
branches after recent changes. After stubbing out some properties that were expected to be present in the OTP and lc_ctrl, I ran some tests onearlgrey_1.0.0
to find potential regressions. The last 5 commits contain bug fixes that address any new test failures that were seen.See the commit messages for more details.