Skip to content

Conversation

jwnrt
Copy link
Contributor

@jwnrt jwnrt commented Oct 17, 2025

Currently we never use the --reset-delay CLI flag. We think it makes more sense to configure the reset delay for a whole target at once rather than each test; for example QEMU needs a longer reset delay than FPGAs or silicon.

This PR splits the reset_target function into two:

  • reset which uses the configured delay
  • reset_with_delay which accepts a custom delay

This PR should be a no-op on all of our tests. The observable change is that there is no longer a --reset-delay flag but there is a reset_delay config field.

@jwnrt jwnrt requested review from AlexJones0 and pamaury October 17, 2025 15:38
@jwnrt jwnrt requested a review from a team as a code owner October 17, 2025 15:38
Copy link
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. It's nice to have less timing assumptions in opentitanlib.

jwnrt added 4 commits October 17, 2025 17:19
The RX clearing mechanism is implemented (in a slightly different
improved way) in `target_reset` so it's okay to defer that to
`reset_target(true)`.

Signed-off-by: James Wainwright <[email protected]>
This allows us to have one function using a default configured reset and
another one that allows specifying the reset.

Signed-off-by: James Wainwright <[email protected]>
Reset delays are now configured once in configuration files and cannot
be changed per-test on the command line.

This commit switches all uses of the `reset_target` function to use
either `reset` or `reset_with_delay` where appropriate. All delays
should be exactly the same after this commit.

Signed-off-by: James Wainwright <[email protected]>
@jwnrt jwnrt force-pushed the earlgrey-1.0.0-reset-delay branch from 2b318ea to fda4184 Compare October 17, 2025 16:19
@jwnrt jwnrt added the CherryPick:master This PR should be cherry-picked to master label Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CherryPick:master This PR should be cherry-picked to master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants