Verilog FSM not working on FPGA

I guess, you got a lot of serious Quartus synthesis warnings with your code. If so, you probably shouldn't ignore them.

You e.g. have a counter in an combinational always block. This can't be expected to ever result in reliable operation.

always @(state)
begin
..
count <= (count + 1'b1);
..
end

A counter may count depending on state machine states and other conditions, but it must be placed in an edge sensitive always block.

P.S:

If the present code seems to give correct results in functional (RTL) simulation, this doesn't mean anything.

Cheers for your reply,

I do get many warnings 337 in fact.

I've changed the always @ (state)

to always @ (negedge clk_in) now - all works fine!

Thanks

Hi Paul,

even if all works fine now this is not the way I would suggest. Your FSM consists of three parts: State register assignment, state actions, and next_state calculation. This is perfect. However, the next_state calculation lacks two items. The first three lines should be:

always @(state or count or whatever appears on the right hand side of an expression in that block)

begin

case(state)

next_state = state;

...

The always construct also can be written this way

always @(*)

in Verilog 2001. Quartus supports that.

Thanks for your reply Harald.

I am unsure about your comment however. Would this not mean that the state would be changed whenever a reg was changed?

I want the FSM to change state synchronously with the clk.

Thanks for your help

Paul

Hi Paul,

the state of the FSM can only change on the rising clock edge since you clearly define:

always @(posedge clk_in)

if (!rst_n)

state <= S0;

else

state <= next_state;

This is the one and only assignment for the state register. What I mentioned is the calculation of next_state. This, in fact, is the input to the state register, and this is combinational logic depending on the input ports and the state. So, the always construct must be triggered every time one of these signals changes. The first statement assigns the current state to next_state. Depending on this, the case switch is evaluated and a new value for next_state is calculated.

Thans Harald,

I understand what you mean now. I am currently triggering the calculation of next_state at every posedge of clock, ready for the state register to be updated on the negedge of the clock. This does seem to work correclty when synthsised on FPGA.

Would it be better to do this combinationally?

Also if possible could you explain further as to why I cannot write...

//State actions

always @(state)

begin

case(state)

S0: begin

clk_out = 1'b1;

count = 4'b0000;

end

S1: clk_out = 1'd1;

S2: begin

count <= (count + 1'b1);

clk_out = 1'b1;

end

The count does not work correctly, but my understanding of the Verilog is that, whenever state changes value, the construct would be run once. This should update the count register once. And so why is this not possible?

Thanks so much for your help in this matter

Paul

Below is the current working code with all the constructs executed on a clk edge:

module Clock_Gen(rst_n,

clk_out,

clk_in,

count);

//Input Ports

input rst_n;

input clk_in;

//****************************************************

//Output Ports

output [3:0] count;

output clk_out;

//****************************************************

//Output Registers

reg clk_out;

//****************************************************

//Internal Registers

reg [40:0] count;

//****************************************************

//State Definition

parameter S0 = 4'd0,

S1 = 4'd1,

S2 = 4'd2,

S3 = 4'd3,

S4 = 4'd4,

S5 = 4'd5,

S6 = 4'd6;

//****************************************************

//Internal state variables

reg [3:0] state;

reg [3:0] next_state;

//****************************************************

//Set initial reg values

//****************************************************

//State changes only at positive edge of clock (synchronous)

always @(negedge clk_in)

if (!rst_n)

state <= S0;

else

state <= next_state; //State Change

//****************************************************

//State actions

always @(posedge clk_in)

begin

case(state)

S0: begin

clk_out = 1'b1;

count = 25000000;

end

S1: clk_out = 1'd1;

S2: begin

count <= (count - 1);

clk_out = 1'b1;

end

S3: begin

clk_out = 1'b0;

count = 25000000;

end

S4: clk_out = 1'b0;

S5: begin

clk_out = 1'b0;

count <= (count - 1);

end

endcase

end

//****************************************************

//State machine using case statement

always @(posedge clk_in)

begin

case(state)

S0: next_state = S1;

S1: begin

if (count == 0)

next_state = S3;

else

next_state = S2;

end

S2: next_state = S1;

S3: next_state = S4;

S4: begin

if (count == 0)

next_state = S0;

else

next_state = S5;

end

S5: next_state = S4;

endcase

end

endmodule

--- Quote Start ---

always @(state or count or whatever appears on the right hand side of an expression in that block)

--- Quote End ---

This suggestion can be basically ignored in synthesis. Although Quartus gives warnings on conditions missing in the always block, the synthesized code won't be different at all. In simulation, the missing trigger in always block most likely causes different behaviour.

Hi FvM,

I totally agree to that. Nevertheless, I still hope that the young folks learn to simulate, simulate, and simulate again before starting Quartus the first time.

Hi Paul,

sure, it works when you trigger the next_state calculation on the falling edge of the clock but this is not the way a FSM is meant to be. Maybe some day you will design an FSM running at *really* high speed and then there is not enough time for this additional register.

Concerning the count issue, please clarify what you mean when you say that is does not work. Simulation or real world? To be honest, I have no clue what Quartus systhesizes out of this source. I would suggest that you have count and the clk_out be triggered on a clock edge.