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

hw: Disable AXI splitting checks in AXI-RT #84

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Conversation

thommythomaso
Copy link
Collaborator

This addresses one point in #4 of AXI-RT.

@thommythomaso thommythomaso added bug Something isn't working enhancement New feature or request labels Oct 13, 2023
@thommythomaso thommythomaso self-assigned this Oct 13, 2023
@paulsc96 paulsc96 changed the title Disable AXI splitting checks in AXI-RT hw: Disable AXI splitting checks in AXI-RT Oct 13, 2023
Copy link
Collaborator

@paulsc96 paulsc96 left a comment

Choose a reason for hiding this comment

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

Please

  • Extend the Xilinx top wrapper, since this parameter has a nonzero default. On that note, maybe it is better to invert the parameter to an enable semantic if erroneously enabling these checks is considered a bug.
  • Amend the documentation.

alex96295
alex96295 previously approved these changes Oct 13, 2023
Copy link
Collaborator

@alex96295 alex96295 left a comment

Choose a reason for hiding this comment

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

Same as Paul, for the rest LGTM

@thommythomaso
Copy link
Collaborator Author

thommythomaso commented Oct 16, 2023

Please

* Extend the Xilinx top wrapper, since this parameter has a nonzero default. On that note, maybe it is better to invert the parameter to an enable semantic if erroneously enabling these checks is considered a bug.

* Amend the documentation.

Enabling it is not a bug. The current parameter solution should be seen as a temporary solution that will eventually be replaced when redesigning the burst splitter. The new parameter defaults to 0, no changes are required to the xilinx wrapper.

Copy link
Collaborator

@alex96295 alex96295 left a comment

Choose a reason for hiding this comment

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

LGTM

@paulsc96 paulsc96 merged commit d4b73a8 into main Oct 17, 2023
18 checks passed
@paulsc96 paulsc96 deleted the disable_checks-tbenz branch October 17, 2023 13:20
Aquaticfuller pushed a commit that referenced this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants