-
Notifications
You must be signed in to change notification settings - Fork 95
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
Contradictory requirements for dmcontrol.ndmreset=1 #944
Comments
This is clearly a contradiction that should be addressed. The left side is very old: 7 years. The right side was added in #594. I believe the intent of the left side was to make it clear that you shouldn't try to access non-debug-module stuff while ndmreset is asserted. E.g. don't execute any abstract commands. @JanMatCodasip's new language sounds good to me. We'll have to run this change by architecture review, so getting it in the spec will be slower than the previous process. |
@rtwfroody, could you please refer me to the description of the architecture review process. What can I do to help with the process? Thank you. |
I don't know of a description of the process after the original review happened. If you can make a PR, that would be helpful. @pdonahue-ventana and I will review it. Once we think it looks good, I'll email ARC and ask for their blessing. |
I'm OK with the proposed change. The RISC-V Lifecycle Guide describes the process. We are just passing the freeze milestone so we're entering the ratification-ready phase now. One of the requirements is "re-review of changes to the document" by the Architecture Review Committee. Rather than sending this to them, I think that we should buffer up all the changes made between now and the end of the 30 day public review period for a single ARC review. |
Thank you for checking this change and referring me to the description of the process. I have opened the pull request here: #951 Please feel free to buffer this change and send it for the ARC review in a batch - as you see fit. |
It appears that the spec contains contradictory requirements for the behavior of
dmcontrol.ndmreset
anddmstatus.ndmresetpending
bits.When
dmcontrol.ndmreset==1
the spec says:ndmreset
is asserted, read ofdmstatus
is undefined.ndmreset
is in progress,dmstatus.ndmresetpending
(if implemented) should be equal to 1, which suggests that read ofdmstatus
is a valid operation at that time (contradiction with the above).To remove the contradiction, we propose to change the left-hand statement to:
While ndmreset [..] is asserted, the only supported DM operations are reading/writing dmcontrol and reading dmstatus.ndmresetpending. The behavior of other accesses is undefined.
Would that be acceptable? If so, I can create the corresponding pull request.
Thank you.
The text was updated successfully, but these errors were encountered: