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

[pwm,dv] Remove lots of dead code from the environment and do some tidying #24934

Merged
merged 6 commits into from
Nov 11, 2024

Conversation

rswarbrick
Copy link
Contributor

This builds on #24929, which should be merged first. Only the last 5 commits are unique to this PR:

  • a379082 [pwm,dv] Remove set_reg_en from pwm_base_vseq
  • ebc2609 [pwm,dv] Remove duration_cycles from the vseqs
  • 870fb50 [pwm,dv] Remove get_pwm_mode and pwm_mode_e from pwm_env_pkg
  • 40b73b8 [pwm,dv] Rewrite pwm_env_cfg using out-of-block declarations
  • d267b5c [pwm,dv] Rewrite pwm_scoreboard using out-of-block declarations

@rswarbrick rswarbrick added Component:DV DV issue: testbench, test case, etc. IP:pwm labels Oct 29, 2024
@rswarbrick rswarbrick requested a review from a team as a code owner October 29, 2024 17:42
@rswarbrick rswarbrick requested review from eshapira and removed request for a team October 29, 2024 17:42
@rswarbrick rswarbrick force-pushed the pwm-remove-cruft branch 2 times, most recently from e48098c to c9ffa9c Compare November 8, 2024 12:21
Copy link
Contributor

@hcallahan-lowrisc hcallahan-lowrisc left a comment

Choose a reason for hiding this comment

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

This is a nice cleanup, thanks @rswarbrick !

hw/ip/pwm/dv/env/pwm_scoreboard.sv Outdated Show resolved Hide resolved
hw/ip/pwm/dv/env/pwm_scoreboard.sv Outdated Show resolved Hide resolved
No functional change, but this allows us to attach proper
documentation comments to the functions/tasks and hopefully make the
class a bit easier to understand at a glance.

This commit also gets rid of get_reg_index, which was replaced by
get_multireg_idx from pwm_env_pkg with 92c5733 in 2021.

Signed-off-by: Rupert Swarbrick <[email protected]>
Since we were touching the code anyway, I've also changed the way that
we randomise the core frequency (divided down from the bus frequency).
The code looked like the scaling was configurable, but it is always
enabled. It also re-randomised on every call of what looks like a
getter (after the rest of the class had been randomised).

This commit changes things so we randomise the scaling ratio (picking
a value between 1/4 and 1) and then apply it with the call to
get_clk_core_freq. This two-stage process is needed because we
randomise the cfg object before connecting up the vif that determines
the bus frequency.

Signed-off-by: Rupert Swarbrick <[email protected]>

[pwm,dv] Remove unused en_random argument from get_clk_core_freq

This is never supplied at the call sites (and I don't think it ever
has been since it was defined in 2021!).

Signed-off-by: Rupert Swarbrick <[email protected]>
The function and enum weren't actually used anywhere (and were
actually inconsistent!). Drop them.

Signed-off-by: Rupert Swarbrick <[email protected]>
This is constrained to equal NUM_CYCLES in every sequence except for
pwm_stress_all_vseq. But the stress all sequence doesn't actually use
the value (because its child sequences have their own values, all set
to NUM_CYCLES).

Chop it out.

Signed-off-by: Rupert Swarbrick <[email protected]>
This task was supposed to work by setting the REGWEN to update the
state. This is a bit odd, since you can only ever clear REGWEN. What's
worse, the DV code was written when the register was rw1c (so it
cleared the register by writing 1).

The register become rw0c in 2021 with commit 25e147d, so this code
has done nothing since then.

Signed-off-by: Rupert Swarbrick <[email protected]>
@rswarbrick
Copy link
Contributor Author

Thanks for the review, @hcallahan-lowrisc. I've just force-pushed with your proposed comment change and a final commit that re-indents the scoreboard task that was a bit of a mess!

@rswarbrick rswarbrick merged commit c1d36f6 into lowRISC:master Nov 11, 2024
3 of 7 checks passed
@rswarbrick rswarbrick deleted the pwm-remove-cruft branch November 11, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. IP:pwm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants