Skip to content

Conversation

@arturum1
Copy link
Contributor

@arturum1 arturum1 commented Dec 19, 2025

'top_module' py2hwsw python parameter.

Update iob_csrs module to only generate CSRs docs if the issuer is the
top module.
Otherwise, always generating CSRs docs would cause conflicts when a
project has multiple cores with CSRs. (Example: iob_eth and its
testbench peripherals).

Add new standard 'top_module' python parameter that gets passed to every
core by py2hwsw automatically.
This allows iob_csrs module to know if the issuer is the top module.
Add new 'CONFIDENTIAL' variable to document/Makefile, by default set to
1 (true).

Allows us to use CONFIDENTIAL=0 in the doc_build.mk file of open-source
cores to generate docs without the confidential tag.
Copy link
Contributor

Copilot AI left a 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 PR implements three improvements to the py2hwsw framework:

Summary: Enhances documentation generation by restricting CSR documentation to top-level modules, makes the confidential tag optional in generated documents, and fixes LaTeX backslash character escaping.

Key Changes:

  • Added top_module parameter to track the top-level module and filter CSR documentation generation to only the top module
  • Made the "Confidential" footer tag conditional based on a CONFIDENTIAL makefile variable
  • Fixed LaTeX backslash escaping to use \textbackslash instead of \\ for proper text mode rendering

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
py2hwsw/scripts/latex.py Fixes backslash escaping in LaTeX by replacing \\ with \textbackslash before escaping other special characters
py2hwsw/scripts/iob_core.py Adds top_module parameter to py_params_dict and filters it from parent parameters to prevent propagation
py2hwsw/lib/hardware/iob_csrs/iob_csrs.py Restricts CSR documentation generation to only run when the issuer is the top module
py2hwsw/document/tsrc/ug.cls Updates footer to conditionally include "Confidential" text based on \CONFIDENTIAL macro definition
py2hwsw/document/Makefile Adds CONFIDENTIAL variable (default=1) and conditionally defines the LaTeX macro in ugtop.tex

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +375 to +377
if (
py_params_dict.get("py2hwsw_target", "") == "setup"
and py_params_dict["top_module"] == py_params_dict["issuer"]["original_name"]
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

This code will raise a TypeError when issuer is None because on line 931 of iob_core.py, issuer is set to an empty string "" when it's None. Attempting to access py_params_dict["issuer"]["original_name"] when issuer is "" will fail. The condition should check if issuer is a dictionary before accessing its keys, or should use py_params_dict.get("issuer", {}).get("original_name") to safely handle the case when issuer is an empty string.

Suggested change
if (
py_params_dict.get("py2hwsw_target", "") == "setup"
and py_params_dict["top_module"] == py_params_dict["issuer"]["original_name"]
issuer = py_params_dict.get("issuer")
issuer_original_name = issuer.get("original_name") if isinstance(issuer, dict) else None
if (
py_params_dict.get("py2hwsw_target", "") == "setup"
and py_params_dict.get("top_module") == issuer_original_name

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in cb10681 by asserting that issuer is defined.

@jjts jjts merged commit 024d4cd into IObundle:main Dec 22, 2025
2 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