-
Notifications
You must be signed in to change notification settings - Fork 78
Move mandatory privilege mode option from CRD family to CRD #182
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
Move mandatory privilege mode option from CRD family to CRD #182
Conversation
| family: MockCRDFamily | ||
|
|
||
| mandatory_priv_modes: | ||
| - M,U |
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.
Assuming you want this to be an array, this should be:
mandatory_priv_modes:
- M
- Uor
mandatory_priv_modes: [M,U]What you've written is an array with one string:
["M,U"]| * Red fields are defined by the RISC-V ISA but not present | ||
| * [green]#Green fields# are present in the associated CRD. | ||
| * [red]#Red fields# are defined by the RISC-V ISA but not required in the associated CRD. | ||
| * [silver]#Grey fields# are reserved for future use by RISC-V (AKA WPRI). Software should ignore the values read from |
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.
Nice
|
|
||
| # @return [true/false] | ||
| def m_mode? | ||
| mandatory_priv_modes.include?('M') |
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.
Just a note: you don't need to change this. You lucked out that String and Array both have an include? method.
|
Not to be a pain, but my preference is for commit messages to have subject lines that reflect what is being changed, rather than "Fixes #181". So, something like "Move mandatory privilege mode option from CRD family to CRD" (if I understand the changes correctly). It makes the |
…ttps://github.com/riscv-software-src/riscv-unified-db into :181-move-priv-modes-present-from-crd-family-to-crd
|
Premature PR |
Fix #181