VHDL coding style

[Topi-ab]

Hi,

Nice project !

Two comments about VHDL coding style:

1. You don't need to introduce "component Synchronizer..." type of declarations, when you instantiate the components this way:
   Sync_AUX1: entity work.Synchronizer ...

2. You have your synchronous reset handling in the beginning of process, and actual work-horse in else-path.
   This either requires you to reset every signal in the process, or the synthesizer implements a clock-enable signal for un-reseted signals. This shall be either native clock enable, or an input multiplexer to the DFF feeding its own output to the other input of MUX.

If you write in style:

process(clk)
begin
  if rising_edge(clk) then
    ...
  end if;
  if RESET = '1' then
    ...
  end if;
end process;

With this coding style the non-reseted signals don't have any effect from the reset signal, and they continue to toggle even when RESET is active (typically for data registers you don't want to have reset).
This helps you when you are coding for high speed logic (e.g. Intel Stratix-10 hyper-registers do not have reset input, so only registers without any reset functionality can be mapped to them).

And you have RESET in your sensitivity list, even though reset is synchronous. That is unnecessary.

[jankae]

Thank you for the feedback :)

As you are probably able to tell, I am not an expert in VHDL. This is only my second "real" project with an FPGA and it was a lot of trial and error. I'll try to keep your suggestions in mind when I make changes to it in the future (but will leave the code as it is now unless I find problems with it).

One question regarding the asynchronous reset you suggested (at least I think that is what you are suggesting): wouldn't that lead to synchronization problems again? The release of the reset signal may happen anytime with respect to the clock and this could cause certain block to react to one rising clock edge and others only to the next edge, right? Or is this not a problem at all because the reset should be synchronous to the clock anyway since it is dependent on the lock signal of the internal PLL? (which I assume will be asserted synchronous to the clock)

[Topi-ab]

About reset:

Using this Dockerfile
https://github.com/Topi-ab/rtl-tools/

You can quickly generate RTL-schematics.

File reset_test.vhdl:

library ieee;
use ieee.std_logic_1164.all;

entity reset_test is
        port(
                clk_in: in std_logic;
                sreset_in: in std_logic; --synchronous reset
                a_in: in std_logic;
                b_in: in std_logic;
                a1_out: out std_logic;
                a2_out: out std_logic;
                b1_out: out std_logic;
                b2_out: out std_logic
        );
end;

architecture rtl of reset_test is
begin
        a_pr: process(clk_in)
        begin
                if rising_edge(clk_in) then
                        if sreset_in = '1' then
                                a1_out <= '0';
                        else
                                a1_out <= a_in;
                                a2_out <= a_in;
                        end if;
                end if;
        end process;

        b_pr: process(clk_in)
        begin
                if rising_edge(clk_in) then
                        b1_out <= b_in;
                        b2_out <= b_in;
                        if sreset_in = '1' then
                                b1_out <= '0';
                        end if;
                end if;
        end process;
end;

run:

docker run -u $(id -u):$(id -g) --rm -it -v $PWD:/prj rtl-tools bash -c "yosys -m ghdl -p 'ghdl --std=08 reset_test.vhdl -e reset_test; write_json reset_test.json' ; netlistsvg reset_test.json -o reset_test.svg"

to get reset_test.svg file:

None of the DFF (D Flip Flops) are asynchronously reseted.
But the key point is:

• a2_out (which is not reseted), gets a hold circuit (MUX working effectively as Clock Enable).
• b2_out doesn't. It is just a register from b_in to b2_out.

a1_out and a2_out on the other hand, are synthesized equally.

[jankae]

Thanks for the example, it illustrates nicely why I should reset all signal with the approach I am using. But I was wondering about this approach you mentioned:

If you write in style:

process(clk)
begin
  if rising_edge(clk) then
     ...
  end if;
  if RESET = '1' then
    ...
  end if;
end process;

This would result in an asynchronous reset, correct? And from my limited understanding of VHDL, this could lead to issues when some circuitry gets out of reset faster than others. Or did I misunderstand something there?

[Topi-ab]

No, my code did not produce async reset.

This code

library ieee;
use ieee.std_logic_1164.all;

entity reset_test is
	port(
		clk_in: in std_logic;
		sreset_in: in std_logic; --synchronous reset
		a_in: in std_logic;
		b_in: in std_logic;
		c_in: in std_logic;
		a1_out: out std_logic;
		a2_out: out std_logic;
		b1_out: out std_logic;
		b2_out: out std_logic;
		c1_out: out std_logic;
		c2_out: out std_logic
	);
end;

architecture rtl of reset_test is
begin
	a_pr: process(clk_in)
	begin
		if rising_edge(clk_in) then
			if sreset_in = '1' then
				a1_out <= '0';
			else
				a1_out <= a_in;
				a2_out <= a_in;
			end if;
		end if;
	end process;
	
	b_pr: process(clk_in)
	begin
		if rising_edge(clk_in) then
			b1_out <= b_in;
			b2_out <= b_in;
			if sreset_in = '1' then
				b1_out <= '0';
			end if;
		end if;
	end process;
	
	c_pr: process(sreset_in, clk_in)
	begin
		if rising_edge(clk_in) then
			c1_out <= c_in;
			c2_out <= c_in;
		end if;
		if sreset_in = '1' then
			c1_out <= '0';
		end if;
	end process;
end;

generates async reset for c1_out.

Vhdl process is executed in simulator like this:

• Any change in any of the signals in the process sensitivity list (process(sens_list)) causes the process to be executed once.
• Final assignment of any signal during process execution is the value the process is driving. (Any non-final signal assignments go to /dev/null, nobody sees them ever).

Note that b1_out reset-if is executed only if there is a rising edge on clk_in.

For c1_out, on the other hand, the process is executed if there is a change on sreset_in or clk_in. And the reset-if is not dependent on clk_in, but sreset_in only => thus generating async reset functionality.

Properly working logic synthesizer (not all tools are like this, though) either

• generates logic which behaves equally to the simulation of the source code; or
• throws an error of unsynthesizable code.

And yes, async reset (as well as sync reset) needs to be properly timed.
Async reset assignment timing (non-active => active) is not critical, but reset release (active => non-active) needs to meet setup and hold time (against clock) of the receiving DFF . Also minimum pulse width of the async reset signal needs to meet requirements.

There are ways to achieve the timing.

Reset signal can be captured by few retiming DFF:s (clock domain crossing), and the timed reset is used for the logic. Works both for sync and async reset.

Clock of the logic is disabled during async reset release (clock gating). Some clock buffers have enable input, which can be used to disable the clock at will. One variation is:

1. Activate reset
2. Allow clock to run for few or more cycles (this allows optional non-reseted pipeline buffers to reach reset state)
3. Disable clock
4. Release reset
5. Enable clock

This way the reset needs not to be timed against clock. Works also in multi-clock designs, but the enable order of different clocks might need proper planning.