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,flash_ctrl] Create ipgen files #21170

Merged
merged 8 commits into from
Feb 8, 2024
Merged

Conversation

matutem
Copy link
Contributor

@matutem matutem commented Feb 2, 2024

Create the flash_ctrl generated files using ipgen, but keep using the legacy flow for now. This is so the full transition is broken into two stages for ease of reviewing. A subsequent change will complete the transition to ipgen and remove the legacy files.

Just like previous IP transitions to ipgen, most of legacy files are simply copied to hw/ip_templates/flash_ctrl, but the following changes are noteworthy:

  • The third commit moves .tpl files to the locations needed for ipgen, and wipes out from ip_templates the files that are generated so the tree will no longer have these useless and confusing files.
  • The fourth commit makes additional files into templates since their generated text contains the name of top.
  • The fifth commit is needed since ipgen uses plain dictionaries, whereas the old flow uses some class objects.
  • The sixth commit fixes core files that build templated files to use instance_vlnv and declare a virtual target, and changes other core files to depend on their virtual name instead.
  • The seventh commit makes azure avoid building under the ip_templates tree since build should only happen in the target generated trees.

@matutem matutem force-pushed the ipgen_flash_ctrl branch 8 times, most recently from 398a6f2 to 9b4b52d Compare February 2, 2024 23:39
@matutem matutem changed the title Ipgen flash ctrl [ipgen,flash_ctrl] Create ipgen files Feb 3, 2024
@matutem matutem marked this pull request as ready for review February 3, 2024 12:50
@matutem matutem force-pushed the ipgen_flash_ctrl branch 3 times, most recently from cd4aaf7 to 1ae5151 Compare February 5, 2024 22:50
Copy link
Contributor

@msfschaffner msfschaffner left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @matutem. Do I understand correctly that the build still uses the old files at the moment, and you are basically going to change the fusesoc dependency to the new ipgen'd files in a follow up PR?

Does it make sense to diff the RTL files (and potentially DV files) to make sure they remained identical during this operation?

hw/ip_templates/flash_ctrl/rtl/flash_ctrl.sv Outdated Show resolved Hide resolved
@matutem
Copy link
Contributor Author

matutem commented Feb 6, 2024

@msf, below are the diffs of the tpl files. Notice flash_ctrl_pkg.sv has an edit for the sake of simplicity: we use already declared variables instead of recreating them from constants.

There are many required differences in the .md and some in .hjson files, since they contain many paths that need adjustment due to the change in hierarchy. The handful of other differences is in comments.

diff hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl_pkg.sv hw/top_earlgrey/ip_autogen/flash_ctrl/rtl/flash_ctrl_pkg.sv 
5,11c5
< // ------------------- 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:
< // Copyright lowRISC contributors.
< // Licensed under the Apache License, Version 2.0, see LICENSE for details.
< // SPDX-License-Identifier: Apache-2.0
< //
< // Flash Controller module.
---
> // Flash Controller package.
31,33c25,27
<     10,
<     1,
<     2
---
>     flash_ctrl_reg_pkg::NumInfos0,
>     flash_ctrl_reg_pkg::NumInfos1,
>     flash_ctrl_reg_pkg::NumInfos2
35,39c29
<   parameter int InfosPerBank    = max_info_pages('{
<     10,
<     1,
<     2
<   });
---
>   parameter int InfosPerBank    = max_info_pages(InfoTypeSize);
78,82d67
<   //parameter logic [PageW-1:0] InfoPartitionEndAddr [InfoTypes] = '{
<   //  9,
<   //  0,
<   //  1
<   //};
diff hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl.sv hw/top_earlgrey/ip_autogen/flash_ctrl/rtl/flash_ctrl.sv 
5,10d4
< // ------------------- 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:
< // Copyright lowRISC contributors.
< // Licensed under the Apache License, Version 2.0, see LICENSE for details.
< // SPDX-License-Identifier: Apache-2.0
< //
diff hw/top_earlgrey/ip/flash_ctrl/data/autogen/flash_ctrl.hjson hw/top_earlgrey/ip_autogen/flash_ctrl/data/flash_ctrl.hjson 
4,9d3
< //
< // ------------------- 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:
< // Copyright lowRISC contributors.
< // Licensed under the Apache License, Version 2.0, see LICENSE for details.
< // SPDX-License-Identifier: Apache-2.0
diff hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl_region_cfg.sv hw/top_earlgrey/ip_autogen/flash_ctrl/rtl/flash_ctrl_region_cfg.sv 
5,10d4
< // ------------------- 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:
< // Copyright lowRISC contributors.
< // Licensed under the Apache License, Version 2.0, see LICENSE for details.
< // SPDX-License-Identifier: Apache-2.0
< //

@msfschaffner
Copy link
Contributor

Thanks for the diff - this all looks reasonable, thanks!

@msfschaffner msfschaffner self-requested a review February 6, 2024 19:21
@matutem matutem force-pushed the ipgen_flash_ctrl branch 4 times, most recently from 36e78b7 to 4e5f761 Compare February 6, 2024 23:36
Populate the default for object parameters with representative data.
Ideally these should have an associated validator function.

Part of rstmgr lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
This is a straight copy with no changes to the files.
The obsolete files under hw/ip/flash_ctrl/util are not copied.

Part of rstmgr lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
In ipgen template files are at the corresponding directory where
their generated file will reside.

There is no use for files in ip_templates that will end up being
generated.

The sec_cm_testplan should not be in ip_template or it will overwrite
the manually updated top-specific sec_cm_testplan.

Part of lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
The files are almost identical to the previous ones, except they use
${topname} instead of a specific name and consecutive '#' are escaped
so they are not considered md comments. This escape replaces things like
`###` by `${"###"}`.

Adjust some relative paths to account for the change in the file locations.

Use {self_dir} to simplify some paths: it points to the location of the hjson
file used to process the contents of a given file.

Part of rstmgr lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
The template core files use instance_vlnv, and declare a virtual target.
Change other core files depending on the template cores to depend on the
corresponding virtual target instead.

Part of rstmgr lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
The custom template generation used class objects, ipgen uses dictionaries
instead.
Make minor changes in the templates causing no functional change.

Part of rstmgr lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
Template files are only intended for code generation, so it makes
no sense to use them to build targets.

- Add flash_ctrl in hw/ip_templates/BUILD all_files
- Add exclusion for flash_ctrl_regs in azure-pipelines.yml

Part of lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
The older way to generate files is preserved.
Remove unused total_bytes field in validate Flash class.
Add FUSESOC_IGNORE in hw/ip_templates/flash_ctrl: it can be
removed once the flash_ctrl change to ipgen is complete.

Part of lowRISC#8440

Signed-off-by: Guillermo Maturana <[email protected]>
@matutem matutem merged commit c735777 into lowRISC:master Feb 8, 2024
32 checks passed
@matutem matutem deleted the ipgen_flash_ctrl branch February 8, 2024 18:44
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