Nigel Noldsworth
Guest
Mon Dec 14, 2009 9:09 pm
Hello there,
I have the following process for the combinatory logic of a state
machine. However during simulation it seems to me that when
rising_edge of the clock, the process is not triggered with "state"
signal assuming that sda (not on the sensitivity list) = 1. Hence the
state machine stays into idle mode. I was guessing that rising_edge of
the clock, that same value is being assigned to "state" signal and
this assignment will wake up the process. It seems I'm wrong. Can you
please shade some light please ?
receive : process (state, count) is
variable i : natural;
begin -- process receive
case state is
when IDLE =>
count <= 0;
storage <= (others => '0');
val <= '0';
--
if sda = '1' then
nxt_state <= CAPTURE;
else
nxt_state <= IDLE;
end if;
when CAPTURE =>
........
sync_receive : process (clock, reset)
begin
if reset = '0' then
state <= IDLE;
output <= (others => '0');
elsif clock'event and clock = '1' then
if enable = '1' then
state <= nxt_state;
output <= storage;
end if;
end if;
end process sync_receive;
Jonathan Bromley
Guest
Mon Dec 14, 2009 10:36 pm
On Mon, 14 Dec 2009 11:09:03 -0800 (PST), Nigel Noldsworth wrote:
Quote:
I have the following process for the combinatory logic of a state
machine. However during simulation it seems to me that when
rising_edge of the clock, the process is not triggered with "state"
signal assuming that sda (not on the sensitivity list) = 1. Hence the
state machine stays into idle mode.
So there is no transition on "state", and therefore the
combinational process is not triggered (at least, not in
simulation; synthesis tools sometimes misinterpret
sensitivity lists in a beguiling but flawed way).
Quote:
I was guessing
VHDL is rather well defined; guessing is rarely necessary or useful.
Quote:
[on a] rising_edge of the clock, that same value is being
assigned to "state" signal
this assignment will wake up the process.
Sensitivity lists are tripped by value changes, not by writes
to a signal.
Quote:
receive : process (state, count) is
variable i : natural;
begin -- process receive
case state is
when IDLE =
count <= 0;
storage <= (others => '0');
val <= '0';
--
if sda = '1' then
Here's the smoking gun. Your process reads (looks at the
value of) SDA. Consequently, if you are trying to imply
combinational logic (which plainly is the case), it is
essential that you include SDA in the sensitivity list.
If history is any guide, you are now likely to be
subjected to a torrent of posts explaining why two-process
state machines like yours are an aberration thrust upon us
by a textbook-writing mafia, and then a few more posts
explaining why you need to use two-process state machines
in order to attain a state of transcendental oneness with
your intended hardware. Personally I'm fairly firmly in
the one-process state machine camp; most of the arguments
in favour of two- (or, horrors, three-) process state
machines are mendacious tosh. However, there are
situations where separating out the combinational logic
can be useful. Don't get too steamed up about it, but
please ensure that your combinational processes exhibit
all three of the truly essential features:
- complete sensitivity list
- complete assignment to all outputs
= no combinational feedback
I strongly suspect that your design may also
fall foul of the last of these. What on earth
is "count" doing in its sensitivity list? What,
precisely, are you doing with "i"? The odour of
rat hangs heavy in the air.
--
Jonathan Bromley
Dave Pollum
Guest
Mon Dec 14, 2009 11:40 pm
On Dec 14, 2:09 pm, Nigel Noldsworth <noldswo...@googlemail.com>
wrote:
Quote:
Hello there,
I have the following process for the combinatory logic of a state
machine. However during simulation it seems to me that when
rising_edge of the clock, the process is not triggered with "state"
signal assuming that sda (not on the sensitivity list) = 1. Hence the
state machine stays into idle mode. I was guessing that rising_edge of
the clock, that same value is being assigned to "state" signal and
this assignment will wake up the process. It seems I'm wrong. Can you
please shade some light please ?
receive : process (state, count) is
variable i : natural;
begin -- process receive
case state is
when IDLE =
count <= 0;
storage <= (others => '0');
val <= '0';
--
if sda = '1' then
nxt_state <= CAPTURE;
else
nxt_state <= IDLE;
end if;
when CAPTURE =
.......
sync_receive : process (clock, reset)
begin
if reset = '0' then
state <= IDLE;
output <= (others => '0');
elsif clock'event and clock = '1' then
if enable = '1' then
state <= nxt_state;
output <= storage;
end if;
end if;
end process sync_receive;
You need to add sda to the receive process's sensitivity list,
otherwise the simulator will not "run" the receive process when sda
changes.
-Dave Pollum
Nigel Noldsworth
Guest
Tue Dec 15, 2009 2:26 pm
On Dec 14, 10:36 pm, Jonathan Bromley wrote:
Quote:
Here's the smoking gun. Your process reads (looks at the
value of) SDA. Consequently, if you are trying to imply
combinational logic (which plainly is the case), it is
essential that you include SDA in the sensitivity list.
Actually the code has been written by a subcontracting company, which
the quality of code left me in an uneasy situation. We have a bug in
the system, hence I'm trying to debug it. That said, due to lack of
time, I won't be able to rewrite that code for him into a one-process,
but it would be great for me to learn those key leakage as well:)
Quote:
please ensure that your combinational processes exhibit
all three of the truly essential features:
- complete sensitivity list
- complete assignment to all outputs
= no combinational feedback
I strongly suspect that your design may also
fall foul of the last of these.
Yes it seems so.
Quote:
What on earth
is "count" doing in its sensitivity list?
The subcontractor placed a 3-bit adder into another case statement to
convert a serial SDA to a 8-bit shift register.
----------------
storage(WIDTH-1 downto 1) <= output(WIDTH-2 downto 0);
storage(0) <= sda;
val <= '0';
if count = WIDTH -1 then
count <= 0;
nxt_state <= PARITY;
else
count <= count +1;
nxt_state <= CAPTURE;
end if;
---------------------
The above code fragment in one of the case statement of the state
machie creates a 3 bit adder. Which is the reason why "count" is in
the sensitivity list. However this is not a proper way to code and
another reason to terminate the contract with this subcontractor.
That said in the case of a two-process state machine, for the output
of this 3 bit adder to be registered at rising_edge of the clock, I
will have to create a new process specially for adder. In the end this
adder implies 3 more DFFs. Which comes to another question. Is it
possible to add an adder in one of the case statement of the 2process
state machine which counts only when rising edge of the clock (in such
a way that the synthesis tool doesn't infer 3 DFFs? I'm interested
from a "design area" point of view.
Quote:
What,precisely, are you doing with "i"?
This is used in the code below but in only one case statement.
for i in storage'range loop
val <= storage(i) xor sda;
end loop;
Jonathan Bromley
Guest
Tue Dec 15, 2009 4:40 pm
On Dec 15, 1:26 pm, Nigel Noldsworth <noldswo...@googlemail.com>
wrote:
Quote:
On Dec 14, 10:36 pm, Jonathan Bromley wrote:
Here's the smoking gun. Your process reads (looks at the
value of) SDA. Consequently, if you are trying to imply
combinational logic (which plainly is the case), it is
essential that you include SDA in the sensitivity list.
Actually the code has been written by a subcontracting company, which
the quality of code left me in an uneasy situation.
Me too. They seem to be suffering a clue shortfall.
Quote:
What on earth
is "count" doing in its sensitivity list?
The subcontractor placed a 3-bit adder into another case statement to
convert a serial SDA to a 8-bit shift register.
----------------
storage(WIDTH-1 downto 1) <= output(WIDTH-2 downto 0);
storage(0) <= sda;
val <= '0';
if count = WIDTH -1 then
count <= 0;
nxt_state <= PARITY;
else
count <= count +1;
nxt_state <= CAPTURE;
end if;
AAAARGH. This is just WRONG. If you write
count <= count + 1;
in a combinational process, you are describing an
incrementer with its output directly connected to
its input. That doesn't sound very sensible to me.
Quote:
The above code fragment in one of the case statement of the state
machie creates a 3 bit adder. Which is the reason why "count" is in
the sensitivity list. However this is not a proper way to code and
another reason to terminate the contract with this subcontractor.
You said it.
Quote:
That said in the case of a two-process state machine, for the output
of this 3 bit adder to be registered at rising_edge of the clock, I
will have to create a new process specially for adder. In the end this
adder implies 3 more DFFs. Which comes to another question. Is it
possible to add an adder in one of the case statement of the 2process
state machine which counts only when rising edge of the clock (in such
a way that the synthesis tool doesn't infer 3 DFFs? I'm interested
from a "design area" point of view.
It's easy; far, far easier if you use a one-process FSM, but easy
anyhow. Just imagine that the counter is part of the state, and
needs handling just like the state. So your combinational process
creates a "next_count" output just in the same way that it creates
a "next_state" output; and you then register it in the clocked
process.
Quote:
What, precisely, are you doing with "i"?
This is used in the code below but in only one case statement.
for i in storage'range loop
val <= storage(i) xor sda;
end loop;
More evidence of your contractor's incompetence. For-loop
counter variables in VHDL don't need to be declared. In
this particular case it's probably harmless because the
declared variable is simply not used at all, but it is
culpably silly to provide it at all.
I'm really sorry, I assumed the code was from a clueless
student. I don't think I have ever before seen anything
so crass from a paid contractor. Sack 'em before they
cost you your reputation.
--
Jonathan Bromley
Andy
Guest
Tue Dec 15, 2009 4:45 pm
There's no need for a torrent, Jonathan's explanation will suffice.
But just to make sure Jonathan cannot be called a liar, and two posts
is the minimum qualifiable torrent, here it is:
Use single process descriptions of sequential logic, and problems just
like the OP has experienced will not happen.
The advantages of a single process description include:
No missing signals in sensitivity lists that cause simulation/
synthesis mismatches
No latches
Easier integration of clock enabled register with behavior
Half the signal declarations
Allows use of variables for code that behaves like it reads (no
suspended signal updates)
Andy