Rework SOC reset

The old reset code was overly complicated and never worked properly.
Replace it with a simpler sequence that uses a couple of shift registers
to assert resets:

- Wait a number of external clock cycles before removing reset from
  the PLL.

- After the PLL locks and the external reset button isn't pressed,
  wait a number of PLL clock cycles before removing reset from the SOC.

Signed-off-by: Anton Blanchard <anton@linux.ibm.com>

Makefile

@@ -2,7 +2,7 @@ GHDL=ghdl
 GHDLFLAGS=--std=08
 CFLAGS=-O2 -Wall
 
-all = core_tb simple_ram_behavioural_tb
+all = core_tb simple_ram_behavioural_tb soc_reset_tb
 # XXX
 # loadstore_tb fetch_tb
 
@@ -41,6 +41,11 @@ wishbone_arbiter.o: wishbone_types.o
 wishbone_types.o:
 writeback.o: common.o
 
+fpga/soc_reset_tb.o: fpga/soc_reset.o
+
+soc_reset_tb: fpga/soc_reset_tb.o fpga/soc_reset.o
+	$(GHDL) -e $(GHDLFLAGS) soc_reset_tb
+
 core_tb: core_tb.o simple_ram_behavioural_helpers_c.o sim_console_c.o
 	$(GHDL) -e $(GHDLFLAGS) -Wl,simple_ram_behavioural_helpers_c.o -Wl,sim_console_c.o $@
 

fpga/clk_gen_bypass.vhd

@@ -3,10 +3,10 @@ use ieee.std_logic_1164.all;
 
 entity clock_generator is
   port (
-    clk        : in  std_logic;
+    ext_clk        : in  std_logic;
-    resetn     : in  std_logic;
+    pll_rst_in   : in  std_logic;
-    system_clk : out std_logic;
+    pll_clk_out : out std_logic;
-    locked     : out std_logic);
+    pll_locked_out : out std_logic);
 
 end entity clock_generator;
 
@@ -14,7 +14,7 @@ architecture bypass of clock_generator is
 
 begin
 
-  locked <= not resetn;
+  pll_locked_out <= pll_rst_in;
-  system_clk <= clk;
+  pll_clk_out <= ext_clk;
 
 end architecture bypass;

fpga/clk_gen_plle2.vhd

@@ -8,10 +8,10 @@ entity clock_generator is
   generic (
     clk_period_hz : positive := 100000000);
   port (
-    clk        : in  std_logic;
+    ext_clk        : in  std_logic;
-    resetn     : in  std_logic;
+    pll_rst_in   : in  std_logic;
-    system_clk : out std_logic;
+    pll_clk_out    : out std_logic;
-    locked     : out std_logic);
+    pll_locked_out : out std_logic);
 end entity clock_generator;
 
 architecture rtl of clock_generator is
@@ -55,17 +55,17 @@ begin
       DIVCLK_DIVIDE      => pll_settings.divclk_divide,
       STARTUP_WAIT       => "FALSE")
     port map (
-      CLKOUT0  => system_clk,
+      CLKOUT0  => pll_clk_out,
       CLKOUT1  => open,
       CLKOUT2  => open,
       CLKOUT3  => open,
       CLKOUT4  => open,
       CLKOUT5  => open,
       CLKFBOUT => clkfb,
-      LOCKED   => locked,
+      LOCKED   => pll_locked_out,
-      CLKIN1   => clk,
+      CLKIN1   => ext_clk,
       PWRDWN   => '0',
-      RST      => not resetn,
+      RST      => pll_rst_in,
       CLKFBIN  => clkfb);
 
 end architecture rtl;

fpga/pp_soc_reset.vhd

@@ -1,69 +0,0 @@
--- The Potato Processor - A simple processor for FPGAs
--- (c) Kristian Klomsten Skordal 2018 <kristian.skordal@wafflemail.net>
-
-library ieee;
-use ieee.std_logic_1164.all;
-use work.pp_utilities.all;
-
---! @brief System reset unit.
---! Because most resets in the processor core are synchronous, at least one
---! clock pulse has to be given to the processor while the reset signal is
---! asserted. However, if the clock generator is being reset at the same time,
---! the system clock might not run during reset, preventing the processor from
---! properly resetting.
-entity pp_soc_reset is
-	generic(
-		RESET_CYCLE_COUNT : natural := 20000000
-	);
-	port(
-		clk : in std_logic;
-
-		reset_n   : in  std_logic;
-		reset_out : out std_logic;
-
-		system_clk        : in std_logic;
-		system_clk_locked : in std_logic
-	);
-end entity pp_soc_reset;
-
-architecture behaviour of pp_soc_reset is
-
-	subtype counter_type is natural range 0 to RESET_CYCLE_COUNT;
-	signal counter : counter_type;
-
-	signal fast_reset : std_logic := '0';
-	signal slow_reset : std_logic := '1';
-begin
-
-	reset_out <= slow_reset;
-
---	process(clk)
---	begin
---		if rising_edge(clk) then
---			if reset_n = '0' then
---				fast_reset <= '1';
---			elsif system_clk_locked = '1' then
---				if fast_reset = '1' and slow_reset = '1' then
---					fast_reset <= '0';
---				end if;
---			end if;
---		end if;
---	end process;
-
-	process(system_clk)
-	begin
-		if rising_edge(system_clk) then
-			if reset_n = '0' then
-				slow_reset <= '1';
-				counter <= RESET_CYCLE_COUNT;
-			else
-				if counter = 0 then
-					slow_reset <= '0';
-				else
-					counter <= counter - 1;
-				end if;
-			end if;
-		end if;
-	end process;
-
-end architecture behaviour;

fpga/soc_reset.vhdl

@@ -0,0 +1,59 @@
+library ieee;
+use ieee.std_logic_1164.all;
+
+entity soc_reset is
+    generic (
+        PLL_RESET_CLOCKS : integer := 32;
+        SOC_RESET_CLOCKS : integer := 32;
+        RESET_LOW        : boolean := true
+        );
+    port (
+        ext_clk       : in std_ulogic;
+        pll_clk       : in std_ulogic;
+
+        pll_locked_in : in std_ulogic;
+        ext_rst_in    : in std_ulogic;
+
+        pll_rst_out : out std_ulogic;
+        rst_out       : out std_ulogic
+        );
+end soc_reset;
+
+architecture rtl of soc_reset is
+    signal ext_rst_n     : std_ulogic;
+    signal rst_n         : std_ulogic;
+    signal pll_rst_reg : std_ulogic_vector(PLL_RESET_CLOCKS downto 0) := (others => '1');
+    signal soc_rst_reg   : std_ulogic_vector(SOC_RESET_CLOCKS downto 0) := (others => '1');
+begin
+    ext_rst_n <= ext_rst_in when RESET_LOW else not ext_rst_in;
+    rst_n <= ext_rst_n and pll_locked_in;
+
+    -- PLL reset is active high
+    pll_rst_out <= pll_rst_reg(0);
+    -- Pass active high reset around
+    rst_out <= soc_rst_reg(0);
+
+    -- 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.
+    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);
+        end if;
+    end process;
+
+    -- Once our clock is stable and the external reset button isn't being
+    -- pressed, assert the SOC reset for long enough for the CPU pipeline
+    -- to clear completely.
+    soc_reset_0 : process(pll_clk)
+    begin
+        if (rising_edge(pll_clk)) then
+            if (rst_n = '0') then
+                soc_rst_reg <= (others => '1');
+            else
+                soc_rst_reg <= '0' & soc_rst_reg(soc_rst_reg'length-1 downto 1);
+            end if;
+        end if;
+    end process;
+end rtl;

fpga/soc_reset_tb.vhdl

@@ -0,0 +1,106 @@
+library ieee;
+use ieee.std_logic_1164.all;
+
+entity soc_reset_tb is
+end soc_reset_tb;
+
+architecture behave of soc_reset_tb is
+    signal ext_clk       : std_ulogic;
+    signal pll_clk       : std_ulogic;
+
+    signal pll_locked_in : std_ulogic;
+    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;
+
+    type test_vector is record
+        pll_locked_in : std_ulogic;
+        ext_rst_in    : std_ulogic;
+        pll_rst_out : std_ulogic;
+        rst_out       : std_ulogic;
+    end record;
+
+    type test_vector_array is array (natural range <>) of test_vector;
+    constant test_vectors : test_vector_array := (
+        -- PLL not locked, reset button not pressed
+        ('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'),
+        ('0', '1', '0', '1'),
+        -- At some point PLL comes out of reset
+        ('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', '1'),
+        ('1', '0', '0', '1'),
+        -- PLL locked, reset button released
+        ('1', '1', '0', '1'),
+        ('1', '1', '0', '1'),
+        ('1', '1', '0', '1'),
+        -- Finally SOC comes out of reset
+        ('1', '1', '0', '0')
+        );
+begin
+    soc_reset_0: entity work.soc_reset
+        generic map (
+            PLL_RESET_CLOCKS => 4,
+            SOC_RESET_CLOCKS => 4,
+            RESET_LOW => true
+            )
+        port map (
+            ext_clk => ext_clk,
+            pll_clk => pll_clk,
+            pll_locked_in => pll_locked_in,
+            ext_rst_in => ext_rst_in,
+            pll_rst_out => pll_rst_out,
+            rst_out => rst_out
+            );
+
+    clock: process
+    begin
+        ext_clk <= '0';
+        pll_clk <= '0';
+        wait for clk_period/2;
+        ext_clk <= '1';
+        pll_clk <= '1';
+        wait for clk_period/2;
+    end process clock;
+
+    stim: process
+    begin
+        for i in test_vectors'range loop
+            (pll_locked_in, ext_rst_in, pll_rst_out_expected, rst_out_expected) <= test_vectors(i);
+
+            --report "pll_locked_in " & std_ulogic'image(pll_locked_in);
+            --report "ext_rst_in " & std_ulogic'image(ext_rst_in);
+            --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 rst_out_expected = rst_out report "rst_out bad";
+
+            wait for clk_period;
+        end loop;
+
+	wait for clk_period;
+
+        assert false report "end of test" severity failure;
+        wait;
+    end process;
+end behave;

fpga/toplevel.vhd

@@ -14,7 +14,9 @@ use work.wishbone_types.all;
 entity toplevel is
 	generic (
 		MEMORY_SIZE   : positive := 524288;
-    RAM_INIT_FILE : string   := "firmware.hex");
+		RAM_INIT_FILE : string   := "firmware.hex";
+		RESET_LOW : boolean := true
+	);
 	port(
 		ext_clk   : in  std_logic;
 		ext_rst   : in  std_logic;
@@ -28,12 +30,12 @@ end entity toplevel;
 architecture behaviour of toplevel is
 
 	-- Reset signals:
-	signal rst : std_logic;
+	signal rst : std_ulogic;
+	signal pll_rst_n : std_ulogic;
 
 	-- Internal clock signals:
-	signal system_clk : std_logic;
+	signal system_clk : std_ulogic;
-	signal timer_clk  : std_logic;
+	signal system_clk_locked : std_ulogic;
-	signal system_clk_locked : std_logic;
 
 	-- wishbone signals:
 	signal wishbone_proc_out: wishbone_master_out;
@@ -137,21 +139,25 @@ begin
 		end case;
 	end process processor_intercon;
 
-	reset_controller: entity work.pp_soc_reset
+	reset_controller: entity work.soc_reset
+		generic map(
+			RESET_LOW => RESET_LOW
+		)
 		port map(
-			clk => system_clk,
+			ext_clk => ext_clk,
-			reset_n => ext_rst,
+			pll_clk => system_clk,
-			reset_out => rst,
+			pll_locked_in => system_clk_locked,
-			system_clk => system_clk,
+			ext_rst_in => ext_rst,
-			system_clk_locked => system_clk_locked
+			pll_rst_out => pll_rst_n,
+			rst_out => rst
 		);
 
 	clkgen: entity work.clock_generator
 		port map(
-			clk => ext_clk,
+			ext_clk => ext_clk,
-			resetn => ext_rst,
+			pll_rst_in => pll_rst_n,
-			system_clk => system_clk,
+			pll_clk_out => system_clk,
-			locked => system_clk_locked
+			pll_locked_out => system_clk_locked
 		);
 
 	processor: entity work.core

microwatt.core

@@ -33,7 +33,7 @@ filesets:
     files:
       - fpga/pp_fifo.vhd
       - fpga/pp_soc_memory.vhd
-      - fpga/pp_soc_reset.vhd
+      - fpga/soc_reset.vhdl
       - fpga/pp_soc_uart.vhd
       - fpga/pp_utilities.vhd
       - fpga/toplevel.vhd