-
Notifications
You must be signed in to change notification settings - Fork 87
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
[general] added cke_i to iob_soc #592
[general] added cke_i to iob_soc #592
Conversation
.clk(clk_i), | ||
.rst(arst_i), |
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.
These lines are used to generate the interconnect instance in every wrapper that needs one.
Therefore, the signals generated must have a common name to all wrappers.
Currently, the iob_soc_sim_wrapper.v
and the iob_soc_fpga_wrapper.v
of the CYCLONEV are the only wrappers that use this interconnect, and they contain the common wires named clk
and rst
.
They do NOT contain the arst_i
signal, therefore trying to generate an interconnect with this signal will give an error (that is why those checks are failing).
I suggest keeping the common clk
and rst
signals.
.clk(clk_i), | |
.rst(arst_i), | |
.clk(clk), | |
.rst(rst), |
Another solution would be to make sure the clk_i
and arst_i
signals exist in the wrappers.
Rst cannot connect to arst
They have a different meaning
Use arst for async reset (posedge)
Use rst for sync reset (no posedge)
…On Fri, Aug 25, 2023, 19:57 arturum1 ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In scripts/iob_soc_create_wrapper_files.py
<#592 (comment)>:
> + .clk(clk_i),
+ .rst(arst_i),
These lines are used to generate the interconnect instance in every
wrapper that needs one.
Therefore, the signals generated must have a common name to all wrappers.
Currently, the iob_soc_sim_wrapper.v and the iob_soc_fpga_wrapper.v of
the CYCLONEV are the only wrappers that use this interconnect, and they
contain the common wires signals clk and rst.
They do NOT contain the arst_i signal, therefore trying to generate an
interconnect with this signal will give an error (that is why those checks
are failing).
I suggest keeping the common clk and rst signals.
⬇️ Suggested change
- .clk(clk_i),
- .rst(arst_i),
+ .clk(clk),
+ .rst(rst),
Another solution would be to make sure the clk_i and arst_i signals exist
in the wrappers.
—
Reply to this email directly, view it on GitHub
<#592 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABLUHO7UCGDJQ2RVIOUZTULXXDYR7ANCNFSM6AAAAAA36UP63E>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Checks fail , can't merge |
the author has no time to fix this - closing |
No description provided.