Search code examples
verilogvivadosynthesis

Vivado linter: inferred latch for signal 'out_reg'

I am new to Verilog. I want to write a simple module to do clock dividing without using PLL. The module is named "uart_brg" since I plan to use it later in an uart module for exercise purpose.

I set the source file uart_brg.v as the top module in Vivado project. The behavior simulations seem ok, but Vivado Linter (2023.1) reports two issues:

  1. clk: Found bit(s) not read for IO port 'clk', first unread bit 0.
  2. out_reg: Inferred latch for signal out_reg.

The first issue is a kind of expected, although I am not sure if there are any risk if I just ignore it.

My question is about the second issue: I did assign to out in every branch. Why does it still complain?

The following is the code. Note that in an attempt to keep the out clock of 50% duty cycle, and also to support 1:1 clock ratio, I increment the count at both rising and falling edges of the input clock, thus the sensitivity list is (clk, rst_n).

module uart_brg #(
    parameter DIVISOR_WIDTH = 16                       // 16-bit integer, per 16550 standard
) (
    input  wire                             clk,       // input clock
    input  wire                             rst_n,     // reset
    input  wire [DIVISOR_WIDTH - 1 : 0]     divisor,   // 
    output reg                              out        // output clock
);

reg unsigned [DIVISOR_WIDTH : 0] count = 0; // count for half clk peroids
// to make duty cycle of out exactly 50%:
reg unsigned [DIVISOR_WIDTH : 0] div = 1;   // local divisor
reg unsigned [DIVISOR_WIDTH : 0] div2 = 2;  // double of div

always @(clk, rst_n) begin // use level-sensitive of both signals
    // make sure that divisor is > 0
    if (divisor > 0) 
        div <= divisor;
    else
        div <= 1;
    div2 <= {div, 1'b0};
    
    if (~rst_n) begin
        count <= {(DIVISOR_WIDTH+1){1'b0}};
        out <= 1'b0;
    end else begin
        if (div == div2 - 1) begin // special case: 1:1 clock ratio
            out <= ~out;
            count <= {(DIVISOR_WIDTH+1){1'b0}};  // preventing "inferred latch for signal xxx"
        end else begin
            count <= count + 1;
            if (count == 0 || count == div) begin
                out <= ~out;
            end else begin
                out <= out;  
                if (count == (div2 - 1))
                    count <= 0;
            end
        end
    end        
end

endmodule

This is the screen shot of behavior simulation when divisor is set to 3:

enter image description here

Please give some hints on the reported issues.

Update:

Based on advices from @Mikef and @toolic, the revised module is much simpler without error report from Vivado Linter.

module uart_brg #(
    parameter DIVISOR_WIDTH = 16                       // 16-bit integer, per 16550 standard
) (
    input  wire                             clk,       // input clock
    input  wire                             rst_n,     // reset
    input  wire [DIVISOR_WIDTH - 1 : 0]     divisor,   // >=2
    output reg                              out        // output clock
);

reg [DIVISOR_WIDTH - 1 : 0] count = 0;

/*
 * the following block is not necessary. when divisor=1,
 * the output is the same as divisor=2, upon simulation.
 */
// make sure that divisor is > 1
//reg [DIVISOR_WIDTH - 1 : 0] div = 2;   // local divisor
//always @(*) begin
//    if (divisor > 1)
//        div = divisor;
//    else
//        div = 2;
//end

// update count
always @(posedge clk, negedge rst_n) begin
    if (~rst_n) 
        count <= {DIVISOR_WIDTH{1'b0}};
    else begin
        if (count == divisor - 1) 
            count <= 0;
        else
            count <= count + 1;
    end    
end

// update output
always @(posedge clk, negedge rst_n) begin
    if (~rst_n) 
        out <= 1'b0;
    else begin
        if (count == 0 || count == divisor / 2)  
            out <= ~out;
    end        
end

endmodule

The only "limitation" is that, for odd divisor values, the duty cycle of the output will be slightly less than 50%. I guess this should not be a problem for uart tx/rx modules.

Solution

  • Always blocks have the ability to infer registers or combinational logic; the syntax is different for each purpose.

    The code in the post always @(clk, rst_n) infers combinational logic.

    To model registers, use always @(posedge clk, negedge rst_n)
    This style infers registers with asynchronous reset.

    The posted code seems designed to model registers because of the use of non blocking <= assignments. However, the keyword posedge is missing, so combinational logic is inferred which is subject to latches. Add the keyword posedge to eliminate latches.

    To model combinational logic, use always @(*) with blocking assignments = rather than non blocking <= assignments.

    These concepts (processes which infer comb logic vs registers, and blocking vs non blocking assignments) are mixed up in the post because of the use of always @(clk, rst_n) (infers combinational logic) with non blocking assignments (should be used in processes intended to infer registers).

    Its fine or even appropriate to omit assignments (for example the else part of an if statement) to variables that are correctly modeled as registers, because it holds its value until the next clock edge by design. When the design intent is clear that a register is modeled, then the tools will not infer latches.

    After removing the unsigned keyword (default is unsigned), and adding the posedge and negedge keywords needed to model synchronous logic\w active low asynchronous reset like this:

    // CODE SNIP
reg  [DIVISOR_WIDTH : 0] count = 0; // count for half clk peroids
// to make duty cycle of out exactly 50%:
reg  [DIVISOR_WIDTH : 0] div = 1;   // local divisor
reg  [DIVISOR_WIDTH : 0] div2 = 2;  // double of div

always @(posedge clk, negedge rst_n) begin // +edge trig flop\w async active low reset    
//CODE SNIP

    Vivado Synthesis on the modified post produces:

    ...
synthesis finished with 0 errors, 0 critical warnings and 0 warnings.
Synthesis Optimization Runtime : Time (s): cpu = 00:00:14 ; elapsed = 00:00:25 . Memory (MB): peak = 2448.074 ; gain = 56.746 ; free physical = 107522 ; free virtual = 203345
Synthesis Optimization Complete : Time (s): cpu = 00:00:14 ; elapsed = 00:00:25 . Memory (MB): peak = 2448.074 ; gain = 56.746 ; free physical = 107522 ; free virtual = 203345
INFO: [Project 1-571] Translating synthesized netlist
Netlist sorting complete. Time (s): cpu = 00:00:00 ; elapsed = 00:00:00 . Memory (MB): peak = 2455.000 ; gain = 0.000 ; free physical = 107608 ; free virtual = 203431
INFO: [Netlist 29-17] Analyzing 16 Unisim elements for replacement
INFO: [Netlist 29-28] Unisim Transformation completed in 0 CPU seconds
INFO: [Project 1-570] Preparing netlist for logic optimization
INFO: [Opt 31-138] Pushed 0 inverter(s) to 0 load pin(s).
Netlist sorting complete. Time (s): cpu = 00:00:00 ; elapsed = 00:00:00 . Memory (MB): peak = 2596.621 ; gain = 0.000 ; free physical = 107500 ; free virtual = 203324
INFO: [Project 1-111] Unisim Transformation Summary:
No Unisim elements were transformed.

INFO: [Common 17-83] Releasing license: Synthesis
14 Infos, 0 Warnings, 0 Critical Warnings and 0 Errors encountered.
...

    Answering follow-up question asked in the comments here:

    Why it's generally not recommended to put both edges into the sensitivity list?

    In Verilog synthesis deign flows, designers model physical hardware. Physical flip flops are either positive edge triggered on clk or negative edge triggered on clk, not both. Same with level sensitive latches,. they are activated by either a logic high or logic low, not both.