-
Notifications
You must be signed in to change notification settings - Fork 877
[hw,entropy_src,rtl] Make noise source data width parametrizable #27429
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
Conversation
4631e50
to
0dfbe02
Compare
79fcd13
to
0af7442
Compare
209e06f
to
7dc9e34
Compare
@vogelpi @andreaskurth Thanks for the review. I added your comments and also implemented a single counter for the bucket test. CI is also almost happy. Could you take another look? Regarding the version bump, I leave that as it is now. It is similar to other IP changes, so nothing critical I think. |
bbc1b2b
to
530ede4
Compare
The full regression suite of entropy_src has been run. Their pass rate is now similar to a full run on the master branh. |
1bf8620
to
63b9550
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments, but LGTM after those. Thx @Razer6!
The version should be incremented before the changes. Otherwise, the changes would technically still be part of the prior version. Could you please make the version bump commit the first commit in this PR? |
63b9550
to
472ef5b
Compare
Thanks @andreaskurth I added your requests and moved the version number to the beginning of the series of commits. Let's get that merged :) |
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
There are 2**N counters necessary for this test, which does not scale with the parametrizable RNG bit width. Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
472ef5b
to
cf475f6
Compare
There was a problem hiding this 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, except for the version increment which needs to be moved to the correct IP core (ENTROPY_SRC) :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Razer6 !
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
cf475f6
to
4eba208
Compare
CHANGE AUTHORIZED: hw/ip/entropy_src/data/entropy_src.hjson ^ Changes to add support for configurable entropy bit widths, which lead to a bump of the major version (now 3.0.0) of the IP. CHANGE AUTHORIZED: hw/top_earlgrey/data/top_earlgrey.hjson ^ No functional changes; structural changes to connect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Razer6!
CHANGE AUTHORIZED: hw/ip/entropy_src/data/entropy_src.hjson ^ Changes to add support for configurable entropy bit widths, which lead to a bump of the major version (now 3.0.0) of the IP. CHANGE AUTHORIZED: hw/top_earlgrey/data/top_earlgrey.hjson ^ No functional changes; structural changes to connect entropy_src ports that have been split. |
This PR makes the RNG data path from the entropy source to the external noise source (AST) parametrizable in the bit width. To do so, the struct defining the interface is removed and explicit signals for each field are added to allow the parametrization of the data width. The same is done for the external health test. Here only data and valid are moved out of the struct.