-
Notifications
You must be signed in to change notification settings - Fork 807
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
[otp_ctrl,doc] Fix some links to otp_ctrl documentation #26070
base: master
Are you sure you want to change the base?
Conversation
I suggest you make this change in otp_ctrl_img-c-tpl: As for the synthesis tcl script, it may only support earlgrey? If so, change the path from If the script knew the topname it may be able to generate the right path. |
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 Rupert. These look good. I have a couple suggestions about the missing paths you mention.
Also, I think it may be best to be able to refer to the html generated from the markup since it provides more anchors. This may be possible when the docs contain all tops (or do they already?). Perhaps we should add an issue to replace reference to md files to references to the generated html if and when docs are multi-top?
Please at least make the change in otp_ctrl_img.c.tpl, otherwise LGTM
These were broken when the block became templated. Signed-off-by: Rupert Swarbrick <[email protected]>
d013ae0
to
e8b021d
Compare
CHANGE AUTHORIZED: hw/ip/sram_ctrl/rtl/sram_ctrl.sv The changes to these files lie in comments in each case, so cause no risk to the design. |
@matutem: I think the docs already contain all tops. I'm not quite sure where this replacement would be relevant though. Can you point to an example? |
@rswarbrick my relevant comment is #26070 (comment) Regarding the synthesis tcl file, we can make it either top_earlgrey-specific, or consult if the flow can take a topname parameter in case it actually makes sense. Another option is to move that script to hw/top_earlgrey/syn. This probably falls outside the scope of this PR, so could you file an issue for it? In any case, I insist this LGTM :) |
These were broken when the block became templated. There are still a couple of hits for "ip/otp_ctrl" as an absolute path in the repository:
hw/syn/tools/dc/testsynth.tcl
(I don't have a DC licence and I'm not sure how this gets run, so can't really fix it)
util/design/data/otp_ctrl_img.c.tpl
This doesn't look quite right, but the generation is tied in with the Bazel SW build. I'm not entirely sure how the link should work.
@matutem: Would you mind glancing through this and checking that the renames (and explanatory comments) match how you think it should work?
@engdoreis: As someone with much more knowledge of the SW world, do you know how the
otp_ctrl_img.c.tpl
file gets used? Does it make more sense for the comment to point at the otp_ctrl template, or should this be some relative path?