Something stupid with a "case"

[Pleg]

Hi everybody, I'm sure I'm doing some stupid error but I can't find it.
It's very simple: I have a FSM that, sometimes, asserts the signal
"ths_counter_updater" to 1 for one clock cycle. I have another process which
is supposed to get that '1' and change the signal "ths_sig_internal"
according to this rule: starting from 0, count up until2, then stay there
(0->1, 1->2, 2->2, 3 is an error). I've written this code:

updating: process(clock)
begin
if((clock'event and clock='1') and (ths_counter_updater='1')) then
case ths_sig_internal is
when B"00" => ths_sig_internal <= B"01";
when B"01" => ths_sig_internal <= B"10";
when B"10" => ths_sig_internal <= B"10";
when others => ths_sig_internal <= B"11";
end case;
end if;
end process updating;

The signals are declared as
signal ths_sig_internal: std_logic_vector(1 downto 0) := B"00";
signal ths_counter_updater: std_logic:='0';


It doesn't work: the first time "ths_sig_internal" goes to 1,
"ths_sig_internal" goes from "00" to "0X", the second time it goes to "XX"
and stays so forever.

Can anybody help me, please?


Pleg

 

[KJ]

Pleg wrote:

Hi everybody, I'm sure I'm doing some stupid error but I can't find it.
It's very simple: I have a FSM that, sometimes, asserts the signal
"ths_counter_updater" to 1 for one clock cycle. I have another process which
is supposed to get that '1' and change the signal "ths_sig_internal"
according to this rule: starting from 0, count up until2, then stay there
(0->1, 1->2, 2->2, 3 is an error). I've written this code:

updating: process(clock)
begin
KJ GACK, NOT LIKE THIS!!: if((clock'event and clock='1') and (ths_counter_updater='1')) then
if rising_edge(clock) then -- KJ added

if (ths_counter_updater='1') then -- KJ added

case ths_sig_internal is
when B"00" => ths_sig_internal <= B"01";
when B"01" => ths_sig_internal <= B"10";
when B"10" => ths_sig_internal <= B"10";
when others => ths_sig_internal <= B"11";
end case;
end if;
end if; -- KJ added
end process updating;

The signals are declared as
signal ths_sig_internal: std_logic_vector(1 downto 0) := B"00";
signal ths_counter_updater: std_logic:='0';
Change to

signal ths_sig_internal: std_ulogic_vector(1 downto 0) := B"00";

It doesn't work: the first time "ths_sig_internal" goes to 1,
KJ: I think you mean when "ths_counter_updater" goes to 1
"ths_sig_internal" goes from "00" to "0X", the second time it goes to "XX"
and stays so forever.
KJ: It looks like it's trying to count but you must have two drivers

for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

By changing the type of 'ths_sig_internal" from std_logic_vector to
std_ulogic_vector the compiler will help you out and point out the two
(or more) places where ths_sig_internal is being driven from (not
immediately obvious to me...but it's late in the day).

Happy hunting!
KJ

 

[Nicolas Matringe]

KJ a écrit :

KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

I think he just forgot to reset his ths_sig_signal at startup.
The initial value you mentionned will make the simulation work but is
not guaranteed in the real hardware.

Nicolas

 

[KJ]

Nicolas Matringe wrote:

KJ a écrit :
KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

I think he just forgot to reset his ths_sig_signal at startup.
The initial value you mentionned will make the simulation work but is
not guaranteed in the real hardware.

I'm not sure what you mean by "ths_sig_signal" but I think you may have

meant "ths_sig_internal" which is initialized and he also said it was
doing things (i.e. 00, 0X, X0, XX)

I agree that most synthesis tools don't honor initial values so the
resulting hardware will act differently than the simulation...but right
now he's still in simulation mode....one step at a time.

KJ

 

[Nicolas Matringe]

KJ a écrit :

Nicolas Matringe wrote:
KJ a écrit :
KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".
I think he just forgot to reset his ths_sig_signal at startup.
The initial value you mentionned will make the simulation work but is
not guaranteed in the real hardware.

I'm not sure what you mean by "ths_sig_signal" but I think you may have
meant "ths_sig_internal" which is initialized and he also said it was
doing things (i.e. 00, 0X, X0, XX)

Right you are, I apologize.

NIcolas

 

[Pleg]

updating: process(clock)
begin
KJ GACK, NOT LIKE THIS!!: if((clock'event and clock='1') and
(ths_counter_updater='1')) then
if rising_edge(clock) then -- KJ added
if (ths_counter_updater='1') then -- KJ added

"(clock'event and clock='1')" worked always fine with me, why do you suggest
using "rising_edge(clock)"? Anyway, this is not the problem...

KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

THIS is the problem! You're right, there are 2 drivers, and I've never fully
understood this thing: these 2 drivers NEVER happen to work at the same time
(from a logical point of view). I explain:
there is a FSM with a "reset" state (which forces "ths_sig_internal" to 0 at
the beginning), and other states; one of these states is the one that
asserts "ths_counter_updater". Therefore, the two drivers should never work
at the same time, and in my mind I can't really figure out why it happens!
Could you please give me a hint of why it happens?

Anyway, I've solved putting all the drivers to "ths_sig_internal" in a
single process (where "reset_ths_sig_internal" is clearly asserted in the
"reset" state of the FSM):


reset_and_update: process(clock)
begin
if clock'event and clock='1' then
if reset_ths_sig_internal='1' then
ths_sig_internal <= B"00";
elsif ths_counter_updater='1' then
case ths_sig_internal is
when B"00" => ths_sig_internal <= B"01";
when B"01" => ths_sig_internal <= B"10";
when B"10" => ths_sig_internal <= B"10";
when others => ths_sig_internal <= B"11";
end case;
end if;
end if;
end process reset_and_update;




Thank you for the help,

Pleg

 

[radarman]

KJ wrote:

Nicolas Matringe wrote:
KJ a écrit :
KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

I think he just forgot to reset his ths_sig_signal at startup.
The initial value you mentionned will make the simulation work but is
not guaranteed in the real hardware.

I'm not sure what you mean by "ths_sig_signal" but I think you may have
meant "ths_sig_internal" which is initialized and he also said it was
doing things (i.e. 00, 0X, X0, XX)

I agree that most synthesis tools don't honor initial values so the
resulting hardware will act differently than the simulation...but right
now he's still in simulation mode....one step at a time.

KJ

Not to put to fine a point on it, but why would you fudge a simulation
for a design intended to be synthesized? As a rule, I never use
simulation only constructs - with a precious few exceptions for models
that rely on the underlying behavior of the device - to solve
simulation problems. If you do, your simulation will essentially be
useless as a diagnostic tool.

This is one of the first things we try to ingraine in new engineers
where I work - because you can simulate plenty of things that won't
synthesize. I wish I had a nickel for every counter I've seen a new
college grad design that works great in Modelsim, but won't even pass
through a synthesis tool without errors.

Note, I DO use sim constructs in test benches, where it's OK to use
simulation only constructs. So, if I need to preinitialize some signal
that is simulating a piece of hardware, that's fine. I just don't do
any of that stuff in the synthesizable code. Shoot, I even modelled a
digital synthesis chip (Analog Devices DDS 9858) in VHDL, and have the
model driving one of my clock domains in a test bench. That clearly
isn't synthesizable - but is useful for debug. You just need to know
where to put the boundary.

 

[Dave Pollum]

Pleg wrote:

updating: process(clock)
begin
KJ GACK, NOT LIKE THIS!!: if((clock'event and clock='1') and
(ths_counter_updater='1')) then
if rising_edge(clock) then -- KJ added
if (ths_counter_updater='1') then -- KJ added

"(clock'event and clock='1')" worked always fine with me, why do you suggest
using "rising_edge(clock)"? Anyway, this is not the problem...


KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

THIS is the problem! You're right, there are 2 drivers, and I've never fully
understood this thing: these 2 drivers NEVER happen to work at the same time
(from a logical point of view). I explain:
there is a FSM with a "reset" state (which forces "ths_sig_internal" to 0 at
the beginning), and other states; one of these states is the one that
asserts "ths_counter_updater". Therefore, the two drivers should never work
at the same time, and in my mind I can't really figure out why it happens!
Could you please give me a hint of why it happens?

Anyway, I've solved putting all the drivers to "ths_sig_internal" in a
single process (where "reset_ths_sig_internal" is clearly asserted in the
"reset" state of the FSM):


reset_and_update: process(clock)
begin
if clock'event and clock='1' then
if reset_ths_sig_internal='1' then
ths_sig_internal <= B"00";
elsif ths_counter_updater='1' then
case ths_sig_internal is
when B"00" => ths_sig_internal <= B"01";
when B"01" => ths_sig_internal <= B"10";
when B"10" => ths_sig_internal <= B"10";
when others => ths_sig_internal <= B"11";
end case;
end if;
end if;
end process reset_and_update;




Thank you for the help,

Pleg

Perhaps I don't understand your code, but it looks like
"ths_sig_internal" has values of 0("00"),1 ("01"), and then is
constantly assigned 2 ("10").

-Dave Pollum

 

[KJ]

"radarman" <jshamlet@gmail.com> wrote in message
news:1160227255.504271.233490@e3g2000cwe.googlegroups.com...

I agree that most synthesis tools don't honor initial values so the
resulting hardware will act differently than the simulation...but right
now he's still in simulation mode....one step at a time.

KJ

Not to put to fine a point on it, but why would you fudge a simulation
for a design intended to be synthesized?

KJ: There was nothing in the orginal post that indicated that the code was
intended to be synthesized so without knowing what the context of the whole
problem was I don't consider it a fudge. Sometimes when replies go off on
tangents the original problem gets either ignored or not properly explained.

As a rule, I never use
simulation only constructs - with a precious few exceptions for models
that rely on the underlying behavior of the device - to solve
simulation problems. If you do, your simulation will essentially be
useless as a diagnostic tool.

KJ: Depends on what you're modelling. I've modelled processors for board
level sims where the code is in no way close to anything synthesizable. In
some ways it mimics how the software may be structured in that I will have
processes that are somewhat like a software thread. For example, there will
be a process that triggers on reset going away and then starts up an
initialization sequence of reads and writes to registers on the board...just
like the way the real system will behave in some fashion. There will be
another process that triggers on the interrupt pin, etc. Finally there
needs to be some form of traffic cop that simply keeps track of which
process 'owns' the address/data bus that every process is going to be
clamoring for once they are activated (which is why each process will have
essentially a 'bus request' signal output and will stall waiting for a 'bus
grant' signal back from the arbiter. That arbiter corresponds to nothing
physical in the device itself (probably but is purely a mechanism to allow
me to model the processor in a way that roughly mimics real code. Note, I'm
not trying to create a model for processor 'X' for use in any application,
just a model for processor 'X' running our particular application. When
modelling something simple (for example, a TTL type part), the model will
tend to be synthesizable even though I'm not intending for it to be
synthesized, just used as a sim model.

This is one of the first things we try to ingraine in new engineers
where I work - because you can simulate plenty of things that won't
synthesize. I wish I had a nickel for every counter I've seen a new
college grad design that works great in Modelsim, but won't even pass
through a synthesis tool without errors.

KJ: Another good rule is to not allow the early grads to use anything other
than std_logic/std_ulogic/signed/unsigned until you believe they are
adequately trained. Use of enumerated types and integers (and subtypes) can
cause simulation mismatches because even without an explicit initialization
the simulator will initialize it 'for you' which will not happen in
synthesis either. You don't necessarily want to ingrain using other types
though as being 'bad' just that they are something that requires more
experience to handle properly. Once the engineer is having no difficulty
'getting rid of those darn 'X' values' they are likely ready to be turned
loose. Use of integers and types results in far more succint and reusable
code if done properly...if not...well...there will be problems that need to
be cleaned up.

Note, I DO use sim constructs in test benches, where it's OK to use
simulation only constructs. So, if I need to preinitialize some signal
that is simulating a piece of hardware, that's fine. I just don't do
any of that stuff in the synthesizable code. Shoot, I even modelled a
digital synthesis chip (Analog Devices DDS 9858) in VHDL, and have the
model driving one of my clock domains in a test bench. That clearly
isn't synthesizable - but is useful for debug. You just need to know
where to put the boundary.

Yep, I agree completely but again there was really no context in the
original post to indicate if this was intended for simulation only or for
synthesis or a homework assignment.

KJ

 

[KJ]

"Pleg" <25mV@300.K> wrote in message
news:MMKVg.16711$pp1.2445@tornado.fastwebnet.it...

updating: process(clock)
begin
KJ GACK, NOT LIKE THIS!!: if((clock'event and clock='1') and
(ths_counter_updater='1')) then
if rising_edge(clock) then -- KJ added
if (ths_counter_updater='1') then -- KJ added

"(clock'event and clock='1')" worked always fine with me, why do you
suggest
using "rising_edge(clock)"? Anyway, this is not the problem...
'rising_edge' is clearer. Every engineer who learns VHDL probably was

taught "clock'event and clock='1'" even though the rising_edge function has
been there darn near forever (it's part of the same ieee std_logic_1164
package that brings us the std_logic type).

Typing 'rising_edge(Clock)' (or falling_edge(Clock)) is quicker.

As a more minor point, when you're simulating, if your clock changes from
'X' to '1' then "clock'event and clock='1'" will be true but
rising_edge(clock) will not. Pondering on that for a moment, 'X' means that
it could be '0' or it could be a '1' the simulator just hasn't been able to
figure it out yet, so if the 'X' happened to be a '1' then there would be no
rising edge in a real system. The same can said about a transition from 'L'
to 'H' (i.e. a weak low to weak high).

'rising_edge' wasn't intended to fix any problem just something I like to
encourage in the interest of productivity and more readable code.

Even the splitting out of "if ths_counter_updater = '1'" itself was not a
problem for your simulation but if you intend to use this code to program a
real
device you'd probably find that some (maybe all) tools would choke on that
line and not know what to do. But again, those are tools for turning VHDL
into bit streams for programming devices....that is far different than what
a simulation tool is intended to do since a simulator is not constrained to
only simulate that which can be synthesized.

KJ: It looks like it's trying to count but you must have two drivers
for the signal "ths_sig_internal" and that 'other' driver is always
trying to drive ths_sig_internal to "00". So when your process above
tries to change it to "01" it conflicts with the "00" producing "0X".

THIS is the problem! You're right, there are 2 drivers, and I've never
fully
understood this thing: these 2 drivers NEVER happen to work at the same
time
(from a logical point of view). I explain:
there is a FSM with a "reset" state (which forces "ths_sig_internal" to 0
at
the beginning), and other states; one of these states is the one that
asserts "ths_counter_updater". Therefore, the two drivers should never
work
at the same time, and in my mind I can't really figure out why it happens!
Could you please give me a hint of why it happens?
Every process has 'outputs'. Each concurrent statement also has an

'output'. There is no logical point of view that would allow for two
outputs to be connected together unless the underlying signal type was one
that allows for this (std_logic does, std_ulogic and most other types do
not). By using the std_logic type you are implicitly saying that, from the
compiler's point of view, it is OK to have two drivers connected to the same
signal. The burden is then on you to make sure that whenever one process is
actively driving the signal to a '0' or a '1' that the 'other' process(es)
all drive that signal to a 'Z'.

By using std_ulogic instead of std_logic you essentially are telling the
compiler that any time you have more than one driver for a signal,
that's an error. Hopefully, if you took my suggestion to switch to
std_ulogic_vector an recompiled the error was pointed right out to you by
the compiler without having to debug it. When you use std_logic instead of
std_ulogic than you run the risk of having to debug (or ask for help) to try
to find the multiple drivers of a signal that is causing the 'X'.

The only time std_logic is required is for true tri-statable outputs (i.e.
'1', '0' or 'Z' are actively driven).....an example would be a memory device
(i.e. SRAM, DRAM, etc)

Anyway, I've solved putting all the drivers to "ths_sig_internal" in a
single process (where "reset_ths_sig_internal" is clearly asserted in the
"reset" state of the FSM):


reset_and_update: process(clock)
begin
if clock'event and clock='1' then
if reset_ths_sig_internal='1' then
ths_sig_internal <= B"00";
elsif ths_counter_updater='1' then
case ths_sig_internal is
when B"00" => ths_sig_internal <= B"01";
when B"01" => ths_sig_internal <= B"10";
when B"10" => ths_sig_internal <= B"10";
when others => ths_sig_internal <= B"11";
end case;
end if;
end if;
end process reset_and_update;




Thank you for the help,

You're welcome

KJ

 

[Pleg]

OK, everything's clear now. Thanks a lot!


Pleg