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

Allow injecting user synchronization modules #56

Open
wants to merge 2 commits into
base: main-next
Choose a base branch
from

Conversation

mkurc-ant
Copy link
Collaborator

@mkurc-ant mkurc-ant commented Feb 15, 2023

This PR allows injection of modules containing synchronization flip-flops (answers #51).

The config script allows to set additional parameters which end up in code as capitalized defines:

tech_specific_rv_sync - enables usage of the user module
user_rv_sync - specifies the module name

The PR is based on #54 and should be reviewed and merged after it.

@varuns-nvidia
Copy link

Hi @mkurc-ant , what is the status of this issue?

@mkurc-ant
Copy link
Collaborator Author

Hi @varuns-nvidia. This PR is ready but marked as draft as it is based on #54 (shared codebase) which has not yet been reviewed.

Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
@mkurc-ant mkurc-ant changed the base branch from main to main-next May 29, 2023 11:54
@mkurc-ant mkurc-ant marked this pull request as ready for review May 29, 2023 11:54
@mkurc-ant
Copy link
Collaborator Author

@varuns-nvidia FYI this is ready for review.

Copy link

@nstewart-amd nstewart-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user_rv_sync placeholder module should not contain an "initial" block.

The user_rv_sync module should not be multi-bit width, since it has no synchronization qualifiers for multi-bit signal synchronization.

Recommend user_rv_sync is single bit in/out.
Recommend user_rv_sync instantiates rvdff behavioural flop model from existing beh_lib.sv.

… initial state

Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
@mkurc-ant
Copy link
Collaborator Author

@nstewart-amd I changed the required user synchronizer cell model to be 1-bit and without any initial blocks. I left the behavioral code in design/dmi/dmi_jtag_to_core_sync.v as it was like that before my changes (It would be best to tackle this in a separate PR if needed). I also left the behavioral code in user_rv_sync as it is not used by default. When enabled it is used only in the testbench.

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.

None yet

3 participants