cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 
kavinduvsomadas
Contributor
Contributor
561 Views
Registered: ‎08-28-2020

Gated clock warning

Jump to solution

Hello, I created a cutom IP named WEIGHT_INPUT_PROVIDER but it unexpectedly issue following warning message during implementation.

[DRC PDRC-153] Gated clock check: Net design_1_i/WEIGHT_INPUT_PROVIDER_0/inst/selection_inst/sel_reg_reg[1]_i_2_n_0 is a gated clock net sourced by a combinational pin design_1_i/WEIGHT_INPUT_PROVIDER_0/inst/selection_inst/sel_reg_reg[1]_i_2/O, cell design_1_i/WEIGHT_INPUT_PROVIDER_0/inst/selection_inst/sel_reg_reg[1]_i_2. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.

I checked the source code (given below - from selection.v file) for the selection_inst which leads to this issue. But I can't find any gated clock there. Can anyone please help me to find out the issue and improve it ? And please inform me if there are any bad coding techniques I used.

 

module selection(clk, rst, sel, ready, valid, sel_0, sel_1, sel_2, SEL_EN, count_data_start, count_data, count_data_max, weight_count_max_enable, count_data_max_enable);

input wire clk, rst;
input wire ready, valid;

output wire [1:0] sel;

wire trans_enable;

reg [1:0] sel_reg;
reg [3:0] count_reg;

input wire [1:0] sel_0, sel_1, sel_2; // selection made under FORWARD PASS CONVOLUTION or BACKWARD PASS LOCAL GRAD CONVOLUTION
input wire SEL_EN;

// COUNT WEIGHT DATA
input wire count_data_start;
input wire [7:0] count_data_max;

output reg weight_count_max_enable;
output reg count_data_max_enable;
output reg [7:0] count_data;

assign sel = sel_reg;
assign trans_enable = ready & valid;

always @(*) begin

        if(count_data_max_enable) weight_count_max_enable <= 1'b1;
        else weight_count_max_enable <= 1'b0;
        
        case(count_reg)   
        4'd0: sel_reg <= sel_0; 
        4'd1: sel_reg <= sel_0;
        4'd2: sel_reg <= sel_0;
        
        4'd3: sel_reg <= sel_1;
        4'd4: sel_reg <= sel_1;
        4'd5: sel_reg <= sel_1;
        
        4'd6: sel_reg <= sel_2;
        4'd7: sel_reg <= sel_2;
        4'd8: sel_reg <= sel_2;
        
        4'd9: sel_reg <= sel_0;
      endcase
end

always @(posedge clk, negedge rst) begin
    if(~rst) begin
        count_data <= 8'd0;
        count_reg <= 4'd0;
        count_data_max_enable <= 1'b0;
    end
    else if(count_data_start) begin
        count_reg <= 4'd0;
        count_data <= 8'd0;
        count_data_max_enable <= 1'b0;
    end
    else begin
    
      if(count_reg == 4'd9 && trans_enable) begin
        count_reg <= 4'd1; // CLOCK AT COUNT_REG = 9 SHOULD NOT BE WASTED
      end
      else if(trans_enable) begin
        count_reg <= count_reg + 1'b1;
      end
      else begin
        count_reg <= count_reg;
      end
      
      count_data <= (trans_enable && ~count_data_max_enable) ? count_data + 1'b1 : count_data;
      
      if(count_data == count_data_max && count_data_max != 8'd0) count_data_max_enable <= 1'b1;
      else count_data_max_enable <= count_data_max_enable;
      
    end
   
end
endmodule

 

Following are the important verilog modules of the complete codebase for the IP I created. 

For implementation what I did was first I created the IP and then created a block design containing zynq processor and connected the IP, and run the implementation. It was completed successfully but ends up with above warning. 

0 Kudos
Reply
1 Solution

Accepted Solutions
dgisselq
Scholar
Scholar
499 Views
Registered: ‎05-21-2015

@kavinduvsomadas ,

A real quick lint check using Verilator (it's much faster than Vivado) reveals two problems with selection.v:

  1. You have used non-blocking assignments (<=) in a combinatorial block when setting sel_reg.  Combinatorial logic should use blocking assignments.  (=)
  2. You haven't completely covered all cases in your case statement.  This will cause a latch to be inferred and other problems (possibly this one) will ensue.  You can fix this by either creating a default: case selection, or by defining sel_reg prior to the case statement.
  3. A third "problem", that "SEL_EN" isn't used, might be just a result of how you copied and pasted the code for the forum.

Dan

View solution in original post

4 Replies
dgisselq
Scholar
Scholar
500 Views
Registered: ‎05-21-2015

@kavinduvsomadas ,

A real quick lint check using Verilator (it's much faster than Vivado) reveals two problems with selection.v:

  1. You have used non-blocking assignments (<=) in a combinatorial block when setting sel_reg.  Combinatorial logic should use blocking assignments.  (=)
  2. You haven't completely covered all cases in your case statement.  This will cause a latch to be inferred and other problems (possibly this one) will ensue.  You can fix this by either creating a default: case selection, or by defining sel_reg prior to the case statement.
  3. A third "problem", that "SEL_EN" isn't used, might be just a result of how you copied and pasted the code for the forum.

Dan

View solution in original post

joancab
Scholar
Scholar
485 Views
Registered: ‎05-11-2015

 

You have a combinatorial process, that always @(*). Because there is no clock, data is registered by the conditions (gates). That's not the best by far... I'd suggest making that process synchronous to a clock. And obviously, include all cases for the 'case' statement to not inferring latches.

kavinduvsomadas
Contributor
Contributor
447 Views
Registered: ‎08-28-2020

@dgisselq Thank you for answering my issue.

The main issue I had was having an inferred latch that comes with avoiding default case in the combinational logic. And other issues you mentioned will also be corrected to follow proper coding techniques. 

0 Kudos
Reply
kavinduvsomadas
Contributor
Contributor
445 Views
Registered: ‎08-28-2020

@joancab Thank you for answering my question.

For my case, defining default case is sufficient to avoid the issue as @dgisselq mentioned.

0 Kudos
Reply