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

[prim,ipgen] Fix up some hierarchies of fusesoc cores #25703

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

a-will
Copy link
Contributor

@a-will a-will commented Dec 18, 2024

Ensure packages with constants are at the leaves of the dependency tree, so other cores may depend on them without creating cycles. Adjust the pwrmgr_reg core to depend on the pwrmgr_pkg core, with pwrmgr_reg_pkg moved to the pwrmgr_pkg core.

Also split the mubi RTL implementations from the package with declarations and standalone utility functions.

In addition, fix up the reg tops for various ipgen'd cores to include a dependency on prim:subreg.

Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

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

This looks really sensible to me! Very happy with it once you're ready with the change.

Place the prim_mubi_pkg package into a separate prim_mubi_pkg
fusesoc core, so it can be used freely for interfaces without
pulling in the RTL. This helps avoid dependency cycles in the build
graph.

Adjust tlul:headers to depend only on the prim_mubi_pkg core.

Signed-off-by: Alexander Williams <[email protected]>
@@ -10,6 +10,7 @@ virtual:
filesets:
files_rtl:
depend:
- lowrisc:prim:subreg
- lowrisc:tlul:headers
Copy link
Contributor

Choose a reason for hiding this comment

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

Funny this depends only on tlul:headers and clkmgr on ip:tlul. It seems it should only one of the two uniformly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the reg_top is here, it should be ip:tlul. Nice catch!

Relatedly, I think I ought to move rstmgr_reg_pkg.sv over to the rstmgr_pkg core, so it can avoid pulling in RTL cores that can create dependency cycles. (similar to what has been done for the pwrmgr_pkg core)

Ensure packages with constants are at the leaves of the dependency tree,
so other cores may depend on them without creating cycles. Adjust the
pwrmgr_reg core to depend on the pwrmgr_pkg core, with pwrmgr_reg_pkg
moved to the pwrmgr_pkg core.

Signed-off-by: Alexander Williams <[email protected]>
Have prim_generic_flash only depend directly on the tlul headers to
avoid dependency cycles. Internally, it only uses tlul_pkg for the
port data types. A different core implements the reg_top with tlul
logic, and prim_generic_flash's dependency to it is virtual.

Signed-off-by: Alexander Williams <[email protected]>
They were all missing prim_subreg_pkg in their dependencies.

Signed-off-by: Alexander Williams <[email protected]>
The adapter_sram core did not claim dependencies on prim_flop and
prim_sec_anchor, even though it uses them in its RTL.

Signed-off-by: Alexander Williams <[email protected]>
@a-will
Copy link
Contributor Author

a-will commented Dec 20, 2024

Well, I'd love for CI to not get broken from this, but I can't debug it, due to the closed-source repo that's required just to call the tool. The terms for ascentlint are antithetical to open-source hardware development, and I feel like we ought to just chuck it from the CI flow. (i.e. relegate it to sign-off of development stages only, and use the less problematic, if less complete tools for lint for PRs)

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