soc_reset: Use counters, add synchronizers

In some cases we need to keep the reset held for much longer,
so use counters rather than shift registers.

Additionally, some signals such as ext_rst and pll_locked
or signals going from the ext_clk domain to the pll_clk
domain need to be treated as async, and testing them without
synchronizers is asking for trouble.

Finally, make the external reset also reset the PLL.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

fpga/soc_reset.vhdl

@@ -1,10 +1,11 @@
 library ieee;
 use ieee.std_logic_1164.all;
+use ieee.numeric_std.all;
 
 entity soc_reset is
     generic (
-        PLL_RESET_CLOCKS : integer := 32;
+        PLL_RESET_BITS   : integer := 5;
-        SOC_RESET_CLOCKS : integer := 32;
+        SOC_RESET_BITS   : integer := 5;
         RESET_LOW        : boolean := true
         );
     port (
@@ -20,26 +21,38 @@ entity soc_reset is
 end soc_reset;
 
 architecture rtl of soc_reset is
-    signal ext_rst_n     : std_ulogic;
+    signal ext_rst0_n    : std_ulogic;
-    signal rst_n         : std_ulogic;
+    signal ext_rst1_n    : std_ulogic := '0';
-    signal pll_rst_reg : std_ulogic_vector(PLL_RESET_CLOCKS downto 0) := (others => '1');
+    signal ext_rst2_n    : std_ulogic := '0';
-    signal soc_rst_reg   : std_ulogic_vector(SOC_RESET_CLOCKS downto 0) := (others => '1');
+    signal rst0_n        : std_ulogic;
+    signal rst1_n        : std_ulogic := '0';
+    signal rst2_n        : std_ulogic := '0';
+    signal pll_rst_cnt   : std_ulogic_vector(PLL_RESET_BITS downto 0) := (others => '0');
+    signal soc_rst_cnt   : std_ulogic_vector(SOC_RESET_BITS downto 0) := (others => '0');
 begin
-    ext_rst_n <= ext_rst_in when RESET_LOW else not ext_rst_in;
+    ext_rst0_n <= ext_rst_in when RESET_LOW else not ext_rst_in;
-    rst_n <= ext_rst_n and pll_locked_in;
+    rst0_n <= ext_rst0_n and pll_locked_in and not pll_rst_out;
 
     -- PLL reset is active high
-    pll_rst_out <= pll_rst_reg(0);
+    pll_rst_out <= not pll_rst_cnt(pll_rst_cnt'left);
     -- Pass active high reset around
-    rst_out <= soc_rst_reg(0);
+    rst_out <= not soc_rst_cnt(soc_rst_cnt'left);
 
     -- Wait for external clock to become stable before starting the PLL
     -- By the time the FPGA has been loaded the clock should be well and
     -- truly stable, but lets give it a few cycles to be sure.
+    --
+    -- [BenH] Some designs seem to require a lot more..
     pll_reset_0 : process(ext_clk)
     begin
         if (rising_edge(ext_clk)) then
-            pll_rst_reg <= '0' & pll_rst_reg(pll_rst_reg'length-1 downto 1);
+            ext_rst1_n <= ext_rst0_n;
+            ext_rst2_n <= ext_rst1_n;
+            if (ext_rst2_n = '0') then
+                pll_rst_cnt <= (others => '0');
+            elsif (pll_rst_cnt(pll_rst_cnt'left) = '0') then
+                pll_rst_cnt <= std_ulogic_vector(unsigned(pll_rst_cnt) + 1);
+            end if;
         end if;
     end process;
 
@@ -49,10 +62,12 @@ begin
     soc_reset_0 : process(pll_clk)
     begin
         if (rising_edge(pll_clk)) then
-            if (rst_n = '0') then
+            rst1_n <= rst0_n;
-                soc_rst_reg <= (others => '1');
+            rst2_n <= rst1_n;
-            else
+            if (rst2_n = '0') then
-                soc_rst_reg <= '0' & soc_rst_reg(soc_rst_reg'length-1 downto 1);
+                soc_rst_cnt <= (others => '0');
+            elsif (soc_rst_cnt(soc_rst_cnt'left) = '0') then
+                soc_rst_cnt <= std_ulogic_vector(unsigned(soc_rst_cnt) + 1);
             end if;
         end if;
     end process;

fpga/soc_reset_tb.vhdl

@@ -12,9 +12,7 @@ architecture behave of soc_reset_tb is
     signal ext_rst_in    : std_ulogic;
 
     signal pll_rst_out          : std_ulogic;
-    signal pll_rst_out_expected : std_ulogic;
     signal rst_out                : std_ulogic;
-    signal rst_out_expected       : std_ulogic;
 
     constant clk_period : time := 10 ns;
 
@@ -32,6 +30,8 @@ architecture behave of soc_reset_tb is
         ('0', '1', '1', '1'),
         ('0', '1', '1', '1'),
         ('0', '1', '1', '1'),
+        ('0', '1', '1', '1'),
+        ('0', '1', '1', '1'),
         -- Reset is removed from the PLL
         ('0', '1', '0', '1'),
         ('0', '1', '0', '1'),
@@ -41,15 +41,27 @@ architecture behave of soc_reset_tb is
         ('1', '1', '0', '1'),
         ('1', '1', '0', '1'),
         ('1', '1', '0', '1'),
+        ('1', '1', '0', '1'),
+        ('1', '1', '0', '1'),
         -- Finally SOC comes out of reset
         ('1', '1', '0', '0'),
         ('1', '1', '0', '0'),
 
         -- PLL locked, reset button pressed
-        ('1', '0', '0', '1'),
+        ('1', '0', '0', '0'),
-        ('1', '0', '0', '1'),
+        ('1', '0', '0', '0'),
-        ('1', '0', '0', '1'),
+        ('1', '0', '0', '0'),
+        ('1', '0', '1', '1'),
         -- PLL locked, reset button released
+        ('1', '1', '1', '1'),
+        ('1', '1', '1', '1'),
+        ('1', '1', '1', '1'),
+        ('1', '1', '1', '1'),
+        ('1', '1', '1', '1'),
+        ('1', '1', '1', '1'),
+        ('1', '1', '0', '1'),
+        ('1', '1', '0', '1'),
+        ('1', '1', '0', '1'),
         ('1', '1', '0', '1'),
         ('1', '1', '0', '1'),
         ('1', '1', '0', '1'),
@@ -59,8 +71,8 @@ architecture behave of soc_reset_tb is
 begin
     soc_reset_0: entity work.soc_reset
         generic map (
-            PLL_RESET_CLOCKS => 4,
+            PLL_RESET_BITS => 2,
-            SOC_RESET_CLOCKS => 4,
+            SOC_RESET_BITS => 2,
             RESET_LOW => true
             )
         port map (
@@ -83,17 +95,29 @@ begin
     end process clock;
 
     stim: process
+        variable tv : test_vector;
     begin
+        -- skew us a bit
+        wait for clk_period/4;
+
         for i in test_vectors'range loop
-            (pll_locked_in, ext_rst_in, pll_rst_out_expected, rst_out_expected) <= test_vectors(i);
+            tv := test_vectors(i);
+
+            pll_locked_in <= tv.pll_locked_in;
+            ext_rst_in <= tv.ext_rst_in;
 
-            --report "pll_locked_in " & std_ulogic'image(pll_locked_in);
+            report " ** STEP " & integer'image(i);
-            --report "ext_rst_in " & std_ulogic'image(ext_rst_in);
+            report "pll_locked_in " & std_ulogic'image(pll_locked_in);
-            --report "pll_rst_out " & std_ulogic'image(pll_rst_out);
+            report "ext_rst_in " & std_ulogic'image(ext_rst_in);
-            --report "rst_out" & std_ulogic'image(rst_out);
+            report "pll_rst_out " & std_ulogic'image(pll_rst_out);
+            report "rst_out" & std_ulogic'image(rst_out);
 
-            assert pll_rst_out_expected = pll_rst_out report "pll_rst_out bad";
+            assert tv.pll_rst_out = pll_rst_out report
-            assert rst_out_expected = rst_out report "rst_out bad";
+                "pll_rst_out bad exp="  & std_ulogic'image(tv.pll_rst_out) &
+                " got=" & std_ulogic'image(pll_rst_out);
+            assert tv.rst_out    =  rst_out     report
+                "rst_out bad exp=" & std_ulogic'image(tv.rst_out) &
+                " got=" & std_ulogic'image(rst_out);
 
             wait for clk_period;
         end loop;