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

[ipgen,pwrmgr] Implement fuseoc instance_vlnv and virtual dependencies #20883

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

matutem
Copy link
Contributor

@matutem matutem commented Jan 18, 2024

Core files that provide generated files use the fusesoc $instance_vlnv mechanism and declare a virtual name. This was overlooked in #19801.

This provides two commits:

  • The first is a minor cleanup in topgen which now implements a convention to call topname things like earlgrey and top_name the corresponding top_earlgrey.
  • The second is the more substantial change, also changing any core that depend on such virtual cores to use the virtual name.

@matutem matutem requested a review from a-will January 18, 2024 19:18
@matutem matutem force-pushed the autogen_vlnv branch 3 times, most recently from e348e80 to 02327a8 Compare January 20, 2024 00:50
@matutem matutem marked this pull request as ready for review January 20, 2024 17:21
@matutem matutem requested review from a team and msfschaffner as code owners January 20, 2024 17:21
@matutem matutem requested review from hcallahan-lowrisc and removed request for a team January 20, 2024 17:21
This pair is uniformly added to all ipgen modules.
Change topgen.py to adopt the convention that, for example, the variable
topname is just earlgrey for example, while top_name is top_earlgrey.

Signed-off-by: Guillermo Maturana <[email protected]>
@matutem matutem changed the title Autogen vlnv [ipgen,pwrmgr] Implement fuseoc instance_vlnv and virtual dependencies Jan 20, 2024
@@ -39,7 +39,7 @@ filesets:
- lowrisc:top_englishbreakfast:xbar_main
- lowrisc:top_englishbreakfast:xbar_peri
- lowrisc:ip:rstmgr
- lowrisc:ip:pwrmgr
- lowrisc:ip_interfaces:pwrmgr
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be the particular implementing core, not the virtual VLNV. It'd probably look better as lowrisc:englishbreakfast:pwrmgr, but I think our current instance_vlnv naming scheme puts it like below, right?

Suggested change
- lowrisc:ip_interfaces:pwrmgr
- lowrisc:opentitan:top_englishbreakfast_pwrmgr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks! Fixed.

@matutem matutem requested a review from a team as a code owner January 22, 2024 22:38
@matutem matutem requested review from engdoreis and removed request for a team January 22, 2024 22:38
@@ -4,8 +4,8 @@
#
# ------------------- W A R N I N G: A U T O - G E N E R A T E D C O D E !! -------------------#
# PLEASE DO NOT HAND-EDIT THIS FILE. IT HAS BEEN AUTO-GENERATED WITH THE FOLLOWING COMMAND:
# util/topgen.py -t hw/top_earlgrey/data/top_earlgrey.hjson
# -o hw/top_earlgrey
# util/topgen.py -t hw/top_englishbreakfast/data/top_englishbreakfast.hjson
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, need to redo topgen for earlgrey again :)

The core files affected are the pwrmgr cores that are templetized.
All other core files referring to the the files above need to depend
on the virtual name instead.
Remove the redundant files for which there is a corresponding template
under ip_templates.

Signed-off-by: Guillermo Maturana <[email protected]>
@matutem matutem merged commit ce648ca into lowRISC:master Jan 23, 2024
31 of 32 checks passed
@matutem matutem deleted the autogen_vlnv branch January 23, 2024 01:35
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.

2 participants