Change tvip_axi_data typedef from bit to logic to handle 4-state value #31
Conversation
|
These TBs use I'd like you to confirm your change does not break the above TBs. |
|
On the latest commit on tnoc / rggen-sample-testbench, test cannot be run with Cadence Xcelium 21.03. As far as I checked, tests can be run on the following commit: According to the commit of rggen below with this PR: Thanks. |
What is happened? Something compile error? |
|
There are some compilation errors with the latest commits at these repositories: |
|
Thank you for uploading log files.
|
src/tvip_axi_ral_adapter.svh
Outdated
| TVIP_AXI_SLAVE_ERROR: return UVM_NOT_OK; | ||
| TVIP_AXI_DECODE_ERROR: return UVM_NOT_OK; | ||
| endcase | ||
| if (axi_item.response[0] inside {TVIP_AXI_SLAVE_ERROR, TVIP_AXI_SLAVE_ERROR}) |
There was a problem hiding this comment.
Please add begin/end block to if statement to follow my coding rule.
There was a problem hiding this comment.
I think we should return UVM_HAX_X when response[0] is unknown. What do you think?
There was a problem hiding this comment.
UVM 1.2 defines UVM_HAS_X as Operation completed successfully bit had unknown bits so my opinion is that it should be treated as UVM_NOT_OK if response[0] contains undefined value.
There was a problem hiding this comment.
Current implementation response is also defined as bit type so we don't need to care the case when response is undefined state. However, in the future, if response type is changed to logic, it needs to be revisited.
| @@ -68,7 +68,7 @@ package tvip_axi_types_pkg; | |||
| TVIP_AXI_WRAPPING_BURST = 'b10 | |||
| } tvip_axi_burst_type; | |||
|
|
|||
| typedef bit [`TVIP_AXI_MAX_DATA_WIDTH-1:0] tvip_axi_data; | |||
| typedef logic [`TVIP_AXI_MAX_DATA_WIDTH-1:0] tvip_axi_data; | |||
There was a problem hiding this comment.
Can you change other types from bit to logic?
There was a problem hiding this comment.
It can be done and ideally signals at Interface boundary should have capability to handle 4-state value. However, VIP internal code needs to care these as 4-state value so it may take some time and effort. I don't think changing bit to logic may be so valuable at the moment.
Anyway, let me check if this should be done in this PR or not.
There was a problem hiding this comment.
In this PR, I want to minimize the difference and I think changing all types of interface to logic is not so valuable at the moment, so signal types other than data should be left as bit.
@taichi-ishitani This PR is related to #29. This change makes RAL adapter return UVM_HAS_X if read data contains X or Z value.