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

Adding ability to define the actor on new_axi_slave function #709

Merged
merged 4 commits into from
Mar 16, 2021

Conversation

rftafas
Copy link
Contributor

@rftafas rftafas commented Feb 28, 2021

This change should not affect any current usage, but adds possibility to define the actor for the axi_slave being created. For some reason on a test needing several AXI Slave VCIs, modelsim put all of them under the same "id" and things got awkward.

This has solved it.

@LarsAsplund
Copy link
Collaborator

There is work being done to standardize some parts of VCs and VCIs to improve interoperability. One of the rules of that standard is to have the ability you're looking for. All present VCs have been modified to comply with this standard but that work hasn't been merged yet. It's been on hold for a while but is number two on my prio list and I will get to it shortly.

We can do this before that as it doesn't break any backward compatibility but please have a look at

actor : actor_t := null_actor;
and modify such that your default value is null_actor to better match that standard.

@rftafas
Copy link
Contributor Author

rftafas commented Mar 9, 2021

I made the changes, but It is still having trouble on some seemingly unrelated checks.

@eine
Copy link
Collaborator

eine commented Mar 9, 2021

@rftafas the linter errors are unrelated to your changes. However, I think that acceptance and vcomponents failures are related to this PR. Precisely, the AXI DMA example is failing in acceptance tests, and AXI Slave tests are failing in vcomponents.

@rftafas
Copy link
Contributor Author

rftafas commented Mar 11, 2021

It seems that the change from 'new_actor' to 'null_actor' caused this. Most probably, the examples will have to be adapted to include an actor now, as it does not do that anymore.

@@ -147,7 +149,7 @@ package body axi_slave_pkg is
max_response_latency : delay_length := 0 ns;
logger : logger_t := axi_slave_logger) return axi_slave_t is
begin
return (p_actor => new_actor,
return (p_actor => actor,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I was a bit unclear here. You still need to call new_actor. p_actor has to be set to

actor when actor /= null_actor else new_actor

Copy link
Collaborator

Choose a reason for hiding this comment

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

Once you've fixed that you can also rebase on master and then, hopefully, the remaining lint issues will disappear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought of that at first, but got worried that it would not be the acceptable way. Will do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants