Few updates to WB VIP

[AakaFosfor]

Hi,
as I was using your code for last few weeks in one of our projects (http://www.ohwr.org/projects/fmc-del-1ns-2cha/) I've made several changes to the WB VIP. Please review these changes and incorporate them if you find them meaningful.
With all the best,
Fosfor

[dovstamler]

Hi,
Could you describe what was the issue here? I have not tested this agent under UVM-1.2. Using UVM-1.1d some simulators require to raise and drop objections in all phases including the build phase.

[dovstamler]

Hi,
I don't see any issue here, file types can be left as is without any functional change so I see no reason to merge this modification to the main branch.

[dovstamler]

Hi,
covert2string: It appears this implementation is similar to the "sprint" built in method of the inherited uvm_object. What was the purpose of this implementation?

get_type_name: The default implementation should return the value type in the string. I have called this method in many of my log calls and see the expected string in the log. Therefore, I see no reason to modify the default implementation here.

[AakaFosfor]

I don't see any issue here, file types can be left as is without any functional change so I see no reason to merge this modification to the main branch.

I've changed file extensions by b937e29 because I found it little bit misleading. .sv is s SV file which needs to be compiled into a project. .svh is a header file, which should not be compiled, as it is included somewhere. All the WB VIP files except the package file (and the IF file) are included inside the package file, thus they should have .svh extension (and they should not be compiled into the project). There is no functional change, but it should help others to use it (i.e. distinguish what to include into the project). Other question is, if the way it is is correct - maybe all these file should be simply .sv, should be compiled and should not be included inside the package... Same goes to I2C core (which I didn't use).

[AakaFosfor]

I have not tested this agent under UVM-1.2. Using UVM-1.1d some simulators require to raise and drop objections in all phases including the build phase.

I've removed raise/drop_objection in 2d1a08d because I was receiving this error:

UVM_ERROR verilog_src/uvm-1.2/src/base/uvm_phase.svh(1835) @ 0: reporter [UVM/PH/NULL_OBJECTION] 'uvm_test_top.Env_h.WbAgent.drv' attempted to raise an objection on 'build', however 'build' is not a task-based phase node!

Tested in Modelsim 10.4d and UVM 1.2. I've found somewhere some information that this has been changed in the version 1.2, but I'm not able to find it again. Also - it depends, what version do you want the repository to be compatible with.

[AakaFosfor]

In connection with c02784d:

covert2string: It appears this implementation is similar to the "sprint" built in method of the inherited uvm_object. What was the purpose of this implementation?

It is not the same, difference is stated in the UVM 1.2 documentation:

5.2 uvm_object -> convert2string: [...] Unlike sprint, there is no
requirement to use a uvm_printer policy object. As such, the format and content of the
output is fully customizable, which may be suitable for applications not requiring the
consistent formatting offered by the print/sprint/do_print API. [...]

Purpose was to make available a short and brief string description of the actual object.

get_type_name: The default implementation should return the value type in the string. I have called this method in many of my log calls and see the expected string in the log. Therefore, I see no reason to modify the default implementation here.

Default implementation (uvm-1.2/src/seq/uvm_sequence_item.svh:57) returns same string "uvm_sequence_item" for all sequence items:

# UVM_INFO TestbenchPackage.sv(56) @ 1237500: uvm_test_top.Env_h.WbSubscriber [UNISUBS] Transaction "uvm_sequence_item" processed: direction_e = WB_B3_DIR_WRITE, address = 0x0001606c, data = 0x00000000, select = 0b1111, response_e = WB_B3_RESPONSE_ACK_OK

The reason to modify this behaviour (by overriding the get_type_name function) is to be able to distinguish between different sequence items, so the output from the modified code looks like this:

# UVM_INFO TestbenchPackage.sv(56) @ 1237500: uvm_test_top.Env_h.WbSubscriber [UNISUBS] Transaction "wishbone_b3_sequence_item" processed: direction_e = WB_B3_DIR_WRITE, address = 0x0001606c, data = 0x00000000, select = 0b1111, response_e = WB_B3_RESPONSE_ACK_OK

[AakaFosfor]

I've also added another change 236ce38 to introduce a time-out check for WB bus transactions. The timeout value has to be set in the WB VIP configuration when building the environment.

Please let me know if you still have any questions.

[dovstamler]

There are a number of changes which doesn't conform to the coding methodology implemented for this agent.

wishbone_b3_master_agent.svh:
change made to the set of the config object - please leave the original code instead of using "" in the set. As a general rule of thumb, config_db "" setting will not be used in this agent, you are welcome to discuss this with me offline.

wishbone_b3_master_driver/monitor.svh:
In order to pass the config object to the common methods implemented in each of these components, please pass the cfg pointer by reference instead of retrieval from the config_db.

Also, could you comment as to the need for this timeout? The UVM phase timeout should suffice since if a request gets stuck, following requests will typically run into similar issues.

Please update these changes into a new pull request, I am in the process of merging the current request up to this change into the master branch and will shortly close this pull request.

[dovstamler]

Regarding the "disable fork" in the common_methods:
the fine grain thread process would be a better approach to halt a running thread. Depending on the simulator, the "disable fork" can create results not intended by the coder.
See an example of the fine grain thread use in the I2C slave driver.