my code is as follows:
module command_FSM(sys_R_Wn,sys_ADSn,cState,sys_REF_REQ,sys_REF_ACK,sys_INIT_DONE,sys_CLK);
input sys_R_Wn;
input sys_CLK;
input sys_ADSn;
output [4:0] cState;
inout sys_INIT_DONE;
input sys_REF_REQ;
output sys_REF_ACK;
wire sys_R_Wn;
wire sys_ADSn;
reg [4:0] cState;
wire sys_INIT_DONE;
wire sys_REF_REQ;
reg sys_REF_ACK;
reg mjet;
integer i;
parameter c_idle=5'b10000;
parameter c_AR=5'b10001;
parameter c_tRFC=5'b10010;
parameter c_rdata=5'b10011;
parameter c_tDAL=5'b10100;
parameter c_cl=5'b10101;
parameter c_ACTIVE=5'b10110;
parameter c_wdata=5'b10111;
parameter c_REDA=5'b11000;
parameter c_tRCD=5'b11001;
parameter c_WRITEA=5'b11010;
initial
begin
cState=c_idle;
end
initial
begin
for(i=0;;i=i+1)
begin
#2;
if (sys_INIT_DONE==1'b1)
if(~sys_REF_REQ && ~sys_ADSn)
begin
case (cState)
5'b10000: begin
cState=c_ACTIVE;
#10;
end
5'b10110:
if(sys_R_Wn)
begin
cState=c_tRCD;
#10;
cState=c_REDA;
end
else
begin
cState=c_tRCD;
#10;
cState=c_WRITEA;
end
5'b11000: begin
cState=c_cl;
end
5'b10101: begin
cState=c_rdata;
end
5'b11010: begin
cState=c_wdata;
end
5'b10111: begin
cState=c_tDAL;
end
endcase
end
end
end
always @(posedge sys_REF_REQ)
begin
sys_REF_ACK=1;
case(cState)
5'b10000: begin
cState=c_AR;
#50;
cState=c_idle;
end
endcase
end
endmodule
here i want "cState" signal to be scanned and updated continuously based on statemachines.Firstly i tried always @* but as it had only output signal cState to be updated.So that block was executed only once because it had no input signal that was being modified in that always @* block.So my doubt is is it good thing to use "infinite for loop" to serve that purpose of scanning and updating continuously
The short answer is no; it is not good style to implement FSMs using an infinite loop.
The long answer is that it very much depends. Firstly, if this FSM is purely for a functional model in simulation and never to be synthesized for an FPGA or ASIC; you might use an infinite loop to implement an FSM. However, you should use the keyword forever to implement these loops, not for (i = 0; ; i = i + 1) or while (1). Or, if they are clocked, you can use an always @(posedge clk) or the like to trigger them (or use forever begin @(posedge clk) if its a forked process or something fancy like that).
However, based on the header of this module, it appears you want to make a synthesizable FSM, it which case, theres quite a lot you need to do to fix your code. Heres a short list of suggestions to make your code synthesizable and have better style:
As a general rule, do not use initial blocks inside an module. While they have a few limited uses (in FPGA synthesis), you should fully understand those uses before using them. If you need a variable to take on an initial state, use a reset line in the element storing that variable (see the next point)
Combinational logic should be placed in an always @* block and sequential logic should be placed in an always @(posedge clk[, negedge rstL]) block (the reset is optional). When making a state machine, I recommend the following style:
reg state, next_state;
// The register that holds the state
always @(posedge clk, negedge rstL) begin
if (~rstL) begin
state <= INIT; // Your starting state here
end
else begin
state <= next_state;
end
end
// The logic that determines the next state and output values based on the inputs and current state
always @* begin
// Defaults
[ Place your outputs here with some default value ]
next_state = state; // As selfloops are common, I typically just set the next state to be the current state
case (state)
[ Your logic for what to do in each state here ]
endcase
end
This form I find to be the best to ensure no latches and makes the synthesis tools happy with simple register blocks (the always @(posedge clk) block).
- Using parameters for state names is good practice, but you should use them everywhere, even in the case statement, like:
case (state)
INIT: begin
...
end
READ: begin
...
end
endcase
Do not use delays in your combinational logic like x = 1'b0; #10; x = 1'b1;. You change cState like this in your initial block but really should have different logic for each state.
You declare sys_REF_REQ as an inout when it should probably be an input.
Don't trigger logic on non-clock things, like always @(posedge sys_REF_REQ). Only use posedge and negedge on clocks and resets within synthesizable code.
Thats all I see right now, but other might add more in the comments. Best of luck!
