1
\$\begingroup\$

The module measures input clocks. It requires some reference clock. There can be from one to five input clocks to measure it. Output values are usual unsigned ones. As expected, it should be reset before starting. If it is used in Vivado, the user can choose the number of ports to show, depending on the number of input clocks to measure.

Example for Vivado block diagram:

enter image description here

Accuracy for 125MHz, 150MHz, 50MHz, 100MHz:

enter image description here

Top module:

   `timescale 1ns / 1ps

module clock_meter_top #
(
  parameter integer CLK_MHZ = 50,
  parameter integer CLK_NUM = 1,
  
  localparam integer MSR_CLK_VAL_WIDTH = 32
)
(
   input  logic                              clk,
   input  logic                              a_rst_n,

   
   input  logic                              msr_clk_0,
   input  logic                              msr_clk_1,
   input  logic                              msr_clk_2,
   input  logic                              msr_clk_3,
   input  logic                              msr_clk_4,
   
   output logic [MSR_CLK_VAL_WIDTH - 1 : 0 ] msr_clk_val_0,
   output logic [MSR_CLK_VAL_WIDTH - 1 : 0 ] msr_clk_val_1,
   output logic [MSR_CLK_VAL_WIDTH - 1 : 0 ] msr_clk_val_2,
   output logic [MSR_CLK_VAL_WIDTH - 1 : 0 ] msr_clk_val_3,
   output logic [MSR_CLK_VAL_WIDTH - 1 : 0 ] msr_clk_val_4
);

  logic                              msr_clk     [CLK_NUM - 1: 0];
  logic [MSR_CLK_VAL_WIDTH - 1 : 0 ] msr_clk_val [CLK_NUM - 1: 0];

  genvar i;
  
  generate
    always_comb
      begin  
        msr_clk[0]     = msr_clk_0;
        msr_clk_val_0 = msr_clk_val[0];
     end
      
    if (CLK_NUM > 1)
      begin
        always_comb
          begin
            msr_clk[1]     = msr_clk_1;
            msr_clk_val_1 = msr_clk_val[1];
          end
      end
      
    if (CLK_NUM > 2)
      begin
        always_comb
          begin
            msr_clk[2]     = msr_clk_2;
            msr_clk_val_2 = msr_clk_val[2];
          end
      end
    
    if (CLK_NUM > 3)
    begin
      always_comb
        begin
          msr_clk[3]     = msr_clk_3;
          msr_clk_val_3 = msr_clk_val[3];
        end
    end
    
    if (CLK_NUM > 4)
    begin
      always_comb
        begin
          msr_clk[4]     = msr_clk_4;
          msr_clk_val_4 = msr_clk_val[4];
        end
    end

    for (i = 0; i < CLK_NUM; i = i + 1)
      begin : gen_clock_meters
        clock_meter #
        (
          .CLK_MHZ (CLK_MHZ)
        )
        clock_meter_inst
        (
          .clk_i         (clk           ),
          .a_rst_n_i     (a_rst_n       ),

          .msr_clk_i     (msr_clk[i]    ),
  
          .msr_clk_val_o (msr_clk_val[i])      
        );
     end
  endgenerate

endmodule

Module:

   `timescale 1ns / 1ps


module clock_meter #
(
  parameter integer CLK_MHZ = 100,
  
  localparam integer MSR_CLK_VAL_WIDTH = 32
)
(
  input  logic                             clk_i,
  input  logic                             a_rst_n_i,
  
  input  logic                             msr_clk_i,
  
  output logic [MSR_CLK_VAL_WIDTH - 1 : 0] msr_clk_val_o
);
  
  
  localparam integer REF_CNT_NUM   = 10000000;
  localparam integer REF_CNT_WIDTH = $clog2(REF_CNT_NUM);
  
  localparam integer DECIM_CLK_MHZ  = CLK_MHZ / 10;
  
  
  logic [REF_CNT_WIDTH - 1 : 0]     ref_cnt;
  
  logic [MSR_CLK_VAL_WIDTH - 1 : 0] msr_cnt;
  logic [MSR_CLK_VAL_WIDTH - 1 : 0] msr_clk_val;
  logic                             msr_cnt_rst;
  logic [MSR_CLK_VAL_WIDTH - 1 : 0] gray_msr_cnt;
  
  logic [MSR_CLK_VAL_WIDTH - 1 : 0] res_msr_cnt;
  
  
  integer i;
  

  always_ff @(posedge clk_i)
    begin
      if (a_rst_n_i == 1'b0)
        begin
          ref_cnt <= 'h0;
        end
      else
        begin
          if (ref_cnt == (REF_CNT_NUM - 1))
            begin
              ref_cnt <= 'h0;
            end
          else
            begin
              ref_cnt <= ref_cnt + 1'b1;
            end
        end
    end
    
  
  always_ff @(posedge clk_i)
    begin
      if (a_rst_n_i == 1'b0)
        begin
          msr_cnt_rst <= 'h0;
        end
      else
        begin
          if (ref_cnt == (REF_CNT_NUM - 1))
            begin
              msr_cnt_rst <= 'h1;
            end
          else
            begin
              msr_cnt_rst <= 1'b0;
            end
        end
    end
  
  always_ff @(posedge msr_clk_i)
    begin
      if (a_rst_n_i == 1'b0)
        begin
          gray_msr_cnt <= 'h0;
          msr_cnt      <= 'h0;
        end
      else
        begin
          if (msr_cnt_rst == 1'b1)
            begin
              msr_cnt      <= 'h0;
              gray_msr_cnt <= 'h0;
            end
          else
            begin
              msr_cnt <= msr_cnt + 1'b1;
              
              for(i = 0; i < MSR_CLK_VAL_WIDTH - 1; i = i + 1)
                begin
                  gray_msr_cnt[i] <=  msr_cnt[i+1] ^ msr_cnt[i];
                end
                
              gray_msr_cnt[MSR_CLK_VAL_WIDTH - 1] <=  msr_cnt[MSR_CLK_VAL_WIDTH - 1];
            end
        end
    end
    
  always_ff @(posedge clk_i)
    begin
      if (a_rst_n_i == 1'b0)
        begin
          res_msr_cnt <= 'h0;
        end
      else if (ref_cnt == (REF_CNT_NUM - 1))
         begin
           for(i = 0;  i < MSR_CLK_VAL_WIDTH; i = i + 1)
               res_msr_cnt[i] <= ^(gray_msr_cnt >> i);
         end
      end
    
  always_ff @(posedge clk_i)
    begin
      if (a_rst_n_i == 1'b0)
        begin
          msr_clk_val <= 'h0;
        end
      else
        begin
          msr_clk_val <= DECIM_CLK_MHZ * res_msr_cnt;
        end
    end
    
  always_comb
    begin
       msr_clk_val_o = msr_clk_val;
    end

endmodule

TB:

`timescale 1ps / 1ps


module clock_meter_tb;

  localparam integer MSR_CLK_VAL_WIDTH = 32;
  
  localparam integer CLOCK_100_00_PS = 10000; //ps == 10MHz
  localparam integer CLOCK_184_32_PS = 5425; //ps == 184.32Hz
  localparam integer CLOCK_368_64_PS = 2712; //ps == 368.64MHz
  
  localparam  integer TEST_ITER_NUM  = 1000000000;

  reg clk_100_00     = 'b0;
  reg a_rst_n        = 'b0;

  reg msr_clk_184_32 = 'b0;
  reg msr_clk_368_64 = 'b0;
  
  wire [MSR_CLK_VAL_WIDTH - 1 : 0] msr_clk_val_184_32;
  wire [MSR_CLK_VAL_WIDTH - 1 : 0] msr_clk_val_368_64;

  clock_meter #
  (
    .CLK_MHZ (100)
  )
  clock_meter_dut_184_32
  (
    .clk_i         (clk_100_00        ),
    .a_rst_n_i     (a_rst_n           ),
    
    .msr_clk_i     (msr_clk_184_32    ),
    
    .msr_clk_val_o (msr_clk_val_184_32)
  );
  
  clock_meter #
  (
    .CLK_MHZ (100)
  )
  clock_meter_dut_368_64
  (
    .clk_i         (clk_100_00               ),
    .a_rst_n_i     (a_rst_n           ),
    
    .msr_clk_i     (msr_clk_368_64    ),
    
    .msr_clk_val_o (msr_clk_val_368_64)
  );
  
  always
    begin
      #(CLOCK_100_00_PS / 2) clk_100_00 = !clk_100_00;
    end
  
  always
    begin
      #(CLOCK_184_32_PS / 2) msr_clk_184_32 = !msr_clk_184_32;
    end
    
  always
    begin
      #(CLOCK_368_64_PS / 2) msr_clk_368_64 = !msr_clk_368_64;
    end
  
  initial
    begin
      a_rst_n <= 'b0;
      @(posedge clk_100_00);
      
      a_rst_n <= 'b1;
      @(posedge clk_100_00);
      
      repeat(TEST_ITER_NUM)
        begin
          @(posedge clk_100_00);
        end
        
        $stop();
        
      
    end



endmodule
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Generally speaking, the code has consistent layout, is easy to understand, uses descriptive variable names and makes good use of parameters for constant values.

You have a good separation between sequential logic and combinational logic, and you take advantage of the additional automatic code check offered by the SystemVerilog always_ff and always_comb keywords. These keywords also convey design intent well.

However, I get compile errors with 2 different simulators due to the integer i declaration in the clock_meter module. Perhaps your simulator is more forgiving. Regardless, to make your code more portable, I recommend declaring the iterator variable locally within each for loop.

You can simplify the for loop iterator using the increment operator, ++. Change all:

i = i + 1

to

i++

As an example, for loop, change:

          for(i = 0; i < MSR_CLK_VAL_WIDTH - 1; i = i + 1)

to:

          for (int i = 0; i < (MSR_CLK_VAL_WIDTH - 1); i++)

I also added an extra set of parentheses to avoid potential operator precedence issues and to make the code easier to read.

Use underscores to make large integer values easier to read:

  localparam  integer TEST_ITER_NUM  = 1000000000;

to:

  localparam  integer TEST_ITER_NUM  = 1_000_000_000;

The `timescale compiler directive is not needed for the clock_meter module since there are no delays specified. Also, it can be confusing if you use multiple different time units: 1ps vs. 1ns. It is better to stick to one.

In the testbench (clock_meter_tb), consider replacing the reg types (4-state) with bit types (2-state) for driving the DUT input ports. It is rare to drive the inputs with x or z, and 2-state has simulation performance benefits. bit types default to 0, so there is no need to initialize them (but this is a preference)

  bit clk_100_00    ;
  bit a_rst_n       ;
  bit msr_clk_184_32;
  bit msr_clk_368_64;
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks toolic for your code review. \$\endgroup\$ Commented Aug 25, 2023 at 5:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.