Heart beat RTL module

Question

Heart rate or blink generator. Clocked from the system frequency, but calculated from a constant of 120MHz. Has a prescaler with values 2, 3, 5, 6, for even heart beat / blinking. The IS_DEBUG parameter is required for debugging for easy control over the rhythm, this parameter reduces 120MHz to 120Hz. IS_BLINK selects between blink and heart beat.

heart_beat.sv

`timescale 1ns / 10ps

module heart_beat #
(
    parameter integer SPEED_GRADE   = 2,
    parameter         IS_DEBUG      = "false",
    parameter         IS_BLINK      = "false"
)
(
    input  logic i_clk,
    input  logic i_s_rst_n,
    
    output logic o_heart_beat
);
    localparam integer                RATE            = (IS_DEBUG == "true") ? 120 : 120_000_000;
    localparam integer                MAX_COUNTER_VAL = RATE / (((SPEED_GRADE != 2 ) && 
                                                                 (SPEED_GRADE != 3 ) &&
                                                                 (SPEED_GRADE != 5 ) &&
                                                                 (SPEED_GRADE != 6 )) ? 2 : SPEED_GRADE);
                                                        
    localparam integer                COUNTER_WIDTH   = $clog2(MAX_COUNTER_VAL);
                                                            
    localparam [COUNTER_WIDTH - 1: 0] CMP_COUNTER_VAL = MAX_COUNTER_VAL - 1'h1;
    
 
    logic                        heart_beat;
    logic                        mask;
    logic [COUNTER_WIDTH - 1: 0] counter;

    assign o_heart_beat = heart_beat & ((IS_BLINK == "true") ? '1 : mask);
    
    always_ff @ (posedge i_clk) begin
        if (i_s_rst_n == '0) begin
            counter    <= '0;
            heart_beat <= '0;
        end
        else begin
            if (counter == CMP_COUNTER_VAL) begin
                heart_beat <= !heart_beat;
                counter    <= '0;
            end
            else begin
                counter++;
            end  
        end
    end
    
    generate 
         if (IS_BLINK == "false") begin
             localparam integer                MASK_COUNTER_PRE     = 4;
             localparam integer                MASK_MAX_COUNTER_VAL = (MAX_COUNTER_VAL / MASK_COUNTER_PRE);
    
             localparam integer                MASK_COUNTER_WIDTH   = $clog2(MASK_MAX_COUNTER_VAL);
             localparam [COUNTER_WIDTH - 1: 0] MASK_CMP_COUNTER_VAL = MASK_MAX_COUNTER_VAL - 1'h1;
             
             logic [MASK_COUNTER_WIDTH - 1: 0] mask_counter;
    
             always_ff @ (posedge i_clk) begin
                 if (i_s_rst_n == '0) begin
                     mask <= '0;
                     mask_counter <= '0;
                 end
                 else begin
                     if (mask_counter == MASK_CMP_COUNTER_VAL) begin
                         mask         <= !mask;
                         mask_counter <= '0;
                     end  
                     else begin
                         mask_counter++;
                     end
                 end
             end
         end
    endgenerate
endmodule

heart_beat_tb.sv

`timescale 1ns / 1ps

module heart_beat_tb;
//-------------------------------------------------- setup
     localparam integer SPEED_GRADE     = 2;
     localparam         IS_DEBUG        = "true";
     localparam         IS_BLINK        = "true";

     localparam integer ITERATION       = 1000;
//-------------------------------------------------- end setup
     
     localparam integer CLOCK_PERIOD    = 120;
   
     localparam integer MAX_COUNTER_VAL = CLOCK_PERIOD - 1;
     localparam integer TEST_ITERATION  = ITERATION * MAX_COUNTER_VAL;

     bit clk        = '0;
     bit s_rst_n    = '1;
     bit heart_beat = '0;
     
     integer counter = 0;

     heart_beat #
     (
         .SPEED_GRADE   (SPEED_GRADE  ),
         .IS_DEBUG      (IS_DEBUG     ),
         .IS_BLINK      (IS_BLINK     ) 
     )
     heart_beat_dut
     (
         .i_clk        (clk       ),
         .i_s_rst_n    (s_rst_n   ),
                       
         .o_heart_beat (heart_beat)
     );
     
     always begin
         #(CLOCK_PERIOD / 2) clk = !clk;
     end

     initial begin
         s_rst_n <= '0;
         @(posedge clk);

         s_rst_n <= '1;
         @(posedge clk);
         
         repeat(TEST_ITERATION) begin
             if (counter == MAX_COUNTER_VAL) begin
                 counter = 0;
             end
             else begin
                 counter++;
             end
            
             @(posedge clk);
         end

         $stop();
     end
endmodule

Answer 1

I get compile errors on 2 different simulators (Cadence and Synopsys) from this line in the testbench:

bit heart_beat = '0;

Since you connect that signal to the module instance output port, you should not set it to any value in the declaration line. Your code has multiple drivers of the signal:

• The module instance output port.
• The continuous driver of your logic declaration.

Just declare the signal without an assignment:

bit heart_beat;

Aside from that, I do not see any major problems with your code. The layout follows good coding practices, and you make good use of parameters.

In the heart_beat module, you use the ++ operator for sequential logic in 2 places. Since ++ behaves as a blocking assignment, and good coding practices advise against using blocking assignments for sequential logic, you should consider using nonblocking assignments.

Change:

            counter++;

to:

            counter <= counter + 1;

Do the same for mask_counter.

Here are some references:

1. Is the ++ operator in System Verilog blocking or non-blocking?
2. Non blocking ++ equivalent in Systemverilog