Properly handle response code in axi_to_detailed_mem #320
Conversation
.github/workflows/gitlab-ci.yml
Outdated
There was a problem hiding this comment.
As a not. Lets not forget the changelog to be updated accordingly.
There was a problem hiding this comment.
Added info in the changelog
Bender.yml
Outdated
| @@ -19,7 +19,7 @@ package: | |||
| - "Florian Zaruba <zarubaf@iis.ee.ethz.ch>" | |||
|
|
|||
| dependencies: | |||
| common_cells: { git: "https://github.com/pulp-platform/common_cells.git", version: 1.27.0 } | |||
| common_cells: { git: "https://github.com/pulp-platform/common_cells.git", rev: "be16fe6efa97af39ca258e398f37b7910e636e02" } | |||
There was a problem hiding this comment.
Improved version of mem_to_banks.
There was a problem hiding this comment.
Updated common_cells to a proper version tag with the required changes
src/axi_to_detailed_mem.sv
Outdated
| logic resp_b_err, resp_b_exokay, resp_r_err, resp_r_exokay; | ||
| for (genvar i = 0; i < NumBanks; i++) begin | ||
| assign meta_buf_bank_strb[i] = meta_buf.strb[i*NumBytesPerBank +: NumBytesPerBank]; | ||
| assign meta_buf_size_enable[i] = i*NumBytesPerBank + NumBytesPerBank > (meta_buf.addr % DataWidth/8) && |
There was a problem hiding this comment.
Quite a full line. Can you maybe explain a bit in more details what is going on here?
There was a problem hiding this comment.
I agree, maybe at least one further level of parentheses would also help with understanding this rather complex condition.
There was a problem hiding this comment.
I expanded the explanation to clarify what is going on and added an additional level of parentheses for clearer grouping.
src/axi_to_detailed_mem.sv
Outdated
| logic resp_b_err, resp_b_exokay, resp_r_err, resp_r_exokay; | ||
| for (genvar i = 0; i < NumBanks; i++) begin | ||
| assign meta_buf_bank_strb[i] = meta_buf.strb[i*NumBytesPerBank +: NumBytesPerBank]; | ||
| assign meta_buf_size_enable[i] = i*NumBytesPerBank + NumBytesPerBank > (meta_buf.addr % DataWidth/8) && |
There was a problem hiding this comment.
I agree, maybe at least one further level of parentheses would also help with understanding this rather complex condition.
src/axi_to_detailed_mem.sv
Outdated
| logic [NumBanks-1:0] meta_buf_bank_strb, meta_buf_size_enable; | ||
| logic resp_b_err, resp_b_exokay, resp_r_err, resp_r_exokay; | ||
| for (genvar i = 0; i < NumBanks; i++) begin | ||
| assign meta_buf_bank_strb[i] = meta_buf.strb[i*NumBytesPerBank +: NumBytesPerBank]; |
There was a problem hiding this comment.
meta_buf.strb is of type axi_strb_t but meta_buf_bank_strb is a 1-bit logic (for every bank).
There was a problem hiding this comment.
I also discovered this and appropriately fixed the issue, ensuring correct mapping:
assign meta_buf_bank_strb[i] = |meta_buf.strb[i*NumBytesPerBank +: NumBytesPerBank];
Fixes #319
Requires pulp-platform/common_cells#194 to be released