From 5c423e6c6b1f5ae648804750e0456fbb14c2becb Mon Sep 17 00:00:00 2001 From: Rutherther Date: Fri, 5 Jan 2024 16:37:09 +0100 Subject: [PATCH] fix: issues on master --- src/i2c/address_generator.vhd | 19 +++++++++--- src/i2c/master.vhd | 4 +-- src/i2c/master_state.vhd | 16 +++++----- src/i2c/startstop_condition_generator.vhd | 7 +++++ src/mcu_slave/counter.vhd | 10 +++--- tb/i2c/master_tb.vhd | 37 +++++++++++++++++++++-- tb/i2c/tb_i2c_slave_pkg.vhd | 2 +- tb/mcu_slave/counter_tb.vhd | 15 +++++---- 8 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/i2c/address_generator.vhd b/src/i2c/address_generator.vhd index 14b3df3..9e03648 100644 --- a/src/i2c/address_generator.vhd +++ b/src/i2c/address_generator.vhd @@ -22,7 +22,7 @@ entity address_generator is end entity address_generator; architecture a1 of address_generator is - type state_t is (IDLE, WAITING_FOR_FALLING, GEN, ACK, DONE); + type state_t is (IDLE, WAITING_FOR_FALLING, GEN, ACK); signal curr_state : state_t; signal next_state : state_t; @@ -31,6 +31,9 @@ architecture a1 of address_generator is signal curr_scl : std_logic; signal next_scl : std_logic; + + signal curr_done : std_logic; + signal next_done : std_logic; begin -- architecture a1 sda_enable_o <= not address_i(6 - curr_index) when curr_index <= 6 and curr_state = GEN else @@ -43,20 +46,24 @@ begin -- architecture a1 unexpected_sda_o <= '1' when curr_state = GEN and curr_index <= 6 and sda_i /= address_i(6 - curr_index) and scl_rising_i = '1' else '0'; noack_o <= '1' when curr_state = ACK and scl_rising_i = '1' and sda_i = '1' else '0'; - done_o <= '1' when curr_state = DONE else '0'; next_scl <= '1' when scl_rising_i = '1' else '0' when scl_falling_delayed_i = '1' else curr_scl; + done_o <= curr_done and not next_done; + next_done <= '1' when curr_state = ACK and next_state /= ACK else + '1' when curr_done = '1' and scl_falling_delayed_i = '0' else + '0'; + set_next_state: process (all) is variable start_gen : std_logic; begin -- process set_next_state next_state <= curr_state; start_gen := '0'; - if curr_state = IDLE then - elsif curr_state = WAITING_FOR_FALLING then + -- if curr_state = IDLE then + if curr_state = WAITING_FOR_FALLING then if scl_falling_delayed_i = '1' then next_state <= GEN; end if; @@ -65,7 +72,7 @@ begin -- architecture a1 next_state <= ACK; end if; elsif curr_state = ACK and scl_rising_i = '1' then - next_state <= DONE; + next_state <= IDLE; end if; if start_i = '1' then @@ -88,10 +95,12 @@ begin -- architecture a1 curr_state <= IDLE; curr_index <= 0; curr_scl <= '1'; + curr_done <= '0'; else curr_state <= next_state; curr_index <= next_index; curr_scl <= next_scl; + curr_done <= next_done; end if; end if; end process set_regs; diff --git a/src/i2c/master.vhd b/src/i2c/master.vhd index 38ae086..d033c31 100644 --- a/src/i2c/master.vhd +++ b/src/i2c/master.vhd @@ -5,8 +5,8 @@ library utils; entity master is generic ( - SCL_FALLING_DELAY : natural := 5; - SCL_MIN_STABLE_CYCLES : natural := 10); + SCL_FALLING_DELAY : natural; + SCL_MIN_STABLE_CYCLES : natural); port ( clk_i : in std_logic; -- Synchronous clock diff --git a/src/i2c/master_state.vhd b/src/i2c/master_state.vhd index f16c8a7..894070b 100644 --- a/src/i2c/master_state.vhd +++ b/src/i2c/master_state.vhd @@ -1,5 +1,6 @@ library ieee; use ieee.std_logic_1164.all; +use ieee.numeric_std.all; entity master_state is @@ -91,7 +92,7 @@ architecture a1 of master_state is begin -- architecture a1 any_err <= curr_err_arbitration or curr_err_noack_data or curr_err_noack_address or curr_err_general; any_terminal_err <= curr_err_noack_data or curr_err_noack_address or curr_err_general; - control_over_bus <= '1' when curr_state /= IDLE and curr_state /= BUS_BUSY; + control_over_bus <= '1' when curr_state /= IDLE and curr_state /= BUS_BUSY else '0'; can_gen_cond <= '1' when curr_state = IDLE or ((curr_state = TRANSMITTING or curr_state = RECEIVING) and (waiting_for_data_i = '1' or tx_done_i = '1' or rx_done_i = '1')) else '0'; req_scl_continuous_o <= '1' when curr_state = GENERATING_ADDRESS or @@ -119,12 +120,12 @@ begin -- architecture a1 rst_i2c_o <= any_err or cond_gen; - next_gen_start_req <= '1' when start_i = '1' and run_i = '1' else - '0' when curr_state = GENERATING_START else + next_gen_start_req <= '0' when curr_state = GENERATING_START or curr_state = GENERATED_START else + '1' when start_i = '1' and run_i = '1' else curr_gen_start_req; - next_gen_stop_req <= '1' when stop_i = '1' else - '0' when curr_state = GENERATING_STOP else + next_gen_stop_req <= '0' when curr_state = GENERATING_STOP or curr_state = GENERATED_STOP else + '1' when stop_i = '1' else curr_gen_stop_req; next_err_general <= '0' when curr_state = GENERATING_START else @@ -219,12 +220,13 @@ begin -- architecture a1 end if; if can_gen_cond = '1' then - if curr_gen_start_req = '1' then + if curr_gen_start_req = '1' and curr_state /= GENERATING_START and curr_state /= GENERATED_START then next_state <= GENERATING_START; + -- if no control over bus, do not -- allow generating stop condition -- TODO consider not checking that? - elsif curr_gen_stop_req = '1' and control_over_bus = '1' then + elsif curr_gen_stop_req = '1' and control_over_bus = '1' and curr_state /= GENERATING_STOP and curr_state /= GENERATED_STOP then next_state <= GENERATING_STOP; end if; end if; diff --git a/src/i2c/startstop_condition_generator.vhd b/src/i2c/startstop_condition_generator.vhd index e929bbf..3bc7388 100644 --- a/src/i2c/startstop_condition_generator.vhd +++ b/src/i2c/startstop_condition_generator.vhd @@ -1,5 +1,6 @@ library ieee; use ieee.std_logic_1164.all; +use ieee.numeric_std.all; library utils; @@ -105,6 +106,8 @@ begin -- architecture a1 elsif curr_count = DELAY then next_state <= PREPARE_SDA; next_count_en <= '0'; + elsif curr_scl = '1' then + next_count_en <= '0'; end if; elsif curr_state = PREPARE_SDA then next_count_en <= '1'; @@ -124,6 +127,8 @@ begin -- architecture a1 elsif curr_count = DELAY then next_state <= GEN_COND; next_count_en <= '0'; + elsif curr_scl = '1' then + next_count_en <= '1'; end if; elsif curr_state = GEN_COND then -- assume correct condition here. If it's the wrong one, @@ -144,6 +149,8 @@ begin -- architecture a1 elsif curr_count = DELAY then next_state <= DONE; next_count_en <= '0'; + elsif curr_scl = '0' then + next_count_en <= '1'; end if; elsif curr_state = DONE then next_count_en <= '0'; diff --git a/src/mcu_slave/counter.vhd b/src/mcu_slave/counter.vhd index fe045e5..5cb058e 100644 --- a/src/mcu_slave/counter.vhd +++ b/src/mcu_slave/counter.vhd @@ -14,8 +14,11 @@ entity counter is port ( clk_i : in std_logic; rst_i : in std_logic; - rst_on : out std_logic; - err_noack_o : out std_logic; + err_noack_data_o : out std_logic; + err_noack_address_o : out std_logic; + err_arbitration_o : out std_logic; + err_general_o : out std_logic; + bus_busy_o : out std_logic; dev_busy_o : out std_logic; sda_io : inout std_logic; @@ -40,7 +43,6 @@ architecture a1 of counter is signal tx_data : std_logic_vector(7 downto 0); begin rst_n <= not rst_sync; - rst_on <= rst_n; sync_reset: entity utils.metastability_filter port map ( @@ -77,7 +79,7 @@ begin tx_stretch_i => '0', tx_clear_buffer_i => '0', - err_noack_o => err_noack_o, + err_noack_o => err_noack_data_o, rw_o => open, dev_busy_o => dev_busy_o, bus_busy_o => bus_busy_o, diff --git a/tb/i2c/master_tb.vhd b/tb/i2c/master_tb.vhd index 16fa3f7..93ffba9 100644 --- a/tb/i2c/master_tb.vhd +++ b/tb/i2c/master_tb.vhd @@ -47,6 +47,8 @@ architecture tb of master_tb is constant SCL_MIN_STABLE_CYCLES : natural := 10; constant TIMEOUT : time := SCL_MIN_STABLE_CYCLES * CLK_PERIOD * 2; + + signal stable_check : std_logic := '0'; begin -- architecture tb clk <= not clk after CLK_PERIOD / 2; @@ -106,6 +108,9 @@ begin -- architecture tb -- TODO ensure active only when no start/stop -- conditions should be generated... -- sda_stability_check: check_stable(clk, one, scl, not_scl, master_sda_enable); + -- + sda_stable: check_stable(clk, stable_check, one, zero, master_sda); + scl_stable: check_stable(clk, stable_check, one, zero, master_scl); main: process is procedure request_start( @@ -195,7 +200,7 @@ begin -- architecture tb check_errors; i2c_slave_check_stop(TIMEOUT, scl, sda); check_errors; - elsif run("waiting") then + elsif run("waiting_write_tx") then request_start("1110101", '0'); i2c_slave_check_start("1110101", '0', TIMEOUT, scl, sda); check_errors; @@ -203,9 +208,10 @@ begin -- architecture tb wait until falling_edge(clk); wait until falling_edge(clk); wait until falling_edge(clk); + wait until falling_edge(clk); for i in 0 to 100 loop - check_equal(waiting, '1'); - check_equal(scl, '0'); + check_equal(waiting, '1', "Waiting wrong"); + check_equal(scl, '0', "SCL wrong"); wait until falling_edge(clk); end loop; -- i @@ -217,6 +223,28 @@ begin -- architecture tb check_errors; i2c_slave_check_stop(TIMEOUT, scl, sda); check_errors; + elsif run("waiting_stop") then + tx_write_data("00000000", tx_data, tx_valid); + request_start("1110101", '0'); + i2c_slave_check_start("1110101", '0', TIMEOUT, scl, sda); + i2c_slave_receive("00000000", 2 * TIMEOUT, scl, sda); + wait until falling_edge(clk); + wait until falling_edge(clk); + wait until falling_edge(clk); + wait until falling_edge(clk); + check_equal(waiting, '1'); + request_stop; + check_errors; + i2c_slave_check_stop(TIMEOUT, scl, sda); + check_errors; + + elsif run("waiting_right_away_stop") then + request_start("1110101", '0', stop => '1'); + i2c_slave_check_start("1110101", '0', TIMEOUT, scl, sda); + check_errors; + i2c_slave_check_stop(TIMEOUT, scl, sda); + check_errors; + elsif run("write_read") then tx_write_data("11101010", tx_data, tx_valid); tx_write_data("00001111", tx_data, tx_valid); @@ -284,6 +312,9 @@ begin -- architecture tb end if; end loop; + stable_check <= '1'; + wait for TIMEOUT * 10; + test_runner_cleanup(runner); end process main; end architecture tb; diff --git a/tb/i2c/tb_i2c_slave_pkg.vhd b/tb/i2c/tb_i2c_slave_pkg.vhd index c217889..bbdd80b 100644 --- a/tb/i2c/tb_i2c_slave_pkg.vhd +++ b/tb/i2c/tb_i2c_slave_pkg.vhd @@ -116,7 +116,7 @@ package body tb_i2c_slave_pkg is wait_for_scl_rise(scl_timeout, scl); if exp_data(i) = '1' then check(sda = '1' or sda = 'H', result("Received data (sda) not as expected.")); - else + elsif exp_data(i) = '0' then check(sda = '0' or sda = 'L', result("Received data (sda) not as expected.")); end if; wait_for_scl_fall(scl_timeout, scl); diff --git a/tb/mcu_slave/counter_tb.vhd b/tb/mcu_slave/counter_tb.vhd index 29b8da4..b51fdf1 100644 --- a/tb/mcu_slave/counter_tb.vhd +++ b/tb/mcu_slave/counter_tb.vhd @@ -38,14 +38,13 @@ begin -- architecture tb generic map ( DELAY => 1) port map ( - clk_i => clk, - rst_i => rst, - rst_on => open, - err_noack_o => err_noack, - dev_busy_o => dev_busy, - bus_busy_o => bus_busy, - sda_io => sda, - scl_io => scl + clk_i => clk, + rst_i => rst, + err_noack_data_o => err_noack, + dev_busy_o => dev_busy, + bus_busy_o => bus_busy, + sda_io => sda, + scl_io => scl ); -- pull up -- 2.49.0