-
Notifications
You must be signed in to change notification settings - Fork 39
Support multiple front-end interface types; Improve User Guide. #327
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
Conversation
Use iob_universal_converter module to add support for other frontend interfaces. Internally, keep using IOb interface. Currently supported frontend interfaces: "IOb", "AXIL", "Wishbone", "APB". The "AXI4" interface was not added due to inefficiency when converting to internal IOb interface (burst transfers are not supported).
Py2HWSW automatically escapes special LaTeX characters, so there is no point in having them in trying to use them in the description lines. Also small fixes in figures.
|
Front-end does not need to support AXI, as nothing will send/receive data in bursts into/from the cache. Use AXI only for memory interfaces and controllers. So only the backend needs AXI |
|
You may also drop APB, as the cache is not peripheral; APB stands for AMBA Peripheral Bus. And to avoid time waste, yo may also drop Wishbone as IObundle is not using it. |
I agree. I have included AXI in the documentation because the iob_universal_converter module already brings support for it. |
These interfaces are not currently desired for the front-end bus. They will likely still work if a user chooses them, but they will be undocumented.
Macros are no longer genereated in <corename>_csrs.h file. Now, they are directly generated in <corename>_csrs_conf.h file (and <corename>_csrs_conf.vh). The <corename>_csrs.vh file is also no longer required. This change was implemented in py2hwsw commit 30ea7f7b7c0da2e19cb58279316c1ba4b58cc59f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for multiple front-end interface types (IOb, AXI-Lite, Wishbone) to the IOb-Cache core by introducing a universal converter module. It also improves documentation with updated user guide content, corrected LaTeX formatting, and enables Doxygen for bare-metal drivers.
Key changes:
- Added configurable front-end interface parameter (
FE_IF) with support for IOb, AXI-Lite, and Wishbone interfaces - Integrated
iob_universal_convertermodule to convert between interface types - Enhanced documentation with detailed simulation architecture, clearer terminology (IOb vs NPI), and improved feature descriptions
Reviewed changes
Copilot reviewed 13 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| iob_cache.py | Added FE_IF parameter, integrated universal converter, updated interface naming and descriptions |
| hardware/simulation/iob_cache_sim_wrapper/iob_cache_sim_wrapper.py | Added FE_IF parameter support and universal converter to simulation wrapper |
| hardware/modules/iob_cache_control/hardware/iob/iob_cache_control_iob.v | Code formatting improvements and removed unused include |
| hardware/modules/iob_cache_control/hardware/axi/iob_cache_control_axi.v | Code formatting improvements and removed unused include |
| document/ug.pdf.license | Added SPDX license header for generated PDF |
| document/tsrc/sim_desc.tex | Enhanced simulation description with testbench architecture details |
| document/tsrc/intro.tex | Updated terminology from "NPI" to "IOb" interface |
| document/tsrc/inst_desc.tex | Clarified interface terminology and support for multiple front-end types |
| document/tsrc/features.tex | Updated features list to reflect multiple interface support |
| document/tsrc/deliverables.tex | Expanded deliverables list with more detailed descriptions |
| document/figures/*.odg | Added/updated block diagram figures (binary files) |
| document/doc_build.mk | Added documentation build configuration file |
| default.nix | Updated py2hwsw dependency to newer commit |
| Makefile | Added FE_IF parameter to build system |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -6,7 +6,6 @@ | |||
|
|
|||
| `include "iob_cache_control_conf.vh" | |||
| `include "iob_cache_iob_csrs_conf.vh" | |||
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An include file has been removed that may still be needed. The removed line include "iob_cache_iob_csrs.vh" could contain macro definitions or declarations that are used elsewhere in the module. Verify that all macros and definitions from this file are either unused or available through other includes.
| `include "iob_cache_iob_csrs_conf.vh" | |
| `include "iob_cache_iob_csrs_conf.vh" | |
| `include "iob_cache_iob_csrs.vh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As explained in commit 1c8e980, the <corename>_csrs.vh files are outdated and no longer generated/used.
The CSRs macros are now directly generated in the <corename>_csrs_conf.vh file.
| @@ -6,7 +6,6 @@ | |||
|
|
|||
| `include "iob_cache_control_conf.vh" | |||
| `include "iob_cache_axi_csrs_conf.vh" | |||
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An include file has been removed that may still be needed. The removed line include "iob_cache_axi_csrs.vh" could contain macro definitions or declarations that are used elsewhere in the module. Verify that all macros and definitions from this file are either unused or available through other includes.
| `include "iob_cache_axi_csrs_conf.vh" | |
| `include "iob_cache_axi_csrs_conf.vh" | |
| `include "iob_cache_axi_csrs.vh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As explained in commit 1c8e980, the <corename>_csrs.vh files are outdated and no longer generated/used.
The CSRs macros are now directly generated in the <corename>_csrs_conf.vh file.
| params = { | ||
| # Confs passed by issuer (iob_cache) | ||
| "cache_confs": [], | ||
| "fe_if": "iob", |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter name 'fe_if' uses inconsistent naming convention. Throughout the codebase, interface type parameters use uppercase (e.g., 'BE_IF', 'FE_IF'), but the parameter value passed to subblocks uses lowercase. Consider using uppercase 'FE_IF' consistently or document the naming convention clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We typically write all python parameters in lower case (as in the shown example).
The internal variable 'FE_IF' in iob_cache.py is in uppercase to show that it is a constant - only defined once at the top of the setup() function, and never modified.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
P-Miranda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some small sugestions to fix a typo and add information about new supported front-end interfaces
Currently supported front-end interfaces: "IOb", "AXI-Lite", "Wishbone".