Merge pull request ipbus#37 from ipbus/enhancement/34

Enhancement/34

components/ipbus_slaves/firmware/hdl/ipbus_ctrlreg_v.vhd

@@ -31,7 +31,11 @@
 -- Provides N_CTRL control registers (32b each), rw
 -- Provides N_STAT status registers (32b each), ro
 --
--- Bottom part of read address space is control, top is status
+-- Address space needed is twice that needed by the largest block of registers, unless
+-- one of N_CTRL or N_STAT is zero.
+--
+-- By default, bottom part of read address space is control, top is status.
+-- Set SWAP_ORDER to reverse this.
 --
 -- Dave Newbold, July 2012
 
@@ -44,7 +48,8 @@ use work.ipbus_reg_types.all;
 entity ipbus_ctrlreg_v is
 	generic(
 		N_CTRL: natural := 1;
-		N_STAT: natural := 1
+		N_STAT: natural := 1;
+		SWAP_ORDER: boolean := false
 	);
 	port(
 		clk: in std_logic;
@@ -71,7 +76,8 @@ architecture rtl of ipbus_ctrlreg_v is
 begin
 
 	sel <= to_integer(unsigned(ipbus_in.ipb_addr(ADDR_WIDTH - 1 downto 0))) when ADDR_WIDTH > 0 else 0;
-	stat_cyc <= ipbus_in.ipb_addr(ADDR_WIDTH) when N_CTRL /= 0 else '1'; -- FIXME; this is incorrect when N_STAT = 0
+	stat_cyc <= '0' when N_STAT = 0 else '1' when N_CTRL = 0 else
+		ipbus_in.ipb_addr(ADDR_WIDTH) when not SWAP_ORDER else not ipbus_in.ipb_addr(ADDR_WIDTH);
 	cw_cyc <= ipbus_in.ipb_strobe and ipbus_in.ipb_write and not stat_cyc;
 
 	process(clk)

components/ipbus_slaves/firmware/hdl/ipbus_syncreg_v.vhd

@@ -26,12 +26,16 @@
 
 -- ipbus_syncreg_v
 --
--- Generic control / status register bank
+-- Clock-domain crossing control / status register bank
 --
 -- Provides N_CTRL control registers (32b each), rw
 -- Provides N_STAT status registers (32b each), ro
 --
--- Bottom part of read address space is control, top is status
+-- Address space needed is twice that needed by the largest block of registers, unless
+-- one of N_CTRL or N_STAT is zero.
+--
+-- By default, bottom part of read address space is control, top is status.
+-- Set SWAP_ORDER to reverse this.
 --
 -- Both control and status are moved across clock domains with full handshaking
 -- This may be overkill for some applications
@@ -41,13 +45,16 @@
 library IEEE;
 use IEEE.STD_LOGIC_1164.ALL;
 use ieee.numeric_std.all;
+use ieee.std_logic_misc.all;
+
 use work.ipbus.all;
 use work.ipbus_reg_types.all;
 
 entity ipbus_syncreg_v is
 	generic(
 		N_CTRL: natural := 1;
-		N_STAT: natural := 1
+		N_STAT: natural := 1;
+		SWAP_ORDER: boolean := false
 	);
 	port(
 		clk: in std_logic;
@@ -69,23 +76,25 @@ architecture rtl of ipbus_syncreg_v is
 	constant ADDR_WIDTH: integer := integer_max(calc_width(N_CTRL), calc_width(N_STAT));
 
 	signal sel: integer range 0 to 2 ** ADDR_WIDTH - 1 := 0;
-	signal ctrl_cyc_w, ctrl_cyc_r, stat_cyc: std_logic;
+	signal s_cyc, ctrl_cyc_w, ctrl_cyc_r, stat_cyc: std_logic;
 	signal cq: ipb_reg_v(2 ** ADDR_WIDTH - 1 downto 0);
 	signal sq, ds: std_logic_vector(31 downto 0);
-	signal cbusy, cack: std_logic_vector(N_CTRL - 1 downto 0);
-	signal sbusy, sack, sre, sstb: std_logic;
-	signal busy, ack, busy_d, pend: std_logic;
+	signal crdy, cack: std_logic_vector(N_CTRL - 1 downto 0);
+	signal sre, srdy, sack, sstb: std_logic;
+	signal rdy, ack, rdy_d, pend: std_logic;
 
 begin
 
+-- Address selects
+
 	sel <= to_integer(unsigned(ipb_in.ipb_addr(ADDR_WIDTH - 1 downto 0))) when ADDR_WIDTH > 0 else 0;
+	s_cyc <= '0' when N_STAT = 0 else '1' when N_CTRL = 0 else
+		ipb_in.ipb_addr(ADDR_WIDTH) when not SWAP_ORDER else not ipb_in.ipb_addr(ADDR_WIDTH);
+	stat_cyc <= ipb_in.ipb_strobe and not ipb_in.ipb_write and s_cyc;
+	ctrl_cyc_r <= ipb_in.ipb_strobe and not ipb_in.ipb_write and not s_cyc;
+	ctrl_cyc_w <= ipb_in.ipb_strobe and ipb_in.ipb_write and not s_cyc;
 
-	ctrl_cyc_w <= ipb_in.ipb_strobe and ipb_in.ipb_write and not ipb_in.ipb_addr(ADDR_WIDTH) when N_CTRL /= 0
-		else '0';
-	ctrl_cyc_r <= ipb_in.ipb_strobe and not ipb_in.ipb_write and not ipb_in.ipb_addr(ADDR_WIDTH) when N_CTRL /= 0
-		else '0';
-	stat_cyc <= ipb_in.ipb_strobe and not ipb_in.ipb_write and ipb_in.ipb_addr(ADDR_WIDTH) when N_CTRL /= 0
-		else ipb_in.ipb_strobe and not ipb_in.ipb_write;
+-- Write registers
 
 	w_gen: for i in N_CTRL - 1 downto 0 generate
 
@@ -94,36 +103,38 @@ begin
 
 	begin
 
-		cwe <= '1' when ctrl_cyc_w = '1' and sel = i and busy = '0' else '0';
+		cwe <= '1' when ctrl_cyc_w = '1' and sel = i and rdy = '1' else '0';
 		ctrl_m <= ipb_in.ipb_wdata and qmask(i);
 
 		wsync: entity work.syncreg_w
 			port map(
 				m_clk => clk,
 				m_rst => rst,
 				m_we => cwe,
-				m_busy => cbusy(i),
+				m_rdy => crdy(i),
 				m_ack => cack(i),
 				m_d => ctrl_m,
-				m_q => cq(i),
 				s_clk => slv_clk,
-				s_q => q(i),
+				s_q => cq(i),
 				s_stb => stb(i)
 			);
 
 	end generate;
 
 	cq(2 ** ADDR_WIDTH - 1 downto N_CTRL) <= (others => (others => '0'));
 
+-- Read register	
+
 	ds <= d(sel) when sel < N_STAT else (others => '0');
-	sre <= '1' when stat_cyc = '1' and busy = '0' else '0';
+
+	sre <= stat_cyc and rdy;
 
 	rsync: entity work.syncreg_r
 		port map(
 			m_clk => clk,
 			m_rst => rst,
 			m_re => sre,
-			m_busy => sbusy,
+			m_rdy => srdy,
 			m_ack => sack,
 			m_q => sq,
 			s_clk => slv_clk,
@@ -142,20 +153,26 @@ begin
 			end if;
 		end loop;
 	end process;
+
+-- Interlock to catch situation where strobe is dropped in middle of write / read cycle
 
 	process(clk)
 	begin
 		if rising_edge(clk) then
-			busy_d <= busy;
-			pend <= (pend or (busy and not busy_d)) and ipb_in.ipb_strobe and not rst;
+			rdy_d <= rdy;
+			pend <= (pend or (not rdy and rdy_d)) and ipb_in.ipb_strobe and not rst and not ack;
 		end if;
 	end process;
 
-	busy <= '1' when cbusy /= (cbusy'range => '0') or sbusy = '1' else '0';
-	ack <= '1' when (cack /= (cack'range => '0') or sack = '1') and pend = '1' else '0';
+	rdy <= and_reduce(crdy) and srdy;
+	ack <= (or_reduce(cack) or sack) and pend;
+
+-- ipbus interface
 
 	ipb_out.ipb_rdata <= cq(sel) when ctrl_cyc_r = '1' else sq;
-	ipb_out.ipb_ack <= ((ctrl_cyc_w or stat_cyc) and ack) or ctrl_cyc_r;
+	ipb_out.ipb_ack <= ((ctrl_cyc_w or stat_cyc) and ack) or (ctrl_cyc_r and rdy);
 	ipb_out.ipb_err <= '0';
 
+	q <= cq(N_CTRL - 1 downto 0);
+
 end rtl;

components/ipbus_slaves/firmware/hdl/syncreg_r.vhd

@@ -26,10 +26,11 @@
 
 -- syncreg_r
 --
--- Clock domain crossing register with full handshaking
--- 	Data are transferred from slave to master when re is asserted
--- 	New requests are ignored while busy is high
--- 	Ack signals completed transfer
+-- Clock domain crossing register with two-way handshaking
+-- Data are transferred from slave to master when re is asserted
+-- New requests are ignored while rdy is low
+-- Ack signals completed transfer
+-- Data is sampled from slv side on cycle after s_stb goes high
 --
 -- Dave Newbold, June 2013
 
@@ -38,71 +39,74 @@ use IEEE.STD_LOGIC_1164.ALL;
 
 entity syncreg_r is
 	generic(
-		size: positive := 32
+		SIZE: positive := 32
 	);
 	port(
 		m_clk: in std_logic;
 		m_rst: in std_logic;
 		m_re: in std_logic;
-		m_busy: out std_logic;
 		m_ack: out std_logic;
-		m_q: out std_logic_vector(size - 1 downto 0);
+		m_rdy: out std_logic;
+		m_q: out std_logic_vector(SIZE - 1 downto 0);
 		s_clk: in std_logic;
-		s_d: in std_logic_vector(size - 1 downto 0);
+		s_d: in std_logic_vector(SIZE - 1 downto 0);
 		s_stb: out std_logic
 	);
 
 end syncreg_r;
 
 architecture rtl of syncreg_r is
 
-	signal we, busy, ack, s1, s2, s3, s4, m1, m2, m3: std_logic;
+	signal we, rdy, cyc, ack, s1, s2, s3, s4, m1, m2, m3: std_logic;
 
 	attribute SHREG_EXTRACT: string;
-	attribute SHREG_EXTRACT of s1: signal is "no"; -- Synchroniser not to be optimised into shreg
-	attribute SHREG_EXTRACT of m1: signal is "no"; -- Synchroniser not to be optimised into shreg
+	attribute SHREG_EXTRACT of s1, m1: signal is "no"; -- Synchroniser not to be optimised into shreg
 	attribute ASYNC_REG: string;
-	attribute ASYNC_REG of s1: signal is "yes";
-	attribute ASYNC_REG of m1: signal is "yes";
+	attribute ASYNC_REG of s1, m1: signal is "yes";
 
 begin
-
-	process(s_clk)
-	begin
-		if rising_edge(s_clk) then
-			if we = '1' then
-				m_q <= s_d;
-			end if;
-		end if;
-	end process;
 
+-- Generate cyc and recover handshake into master domain
+
 	process(m_clk)
 	begin
 		if rising_edge(m_clk) then
-			m1 <= s3; -- Clock domain crossing for ack handshake
+			m1 <= s4; -- CDC, with synchroniser
 			m2 <= m1;
 			m3 <= m2;
-			busy <= (busy or m_re) and not (ack or m_rst);
+			cyc <= (cyc or (m_re and rdy)) and not (ack or m_rst);
+			rdy <= (rdy or m_rst or (m3 and not m2)) and not m_re;
 		end if;
 	end process;
 
 	ack <= m2 and not m3;
+	m_ack <= ack;
+	m_rdy <= rdy;
+
+-- Move cyc into slave domain, generate handshake
 
 	process(s_clk)
 	begin
 		if rising_edge(s_clk) then
-			s1 <= busy; -- Clock domain crossing for we handshake
+			s1 <= cyc; -- CDC, with synchroniser
 			s2 <= s1;
 			s3 <= s2;
 			s4 <= s3;
 		end if;
 	end process;
 
+-- Capture register, in slave domain
+
 	we <= s3 and not s4;
 	s_stb <= s2 and not s3;
 
-	m_busy <= busy;
-	m_ack <= ack;
-
+	process(s_clk)
+	begin
+		if rising_edge(s_clk) then
+			if we = '1' then
+				m_q <= s_d;
+			end if;
+		end if;
+	end process;
+
 end rtl;
-