Is this state machine written correctly?

[Jaco Naude]

Hi there

I've been struggling with a design containing 3 state machines (Moore)
working together to form a dynamic memory map. In behavioral
simulation the simulation works perfectly but in structural simulation
the design does not work. We've also rewrote the design from scratch
and used the ISE Moore state machine templates as the basis of the 3
state machines. Our synthesis report does not give anything suspicious
as far as I can see. We've checked in both XST and Synplify and both
of them give a netlist which behaves different from the behavioral
model (the XST netlist also behaves different compared to the Synplify
netlist). I've attached the XST synthesis report which might be
usefull, and I will give the code for one of our state machines below
to see if anyone can find something fundamentally wrong in the way its
been done.

Any help would be greatly appreciated.
Thanks
Jaco


State machine - In structural simulation the state machine gets stuck
in the following state: st_init_response

=============================================================================

ENTITY DMM_SPLITTER IS
GENERIC(
G_DATA_WIDTH : natural := 32;
G_ADDR_WIDTH : natural := 24
);
PORT(
S_DATA : IN transmit_bus; --! Data received from a
splitter or interface feeding this splitter.
S_RESPONSE : OUT response_bus; --! Data sent to a splitter
or interface feeding this splitter.
M0_DATA : OUT transmit_bus; --! Data sent from a leaf
node which is fed by this splitter.
M0_RESPONSE : IN response_bus; --! Data received from a
leaf node which is fed by this splitter.
M1_DATA : OUT transmit_bus; --! Data sent from a leaf
node which is fed by this splitter.
M1_RESPONSE : IN response_bus; --! Data received from a
leaf node which is fed by this splitter.
CLK : IN std_logic; --! Clock input.
RESET_N : IN std_logic --! Active-low reset input.
);

-- Declarations

END DMM_SPLITTER ;
-- hds interface_end
-- hds interface_end

--! @brief Architure definition DDM_SPLITTER
--! @details This architecture definition should work on any device
from any vendor.
architecture RTL of DMM_SPLITTER is

type state_type is (st_startup, st_init, st_init_m0, st_init_m1,
st_init_response, st_ready, st_request_m0, st_request_m1,
st_request_idle, st_response_m0, st_response_m1, st_error);
signal state, next_state : state_type;

signal new_state_valid_i : std_logic := '0';
signal m0_size : integer := 0;
signal m1_size : integer := 0;
signal s_data_i : transmit_bus := ((others => '0'), (others
=> '0'), '0', '0');
signal s_response_i : response_bus := ((others => '0'),
'0');
signal m0_data_i : transmit_bus := ((others => '0'),
(others => '0'), '0', '0');
signal m0_response_i : response_bus := ((others => '0'),
'0');
signal m1_data_i : transmit_bus := ((others => '0'),
(others => '0'), '0', '0');
signal m1_response_i : response_bus := ((others => '0'),
'0');
signal tmp_addr : integer := 0;

begin

SYNC_PROC: process (CLK)
begin
if (CLK'event and CLK = '1') then
if (RESET_N = '0') then
state <= st_startup;
S_RESPONSE.DATA <= (others => '0');
S_RESPONSE.DVAL <= '0';
M0_DATA.DATA <= (others => '0');
M0_DATA.ADDR <= (others => '0');
M0_DATA.DVAL <= '0';
M0_DATA.RNW <= '0';
M1_DATA.DATA <= (others => '0');
M1_DATA.ADDR <= (others => '0');
M1_DATA.DVAL <= '0';
M1_DATA.RNW <= '0';
else
-- This will generate a valid pulse for new states.
if next_state = state then
new_state_valid_i <= '0';
else
new_state_valid_i <= '1';
end if;
-- Sets the current state equal to the next_state.
state <= next_state;
-- Assign other outputs to internal signals.
S_RESPONSE <= s_response_i;
M0_DATA <= m0_data_i;
M1_DATA <= m1_data_i;
end if;
end if;
end process;

OUTPUT_DECODE: process (state)
begin
-- Insert statements to decode internal output signals
if state = st_init_m0 then
m0_data_i.DATA <= (others => '1');
m0_data_i.ADDR <= (others => '1');
m0_data_i.DVAL <= new_state_valid_i;
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= (others => '0');
s_response_i.DVAL <= '0';
elsif state = st_init_m1 then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '1');
m1_data_i.ADDR <= (others => '1');
m1_data_i.DVAL <= new_state_valid_i;
m1_data_i.RNW <= '0';
s_response_i.DATA <= (others => '0');
s_response_i.DVAL <= '0';
elsif state = st_init_response then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= std_logic_vector(conv_unsigned
(m0_size + m1_size, G_DATA_WIDTH));
s_response_i.DVAL <=
new_state_valid_i;
elsif state = st_ready then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= (others => '0');
s_response_i.DVAL <= '0';
elsif state = st_request_idle then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= (others => '0');
s_response_i.DVAL <= '0';
elsif state = st_request_m0 then
m0_data_i.DATA <= s_data_i.DATA;
m0_data_i.ADDR <= s_data_i.ADDR;
m0_data_i.DVAL <= new_state_valid_i;
m0_data_i.RNW <= s_data_i.RNW;
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= (others => '0');
s_response_i.DVAL <= '0';
elsif state = st_request_m1 then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= s_data_i.DATA;
-- Subtract m0_size from address to compensate for size
of first leaf node.
m1_data_i.ADDR <= std_logic_vector(conv_unsigned
(tmp_addr,G_ADDR_WIDTH));
m1_data_i.DVAL <= new_state_valid_i;
m1_data_i.RNW <= s_data_i.RNW;
s_response_i.DATA <= (others => '0');
s_response_i.DVAL <= '0';
elsif state = st_response_m0 then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= m0_response_i.DATA ;
s_response_i.DVAL <=
m0_response_i.DVAL;
elsif state = st_response_m1 then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
s_response_i.DATA <= m1_response_i.DATA ;
s_response_i.DVAL <= m1_response_i.DVAL;
elsif state = st_error then
m0_data_i.DATA <= (others => '0');
m0_data_i.ADDR <= (others => '0');
m0_data_i.DVAL <= '0';
m0_data_i.RNW <= '0';
m1_data_i.DATA <= (others => '0');
m1_data_i.ADDR <= (others => '0');
m1_data_i.DVAL <= '0';
m1_data_i.RNW <= '0';
s_response_i.DATA <= error_code;
s_response_i.DVAL <= new_state_valid_i;
end if;
end process;

NEXT_STATE_DECODE: process (state, new_state_valid_i, RESET_N,
S_DATA.DVAL, M0_RESPONSE.DVAL, M1_RESPONSE.DVAL)
begin
-- Declare default state for next_state to avoid latches
next_state <= state; -- Default is to stay in current state
case (state) is
when st_startup =>
if RESET_N = '1' then
next_state <= st_init;
else
next_state <= st_startup;
end if;
when st_init =>
if S_DATA.DVAL = '1' then
next_state <= st_init_m0;
end if;
when st_init_m0 =>
if M0_RESPONSE.DVAL = '1' then
m0_size <= conv_integer(unsigned(M0_RESPONSE.DATA(15
downto 0)));
m0_response_i.DATA <= M0_RESPONSE.DATA;
m0_response_i.DVAL <=
M0_RESPONSE.DVAL;
next_state <= st_init_m1;
end if;
when st_init_m1 =>
if M1_RESPONSE.DVAL = '1' then
m1_size <= conv_integer(unsigned(M1_RESPONSE.DATA(15
downto 0)));
m1_response_i.DATA <= M1_RESPONSE.DATA;
m1_response_i.DVAL <=
M1_RESPONSE.DVAL;
next_state <= st_init_response;
end if;
when st_init_response =>
next_state <= st_ready;
when st_ready =>
if (S_DATA.DVAL = '1' and (unsigned(S_DATA.ADDR) <
conv_unsigned(m0_size,G_ADDR_WIDTH))) then
s_data_i <= S_DATA;
next_state <= st_request_m0;
elsif (S_DATA.DVAL = '1' and (unsigned(S_DATA.ADDR) >=
conv_unsigned(m0_size,G_ADDR_WIDTH)) and (unsigned(S_DATA.ADDR) <
conv_unsigned(m1_size+m0_size,G_ADDR_WIDTH))) then
s_data_i <= S_DATA;
tmp_addr <= conv_integer(unsigned(S_DATA.ADDR)) -
m0_size;
next_state <= st_request_m1;
elsif (S_DATA.DVAL = '1' and (unsigned(S_DATA.ADDR) >=
conv_unsigned(m1_size+m0_size,G_ADDR_WIDTH))) then
-- The received address is too big for this splitter.
Something went wrong upstream in the DMM. Go into error state.
next_state <= st_error;
else
next_state <= st_ready;
end if;
when st_request_m0 =>
next_state <= st_request_idle;
when st_request_m1 =>
next_state <= st_request_idle;
when st_request_idle =>
if M0_RESPONSE.DVAL = '1' then
next_state <= st_response_m0;
m0_response_i.DATA <= M0_RESPONSE.DATA;
m0_response_i.DVAL <=
M0_RESPONSE.DVAL;
elsif M1_RESPONSE.DVAL = '1' then
next_state <= st_response_m1;
m1_response_i.DATA <= M1_RESPONSE.DATA;
m1_response_i.DVAL <=
M1_RESPONSE.DVAL;
else
next_state <= st_request_idle;
end if;
when st_response_m0 =>
next_state <= st_ready;
when st_response_m1 =>
next_state <= st_ready;
when st_error =>
next_state <= st_ready;
when others =>
next_state <= st_ready;
end case;
end process;

end RTL;

 

[KJ]

On Mar 18, 7:36 am, Jaco Naude <naude.j...@gmail.com> wrote:

Hi there

I've been struggling with a design containing 3 state machines (Moore)
working together to form a dynamic memory map. In behavioral
simulation the simulation works perfectly but in structural simulation
the design does not work.

Peruse your synthesis report for some form of message that talks about
an incomplete sensitivity list. Quick perusal shows that the
"OUTPUT_DECODE" process is missing several signals,
"NEXT_STATE_DECODE" is missing at least one and maybe a few more (not
sure if 'S_DATA.ADDR' is a signal or constant since it's not defined
in your posted code).

Consider not using unclocked processes for exactly this reason, it
leads to difference between simulation and synthesis. Far better to
have all synchronous processes like your "SYNC_PROC" coupled with
concurrent statements for things that must be unclocked.

If you really prefer the form of "if...then" and "case" statements
that you get to use inside a process but are not available as
concurrent statements, then consider using functions or procedures
instead. Functions and procedures will allow you to use 'if..then',
'case', etc. statements but you won't be able to have the equivalent
of an incomplete sensitivity list since this would imply use of some
signal that is not defined as an interface to the function/procedure
which the compiler will immediately flag as an error. That way you
won't get stuck perusing synthesis reports for incomplete sensitivity
lists to catch this design error.

Also, check your testbench to see if the stimulus is being generated
at an acceptable time relative to the clock. In order to use the post
route simulation model you must make sure that the inputs to the
device meet the setup and hold time requirements that the timing
report says is acceptable. If not, then you will get differences
between pre- and post-route simulation models.

Consider using "if rising_edge(CLK)..." rather than "if (CLK'event and
CLK = '1')...", it's more descriptive. Has nothing to do with your
current problem though.

Kevin Jennings

 

[Jaco Naude]

On Mar 18, 2:48 pm, KJ <kkjenni...@sbcglobal.net> wrote:

On Mar 18, 7:36 am, Jaco Naude <naude.j...@gmail.com> wrote:

Hi there

I've been struggling with a design containing 3 state machines (Moore)
working together to form a dynamic memory map. In behavioral
simulation the simulation works perfectly but in structural simulation
the design does not work.

Peruse your synthesis report for some form of message that talks about
an incomplete sensitivity list.  Quick perusal shows that the
"OUTPUT_DECODE" process is missing several signals,
"NEXT_STATE_DECODE" is missing at least one and maybe a few more (not
sure if 'S_DATA.ADDR' is a signal or constant since it's not defined
in your posted code).

Consider not using unclocked processes for exactly this reason, it
leads to difference between simulation and synthesis.  Far better to
have all synchronous processes like your "SYNC_PROC" coupled with
concurrent statements for things that must be unclocked.

If you really prefer the form of "if...then" and "case" statements
that you get to use inside a process but are not available as
concurrent statements, then consider using functions or procedures
instead.  Functions and procedures will allow you to use 'if..then',
'case', etc. statements but you won't be able to have the equivalent
of an incomplete sensitivity list since this would imply use of some
signal that is not defined as an interface to the function/procedure
which the compiler will immediately flag as an error. That way you
won't get stuck perusing synthesis reports for incomplete sensitivity
lists to catch this design error.

Also, check your testbench to see if the stimulus is being generated
at an acceptable time relative to the clock.  In order to use the post
route simulation model you must make sure that the inputs to the
device meet the setup and hold time requirements that the timing
report says is acceptable.  If not, then you will get differences
between pre- and post-route simulation models.

Consider using "if rising_edge(CLK)..." rather than "if (CLK'event and
CLK = '1')...", it's more descriptive.  Has nothing to do with your
current problem though.

Kevin Jennings

Thanks for the reply Kevin. After taking some of your ideas into
consideration I've got a working version now:


-
ENTITY DMM_SPLITTER IS
GENERIC(
G_DATA_WIDTH : natural := 32;
G_ADDR_WIDTH : natural := 24
);
PORT(
S_DATA : IN transmit_bus; --! Data received from a
splitter or interface feeding this splitter.
S_RESPONSE : OUT response_bus; --! Data sent to a splitter
or interface feeding this splitter.
M0_DATA : OUT transmit_bus; --! Data sent from a leaf
node which is fed by this splitter.
M0_RESPONSE : IN response_bus; --! Data received from a
leaf node which is fed by this splitter.
M1_DATA : OUT transmit_bus; --! Data sent from a leaf
node which is fed by this splitter.
M1_RESPONSE : IN response_bus; --! Data received from a
leaf node which is fed by this splitter.
CLK : IN std_logic; --! Clock input.
RESET_N : IN std_logic --! Active-low reset input.
);

-- Declarations

END DMM_SPLITTER ;
-- hds interface_end
-- hds interface_end

--! @brief Architure definition DDM_SPLITTER
--! @details This architecture definition should work on any device
from any vendor.
architecture RTL of DMM_SPLITTER is

type state_type is (st_startup, st_init, st_init_m0, st_init_m1,
st_ready, st_request_m0, st_request_m1, st_request_idle);
signal state : state_type;

signal m0_size : integer := 0;
signal m1_size : integer := 0;

begin

SYNC_PROC: process (CLK, RESET_N)
begin
if (CLK'event and CLK = '1') then
if (RESET_N = '0') then
state <= st_startup;
S_RESPONSE <= ((others => '0'),'0');
M0_DATA <= ((others => '0'), (others => '0'), '0', '0');
M1_DATA <= ((others => '0'), (others => '0'), '0',
'0');
else
-- This will generate a valid pulse for new
states.
case (state) is
when st_startup =>
if RESET_N = '1' then
state <= st_init;
else
state <= st_startup;
end if;
when st_init =>
if S_DATA.DVAL = '1' then
state <= st_init_m0;
M0_DATA <= ((others => '0'), (others => '0'),
'1', '0');
end if;
when st_init_m0 =>
if M0_RESPONSE.DVAL = '1' then
m0_size <= conv_integer(unsigned(M0_RESPONSE.DATA
(15 downto 0)));
M1_DATA <= ((others => '0'), (others => '0'),
'1', '0');
state <= st_init_m1;
else
M0_DATA <= ((others => '0'), (others => '0'),
'0', '0');
end if;
when st_init_m1 =>
if M1_RESPONSE.DVAL = '1' then
m1_size <= conv_integer(unsigned(M1_RESPONSE.DATA
(15 downto 0)));
S_RESPONSE <= (std_logic_vector(conv_unsigned
(m0_size + conv_integer(unsigned(M1_RESPONSE.DATA(15 downto 0))),
G_DATA_WIDTH)), '1');
state <= st_ready;
else
M1_DATA <= ((others => '0'), (others => '0'),
'0', '0');
end if;
when st_ready =>
S_RESPONSE <= ((others => '0'), '0');
M0_DATA <= ((others => '0'), (others => '0'), '0',
'0');
M1_DATA <= ((others => '0'), (others => '0'), '0',
'0');
if (S_DATA.DVAL = '1' and (unsigned(S_DATA.ADDR) <
conv_unsigned(m0_size,G_ADDR_WIDTH))) then
M0_DATA <= S_DATA;
state <= st_request_m0;
elsif (S_DATA.DVAL = '1' and (unsigned
(S_DATA.ADDR) >= conv_unsigned(m0_size,G_ADDR_WIDTH)) and (unsigned
(S_DATA.ADDR) < conv_unsigned(m1_size+m0_size,G_ADDR_WIDTH))) then
M1_DATA <= S_DATA;
M1_DATA.ADDR <= std_logic_vector(conv_unsigned
(conv_integer(unsigned(S_DATA.ADDR)) - m0_size,G_ADDR_WIDTH));
state <= st_request_m1;
elsif (S_DATA.DVAL = '1' and (unsigned
(S_DATA.ADDR) >= conv_unsigned(m1_size+m0_size,G_ADDR_WIDTH)))
then
-- The received address is too big for this
splitter. Something went wrong upstream in the DMM. Go into error
state.
S_RESPONSE <= (error_code, '1');
end if;
when st_request_m0 =>
if M0_RESPONSE.DVAL = '1' then
S_RESPONSE <= M0_RESPONSE;
state <= st_ready;
else
M0_DATA <= ((others => '0'), (others => '0'),
'0', '0');
state <= st_request_m0;
end if;
when st_request_m1 =>
if M1_RESPONSE.DVAL = '1' then
S_RESPONSE <= M1_RESPONSE;
state <= st_ready;
else
M0_DATA <= ((others => '0'), (others => '0'),
'0', '0');
state <= st_request_m1;
end if;
when others =>
state <= st_ready;
end case;
end if;
end if;
end process;

end RTL;


Thanks again,
Jaco