State machine going into unknown state

A

axr0284

Hey guys,
I have a weird problem with a spartan 3E FPGA. I don't know if you
have ever seen this.
I have a simple state machine to read data from an ADC. The code is
below:

<CODE>
--
-- Sequential part of state machine
--
seq_state_machine: process(reset, clk_120, nextState,
next_write_flag, next_data_valid_internal,
next_cs_zero_l, next_cs_one_l, next_rd_l, next_conv_l,
next_adc_zero_data_A_one, next_adc_zero_data_A_two,
next_adc_one_data_A_one, next_adc_one_data_A_two,
next_gamma_sign, next_reset_skip_count, next_busy_flag)
begin
if (clk_120'event and clk_120 = '1') then
if (reset='1') then -- Synchronous active low reset
currentState <= IDLE;

internal_cs_zero_l <= '1';
internal_cs_one_l <= '1';
internal_rd_l <= '1';
internal_conv_l <= '1';
internal_adc_zero_data_A_one <= (others => '0');
internal_adc_zero_data_A_two <= (others => '0');
internal_adc_one_data_A_one <= (others => '0');
internal_adc_one_data_A_two <= (others => '0');
internal_gamma_sign <= '0';
reset_skip_count <= '1';
busy_flag <= '0';
write_flag <= '0';
data_valid_internal <= '0';

else
currentState <= nextState;

internal_cs_zero_l <= next_cs_zero_l;
internal_cs_one_l <= next_cs_one_l;
internal_rd_l <= next_rd_l;
internal_conv_l <= next_conv_l;
internal_adc_zero_data_A_one <= next_adc_zero_data_A_one;
internal_adc_zero_data_A_two <= next_adc_zero_data_A_two;
internal_adc_one_data_A_one <= next_adc_one_data_A_one;
internal_adc_one_data_A_two <= next_adc_one_data_A_two;
internal_gamma_sign <= next_gamma_sign;
reset_skip_count <= next_reset_skip_count;
busy_flag <= next_busy_flag;
write_flag <= next_write_flag;
data_valid_internal <= next_data_valid_internal;

end if;
end if;
end process seq_state_machine;

--
-- Combinational part of state machine
--
comb_state_machine: process(currentState, adc_busy_zero,
adc_busy_one, adc_data, data_valid_internal,
internal_cs_zero_l, internal_cs_one_l, internal_rd_l,
internal_conv_l, gamma_sign,
internal_adc_zero_data_A_one, internal_adc_zero_data_A_two,
internal_adc_one_data_A_one, internal_adc_one_data_A_two,
internal_gamma_sign, reset_skip_count, busy_flag, skip_count,
write_flag, convert_count)
begin

nextState <= currentState;
next_cs_zero_l <= internal_cs_zero_l;
next_cs_one_l <= internal_cs_one_l;
next_rd_l <= internal_rd_l;
next_conv_l <= internal_conv_l;
next_adc_zero_data_A_one <= internal_adc_zero_data_A_one;
next_adc_zero_data_A_two <= internal_adc_zero_data_A_two;
next_adc_one_data_A_one <= internal_adc_one_data_A_one;
next_adc_one_data_A_two <= internal_adc_one_data_A_two;
next_gamma_sign <= internal_gamma_sign;
next_reset_skip_count <= reset_skip_count;
next_busy_flag <= busy_flag;
next_write_flag <= write_flag;
next_data_valid_internal <= data_valid_internal;

case currentState is
when IDLE =>
next_write_flag <= '0';
next_conv_l <= '1';
next_cs_zero_l <= '1';
next_cs_one_l <= '1';
next_rd_l <= '1';
next_reset_skip_count <= '1';
next_busy_flag <= '0';
next_data_valid_internal <= '0';
nextState <= START_THE_CONVERSION_ZERO;

when START_THE_CONVERSION_ZERO =>
if(convert_count = b"0110") then -- assert convert for
50 ns
next_conv_l <= '1';
nextState <= WAIT_FOR_BUSY_HIGH;--
STOP_THE_CONVERSION_ZERO;--
else
next_conv_l <= '0';
nextState <= START_THE_CONVERSION_ZERO;
end if;

when WAIT_FOR_BUSY_HIGH =>
if(adc_busy_zero = '1' AND adc_busy_one = '1') then --
Wait for busy signal to go high
nextState <= WAIT_FOR_BUSY_LOW;

else
nextState <= WAIT_FOR_BUSY_HIGH;

end if;

when WAIT_FOR_BUSY_LOW =>
if(adc_busy_zero = '0' AND adc_busy_one = '0') then --
Wait for busy signal to go high
next_cs_zero_l <= '0';
next_rd_l <= '0';
next_reset_skip_count <= '0';
nextState <= READ_DATA_ZERO; -- Read the data

else

nextState <= WAIT_FOR_BUSY_LOW;

end if;

when READ_DATA_ZERO =>
next_reset_skip_count <= '0';
if(skip_count = b"0001100") then
next_adc_zero_data_A_one <= adc_data; -- Store the data A1
nextState <= READ_DATA_ZERO; -- Read the data

elsif(skip_count = b"0001111") then
next_rd_l <= '1';
nextState <= READ_DATA_ZERO; -- Read the data

elsif(skip_count = b"0010011") then
next_rd_l <= '0';
nextState <= READ_DATA_ZERO; -- Read the data

elsif(skip_count = b"0010111") then
next_adc_zero_data_A_two <= adc_data; -- Store the
data A1
nextState <= READ_DATA_ZERO; -- Read the data

elsif(skip_count = b"0100011") then -- 2 cycles later
deassert chip select
next_rd_l <= '1';
next_cs_zero_l <= '1';
nextState <= SWITCH_TO_ADC_ONE; -- Read the data
else
nextState <= READ_DATA_ZERO; -- Read the data
end if;

when SWITCH_TO_ADC_ONE =>
if(skip_count = b"0101011") then
next_cs_one_l <= '0';
next_rd_l <= '0';
next_reset_skip_count <= '1';
nextState <= READ_DATA_ONE; -- Read the
data

else
nextState <= SWITCH_TO_ADC_ONE; -- Read the
data

end if;

when READ_DATA_ONE =>
next_reset_skip_count <= '0';
if(skip_count = b"0001100") then
next_adc_one_data_A_one <= adc_data; -- Store the
data A1
nextState <= READ_DATA_ONE; -- Read the data

elsif(skip_count = b"0001111") then
next_rd_l <= '1';
nextState <= READ_DATA_ONE; -- Read the data

elsif(skip_count = b"0010011") then
next_rd_l <= '0';
nextState <= READ_DATA_ONE; -- Read the data

elsif(skip_count = b"0010111") then
next_adc_zero_data_A_two <= adc_data; -- Store the
data A1
next_gamma_sign <= gamma_sign;
nextState <= READ_DATA_ONE; -- Read the data

elsif(skip_count = b"100001") then -- Skip 60 cycles
next_cs_one_l <= '1';
next_rd_l <= '1';
next_write_flag <= '1';
next_data_valid_internal <= '1';
nextState <= READ_DATA_ONE; -- Read the data

elsif(skip_count = b"1011101") then -- Skip 60 cycles
next_reset_skip_count <= '1';
nextState <= IDLE; -- Read the data

else
nextState <= READ_DATA_ONE; -- Read the data
end if;

when OTHERS =>
nextState <= IDLE; -- Read the data

end case;
end process comb_state_machine;
</CODE>

In the state machine if I don't have
when OTHERS =>
nextState <= IDLE;

the state machine CONSISTENTLY goes into an unknown state and locks
up. That does not seem right to me. I am wondering if you guys can
take a look at the code and tell me if there is a bug because I can't
find any. Thanks,
Amish
 
K

KJ

Hey guys,
 I have a weird problem with a spartan 3E FPGA. I don't know if you
have ever seen this.
I have a simple state machine to read data from an ADC. The code is
below:

<SNIP CODE>
In the state machine if I don't have
when OTHERS =>
        nextState <= IDLE;

the state machine CONSISTENTLY goes into an unknown state and locks
up. That does not seem right to me.

It's not right...there are no 'unknown' states in real hardware, there
are no 'unknown' states in simulation either when the state variable
is an enumerated type. What you most likely mean is YOU don't know
what state things are in. (Hint: Take a look at the state signals,
they are not unknowns).

I'm assuming you've simulated the design, if not do that first and get
it working.

If you're talking about real hardware, then most likely the state
machine is one hot encoded and the 'unknown' state is occurring
because more than one state bit is getting set. Then because of the
'when others' taking you back to the idle state you can recover. But
one hot encoded states with more than one bit set is the design issue
that needs fixing.

Assuming the sim works, but real hardware doesnt' then the design
problem you have that causes more than one state bit to get set is
failing timing analysis. In this particular case, the timing problem
is most likely because the inputs to your state machine have not been
synchronized to the clock. Based on the list-o-signals in your
sensitivity list any one of the following signals could be the
culprit:

currentState, adc_busy_zero, adc_busy_one, adc_data,
data_valid_internal, internal_cs_zero_l, internal_cs_one_l,
internal_rd_l, internal_conv_l, gamma_sign,
internal_adc_zero_data_A_one, internal_adc_zero_data_A_two,
internal_adc_one_data_A_one, internal_adc_one_data_A_two,
internal_gamma_sign, reset_skip_count, busy_flag, skip_count,
write_flag, convert_count

If any of these are not outputs of a flip flop that is clocked by your
signal 'clk_120' then you'll get a flaky design that fails in ways
that you're seeing as well as in other 'mysterious' manners.

The other possibility is that you are using a signal inside that
combinatorial process that is not in the sensitivity list. This would
cause a mismatch between sim and reality. To detect if this is the
case, peruse your synthesis report looking for something like
"incomplete sensitivity list". To avoid ever having to have this
particular problem ever bite you, then do away with the two process
style and use a single clocked process.
I am wondering if you guys can
take a look at the code and tell me if there is a bug because I can't
find any. Thanks,

No thanks, look at my suggestions and then go back and look at your
own code.

Kevin Jennings
 
A

axr0284

Thanks for the answer, I will make sure to check the code carefully.
Now that you mention it, I never synchronize the asynchronous signals.
I'll try that out. Thanks.
then do away with the two process
style and use a single clocked process.

Could you elaborate why a single process is better. Thanks,
 
A

axr0284

It's not right...there are no 'unknown' states in real hardware, there
are no 'unknown' states in simulation either when the state variable
is an enumerated type.  What you most likely mean is YOU don't know
what state things are in.  (Hint:  Take a look at the state signals,
they are not unknowns).

I'm assuming you've simulated the design, if not do that first and get
it working.

If you're talking about real hardware, then most likely the state
machine is one hot encoded and the 'unknown' state is occurring
because more than one state bit is getting set.  Then because of the
'when others' taking you back to the idle state you can recover.  But
one hot encoded states with more than one bit set is the design issue
that needs fixing.

Assuming the sim works, but real hardware doesnt' then the design
problem you have that causes more than one state bit to get set is
failing timing analysis.  In this particular case, the timing problem
is most likely because the inputs to your state machine have not been
synchronized to the clock.  Based on the list-o-signals in your
sensitivity list any one of the following signals could be the
culprit:

currentState, adc_busy_zero, adc_busy_one, adc_data,
data_valid_internal, internal_cs_zero_l, internal_cs_one_l,
internal_rd_l, internal_conv_l, gamma_sign,
internal_adc_zero_data_A_one, internal_adc_zero_data_A_two,
internal_adc_one_data_A_one, internal_adc_one_data_A_two,
internal_gamma_sign, reset_skip_count, busy_flag, skip_count,
write_flag, convert_count

If any of these are not outputs of a flip flop that is clocked by your
signal 'clk_120' then you'll get a flaky design that fails in ways
that you're seeing as well as in other 'mysterious' manners.

The other possibility is that you are using a signal inside that
combinatorial process that is not in the sensitivity list.  This would
cause a mismatch between sim and reality.  To detect if this is the
case, peruse your synthesis report looking for something like
"incomplete sensitivity list".  To avoid ever having to have this
particular problem ever bite you, then do away with the two process
style and use a single clocked process.


No thanks, look at my suggestions and then go back and look at your
own code.

Kevin Jennings

You were correct. The asynchronous signals were causing the issue. I
added double buffering on all my asynchronous lines and it seems to
have solved the problem. Thanks a lot for the help.
Amish
 
K

KJ

Could you elaborate why a single process is better. Thanks,

1. Less typing.
2. Less chance for creating a design error because of an incomplete
sensitivity list.

Whenever you have a combinatorial (i.e. unclocked) process, you can
unintentionally create the following problems that then need to be
debugged in simulation or (even worse) by perusing synthesis
warnings. Neither of these problems occur if you simply use a single
clocked process for your state machine:

Prob 1: Forget to assign a signal under all conditions. Typically one
would handle this with a "Next_State <= Current_State" assignment at
the begining of the process and then go into your 'real' logic. That
way, there will always be some assignment made whenver the process
triggers which will prevent generating latches which are a big no-no
in FPGA/CPLDs. The "Next_State <= Current_State" statement is also
'extra' typing, in that it is simply not needed if you have a single
clocked process.

Prob 2: Forget to put a signal into the sensitivity list of the
process. The simulator won't care, but the synthesis tool will create
different logic since it will assume that you just forgot to put the
signal in and create logic as if you had put it into the sensitivity
list. Thus your simulation and 'real world' designs will do different
things.

Ex:

process(a)
begin
x1 <= a and b;
end process;

process(a,b)
begin
x2 <= a and b;
end process;

x3 <= a and b;

a <= '0', '1' after 10 ns;
b <= '0', '1' after 20 ns;

The first above process will not change the value of 'x' when 'b'
changes unless it also changes at the exact same time as 'a'.
Simulate the above and check it out, noticing that x2 and x3 will do
the same thing, but x1 will not change at t=20ns. Synthesis of x1, x2
and x3 however will produce the exact same logic for all three. While
the synthesis tool's actions might be what you had really intended to
do anyway, it is bad practice to have a simulation model that
functionally differs from reality.

Now ask yourself if those are the types of problems you want to spend
your time debugging or not.

KJ
 
A

Andy Peters

Thanks for the answer, I will make sure to check the code carefully.
Now that you mention it, I never synchronize the asynchronous signals.
I'll try that out. Thanks.


Could you elaborate why a single process is better. Thanks,

Because you avoid potential combinatorial loops when you forget to
assign to a signal in a state.

Also, your synchronous process should NOT have every signal that's on
the RHS of an assignment in the sensitivity list. When using a
synchronous reset, the ONLY signal that should be on the list is the
clock.

-a
 
M

Mike Treseler

axr0284 said:
Could you elaborate why a single process is better.

It would eliminate half of the questions
to this newsgroup, for one thing.

-- Mike Treseler
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,898
Latest member
BlairH7607

Latest Threads

Top