-
Notifications
You must be signed in to change notification settings - Fork 276
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
Add option to enable internal pipelining to xbar
.
#268
Conversation
185d4f4
to
42473bf
Compare
test/tb_axi_xbar.sv
Outdated
axi_xbar #( | ||
.Cfg ( xbar_cfg ), | ||
.slv_aw_chan_t( aw_chan_mst_t ), | ||
.mst_aw_chan_t( aw_chan_slv_t ), | ||
.w_chan_t ( w_chan_t ), | ||
.slv_b_chan_t ( b_chan_mst_t ), | ||
.mst_b_chan_t ( b_chan_slv_t ), | ||
.slv_ar_chan_t( ar_chan_mst_t ), | ||
.mst_ar_chan_t( ar_chan_slv_t ), | ||
.slv_r_chan_t ( r_chan_mst_t ), | ||
.mst_r_chan_t ( r_chan_slv_t ), | ||
.slv_req_t ( mst_req_t ), | ||
.slv_resp_t ( mst_resp_t ), | ||
.mst_req_t ( slv_req_t ), | ||
.mst_resp_t ( slv_resp_t ), | ||
.rule_t ( rule_t ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue we stick to testing the axi_xbar_intf
, as anything that tests it will also test the axi_xbar
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will change back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
/// Address rule type for the address decoders from `common_cells:addr_decode`. | ||
/// Example types are provided in `axi_pkg`. | ||
/// Required struct fields: | ||
/// ``` | ||
/// typedef struct packed { | ||
/// int unsigned idx; | ||
/// axi_addr_t start_addr; | ||
/// axi_addr_t end_addr; | ||
/// } rule_t; | ||
/// ``` | ||
parameter type rule_t = axi_pkg::xbar_rule_64_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed removing this in a future update, correct? I don't remember if it was planned for this update or the next.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter struct should be removed soon. But that will be the last two PRs of Wolfgang.
42473bf
to
66e5200
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Let's wait for CI, can be merged afterwards
Replaces #116.