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

ROM cleanups and fixes #22398

Open
wants to merge 3 commits into
base: integrated_dev
Choose a base branch
from
Open

Conversation

sameo
Copy link
Contributor

@sameo sameo commented Apr 3, 2024

  • Remove the AST copy from the base ROM startup code
  • Remove one extra CFI counter increment
  • Typo fix for the ROM1 base address in tb.sv

@sameo sameo requested a review from a team as a code owner April 3, 2024 07:04
@sameo sameo requested review from jon-flatley and removed request for a team April 3, 2024 07:04
sameo added 3 commits April 12, 2024 15:11
Integrated has custom AST IP blocks and blindly copying the AST OTP
values in the AST memory mapped region may generate undefined behaviour.

Signed-off-by: Samuel Ortiz <[email protected]>
@sameo sameo force-pushed the topic/rom-cleanup branch from 5a9f023 to 0a5d9ad Compare April 12, 2024 13:12
@sameo sameo requested a review from a team as a code owner April 12, 2024 13:12
@sameo sameo requested review from hcallahan-lowrisc and removed request for a team April 12, 2024 13:12
@sameo sameo changed the title ROM cleanups ROM cleanups and fixes Apr 12, 2024
@sameo sameo requested a review from rswarbrick April 13, 2024 12:54
Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

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

This all seems reasonable, but I'm not sure I've got enough background info to review it properly.

For the second patch in particular ("Remove AST initialization"), do we have a document that explains what the ROM is supposed to be doing at this point? Presumably, something else has configured the AST already? Does any OT software or hardware have to do anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants