Verilog code, same structure, same style. How come one works but the other doesnt? Where did I go wrong?

Question

I will get straight to the point. I have a simple counter that is trying to mimic how a clock works pretty much. I have a module called counter60sec and another one called counter12hr

counter12hr

    module counter12hr(reset, hourInc, overflowOut, hrCounter);
        input reset;
        input hourInc;
        output overflowOut;
        output [3:0] hrCounter;
        reg overflowOut;
        reg [3:0] hrCounter; //0'b1101 == 13 hours

        //Initialize counter
        initial begin
            overflowOut = 1'b0;
            hrCounter = 4'b0; //once hour reaches 12:59:59, it is supposed to go back to zero
            end
        //Everytime hrInc is one, increment hrCounter
        always@(negedge reset or posedge hourInc) begin
            overflowOut = 1'b0;
            if(reset == 1'b0) begin
                overflowOut = 1'b0;
                hrCounter = 4'b0;
            end
            else begin
                if (hourInc == 1'b1) begin
                    hrCounter = hrCounter + 1'b1;
                end 
            end
        end

        always@(negedge hrCounter) begin
            if (hrCounter == 4'b1100) begin
                overflowOut = 1'b1;
                hrCounter = 4'b0;
            end
        end
    endmodule

counter60sec

    module counter60sec(reset, secInc, minOut, secCounter);
        input reset;
        input secInc;
        output minOut;
        output [5:0] secCounter;
        reg [5:0] secCounter; //0'b111100 == 60 seconds. 
        reg minOut;

        //Initialize counter
        initial begin
            minOut = 1'b0;
            secCounter = 6'b0;
            end
        //Everytime secInc is one, increment secCounter
        always@(posedge secInc or negedge reset) begin
            minOut = 1'b0;
            if(reset == 1'b0) begin
                minOut = 1'b0;
                secCounter = 6'b0;
            end
            else begin
                if (secInc == 1'b1) begin
                    secCounter = secCounter + 1'b1;
                end
            end
        end

        //output minOut to 1 to signal minute increase when secCounter hits 111100 in binary
        always@(negedge secCounter) begin
            if(secCounter == 6'b111100) begin
                minOut = 1'b1;
                secCounter = 6'b0;
            end
        end
    endmodule

I have test bench set up for both. The counter60sec one works fine (Where when secCounter is at value of 60, the minOut becomes 1). The counter12hr follows the same concept, but the value of overflowOut never becomes 1.

For my hrCounter conditional statement in counter12hr.v, I have tried both 4'b1101 and 4'b1100 and neither of them worked. (Trying to get the overflowOut to become one when the hrCounter hits 13)

I've spent hours on this, taking break to relax my eyes etc. I still can't see where I am going wrong. Any help would be appreciated

Serge · Accepted Answer · 2017-12-16 14:52:09Z

you have a few issues. The main one is that you have a multiple-driven register in both cases:

    always@(negedge reset or posedge hourInc) begin
        overflowOut = 1'b0;
        ...

    always@(negedge hrCounter) begin
        if (hrCounter == 4'b1100) begin
            overflowOut = 1'b1;

the overflowOut is driven from 2 different alsways blocks. Same as minOut in the second counter. The order in which those statements are executed is undefined and the resulting value would depend on the order.

You need to restructure your program in such a way that the registers are assigned in a single always block only.

Secondly, i think, that you have a logic bug, assigning your overflow to '0' in the first statement of the first block (same as in the second one).

Thirdly, you should have uset non-blocking assighments '<=' in finlal assignments to the registers.

something like the following should work.

  always@(negedge reset or posedge hourInc) begin
        if(reset == 1'b0) begin
            hrCounter <= 4'b0;
            overflowOut <= 1'b0;
        end
        else begin
          if (hrCounter == 4'b1100) begin 
            overflowOut <= 1'b1;
            hrCounter <= 4'b0;
          end
          else 
            hrCounter <= hrCounter + 1'b1;
            overflowOut <= 1'b0;
          end 
       end
   end

Oldfart · Accepted Answer · 2017-12-16 08:03:50Z

Your code is wrong in so many ways...
Most likely it does not work because of:

@(negedge hrCounter)

You are using a vector but @... works with bits. So it will look at the LS bit only.

As to your code:first and most start using a clock. Do not use posedge and negedge of your signals to drive the other signals.
You use:

always@(posedge secInc ...
   ...
   if (secInc == 1'b1)

Remove the 'if'. you have that condition already in your always statement. (But the signal should not be the 'always' anyway, it should be your clock)
Remove the 'initial' sections You have a reset which defines the start condition.
If you have an active low reset reflect that in the name. Call it reset_n or n_reset anything but 'reset' which is per convention a positive reset signal.
If your drive ANYTHING from an edge, be it from a clock or from other signals, use non-blocking assignments "<="
Do not use your generated signal edges to generate other signals. As mentioned use a clock for everything an in the clocking section use an if. Also do not drive signals from different 'always'. You can never know in which order they are executed and then you have a race condition. Thus the clear and the increment are all in one always block:

if (secCounter == 6'b111100) begin
   minOut <= 1'b1;
   secCounter <= 6'b0;
end
else
   secCounter <= secCounter  + 6'b000001;

Because of the timing aspect you now have to go to 59 not 60. Which is as you should expect as digital clocks and watches run from 00 to 59, not 00 to 60. You are allowed to use decimal numbers which will, again, make the code more readable: 6'd59, 4'd11

Collectives™ on Stack Overflow

Verilog code, same structure, same style. How come one works but the other doesnt? Where did I go wrong?

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related