Verilog counting problem - posedge detector with if statement not working as expected

Question

First I am doing reset with reset input. Then I check the led position by turning on pinA. Next, a shift posedge to switch to the next led but the counting not going to its next led.

After doing reset from "00" it goes to "10" or "11" on shift posedge. But as I am adding just 1 so that should have gone to "01"

Where am I going wrong?

module syScore(
 input clk,
 input shift,
 input enb,
 input reset,
 input pinA,
 input pinB,
 input pinC,
 input pinD,
 input pinE,
 output reg out1,
 output reg out2,
 output reg out3,
 output reg out4,
 output reg out5,
 output reg out6,
 output reg out7,
 output statusBit
);
reg [1:0] enPosition;
reg out;
wire w1,w2;
wire w3,w4;
reg j;
always @ (posedge clk) begin
 // posedge detector block with if statement
 if (j == 0 && shift == 1) begin
 j<=1;
 enPosition <= enPosition + 2'b01;
 out3 <= 1;
 end
 if (shift == 0) begin
 j<=0;
 out3 <= 0;
 end
 if (reset == 1) begin
 enPosition <= 2'b00;
 out1 <= 0;
 out2 <= 0;
 out3 <= 0;
 out4 <= 1;
 out6 <= 0;
 out7 <= 0;
 j <= 0;
 end
 else begin
 out4 <= 0;
 end
 case(enPosition)
 2'b00: out1 <= statusBit;
 2'b01: out2 <= statusBit;
 2'b10: out6 <= statusBit;
 2'b11: out7 <= statusBit;
 default: out5 <= statusBit;
 endcase
end
assign statusBit = pinA;
endmodule

• nanofarad: I don't really understand the intent here. If you're targeting an FPGA, why don't you either oversample the signal using an existing clock, or (if it's going to a GCLK pin and is suitably debounced and conditioned) just use it as a clock directly?
• Mitu Raj: Because there is no clock in your design to 'count' and 'shift'.
• Mitu Raj: The circuit NEEDS clock regardless of what happened when you used clock. That's secondary. Update you question with details/code.
• tim: Are you trying to detect a positive edge on the shift input when j is zero? Where does shiftcome from? Is it debounced?
• Mitu Raj: Maybe you should explain what each signals are on your code. It is not clear what is the end-objective.

Answer 1

I'm having difficulty understanding your intent, but I think you are trying to detect a positive edge on the shift input, and then use that edge to increment a counter. The counter is then used to connect the statusBit input to different outputs, i.e. out7, out6, out2, out1.

Firstly, you need a clear demarcation between the reset condition and normal operation:

always @(posedge clk)
begin
 if (reset == 1)
 begin
 // Do reset condition.
 end
 else
 begin
 // Do normal operation.
 end
end

Secondly, you need a to detect the positive edge on shift. If shift is coming from an external button or switch, it needs to be debounced. The positive edge detector could be implemented like this:

// Positive edge detection of shift input.
assign shift_posedge = (shift == 1) & (shift_previous == 0);

...where shift_previous is a single bit register that is assigned the value of shift every clock cycle in "normal operation" in the above always block:

shift_previous <= shift;

Lastly, you need to decode the counter value to connect the statusBit input to the correct output, and zero the other outputs:

always @*
begin
 case (enPosition)
 2'b00: {out7, out6, out2, out1 } = { 1'b0, 1'b0, 1'b0, statusBit};
 2'b01: {out7, out6, out2, out1 } = { 1'b0, 1'b0, statusBit, 1'b0};
 2'b10: {out7, out6, out2, out1 } = { 1'b0, statusBit, 1'b0, 1'b0};
 2'b11: {out7, out6, out2, out1 } = { statusBit, 1'b0, 1'b0, 1'b0};
 default: out5 = statusBit; // Indicate invalid enPosition.
 endcase
end

Putting it all together it could look something like the following code just to give you the general idea. In response to comments, I've removed the multiple drivers and changed the assignments in the synchronous reset block to non-blocking. As an alternative for synchronous and registered outputs, the case statement that decodes the enPosition counter could be moved to the normal operation block and the multiple drivers uncommented in the synchronous reset block:

module syCore
(
 input reset,
 input clk,
 input shift, // Debounced signal from switch or button.
 input statusBit,
 output reg out1,
 output reg out2,
 output reg out3,
 output reg out4,
 output reg out5,
 output reg out6,
 output reg out7,
 output reg out8
);
 reg shift_previous;
 wire shift_posedge;
 reg [1:0] enPosition;
 always @(posedge clk)
 begin
 if (reset == 1)
 begin
 //
 // Synchronous reset.
 //
 
 shift_previous <= shift;
 enPosition <= 2'b00;
 //out1 <= 0; // Removed multiple driver.
 //out2 <= 0; // Removed multiple driver.
 out3 <= 0;
 out4 <= 1; // Indicate reset condition.
 out5 <= 0;
 //out6 <= 0; // Removed multiple driver.
 //out7 <= 0; // Removed multiple driver.
 out8 <= 0;
 end
 else
 begin
 //
 // Normal operation.
 //
 
 out4 <= 0; // Indicate normal operation.
 if (shift_posedge)
 begin
 // Indicate positive edge on shift.
 // Will go high for one clock cycle.
 out3 <= 1;
 // Increment counter.
 enPosition <= enPosition + 2'b01;
 end
 else
 begin
 out3 <= 0; // Indicate level voltage on shift.
 end
 shift_previous <= shift;
 end
 end
 // Positive edge detection of shift input.
 assign shift_posedge = (shift == 1) & (shift_previous == 0);
 always @*
 begin
 // Decode the counter and connect statusBit to the correct output.
 case (enPosition)
 2'b00: {out7, out6, out2, out1 } = { 1'b0, 1'b0, 1'b0, statusBit};
 2'b01: {out7, out6, out2, out1 } = { 1'b0, 1'b0, statusBit, 1'b0};
 2'b10: {out7, out6, out2, out1 } = { 1'b0, statusBit, 1'b0, 1'b0};
 2'b11: {out7, out6, out2, out1 } = { statusBit, 1'b0, 1'b0, 1'b0};
 default: out5 = statusBit; // Indicate invalid enPosition.
 endcase
 end
endmodule

Simulation

Figure 1 – Simulation waveforms

RTL Synthesis

Figure 2 – RTL synthesis schematic

• Dipnarayan Das: Hi tim, Thank you so much for this explanation. How to fix "connected to multiple drivers problem" do you know? It is showing "Multi-source in Unit <syScore> on signal <out5>; this signal is connected to multiple drivers.". I already faced this problem before.
• Mitu Raj: You have multiple drivers at all out signals @tim
• tim: Thank you for highlighting that. When I synthesised it in Quartus, it found multiple drivers on out1, out2, out6 and out7. When I removed them, it synthesised ok.
• Mitu Raj: And inside the synchronous reset block, non-blocking assignments are recommended.
• tim: Thanks @DipnarayanDas. I've changed it to wire now. Don't forget to accept it as the answer if it answers your question.