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

initial values on outgoing ram_master ports #718

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

initial values on outgoing ram_master ports #718

wants to merge 1 commit into from

Conversation

embed-me
Copy link

Avoid undefined signals on outgoing ports of the ram interface.

@umarcor
Copy link
Member

umarcor commented Jul 23, 2021

I'm not sure it is correct to avoid undefined signals. The undefined value exist for it to be used... See stnolting/neorv32#128 (comment).

@embed-me
Copy link
Author

Undefined initial signals propagate to the UUT. Simple checks like "check_not_unknown" will fail. Other master modules (eg. ram_master, axi_stream_master,...) all have their output values defined, so I still think it makes sense to add them.

@eschmidscs
Copy link
Contributor

As you mention axi_stream_master: You have the option to have the outputs undefined if invalid.
The ram_master already has the en output initialized to zero. If the UUT fails due to undefined values while en is zero, the UUT should be fixed to not consider any other signals while en is zero.

@embed-me
Copy link
Author

In general, this makes sense of course, however, the enable port for blk-mem is optional (at least for Xilinx, but I think also for Altera/Intel). If you do not use this signal, for example, because the Memory is always enabled (like it is the case for me), default values would help.
Also, if you follow your logic, why does the "axis_stream_master" define default values for "tdata" (and other ports) if "tvalid" is already set to a default value of '0'?

@eschmidscs
Copy link
Contributor

Well, the possibility to drive invalids was added on our request. So the default is to have it drive zeros (for backward compatibility), but you can make it drive X. Which would solve your issue, if ram_master would behave the same way.
I personally would prefer to have the default drive X or U and add an option to drive 0. But we came late and are not the initiators, so it is the other way round ;)

I have to admit that I never used ram_master, so I'm not really competent when it comes to its application. I was triggered by axi_stream_master in your post, and wanted to point out that this block can drive U as default as well.

The enable port becomes a philosophical topic indeed, because the default assignment of the Xilinx memory is '1', but the ram_masters default assignment is '0'. Maybe the default of '0' is not good either...

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.

3 participants