Skip to content

Conversation

@Razer6
Copy link
Member

@Razer6 Razer6 commented Apr 22, 2025

Instead of using default values from a package, create a top-level parameter to define these and pass them down. This allows integrators to specify them on a per-instance basis.

@Razer6 Razer6 force-pushed the ibex-mvendorid-mimpid-param branch from 6f9651b to af2f400 Compare April 22, 2025 18:21
Copy link

@davidschrammel davidschrammel left a comment

Choose a reason for hiding this comment

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

LGTM

@SamuelRiedel
Copy link
Contributor

This looks good to me, thanks.

I looked into the CI failure and can't see how it relates to your changes, so I assume it’s a quirk of our current (older) Verilator version. I created a PR addressing this failure here: #2270. If that gets merged, CI should be fixed and this PR should be good to go, IMO. If we decide not to implement the fix, we could disable the linter on the specific line where Verilator is currently complaining.

@Razer6 Razer6 force-pushed the ibex-mvendorid-mimpid-param branch from af2f400 to 7c85b9f Compare April 25, 2025 11:50
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.

Two small comments, otherwise LGTM

@Razer6 Razer6 force-pushed the ibex-mvendorid-mimpid-param branch from 7c85b9f to cbade62 Compare April 25, 2025 12:50
Instead of using default values from a package, create a top-level
parameter to define these and pass them down. This allows integrators
to specify them on a per-instance basis.

Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
@Razer6 Razer6 force-pushed the ibex-mvendorid-mimpid-param branch from cbade62 to 66ca376 Compare April 25, 2025 13:08
@andreaskurth andreaskurth added this pull request to the merge queue Apr 25, 2025
Merged via the queue into lowRISC:master with commit 0369438 Apr 25, 2025
11 checks passed
@Razer6 Razer6 deleted the ibex-mvendorid-mimpid-param branch April 25, 2025 13:51
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.

4 participants