Skip to content
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

Gearbox.vhd: changed the type of the generic parameter RST_POLARITY_G from sl to boolean #1118

Closed
wants to merge 1 commit into from

Conversation

king-pietro
Copy link
Contributor

Gearbox.vhd: changed the type of the generic parameter RST_POLARITY_G from sl to boolean

Description

The type of generic parameter RST_POLARITY_G in Gearbox.vhd has been changed from sl to boolean.
RST_POLARITY_G now controls a new constant sl called RST_POLARITY_C, which acts exactly as the generic acted previously.

Details

When using a Verilog wrapper around the gearbox, if you tried to set the RST_POLARITY_G generic to either '0' or '1', the simulation would give you a wrong result as its not possible to use sl as parameters in verilog: https://stackoverflow.com/questions/32218950/instantiate-vhdl-in-verilog-with-generics-containing-std-logic
Changing its type to boolean fixes the problem

@king-pietro king-pietro requested a review from ruck314 October 11, 2023 00:38
@king-pietro
Copy link
Contributor Author

king-pietro commented Oct 11, 2023

@ruck314 another "bug" I saw while changing the reset is that:
If the gearbox is in reset state, but I still give clk and slaveValid, the signal slaveReady is moving nonetheless (while the other signals are reset as they should).
This is due to the fact that in the FSM, slaveReady is asserted before the reset of v.

slaveReady <= v.slaveReady;
if (RST_ASYNC_G = false and rst = RST_POLARITY_G) then
v := REG_INIT_C;
end if;
rin <= v;

I don't know if it was done on purpose or not. If it's a bug the bug-fix is to just move down that line of code aftere the reset

@ruck314 ruck314 closed this Nov 1, 2023
@ruck314 ruck314 deleted the gearbox-fix branch November 1, 2023 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants