Something stupid with a "case"

Discussion in 'VHDL' started by Pleg, Oct 6, 2006.

  1. Pleg

    Pleg Guest

    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
     
    Pleg, Oct 6, 2006
    #1
    1. Advertising

  2. Pleg

    KJ Guest

    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
     
    KJ, Oct 6, 2006
    #2
    1. Advertising

  3. 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
     
    Nicolas Matringe, Oct 6, 2006
    #3
  4. Pleg

    KJ Guest

    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
     
    KJ, Oct 6, 2006
    #4
  5. 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
     
    Nicolas Matringe, Oct 7, 2006
    #5
  6. Pleg

    Pleg Guest

    > > 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
     
    Pleg, Oct 7, 2006
    #6
  7. Pleg

    radarman Guest

    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.
     
    radarman, Oct 7, 2006
    #7
  8. Pleg

    Dave Pollum Guest

    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
     
    Dave Pollum, Oct 8, 2006
    #8
  9. Pleg

    KJ Guest

    "radarman" <> wrote in message
    news:...
    > 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, Oct 8, 2006
    #9
  10. Pleg

    KJ Guest

    "Pleg" <> wrote in message
    news:MMKVg.16711$...
    >> > 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
     
    KJ, Oct 9, 2006
    #10
  11. Pleg

    Pleg Guest

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


    Pleg
     
    Pleg, Oct 11, 2006
    #11
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Brandon McCombs
    Replies:
    4
    Views:
    515
    Richard Wheeldon
    Aug 28, 2006
  2. mcl
    Replies:
    5
    Views:
    330
    Justin Ezequiel
    Sep 10, 2007
  3. rincewind

    stupid, STUPID question!

    rincewind, Apr 19, 2009, in forum: HTML
    Replies:
    25
    Views:
    1,023
  4. david jensen
    Replies:
    10
    Views:
    392
  5. Adam Short

    HELP! I've done something really stupid

    Adam Short, Mar 22, 2005, in forum: ASP General
    Replies:
    1
    Views:
    92
    Adam Short
    Mar 22, 2005
Loading...

Share This Page