Sdtrig: Clarify that tinfo.version is a global constant, not per-trigger #1081
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
en-sc
approved these changes
Nov 8, 2024
xml/hwbp_registers.xml
Outdated
| @@ -176,7 +177,9 @@ same project unless stated otherwise. | |||
|
|
|||
| <field name="0" bits="XLEN-1:32" access="R" reset="0" /> | |||
| <field name="version" bits="31:24" access="R" reset="Preset"> | |||
| Contains the version of the Sdtrig extension implemented. | |||
| Contains the version of the Sdtrig extension implemented. The read-only | |||
| value of this field is refers to the implemented version of the whole | |||
There was a problem hiding this comment.
Suggested change
| value of this field is refers to the implemented version of the whole | |
| value of this field refers to the implemented version of the whole |
There was a problem hiding this comment.
Thanks, fixed.
|
The confusion is that "the version of the Sdtrig extension implemented" might change dynamically because part of the trigger module might support 0.13 and another part supports 1.0? |
The purpose of "tinfo.version" field is apparently to describe the version of the whole Sdtrig implementation. That is, it is a constant unaffected by the current value of tselect CSR. Judging form the original commit that introduced it (0374fd6) and from the related mailing list discussion (https://lists.riscv.org/g/tech-debug/topic/96888390). However, the sentences `This register provides access to the trigger selected by tselect. The reset values listed here apply to every underlying trigger.` leave certain room for interpretation that "tinfo.version" is a per-trigger constant, not a global one, and could therefore change depending on "tselect". Remove this ambiguity by rewording the description of "tinfo" CSR and "tinfo.version" field.
JanMatCodasip
force-pushed
the
clarify-tinfo-version
branch
from
c56c4f7 to
9d0e6e0
Compare
|
Yes. The sentence |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The purpose of "tinfo.version" field is apparently to describe the version of the whole Sdtrig implementation. That is, it is a constant unaffected by the current value of tselect CSR. Judging form the original commit that introduced it (0374fd6) and from the related mailing list discussion
(https://lists.riscv.org/g/tech-debug/topic/96888390).
However, the sentences
This register provides access to the trigger selected by tselect.The reset values listed here apply to every underlying trigger.leave certain room for interpretation that "tinfo.version" is a per-trigger constant, not a global one, and could therefore change depending on "tselect".
Remove this ambiguity by rewording the description of "tinfo" CSR and "tinfo.version" field.