32-bit counter and test bench

How could this VHDL counter and its test bench be improved? I am interested in anything you see that could be done better, but especially in the test bench:

• Is wait for 10 ns better or worse than any other time delay?

• The test is very minimal. Should it do more?

counter32.vhdl:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity counter32 is
  port (
    clk: in std_ulogic;
    ena: in std_ulogic;
    rst: in std_ulogic;
    q  : out std_ulogic_vector(31 downto 0));
end entity;

architecture rtl of counter32 is

  signal count : unsigned(31 downto 0);

begin

  process(clk, rst)
  begin
    if rst = '1' then
      count <= X"00000000";
    elsif rising_edge(clk) then
      if ena = '1' then
        count <= count + 1;
      end if;
    end if;
  end process;

  q <= std_ulogic_vector(count);

end architecture;

counter32_tb.vhdl:

```
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity counter32_tb is
end counter32_tb;

architecture tb of counter32_tb is

component counter32
port (
clk: in std_ulogic;
ena: in std_ulogic;
rst: in std_ulogic;
q : out std_ulogic_vector(31 downto 0));
end component;

signal clk: std_ulogic;
signal ena: std_ulogic;
signal rst: std_ulogic;
signal q : std_ulogic_vector(31 downto 0);

begin

dut : counter32 port map (
clk => clk,
ena => ena,
rst => rst,
q => q);

process
begin

-- Reset
clk <= '0';
ena <= '1';
rst <= '1';
wait for 10 ns;
rst <= '0';
wait for 10 ns;

-- Counts on clock leading edge if enabled
clk <= '1';
wait for 10 ns;
assert q = x"00000001";

-- Does not count on clock trailing edge
clk <= '0';
wait for 10 ns;
assert q = x"00000001";

-- Does not count when not enabled
ena <= '0';
clk <= '1';
wait for 10 ns;
assert q = x"00000001";

-- Clears on reset
rst <= '1';
wait for 10 ns;
assert q = x"00000

Nice with some HDL on Code Review. I'll contribute with my opinions. All-in-all, I find the code all well-written.
Opinions not already covered in previous answers

-
A more standard way of setting count at reset would be (others => '0').

• That is, as opposed to hard-coding a bit/hex literal - even if you for some reason hard-code the width as you have done, (others => '0') is more standard in this case.

-
You might have a good reason for asynchronous reset, but if not, synchronous reset is a better habit.

• You often see these asynchronous resets, it is unfortunately a more common habit, since that's what is (or was) most often taught.

• See my motivations here: http://www.fpga-dev.com/resets-make-them-synchronous-and-local/

Opinions already covered/partly covered

Somewhat in order of importance (in my subjective opinion).

-
Define the roll-over behaviour.

• The behaviour is actually defined for an unsigned in numeric_std: any carry bits will be ignored so the unsigned will roll-over. However, neither simulators nor synthesis tools should be trusted on following this. (And even less, humans reading the code.) Also; it is good to really think about what behaviour you desire, or is the most stable: roll-over or saturate? I'll take the chance to give two comments on this choice:

• For synthesis, roll-over rather than saturation will yield less logic and improved timing. The tool will be able to use a counter macro right-off.

• If reaching max count is expected never to happen, add an assertion so a simulation will flag in that case. Then still code the RTL behaviour explicitly to roll-over (less logic, better timing) if there aren't stability reasons to code it to saturate.

-
Parameterize the width. Put it in a constant or generic.

-
Generate the clock separately (e.g. clk



• Then, when coding your stimuli, do not dead-reckon the time for stimuli assignments, instead, use wait until rising_edge(clk); etc. This makes code more stable, and gives deterministic and proper times for the stimuli changes (not only in regards to time, but also to simulation deltas, which is equally important).

-
Use entity instantiation

• More details and motivation: http://www.fpga-dev.com/leaner-vhdl-with-entity-instantiation/

Simulation performance

• There's nothing wrong in coding the RTL of the counter like this, but it could be noted that there is a simulation performance gain possible: Make count a variable, local to the process, do tests and arithmetics on this variable, and in the end of the process drive the variable to a counter signal` (defined in the architecture).

Others

• Thumbs up for self-checking test benches.

StackExchange Code Review Q#28784, answer score: 8