-
Notifications
You must be signed in to change notification settings - Fork 12
earlgrey, darjeeling: rework machine reset management #186
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
earlgrey, darjeeling: rework machine reset management #186
Conversation
…G machine Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
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.
LGTM. I tested resetting Earlgrey via the monitor and it works great now, thank you!
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.
Spotted some stuff in comments otherwise LGTM
…s not available Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
1dc5f03
to
53669d6
Compare
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.
LGTM but it seems to be breaking some EG tests
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 for looking into this! I feel like I lack the full understanding of QEMU's reset sequence to give a thorough review, but the changes seem sensible and well-documented, and all existing passing Earlgrey tests remain passing.
The CI failure is likely my fault - I merged a new test in upstream OpenTitan @jwnrt I wonder if we should make passing tests (not marked as passing) non-failing in CI but give a loud warning, because otherwise CI is fragile to changes in |
Yes, that sounds sensible. In any case this PR didn’t cause any problems so it looks okay to merge |
Rework machine reset management, including board and SoC: