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

Stream checker rule5 fix #799

Merged
merged 2 commits into from
Nov 1, 2023
Merged

Conversation

bewimm
Copy link
Contributor

@bewimm bewimm commented Feb 11, 2022

Fix for #798

@@ -42,7 +42,7 @@ architecture a of tb_axi_stream_protocol_checker is
constant logger : logger_t := get_logger("protocol_checker");
constant protocol_checker : axi_stream_protocol_checker_t := new_axi_stream_protocol_checker(
data_length => tdata'length, id_length => tid'length, dest_length => tdest'length, user_length => tuser'length,
logger => logger, actor => new_actor("protocol_checker"), max_waits => max_waits
logger => logger, actor => new_actor("protocol_checker"), max_waits => max_waits, allow_x_in_null_bytes => true
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'm not very happy with this solution.
I think it would be better to not mix the extension with the rest, but I'm not sure what the cleanest way of doing that is

logger : logger_t := axi_stream_logger;
actor : actor_t := null_actor;
max_waits : natural := 16;
allow_x_in_null_bytes : boolean := false
Copy link
Contributor

Choose a reason for hiding this comment

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

Align

Suggested change
allow_x_in_null_bytes : boolean := false
allow_x_in_null_bytes : boolean := false

begin
ret := data;
for i in keep'range loop
if keep(i) = '0' and strb(i) = '0' then
Copy link
Contributor

Choose a reason for hiding this comment

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

Per discussion in #798 this should perhaps be

Suggested change
if keep(i) = '0' and strb(i) = '0' then
if keep(i) = '0' or strb(i) = '0' then

Null bytes must be ignored by the slave and the interconnect is
allowed to remove these so checking that they contain valid data
is not necessary.
This test checks that no errors are reported for
tdata bytes that are masked by tkeep or tstrb if
allow_x_in_non_data_bytes is set.
@LukasVik
Copy link
Contributor

I looked through it again, and after @bewimm's fixes it looks all good to me. I agree with the nomenclature and functionality, per the discussion in #798, and I agree with the implementation. IMHO ready to merge.

@umarcor
Copy link
Member

umarcor commented Mar 11, 2024

See #997.

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.

5 participants