Skip to content

Conversation

@Razer6
Copy link
Member

@Razer6 Razer6 commented Oct 20, 2025

This PR adds a new template to automatically render the address map of a top to a dedicated file. The template will for all address spaces render the IP regions and the memory blocks into separate tables.

@adurbin-rivos This is what we couldn't find at some point in the documentation.

Signed-off-by: Robert Schilling <[email protected]>
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.

A couple of nitty comments, but I really like this.

%>
${mem_table_str}
% endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but I'd be inclined to drop this newline.

% for addr_space in top['addr_spaces']:
${"##"} ${addr_space['name'].capitalize()} Address Space

% if 'desc' in addr_space:
Copy link
Contributor

Choose a reason for hiding this comment

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

I always find these files quite difficult to read ("Give me more curly braces!!"). Sometimes, I've seen things that be slightly easier if they indent the stuff inside. So this line would be % if ... (gaining 2 spaces because it's inside the for loop). Definitely not a requirement, but I thought I'd suggest it.

@pamaury
Copy link
Contributor

pamaury commented Oct 20, 2025

We already have an automatically generated memory map here: https://opentitan.org/book/hw/top_earlgrey/doc/design/index.html?highlight=address%20map#memory-map
If you look at the source, you'll see that it invokes a tool:
https://github.com/lowRISC/opentitan/blob/master/hw/top_earlgrey/doc/design/README.md?plain=1#L390

The output is a bit different I guess but maybe it's better to modify the existing tool to make the register better, instead of creating a new one?

@Razer6
Copy link
Member Author

Razer6 commented Oct 20, 2025

Oh we have? We need to make that more prominent. Let me change that.

@pamaury
Copy link
Contributor

pamaury commented Oct 20, 2025

Yes I agree it's a bit hidden at the moment, and also I prefer your new memory map to the old one so feel free to update the old python script and put the memory map somewhere else ;)

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