VHDL simulation does not work

Question

With no experience in VHDL, I am trying to convert a C++ code for driving a CCD to VHDL based code. This is very likely not the best way to implement it, but it's intuitive to me. So, my counter counts clock cycles and based on the clock cycle number, I assign the specific state to the output. I am able to implement the design successfully:

However, when I run simulation, the ALLOUT does not change, in fact it stays in ST3 state. The warnings from the test bench console:

They repeat every 25ns up to the end of the simulation. Any suggestions on where I made the mistake?

Answer 1

There are a number of style issues here and one major problem.

Fru1tbat has pointed out that you never initialise MainCounter : one way to do so would be to drive the Reset input with a simple waveform :

RESET <= '0', '1' after 100 ns, '0' after 200 ns;

in the parallel execution region (after the architecture "begin") would do so. Note that this is for simulation only, and it would be MUCH better to make RESET an input port and drive it from your simulation testbench.

Secondly (and this is the major problem, waiting to bite you randomly in real hardware) the RESET clause in process COUNT should be simplified to ONLY clear MainCounter on reset. Then the wrapround clear should be moved into the clocked part of the process:

if Reset = '1' then 
   MainCounter <= (others => '0');
elsif rising_edge(Clk) then
   if MainCounter >= 12345 then
      MainCounter <= (others => '0');
   else
      MainCounter <= MainCounter + 1;
   end if;
end if;

The reason for this is subtle if you're not used to hardware design : as expressed, you have a pile of asynchronous logic waiting to clear the counter if it momentarily transitions to a high number while counting. So counting from X"7FFF" to X"8000" it will probably momentarily pass through X"FFFF" ... and reset itself.

By moving that logic to the clocked part, it will only reset if the temporary state persists until the next clock edge : and the timing analysis tools will report timing failure if that is even a possibility.

Now the warnings : Metavalues at 0 ns are basically harmless. Metavalues persisting AFTER reset is complete are a different matter...

And the style issues :

You can compare Unsigned against simple integer literals (because numeric_std overloads comparison operators with mixed operand types) and that would make the code MUCH more readable. Better : use constant declarations instead of magic numbers. (As you discovered, you can't assign an integer literal to an unsigned; because assignment is not an operator. There are to_unsigned and to_integer conversion functions for that purpose)

There is no need for parentheses around boolean expressions e.g. in if-statements. Some C programmers use them out of habit, but it looks needlessly cluttered to me.

Your SLOW process is unclocked, despite the clock in its sensitivity list... I prefer simple clocked processes; they avoid the problems that plague combinational processes such as SLOW - which will not work correctly unless you add all the RHS signals (here - only Maincounter) to the sensitivity list.

Answer 2

MainCounter is not initialized, and is never set anywhere else (you don't drive RESET to '1' anywhere). The "metavalue" message indicates you have 'U', 'X', etc. in your inputs to the comparison (a web search should have told you that much, by the way).

You may want to consider just using integer literals, by the way. numeric_std and unsigned let you do that, and it's much easier to read. For example:

if (RESET = '1' or MainCounter >= 110000) then