Error in this Countdown module? (Verilog)

Question

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

Answer 1

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

Answer 2

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