# Error in this Countdown module? (Verilog)

My professor looked at this code for a good 10 minutes, but could not find the problem. So, I'm hoping a fresh pair of eyes will see something both of us missed. As always, I'll be grateful for any hints you can provide.

Context: A module that counts from FFF to 000, then repeats.

Problem: Only lowest 2 displays decrement, skipping several numbers at a time and then jump back up to FF at a seemingly random point.

Assumptions: 1. SevenSegment driver is working properly. It has been checked manually and will display the hex number it is given 2. The ClockDivider module works as intended. The countdown decreases every 1 second as it should (CLOCK_50 = 50 MHz)

Possible Hint: Verilog warns that "cd" is an inferred latch and retains its value through one or more paths in the "always" block

module ClockDivider( input CLOCK_50, output reg[ 31:0] count );
parameter clockDivisor = 50_000_000;

always @( posedge CLOCK_50 )
if ( count == 0)
count <= clockDivisor;
else
count <= count - 1;
endmodule

module Test (

//////////// CLOCK //////////
input                       CLOCK_50,
//////////// SEG7 //////////
output           [6:0]      HEX0,
output           [6:0]      HEX1,
output           [6:0]      HEX2
);

//ClockDivider Output
wire                [31:0]          cout;

reg                 [11:0]            cd;

ClockDivider a( CLOCK_50, cout);

always @(cout)
begin

if (cout == 32'h0)
if (cd == 12'h0)
cd <= 12'hFFF;
else
cd <= (cd - 12'h001);

end

SevenSegment    C2( cd[11:8],  1'b0, HEX2 );
SevenSegment    C1( cd[7:4],   1'b0, HEX1 );
SevenSegment    C0( cd[3:0],   1'b0, HEX0 );

endmodule

• You may be interested in this Stack Overflow question: What is inferred latch and how it is created when it is missing else statement in if condition.can anybody explain briefly? – The Photon Nov 7 '15 at 5:46
• As I see it the basic problem is you have cout in the sensitivity list, but it never appears in the r.h.s. of any of the assignments below it. So what can the tool infer except that you want to use cout as a clock signal for block. – The Photon Nov 7 '15 at 5:48
• @ThePhoton Thank you. So the program was using the least significant bit of cout as clock input. Glad you caught that one. – Meridian Nov 7 '15 at 6:56

Personally, I have had problems with always @(cout) that can be flakey.

I would always use always @(posedge CLOCK_50) . In my opinion, you are guaranteed proper results this way...

EDIT: added clearer code for above suggestion

always @(cout) -> always @(posedge CLOCK_50)


cout should only change with the positive edge of the clock in theory (or code) but what is actually happening maybe different. Your timing report will tell you about setup time and slack - and this may be your problem. change your logic to all "synchronous" and I'll bet you it works.

also, get rid of the inferred latch as good practice. all this would take is an else statement:

if (cout == 32'h0)
if (cd == 12'h0)
cd <= 12'hFFF;
else
cd <= (cd - 12'h001);
else
cd <= cd;


EDIT: suggested working code

module ClockDivider(input CLOCK_50,
output reg[ 0:0] en
);
parameter clockDivisor = 50_000_000;
reg [31:0] i;

always @( posedge CLOCK_50 ) begin
if ( i == clockDivisor ) begin
en <= 1;
i <= 0;
end
else begin
en <= 0;
i <= i + 32'b1;
end
end
endmodule

//1Hz clock output
wire                [0:0]           en;
//Holds the countdown value FFF-000
reg                 [11:0]              cd;

ClockDivider CLOCK_1Hz( CLOCK_50, en);

always @(posedge CLOCK_50)
if (en) begin
if (cd == 12'h0)
cd <= 12'hFFF;
else
cd <= (cd - 12'h001);
end
else
cd <= cd;
end

• johnnymopo, thank you for your reply. The revised code still brought about an inferred latch (see ThePhoton above). And CLOCK_50 (50 MHz) would have been too fast for the countdown. But thanks to your suggestion of using posedge, I managed to figure it out. So wherever you are in this world, I wish the best of luck to you and thanks again for helping out a random stranger. – Meridian Nov 7 '15 at 7:05
• Just to be clear, adding else cd <= cd; still creates a latch; all you've done is make it explicit rather than implicit. In general, combinatorial latches created in this way should be avoided. – Dave Tweed Nov 7 '15 at 12:01
• @dave tweed, how would you write this to avoid a latch at all if you want to make synchronous logic with a common clock domain? cd has to be something.... I ask as I do this and it seems to work well, at least to 125 MHz where I've been working at. But I have no formal vhdl training. – johnnymopo Nov 7 '15 at 15:16
• @johnnymopo, I think your answer would be more clear if you included the always @(<xxx>) that you recommend. – The Photon Nov 7 '15 at 15:43
• Added code suggestion – johnnymopo Nov 7 '15 at 18:38

Thanks to ThePhoton and johhnymopo I was able to figure out the solution. The solution was to have ClockDivisor act as a slower clock itself. The code becomes:

module ClockDivider( input CLOCK_50, output reg[ 0:0] count );
parameter clockDivisor = 50_000_000;
reg [31:0] i;

always @( posedge CLOCK_50 )
begin
i <= i + 32'b1;
if ( i == clockDivisor )
begin
count <= 1;
i <= 0;
end
else
count <= 0;
end

endmodule


In this case, this is a 1Hz Clock. The countdown decrement can then be triggered using the posedge of that slower clock:

//1Hz clock output
wire                [0:0]           slowClock;

//Holds the countdown value FFF-000
reg                 [11:0]              cd;

ClockDivider CLOCK_1Hz( CLOCK_50, slowClock);

always @(posedge slowClock)
begin

if (cd == 12'h0)
cd <= 12'hFFF;
else
cd <= (cd - 12'h001);

end

• Glad it worked. I would personally try to make it all work with one clock. I believe this would infer a new clock and on a larger design this could make timing more difficult. Could you post the exact inferred latch warning? – johnnymopo Nov 7 '15 at 15:31
• @johnnymopo The exact warning was: "inferring latch(es) for variable "cd", which holds its previous value in one or more paths through the always construct". – Meridian Nov 7 '15 at 16:14
• I wonder if this happens because slowClock is not a new clock domain in synthesis? if that were the case, cd changes once per second but still based on the 50 MHz clock, and hence, still a latch - but that seems weird. I still recommend using a common clock for all logic when possible. – johnnymopo Nov 7 '15 at 17:51