I’m working on verifying a simple valid/ready handshaking design. I’m learning to use clocking blocks, interfaces and modports. The testbench will assert valid and keep it asserted for one clock cycle after the DUT asserts ready. The DUT can assert ready before or after the TB asserts valid. I’d like to know if my approach to testing this with the task is correct. Is there any issues with it? It appears to be working and I’ve also posted the code here: EDA Playground link
I’ve actually posted this on another forum (Original post) but I should have started here.
Below is the code. I’d like to know if my approach to testing this with the task is correct. Is there any issues with it?
`define CYCLE 10 //10.0001
module tb();
reg sys_clk;
// interface connection
ar_intf ar_intf_inst (.axi_slv_aclk(sys_clk));
ar_handshake ar_handshake_inst (
.aclk(ar_intf_inst.axi_slv_aclk),
.areset(ar_intf_inst.s_axi_areset),
.arvalid(ar_intf_inst.s_axi_arvalid),
.arrdy_high_en(ar_intf_inst.arrdy_high_en),
.arready(ar_intf_inst.s_axi_arready)
);
// Clock generation
initial begin
sys_clk <= 0;
forever #(`CYCLE/2.0) sys_clk = ~sys_clk;
end
// Reset generation followed by task to check arvalid/arready handshaking
initial begin
ar_intf_inst.initialize_sigs;
//Use $strobe to display values of variables assigned by nonblocking assignments
$strobe("\nAt Time %0t: $strobe System Reset Asserted", $realtime);
repeat(10) @(posedge sys_clk);
@(ar_intf_inst.cb_axil_slv) ar_intf_inst.cb_axil_slv.s_axi_areset <= 0;
$strobe("At Time %0t: System Reset De-Asserted \n", $realtime);
repeat (10) @(posedge sys_clk);
$display("At Time %0t: Begin ar_handshake, arrdy_high_en=%0b", $realtime, ar_handshake_inst.arrdy_high_en);
repeat (3) ar_check;
repeat (5) @(posedge sys_clk);
@(ar_intf_inst.cb_axil_slv);
ar_intf_inst.cb_axil_slv.arrdy_high_en <= 1;
$display("At Time %0t: Begin ar_handshake, arrdy_high_en=%0b", $realtime, ar_handshake_inst.arrdy_high_en);
repeat (3) ar_check;
repeat (5) @(posedge sys_clk);
ar_check;
@(posedge sys_clk);
//Assert reset to verify when arready resets to high
@(ar_intf_inst.cb_axil_slv) ar_intf_inst.cb_axil_slv.s_axi_areset <= 1;
$strobe("\nAt Time %0t: $strobe System Reset Asserted", $realtime);
repeat(5) @(posedge sys_clk);
@(ar_intf_inst.cb_axil_slv) ar_intf_inst.cb_axil_slv.s_axi_areset <= 0;
$strobe("At Time %0t: System Reset De-Asserted\n", $realtime);
repeat (5) @(posedge sys_clk);
$display("At Time %0t: Begin ar_handshake, arrdy_high_en=%0b", $realtime, ar_handshake_inst.arrdy_high_en);
repeat (3) ar_check;
repeat (5) @(posedge sys_clk);
ar_check;
@(posedge sys_clk);
$display("\nAt Time %0t: Stop sim", $realtime);
$stop(2);
end
initial
begin
$dumpfile("dump.vcd");
$dumpvars;
end
task ar_check;
begin
@(ar_intf_inst.cb_axil_slv);
ar_intf_inst.cb_axil_slv.s_axi_arvalid <= 1;
$display("At Time %0t: ar_intf_inst.cb_axil_slv.s_axi_arvalid=%x",$realtime, ar_intf_inst.cb_axil_slv.s_axi_arvalid);
//***************************************************************
// NOTE: CLOCKING BLOCKS:
// CLOCKING INPUTS MAYBE SAMPLED, BUT MUST NOT BE DRIVEN
// CLOCKING OUTPUTS MAY BE DRIVEN BUT MUST NOT BE SAMPLED
//***************************************************************
@(ar_intf_inst.cb_axil_slv);
wait(ar_intf_inst.cb_axil_slv.s_axi_arready);
$display("at Time %0t: ar_intf_inst.s_axi_arvalid=%0h", $realtime, ar_intf_inst.s_axi_arvalid);
$display("at Time %0t: ar_intf_inst.s_axi_arready=%0h", $realtime, ar_intf_inst.s_axi_arready);
$display("at Time %0t: ar_intf_inst.cb_axil_slv.s_axi_arready=%0h", $realtime, ar_intf_inst.cb_axil_slv.s_axi_arready);
ar_intf_inst.cb_axil_slv.s_axi_arvalid <= 0;
$display("at Time %0t: ar_intf_inst.cb_axil_slv.s_axi_arvalid=%0h", $realtime, ar_intf_inst.cb_axil_slv.s_axi_arvalid);
end
endtask
endmodule
interface ar_intf (input logic axi_slv_aclk);
logic s_axi_areset;
logic s_axi_arready;
logic arrdy_high_en;
logic s_axi_arvalid;
clocking cb_axil_slv @(posedge axi_slv_aclk);
default input #1step output #1;
// input is from DUT
input s_axi_arready;
// output is to DUT
output arrdy_high_en;
output s_axi_areset;
output s_axi_arvalid;
endclocking
modport axil_slave_mp (clocking cb_axil_slv);
task initialize_sigs(); // Only do this once at time-0 : Drive clk signals and not clkvars to initialize
s_axi_areset <= '1;
arrdy_high_en <= '0;
s_axi_arvalid <= '0;
endtask : initialize_sigs
// Interface acts as tb hook to each set of bus signals that follow this
// protocol. It is an ideal place for assertions that check protocol validity
arvalid_deassert_after_arready: assert property (@(cb_axil_slv) disable iff (s_axi_areset)
s_axi_arvalid && s_axi_arready |-> ##1 !s_axi_arvalid)
else $fatal(1);
endinterface
module ar_handshake (
input wire aclk,
input wire areset,
input wire arvalid,
input wire arrdy_high_en, // select between arready high until arvalid
output reg arready
);
always @( posedge aclk )
begin
if ( areset )
arready <= 1'b1;
else begin
if(!arrdy_high_en) begin
if (~arready && arvalid)
arready <= 1'b1;
else
arready <= 1'b0;
end
else begin
// Below have arready high
if (arready && arvalid)
arready <= 1'b0;
else
arready <= 1'b1;
end
end
end
endmodule
Your use of assertions inside the interface is good.
clocking block usage is OK too.
I particularly (my preference) don’t like too many begin … end keywords unless absolutely necessary. Specificaly, if the if else statement is of one term, why do you need the begin end?. Also, if you do use lots of begin end, label them
// instead of
always @( posedge aclk )
begin
if ( areset )
arready <= 1'b1;
else begin
if(!arrdy_high_en) begin
if (~arready && arvalid)
arready <= 1'b1;
else
arready <= 1'b0;
end
else begin
// Below have arready high
if (arready && arvalid)
arready <= 1'b0;
else
arready <= 1'b1;
end
end
end
// simplify to
always @( posedge aclk ) begin : alwy1
if (areset) arready <= 1'b1;
else
if(!arrdy_high_en)
if (~arready && arvalid) arready <= 1'b1;
else arready <= 1'b0;
else // Below have arready high
if (arready && arvalid) arready <= 1'b0;
else arready <= 1'b1;
end : alwy1
//
// BTW, maybe a case statement might be better here?
// The code was hard to follow.
Unless absolutely necessary, I prefer constrained-random tests instead of directed tests. For example, the template I generally use to test my assertions. I then modify the variables and the statistics.
initial begin
repeat(200) begin
@(posedge clk);
if (!randomize(a, b) with
{ a dist {1'b1:=1, 1'b0:=1};
b dist {1'b1:=1, 1'b0:=2};
}) `uvm_error("MYERR", "This is a randomize error");
end
$finish;
end
In reply to ben@SystemVerilog.us:
Thank you for the feedback Ben. Can you point out the use of “bitwise instead of the logical operator”? I’m not finding it.
For the DUT, initially I had a simple logic shown below but I added to it to verify my task ar_check was working properly. I’m not concerned with it but with the testing code.
always @( posedge aclk )
begin
if ( areset )
arready <= 1'b1;
else
begin
if (~arready && arvalid)
arready <= 1'b1;
else
arready <= 1'b0;
end
end
I’m really interested to know if my task ar_check is correct and/or needs improvement. So, I’m starting with the directed tests so that I understand it and can quickly test a module. I would like to move to constrained-random tests. Can you provide some guidance?
You don’t need a begin end for a single if else statement (my preference
I took another look at your use of the task and I DO have comments:
- You have
verilog task ar_check; begin // <--- NOT NEEDED, delete @(ar_intf_inst.cb_axil_slv); ... endtask // You fire the task in an initial with repeat (3) ar_check;
Though it is syntactically correct, you may eventually migrate to UVM where tasks are automatic because they are in a class, and in a driver, a task have a purpose or a job to do, as specified by an instruction. The task may also identify an outcome. Consider this example (a counter):
verilog // in a package, I have a typedef for the type of stuff I want to expose my counter to typedef enum {CT_LOAD, CT_RESET, CT_WAIT, CT_DONE} ct_scen_e; // The transaction, the job with details class counter_xactn extends uvm_sequence_item; rand logic[3:0] data=0; logic[3:0] data_collected, data_viewed; rand logic ld=0, rst_n; rand int reset_cycles, idle_cycles=1; rand int xx; rand ct_scen_e kind; ... enclass // The start of of the driving machine, like opening the store task get_and_drive(); // portion of the code shown here counter_xactn t; // to hold gotten item as a copy, keeping original pristine // do a create (like a new of t forever begin //wait(vif.reset==0); //block until reset released seq_item_port.get_next_item(req); // New item (or job) at every call, send_to_dut(t); // The actual job to do ... seq_item_port.put_response(rsp); // put the response (e.g., here is data) seq_item_port.item_done(); end endtask : get_and_drive // the job task send_to_dut(input counter_xactn item); uvm_report_info(tID,$psprintf("%s : item sent is %0s",tID,item.sprint()),UVM_MEDIUM); copy_items(item_copy, item); // Do this for viewing in waveform view this.vif.driver_cb.kind_cp<=item.kind; // put into interface for debug case(item.kind) CT_LOAD : begin this.load_task(item.data); end CT_RESET : begin this.reset_task(1, item); end CT_WAIT : begin this.idle_task(item.data); end CT_DONE : begin this.done_task(item); end endcase endtask : send_to_dut // more details task load_task(int data); this.vif.driver_cb.data_in <= data; this.vif.driver_cb.rst_n <= 1'b1; this.vif.driver_cb.ld <= 1'b1; @(this.vif.driver_cb)this.vif.driver_cb.ld <= 1'b0; endtask : load_task
- The point here is that by being more hierarchical, you attack the problem from a top-down viewpoint – stating the engine, feeding the stuff to do (reset, a load, a count, …), sending the details on what a load or a count is to separate tasks. You can do all of that without UVM, until you learn UVM (if you need to).
- Recommendations:
verilog // Add in the interface something like typedef enum {INIT, DO,RESULT} scen_e; // << scen_e kind; // << // You may want to use classes, but that is not necessary. // Put your task ar_check in your interface // Use hierarchy as described above.
What I said above sounds overkill, and it is for this model. However, consider it for a more complex model.
would like to move to constrained-random tests. Can you provide some guidance?
I copied into my SVA book open source code examples on constrained-randomization. The source of that book is System Verilog Functional Verification 1st Edition
by Sasan Iman (Author), Warren J. Smith (Author)
Since the examples are open-source, I am providing a link to the models.
That should help a bit. Of course, 1800’2017 has info on that too.
I really appreciate the information! It’s very helpful.
You had initially mentioned “You tend to use the bitwise instead of the logical operator”. In my response I had meant that in my code I didn’t find the use of bitwise and was asking you to indicate where I was using bitwise operator.
In reply to ben@SystemVerilog.us:
I really appreciate the information! It’s very helpful.
… I didn’t find the use of bitwise and was asking you to indicate where I was using bitwise operator.
forever #(`CYCLE/2.0) sys_clk = ~sys_clk; // The "~"
...
if (~arready && arvalid) // The "~"
In reply to markylew:
The question is not about “correctness” since ~a_bit_var produces the same result of !a_bit_var. The issue is the intent, or how it reads /visualize. When I see the “~” I visualize bit-flipping. When I see"!" I since the logical result (true/false).
As Dave said, “It should be what the intent requires. In general, I would stick with logical operators and only use bitwise on vectors greater than 1-bit wide AND the operation requires it, like masking.”
I’ll just add that for single bit operands of unary operators, it really doesn’t matter that much other than showing your intent. But when it comes to the binary operators, precedence starts to matter even with single bit operands.