Reset If-Else statement produces improper results

Question

I've been trying to debug this code, but I cannot find an error in it. However, it's not producing the results that I expect. Here's the code:

module countAdd( btn, reset, thousands, hundreds, tens, ones, led);

  //Port Assignments
  input [2:0] btn;
  input reset;

  output [0:7] thousands, hundreds, tens, ones;
  output reg led;

  //Internal Port Assignments
  reg [15:0] sum;
  wire [15:0] sumBCD;
  reg [15:0] inc;

  //Add some stuff
  always @(btn or reset)
    begin

        //Determine which button is active
        case(btn)
            3'b110: inc <= 1;
            3'b101: inc <= 10;
            3'b011: inc <= 100;
            default: inc <= 0;
        endcase

        //Add
        sum <= sum + inc;

        //Should we reset?
        if(reset)
            begin
                sum <= 0;
                led <= 1;
            end
        else
            begin
                led <= 0;
            end

    end

  //translate sum to sumBCD
  binToBCD translate (sum, sumBCD[15:12], sumBCD[11:8], sumBCD[7:4], sumBCD[3:0]);

  //display results on SS display
  ssFromBCD seg0 (sumBCD[3:0], ones);
  ssFromBCD seg1 (sumBCD[7:4], tens);
  ssFromBCD seg2 (sumBCD[11:8], hundreds);
  ssFromBCD seg3 (sumBCD[15:12], thousands);
endmodule


module binToBCD(sum, thousands, hundreds, tens, ones);
  //Port Assignments
  input [15:0] sum;
  output reg [3:0] thousands;
  output reg [3:0] hundreds;
  output reg [3:0] tens;
  output reg [3:0] ones;

  //Begin conversion
  integer i;
  always @(sum)
    begin
        //set 1000's, 100's, 10's, and 1's to 0
        thousands = 4'd0;
        hundreds = 4'd0;
        tens = 4'd0;
        ones = 4'd0;

        for(i = 15; i >= 0; i=i-1)
            begin
                //Add 3 to columns >= 5
                if(thousands >= 5)
                    thousands = thousands + 3;
                if(hundreds >= 5)
                    hundreds = hundreds + 3;
                if(tens >= 5)
                    tens = tens + 3;
                if(ones >= 5)
                    ones = ones + 3;

                //Shift left ones
                thousands = thousands << 1;
                thousands[0] = hundreds[3];
                hundreds = hundreds << 1;
                hundreds[0] = tens[3];
                tens = tens << 1;
                tens[0] = ones[3];
                ones = ones << 1;
                ones[0] = sum[i];   
            end

    end 
endmodule

module ssFromBCD(in, ssOut);
  //Port Assignments
  input [3:0] in;
  output reg [0:7] ssOut;

  always @(in)
    begin
        case(in)
            4'b0000: ssOut = 8'b00000011;
            4'b0001: ssOut = 8'b10011111;
            4'b0010: ssOut = 8'b00100101;
            4'b0011: ssOut = 8'b00001101;
            4'b0100: ssOut = 8'b10011001;
            4'b0101: ssOut = 8'b01001001;
            4'b0110: ssOut = 8'b01000001;
            4'b0111: ssOut = 8'b00011111;
            4'b1000: ssOut = 8'b00000001;
            4'b1001: ssOut = 8'b00011001;
            default: ssOut = 8'b11111111;
        endcase
    end
endmodule

This code does not have a testbench; it's instead loaded onto a DE0 board. The idea is that when I press button 1, the sum is supposed to increment by 1. Button 2 increments it by 10, and button 3 increments it by 100. However, the if-else module that encapsulates reset seems to be causing some serious issues.

For instance, when I activate the switch controlling reset, the sum goes to 0 as expected. However, when I try to increment the sum with any of the three buttons, a random number is added - not a 1, 10, or 100.

I've also tried moving the if-else statement before the case statement. Another issue arises. Even if the switch is not activated, the sum is set to 0 as if it ignores the reset condition. However, if I activate the switch, an led turns on. If I turn it off, the led deactivates. However, sum will always be set to 0 regardless of the state of the switch. In this case, though, if I hold a button I see that a 1, 10, or 100 is added to the sum. As soon as I release it, the sum is reset to 0.

Obviously, I believe something is wrong with my if-else statement. I'm not sure why it's not working as I intended. Basically, the logic is as follows:

1. Determine which button is depressed, assign an increment accordingly
2. Add the increment value to sum
3. Should the sum be reset? If yes, set sum to 0.

Note that the last two modules are straightforward. bintoBCD converts the binary string sum into its BCD equivalency. Then ssFromBCD is simply a seven segment display encoder.

Answer 1

A random number is being displayed because sum is continuously being incremented while btn is changing states. A clear start and stop point must be explicit.

Try sampling btn with a clock. In a combination block assign inc. If past_btn equals btn, then assign inc to zero. sum should be assigned in a clocked block.

alwasy @* begin
 if (past_btn == btn) inc = 'b0;
 else begin
   // case(btn) ...
 end
end

always @(posedge clk) begin
  past_btn <= btn;
  if (!reset)  sum <= 'b0;
  else         sum <= sum + inc;
end

If you are still getting random numbers, then the buttons are likely not denounced. In this case add another stage to the button sampling and use only the sampled values.

alwasy @* begin
 if (past_btn == sync_btn) inc = 'b0;
 else begin
   // case(sync_btn) ...
 end
end

always @(posedge clk) begin
  sync_btn <= btn;
  past_btn <= sync_btn;
  if (!reset)  sum <= 'b0;
  else         sum <= sum + inc;
end

Always make a test bench and run in simulation. Debugging from FPGA is difficult with logic bugs and implementation issues.

Answer 2

It depends what you want from the following line:

always @(btn or reset)

Combinatorial logic or Sequential (flip-flops)?

Combinatorial use always @* or always_comb if SystemVerilog is supported. The automatic sensitivity list behaves as combinatorial hardware does cutting down on simulation mismatches.

Also use = Blocking assignments for combinatorial.

Sequential use something like always @(posedge clk or negedge rst) and do use <= This will allow simulation to match hardware.

---

You issues look like they could be coming from sum <= sum + inc; which is then followed by optional reset statements. The use of <= is incorrect here as it is a combinatorial block with a manual sensitivity list. The scheduling of <= assignments might be throwing some thing off.

As it stands think about what hardware you might expect from :

always @* begin
  sum = sum + 1;
end

Against what you get from:

always @(posedge clk or negedge rst_n) begin
  if (~rst) begin
    sum <= 'b0;
  end
  else begin
    sum <= sum + 1;
  end
end

I think if you add a clock to your design and imply a flip-flop for the counter (sum) you will have a big step in the right direction.

Answer 3

This code does not have a testbench

Firstly: Create a testbench! It's not difficult (you can even do it online) and for logical errors it is way faster to debug in simulation than iterating on real hardware.

Secondly: There's no clock in your code. Is this the top level? FPGAs aren't very good at asynchronous design particularly if you are trying to store state (in this case sum). It's not clear what Quartus will have inferred - it might have hooked one of your inputs up to a clock network. You should check the synthesised design in the RTL viewer and go through the synthesis logs.

However, when I try to increment the sum with any of the three buttons, a random number is added

Are you debouncing your switch input? If you aren't then it's very likely that for each button press you'll get a random number of toggles.

Note that your sensitivity list is incorrect here - it should include sum too:

always @(btn or reset)

Ironically (given my opening comment) this won't affect synthesis but would change the behaviour in simulation. You're better off using always_comb (or always @(*) if you're stuck with Verilog).