Verilog - digital clock- Minute doesnt work

Question

im implementing a digital clock with verilog. I count clk's and so count seconds. Then i sent outputs to seven segment display. My second display works perfectly, but minute's doesnt work . Some times , it displays like , in first increase 60, in second 2 , third 45 60 anything, fourth 4.

I created split module which takes 1 input give 2 output. Like

if input = 56 , output 1 = 5 , output 2=6 . Works perfect in simulation and for second.

    `timescale 1ns / 1ps
//////////////////////////////////////////////////////////////////////////////////
// Company: 
// Engineer: 
// 
// Create Date:    23:36:40 11/05/2015 
// Design Name: 
// Module Name:    Top 
// Project Name: 
// Target Devices: 
// Tool versions: 
// Description: 
//
// Dependencies: 
//
// Revision: 
// Revision 0.01 - File Created
// Additional Comments: 
//
//////////////////////////////////////////////////////////////////////////////////
module Top(input clk,reset,input in0, in1, in2, in3,output a, b, c, d, e, f, g, dp,output [3:0] an
    );
     wire [3:0] minLeft,minRight;
     wire [3:0] secLeft,secRight;

    wire [6:0] second,minute;
    wire[4:0] hour;
    wire newDay;

    split_output sec(second,secLeft,secRight);
    split_output split(minute,minLeft,minRight);
    Clock timer(clk,second,minute,hour,newDay);

    sevenseg  decoder(clk,reset,minLeft,minRight,secLeft,secRight,a,b,c,d,e,f,g,dp,an);

endmodule

CLOCK MODULE

`timescale 1ns / 1ps
//////////////////////////////////////////////////////////////////////////////////
// Company: 
// Engineer: 
// 
// Create Date:    21:26:35 11/05/2015 
// Design Name: 
// Module Name:    Clock 
// Project Name: 
// Target Devices: 
// Tool versions: 
// Description: 
//
// Dependencies: 
//
// Revision: 
// Revision 0.01 - File Created
// Additional Comments: 
//
//////////////////////////////////////////////////////////////////////////////////
module Clock(input clk,output [6:0] second,minute,output [4:0] hour,output  reg newDay
    );

     //Clock counter for second
      reg [25:0]cnt_clk=0;
     //Second counter
     reg [6:0]cnt_second=0;
     //Minutes counter
     reg [6:0]cnt_minute=0;
     //Hour counter
     reg [4:0]cnt_hour=0;

     assign second=cnt_second;
     assign minute=cnt_minute;
     assign hour=cnt_hour;


    //COUNT CLOCK, INCREASE SECOND
    always@(*)
    begin
        // IF CLOCK COUNT İS 1 SECOND
        if(cnt_clk==26'd5000000)
            begin
                cnt_clk=26'd0;

                // IF SECOND COUNT İS 60, RESET İT
                if(cnt_second==7'b0111100)
                    begin
                        cnt_second<=7'b0000000;
                    end
                else
                    begin
                        cnt_second<=cnt_second+1;
                    end
            end
        else
            begin
                cnt_clk=cnt_clk+1;
            end
    end

    // UPDATE MİNUTES, AS SECONDS INCREASE
    always@(cnt_second)
    begin
        //IF ITS 1 MINUTES
        if(cnt_second==7'd60)
            begin

                if(cnt_minute==7'd60)
                begin
                    cnt_minute<=0;
                end
                else
                    begin
                            cnt_minute=cnt_minute+1;
                    end
            end 
    end

    //UPDATE HOURS,AS MİNUTES INCREASE

    always@(cnt_minute)
    begin
        //IF ITS 60 MINUTES
        if(cnt_minute==7'b0111100)
            begin

                if(cnt_hour==5'b11000)
                    begin
                        cnt_hour<=5'b00000;
                    end
                else
                    begin
                            cnt_hour<=cnt_hour+1;
                    end
            end         
    end


    //  IF THE DAY İS OVER
    always@(cnt_hour)
    begin
        if(cnt_hour==5'b11000)
            begin
                newDay=1;
            end
        else
            begin
                newDay=0;
            end

    end




endmodule

SPLİT MODULE

`timescale 1ns / 1ps
//////////////////////////////////////////////////////////////////////////////////
// Company: 
// Engineer: 
// 
// Create Date:    18:51:22 11/09/2015 
// Design Name: 
// Module Name:    split_output 
// Project Name: 
// Target Devices: 
// Tool versions: 
// Description: 
//
// Dependencies: 
//
// Revision: 
// Revision 0.01 - File Created
// Additional Comments: 
//
//////////////////////////////////////////////////////////////////////////////////
module split_output(input [7:0] total,output reg[3:0] left,right
    );


     always@(total)
     begin


        if(total>=8'b00110010&&total<8'b00111100)
        begin
            assign left=4'b0101;
            assign right=total-50;
        end


         if(total>=8'b00101000&&total<8'b00110010)
        begin
            assign left=4'b0100;
            assign right=total-40;
        end

        if(total>=8'b00011110&&total<8'b00101000)
        begin
            assign left=4'b0011;
            assign right=total-30;
        end

        if(total>=8'b00010100&&total<8'b00011110)
        begin
            assign left=4'b0010;
            assign right=total-20;
        end

        if(total>=8'b00001010&&total<8'b00010100)
        begin
            assign left=4'b0001;
            assign right=total-10;
        end
        if(total<8'b00001010)
        begin
            assign left=0;
            assign right=total;
        end
        if(total==8'b00111100)
        begin
            assign left=4'b0110;
            assign right=0; 
        end



     end

     endmodule

7seg decoder- i found in that site- it works perfect( THANKS TO WHO PUBLİSHED AGAİN)

`timescale 1ns / 1ps
//////////////////////////////////////////////////////////////////////////////////
// Company: 
// Engineer: 
// 
// Create Date:    23:31:47 11/05/2015 
// Design Name: 
// Module Name:    sevenseg 
// Project Name: 
// Target Devices: 
// Tool versions: 
// Description: 
//
// Dependencies: 
//
// Revision: 
// Revision 0.01 - File Created
// Additional Comments: 
//
//////////////////////////////////////////////////////////////////////////////////

module sevenseg(
 input clock, reset,
 input [3:0] in0, in1, in2, in3,  //the 4 inputs for each display
 output a, b, c, d, e, f, g, dp, //the individual LED output for the seven segment along with the digital point
 output [3:0] an   // the 4 bit enable signal
 );

localparam N = 18;

reg [N-1:0]count; //the 18 bit counter which allows us to multiplex at 1000Hz

always @ (posedge clock or posedge reset)
 begin
  if (reset)
   count <= 0;
  else
   count <= count + 1;
 end

reg [6:0]sseg; //the 7 bit register to hold the data to output
reg [3:0]an_temp; //register for the 4 bit enable

always @ (*)
 begin
  case(count[N-1:N-2]) //using only the 2 MSB's of the counter

   2'b00 :  //When the 2 MSB's are 00 enable the fourth display
    begin
     sseg = in0;
     an_temp = 4'b1110;
    end

   2'b01:  //When the 2 MSB's are 01 enable the third display
    begin
     sseg = in1;
     an_temp = 4'b1101;
    end

   2'b10:  //When the 2 MSB's are 10 enable the second display
    begin
     sseg = in2;
     an_temp = 4'b1011;
    end

   2'b11:  //When the 2 MSB's are 11 enable the first display
    begin
     sseg = in3;
     an_temp = 4'b0111;
    end
  endcase
 end
assign an = an_temp;


reg [6:0] sseg_temp; // 7 bit register to hold the binary value of each input given

always @ (*)
 begin
  case(sseg)
   4'd0 : sseg_temp = 7'b1000000; //to display 0
   4'd1 : sseg_temp = 7'b1111001; //to display 1
   4'd2 : sseg_temp = 7'b0100100; //to display 2
   4'd3 : sseg_temp = 7'b0110000; //to display 3
   4'd4 : sseg_temp = 7'b0011001; //to display 4
   4'd5 : sseg_temp = 7'b0010010; //to display 5
   4'd6 : sseg_temp = 7'b0000010; //to display 6
   4'd7 : sseg_temp = 7'b1111000; //to display 7
   4'd8 : sseg_temp = 7'b0000000; //to display 8
   4'd9 : sseg_temp = 7'b0010000; //to display 9
   default : sseg_temp = 7'b0111111; //dash
  endcase
 end
assign {g, f, e, d, c, b, a} = sseg_temp; //concatenate the outputs to the register, this is just a more neat way of doing this.
// I could have done in the case statement: 4'd0 : {g, f, e, d, c, b, a} = 7'b1000000;
// its the same thing.. write however you like it

assign dp = 1'b1; //since the decimal point is not needed, all 4 of them are turned off


endmodule

MY UCF

NET "reset" LOC = "a7";

# Pin assignment for 7-segment displays
NET "a" LOC = "l14" ;
NET "b" LOC = "h12" ;
NET "c" LOC = "n14" ;
NET "d" LOC = "n11" ;
NET "e" LOC = "p12" ;
NET "f" LOC = "l13" ;
NET "g" LOC = "m12" ;
NET "dp" LOC = "n13" ;

NET "an[0]" LOC = "k14";
NET "an[1]" LOC = "m13";
NET "an[2]" LOC = "j12";
NET "an[3]" LOC = "f12";

# Pin assignment for clock
NET "clk" LOC = "b8";

There is some nasty latching logic and bad coding practice in Clock and split_output. Re-code them using sevenseg as a coding style reference. — Greg
Anytime you have always @(*), you should not use NBA (<=) nor should it be non-combinational logic. If you mean to have a register/ff, ie a value that lasts across clock cycles such as a counter, you need to use always @(posedge clk) and NBA to assign it. The Clock module does not do this, so it likely will not work on an FPGA. Also, the split_output module uses assign incorrectly. — Unn
Please, can you be more specific with examples? I'm a newbie in verilog, just trying to learn my self. Its my course project. Did not have lectures about it.Thank you. — Ferit Özcan

Greg Greg · Accepted Answer · 2015-11-10T19:48:42

Blocking (=) vs non-blocking (<=) been answered may times:

Latches are inferred when a reg is not assigned in all possible branches within a otherwise conbinational block. If you want latches, put them in a separate always block, away from the combinational logic, use non-blocking (<=) assignments, and keep them as simple assignments. Doing so will remove the confusion of what is intended to be decoding logic, level sensitive latches, and edge sensitive flip-flops. Combinational loops are another hazard. This is where you get different results if you run the same combinational block two or more times with the same inputs in the same time stamp and get different outputs.

When it comes to RTL coding style, I typically follow Cliff Cummings' recommendations, such as:

There are additional links to useful Verilog/SystemVerilog references in my profile. I personally do not like nested conditional statements (?:). From experience, I get better synthesis results using case statements when there are more then two possibilities.

Here is an incomplete example how I would code Clock and split_output. I'll leave the rest for you figure out and learn on your own.

module Clock(
    input clk,
    output reg [5:0] second, minute,
    output reg [3:0] hour,
    output reg newDay 
  );

  // ... declare local regs

  // SYNCHRONOUS ASSIGNMENTS
  always @(posedge clk) begin
    cnt_clk <= next_cnt_clk;
    second <= next_second;
    // ... other assignments
  end

  // COMBINATIONAL CALCULATIONS
  always @* begin
      // DEFAULT VALUES
      next_cnt_clk = cnt_clk + 1;
      next_second = second;
      // ... other default

      // IF CLOCK COUNT İS 1 SECOND
      if (next_cnt_clk == 24'd5000000) begin
        next_cnt_clk = 24'd0;
        next_second = second + 1;
      end

      // IF SECOND COUNT İS 60, RESET İT
      if (next_second == 6'd60) begin
        next_second = 6'd0;
        next_minute = minute + 1;
      end

      // ... other calculations
  end
endmodule

module split_output(
    input [5:0] total,
    output reg [3:0] left, right
  );

  always @* begin
    if (total < 8'd10) begin
        left  = 4'b0000;
        right = total[3:0];
    end
    else if (total < 8'd20) begin
        left  = 4'b0001;
        right = total-10;
    end
    // ... other 'else if'
    else begin // final is 'else'
        left  = 4'b0110;
        right = 4'b0000; 
    end
  end
endmodule

Verilog - digital clock- Minute doesnt work

1 Answers