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

[topgen] Only create pwrmgr/pinmux/alert_handler if there is an instance #25432

Merged
merged 7 commits into from
Nov 29, 2024

Conversation

Razer6
Copy link
Member

@Razer6 Razer6 commented Nov 27, 2024

Not all tops may have a pwrmgr, pinmux, alert_handler or clkmgr. Only create those IPs and the related connections if the top really specifies those IPs.

@Razer6 Razer6 requested review from matutem and andreaskurth and removed request for msfschaffner November 27, 2024 16:57
@Razer6 Razer6 force-pushed the topgen-pwrmgr branch 4 times, most recently from 4c6fde5 to e7b74af Compare November 27, 2024 19:28
@Razer6 Razer6 changed the title [topgen] Only create pwrmgr if there is an instance [topgen] Only create pwrmgr and pinmux if there is an instance Nov 27, 2024
@Razer6 Razer6 force-pushed the topgen-pwrmgr branch 4 times, most recently from 89acca7 to bb8af72 Compare November 27, 2024 20:27
util/topgen.py Outdated Show resolved Hide resolved
@Razer6 Razer6 changed the title [topgen] Only create pwrmgr and pinmux if there is an instance [topgen] Only create pwrmgr/pinmux/alert_handler if there is an instance Nov 29, 2024
Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

LGTM in the sense that it doesn't negatively impact currently generated tops. Whether the result is correct for tops that make use of the added functionality I cannot say, though

clkmgrs = [m for m in top['module'] if m['type'] == 'clkmgr']
rstmgrs = [m for m in top['module'] if m['type'] == 'rstmgr']

if len(pwrmgrs) == 1 * len(clkmgrs) == 1 * len(rstmgrs) != 1:
Copy link
Member Author

Choose a reason for hiding this comment

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

This check was broken as it only checked the rstmgr but not the others due to operator precedence.

@Razer6 Razer6 force-pushed the topgen-pwrmgr branch 4 times, most recently from cdb05f4 to 7613b07 Compare November 29, 2024 13:55
@Razer6 Razer6 requested a review from a team as a code owner November 29, 2024 13:55
@Razer6 Razer6 requested review from engdoreis and removed request for a team November 29, 2024 13:55
@Razer6
Copy link
Member Author

Razer6 commented Nov 29, 2024

I used it downstream, and it works. The used top-level template though needs some tweaks, but I leave that as a downstream issue.

One thing I needed to change was the order of the alert and plic mappings in toplevel.c.tpl. The alert mappings were at the end of that file, and gating it with an if caused mako to render two newlines. One for the mapping and one for the endif. This then caused a linting issue. The easy fix was to put the un-gated plic mapping at the end.

I had to add a special case for Englishbreakfast. Although this top does not have an alert handler in the design it uses SW from it. It copies its own alert handler config over to Earlgrey and then build SW from that. Here, In this PR we ensure that the alert handler is generated for Englishbreakfast even though the design does not have an actual alert handler.

@Razer6 Razer6 force-pushed the topgen-pwrmgr branch 2 times, most recently from a980cff to bbffa89 Compare November 29, 2024 15:15
@andreaskurth andreaskurth merged commit f41f057 into lowRISC:master Nov 29, 2024
18 of 37 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