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

[multitop] Refactor DIF and testutils #26252

Merged
merged 10 commits into from
Feb 13, 2025
Merged

Conversation

pamaury
Copy link
Contributor

@pamaury pamaury commented Feb 11, 2025

This PR refactors the DIF BUILD file and the autogen, it also refactor the testutils and switches them to a bazel rule.
On the technical side, an important change is that the isr_tesutils now only depends on the autogen part of the DIF which means that it can be used even for IP which have no manual DIFs.

NOTE: the testutils don't compile yet for Darjeeling due to a hardcoded dependency on earlgrey in the isr testutils structure. This will need to switch to DT and will be done in a follow-up PR.

They do not compile because those files were forgotten in the DIF
unittest 'srcs' of the corresponding tests.

Signed-off-by: Amaury Pouly <[email protected]>
@pamaury pamaury requested review from cfrantz and a team as code owners February 11, 2025 17:00
@pamaury pamaury requested review from jon-flatley, a-will, timothytrippel, engdoreis, AlexJones0, jwnrt and nbdd0121 and removed request for a team February 11, 2025 17:00
@pamaury pamaury force-pushed the multitop_testutils branch 3 times, most recently from 93f371f to dde0263 Compare February 11, 2025 17:27
Copy link
Contributor

@jwnrt jwnrt left a comment

Choose a reason for hiding this comment

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

Reviewed the Bazel but not the Python and it looks good, especially the DIF generation cleanup. I'll double check again when it passes CI.

@pamaury
Copy link
Contributor Author

pamaury commented Feb 12, 2025

Closes #25752

With the upcoming change to the isr_testutils, the DIF are no longer
included as dependencies. The following two dependencies are affected
and need to explicitely include more DIFs.

Signed-off-by: Amaury Pouly <[email protected]>
Virtually all DIFs use the same structure so instead of repeating
the same thing many times over (and forget some files), use a loop
to describe all DIFs.

Signed-off-by: Amaury Pouly <[email protected]>
With this change, the tool becomes more flexible so it can be run
by bazel. More precisely, we can select the output directory, we
can also explicitely provide the path to the IP's Hjson files so
that the tool doesn't have to guess.

Signed-off-by: Amaury Pouly <[email protected]>
This rules runs util/autogen_testutils.py

Signed-off-by: Amaury Pouly <[email protected]>
Some dependencies depend on the top. For now manually select the right
ones.

Signed-off-by: Amaury Pouly <[email protected]>
The heartbeat mode modules the blink mode. This was fixed in:
  [dif,pwm] Heartbeat mode operates only when blinking enabled.
but the unittest was not updated

Signed-off-by: Amaury Pouly <[email protected]>
@pamaury pamaury merged commit afa6ed1 into lowRISC:master Feb 13, 2025
42 checks passed
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.

3 participants