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

[hw,otp_ctrl,rtl] Provide a DFT config and response port #25897

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

Razer6
Copy link
Member

@Razer6 Razer6 commented Jan 16, 2025

This port can be overwritten by the integrator with their tech lib. This allows to place custom DFT and response signals. In the future, existing status control and monitor ports should be incorporated to the cfg/cfg_rsp ports.

Copy link
Contributor

@vogelpi vogelpi 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 good to me, thanks @Razer6 .

As you noted in an offline discussion, there are already the test_xyz signals for OTP. AFAIK, this are mostly used for the vendor test partition but I don't have any visibility into this so can't say for sure how this interplays with DFT. Adding a separate interface as you did now and inline with what we do for the SRAMs is very reasonable in my view. If really required, one could still combine the two interfaces later on.

@meisnere, @sha-ron , please note that you will have to add the same ports to your OTP macro wrapper.

hw/ip_templates/otp_ctrl/rtl/otp_ctrl.sv Outdated Show resolved Hide resolved
@@ -444,6 +444,21 @@ otp_size_as_uint32 = otp_size_as_bytes // 4
package: ""
desc: "AST observability bus."
}
// DFT CFG and RSP siognals
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: siognals -> signals

@vogelpi
Copy link
Contributor

vogelpi commented Jan 16, 2025

CHANGE AUTHORIZED: hw/ip/prim_generic/rtl/prim_generic_otp.sv
CHANGE AUTHORIZED: hw/ip/prim_generic/rtl/prim_otp_cfg_pkg.sv
CHANGE AUTHORIZED: hw/top_earlgrey/ip_autogen/otp_ctrl/rtl/otp_ctrl.sv
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

This PR adds a new interface to the OTP controller and the generic macro. Earlgrey functionality is not altered but partners will have to align their macro wrappers.

Copy link
Contributor

@a-will a-will left a comment

Choose a reason for hiding this comment

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

LGTM

Someday, I think it'd be good to evict the macros from the controllers, though. The dependency of a so-called "prim" on ipgen cores seems backwards, and the controller's prim interface's CSR definitions don't make sense to any but one integrator.

@a-will
Copy link
Contributor

a-will commented Jan 16, 2025

CHANGE AUTHORIZED: hw/ip/prim_generic/rtl/prim_generic_otp.sv
CHANGE AUTHORIZED: hw/ip/prim_generic/rtl/prim_otp_cfg_pkg.sv
CHANGE AUTHORIZED: hw/top_earlgrey/ip_autogen/otp_ctrl/rtl/otp_ctrl.sv
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

@Razer6
Copy link
Member Author

Razer6 commented Jan 17, 2025

@meisnere, @sha-ron , please note that you will have to add the same ports to your OTP macro wrapper.

Similar changes where added to the SRAMs.

@Razer6
Copy link
Member Author

Razer6 commented Jan 17, 2025

Someday, I think it'd be good to evict the macros from the controllers, though. The dependency of a so-called "prim" on ipgen cores seems backwards, and the controller's prim interface's CSR definitions don't make sense to any but one integrator.

Yes, this definitively comes from a first product and requires some generalization. But an adventure for a different time :)

This port can be overwritten by the integrator with their tech lib.
This allows to place custom DFT and response signals. In the future,
existing status control and monitor ports should be incorporated to the
cfg/cfg_rsp ports.

Signed-off-by: Robert Schilling <[email protected]>
@vogelpi vogelpi requested review from sha-ron and meisnere January 17, 2025 15:09
@vogelpi vogelpi merged commit 5d16cb2 into lowRISC:master Jan 17, 2025
38 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