From 380887154f0c912f143363d71976c471f2158757 Mon Sep 17 00:00:00 2001 From: Rutherther Date: Thu, 2 Jan 2025 15:42:59 +0100 Subject: [PATCH] fix: short last sck pulse on slower clock --- hdl_spi/src/spi_clkgen.vhd | 108 +++++++++++++++++++------- hdl_spi/src/spi_master.vhd | 27 +++---- hdl_spi/src/spi_master_ctrl.vhd | 63 ++++++++------- hdl_spi/src/spi_masterslave.vhd | 54 ++++++------- hdl_spi/tests/test_spi_masterslave.py | 18 +++-- hdl_spi/tests/test_spi_peripheral.py | 9 ++- 6 files changed, 174 insertions(+), 105 deletions(-) diff --git a/hdl_spi/src/spi_clkgen.vhd b/hdl_spi/src/spi_clkgen.vhd index 88061ba..a96edb6 100644 --- a/hdl_spi/src/spi_clkgen.vhd +++ b/hdl_spi/src/spi_clkgen.vhd @@ -11,30 +11,34 @@ entity spi_clkgen is ); port ( - clk_i : in std_logic; - rst_in : in std_logic; - start_i : in std_logic; + clk_i : in std_logic; + rst_in : in std_logic; + start_i : in std_logic; -- next_data_i : in std_logic; - div_sel_i : in std_logic_vector(DIVISORS_LOG2 - 1 downto 0); - clock_polarity_i : in std_logic; - clock_phase_i : in std_logic; - sck_o : out std_logic; - sck_mask_i : in std_logic; - clock_rising_o : out std_logic; - sample_data_o : out std_logic; - change_data_o : out std_logic); + div_sel_i : in std_logic_vector(DIVISORS_LOG2 - 1 downto 0); + clock_polarity_i : in std_logic; + clock_phase_i : in std_logic; + sck_o : out std_logic; + sck_mask_i : in std_logic; + counter_overflow_o : out std_logic; + sample_data_o : out std_logic; + change_data_o : out std_logic); end entity spi_clkgen; architecture a1 of spi_clkgen is constant MAX : natural := get_max_natural(DIVISORS); - signal curr_running : std_logic; - signal next_running : std_logic; + type state_t is (IDLE, CLK_GEN, SCK_GEN); + + signal running : std_logic; signal selected_divisor : natural range 0 to MAX := 1; signal changing : std_logic; + signal curr_state : state_t; + signal next_state : state_t; + signal curr_counter : integer range 0 to MAX - 1; signal next_counter : integer range 0 to MAX - 1; @@ -47,42 +51,90 @@ begin -- architecture a1 begin -- process set_data if rising_edge(clk_i) then -- rising clock edge if rst_in = '0' then -- synchronous reset (active low) - curr_running <= '0'; curr_sck <= '0'; curr_counter <= 0; + curr_state <= IDLE; else - curr_running <= next_running; curr_sck <= next_sck; curr_counter <= next_counter; + curr_state <= next_state; end if; end if; end process set_data; + state: process (all) is + begin -- process state + next_state <= curr_state; + running <= '0'; + + sample_data_o <= '0'; + change_data_o <= '0'; + + case curr_state is + when IDLE => + if start_i = '1' then + if sck_mask_i = '1' then + next_state <= SCK_GEN; + change_data_o <= not clock_phase_i; + else + next_state <= CLK_GEN; + end if; + end if; + when CLK_GEN => + running <= '1'; + if start_i = '0' then + next_state <= IDLE; + elsif sck_mask_i = '1' then + next_state <= SCK_GEN; + end if; + when SCK_GEN => + running <= '1'; + + if changing = '1' then + if curr_sck = clock_phase_i then + sample_data_o <= '1'; + else + change_data_o <= '1'; + end if; + end if; + + if start_i = '0' and ((changing = '1' and curr_sck /= '0') or (curr_sck = '0')) then + next_state <= IDLE; + sample_data_o <= '0'; + change_data_o <= '0'; + elsif sck_mask_i = '0' and ((changing = '1' and curr_sck /= '0') or (curr_sck = '0')) then + next_state <= CLK_GEN; + sample_data_o <= '0'; + change_data_o <= '0'; + end if; + when others => null; + end case; + end process state; + selected_divisor <= DIVISORS(to_integer(unsigned(div_sel_i))); - changing <= '1' when curr_counter = 0 and curr_running = '1' else '0'; + changing <= '1' when curr_counter = 0 and running = '1' and curr_state = SCK_GEN else '0'; next_counter <= selected_divisor - 2 when changing = '1' else 0 when curr_counter = 0 else - curr_counter - 1 when curr_running = '1' else + curr_counter - 1 when running = '1' else selected_divisor - 1; - sample_data_o <= sck_mask_i when curr_sck = clock_phase_i and changing = '1' else '0'; - change_data_o <= sck_mask_i when curr_sck /= clock_phase_i and changing = '1' else - '1' when clock_phase_i = '0' and start_i = '1' and curr_running = '0' else - -- '1' when next_data_i = '1' else - '0'; + -- sample_data_o <= '1' when curr_sck = clock_phase_i and changing = '1' else '0'; + -- change_data_o <= '1' when curr_sck /= clock_phase_i and changing = '1' else + -- '1' when clock_phase_i = '0' and start_i = '1' and running = '0' else + -- -- '1' when next_data_i = '1' else + -- '0'; - next_sck <= not curr_sck when changing = '1' - else curr_sck when curr_running = '1' else + next_sck <= '0' when curr_state /= SCK_GEN else + not curr_sck when changing = '1' + else curr_sck when running = '1' else '0'; - next_running <= start_i; - - sck_o <= clock_polarity_i when sck_mask_i = '0' else + sck_o <= clock_polarity_i when curr_state /= SCK_GEN else curr_sck when clock_polarity_i = '0' else not curr_sck; - clock_rising_o <= '1' when curr_sck = clock_phase_i and changing = '1' else '0'; + counter_overflow_o <= '1' when running = '1' and curr_counter = 0 else '0'; end architecture a1; diff --git a/hdl_spi/src/spi_master.vhd b/hdl_spi/src/spi_master.vhd index 06a1db1..af7afa3 100644 --- a/hdl_spi/src/spi_master.vhd +++ b/hdl_spi/src/spi_master.vhd @@ -56,7 +56,7 @@ architecture a1 of spi_master is signal ctrl_rst : std_logic; signal latch_sample_data : std_logic; - signal clock_rising : std_logic; + signal counter_overflow : std_logic; signal latch_change_data_out : std_logic; signal latch_new_tx_data : std_logic; @@ -93,8 +93,9 @@ begin -- architecture a1 en_i => en_i, size_sel_i => size_sel_i, div_sel_i => div_sel_i, + clock_phase_i => clock_phase_i, pulse_csn_i => pulse_csn_i, - clock_rising_i => clock_rising, + counter_overflow_i => counter_overflow, rx_block_on_full_i => rx_block_on_full_i, rx_en_i => rx_en_i, rx_ready_i => rx_ready_i, @@ -120,17 +121,17 @@ begin -- architecture a1 DIVISORS => DIVISORS, DIVISORS_LOG2 => DIVISORS_LOG2) port map ( - clk_i => clk_i, - rst_in => rst_n, - start_i => start_clock, - div_sel_i => div_sel_i, - clock_polarity_i => clock_polarity_i, - clock_phase_i => clock_phase_i, - sck_o => sck, - sck_mask_i => sck_mask, - clock_rising_o => clock_rising, - sample_data_o => latch_sample_data, - change_data_o => latch_change_data_out); + clk_i => clk_i, + rst_in => rst_n, + start_i => start_clock, + div_sel_i => div_sel_i, + clock_polarity_i => clock_polarity_i, + clock_phase_i => clock_phase_i, + sck_o => sck, + sck_mask_i => sck_mask, + counter_overflow_o => counter_overflow, + sample_data_o => latch_sample_data, + change_data_o => latch_change_data_out); shift_register: entity work.shift_register generic map ( diff --git a/hdl_spi/src/spi_master_ctrl.vhd b/hdl_spi/src/spi_master_ctrl.vhd index cdc9337..4801904 100644 --- a/hdl_spi/src/spi_master_ctrl.vhd +++ b/hdl_spi/src/spi_master_ctrl.vhd @@ -20,7 +20,8 @@ entity spi_master_ctrl is size_sel_i : in std_logic_vector(SIZES_2LOG - 1 downto 0); div_sel_i : in std_logic_vector(DIVISORS_LOG2 - 1 downto 0); pulse_csn_i : in std_logic; - clock_rising_i : in std_logic; + clock_phase_i : in std_logic; + counter_overflow_i : in std_logic; rx_block_on_full_i : in std_logic; rx_en_i : in std_logic; rx_valid_o : out std_logic; @@ -48,7 +49,7 @@ architecture a1 of spi_master_ctrl is constant MAX_SIZE : natural := get_max_natural(SIZES); constant MAX_DIVISOR : natural := get_max_natural(DIVISORS); - type states_t is (RESET, IDLE, SHIFTING, NEXT_DATA, CSN_PULSE, CSN_RISING); + type states_t is (RESET, IDLE, SHIFTING, NEXT_DATA, CSN_RISING); type tx_states_t is (IDLE, TX_LATCHING_DATA, TX_LATCHED, TX_WAITING); type rx_states_t is (IDLE, RX_GOT_DATA, RX_INVALID_DATA); @@ -74,7 +75,7 @@ architecture a1 of spi_master_ctrl is signal transmission_done : std_logic; - signal shifting_length : integer range 0 to MAX_SIZE; + signal shifting_length : integer range 0 to MAX_SIZE * 2; signal selected_divisor : integer range 0 to MAX_DIVISOR; signal clear_lost_rx_data : std_logic; begin -- architecture a1 @@ -104,10 +105,29 @@ begin -- architecture a1 next_counter <= counter; end procedure switch_to; + procedure switch_to_shifting(constant is_next_data: boolean) is + variable count : natural; + begin -- procedure switch_to_shifting + if is_next_data then + if selected_divisor = 2 then + count := shifting_length * 2 - 2; + else + count := shifting_length * 2; + end if; + else + count := shifting_length * 2 - 1; + if clock_phase_i = '1' then + count := count + 1; + end if; + end if; + + switch_to(SHIFTING, count); + end procedure switch_to_shifting; + variable zero : std_logic; begin -- process state_sel next_counter <= curr_counter; - if curr_counter /= 0 and clock_rising_i = '1' then + if curr_counter /= 0 and counter_overflow_i = '1' then next_counter <= curr_counter - 1; end if; @@ -125,7 +145,7 @@ begin -- architecture a1 rst_on <= '1'; - sck_mask_o <= '0'; + sck_mask_o <= '1'; busy_o <= '1'; csn_o <= '1'; @@ -140,8 +160,9 @@ begin -- architecture a1 busy_o <= '0'; gen_clk_en_o <= '0'; - if tx_got_data = '1' then - switch_to(SHIFTING, shifting_length); + if zero = '1' and tx_got_data = '1' then + switch_to_shifting(false); + gen_clk_en_o <= '1'; ack_tx_got_data <= '1'; end if; when SHIFTING => @@ -150,37 +171,21 @@ begin -- architecture a1 if zero = '1' then transmission_done <= '1'; - if pulse_csn_i = '1' then - switch_to(CSN_PULSE, CSN_PULSE_CYCLES); - else - switch_to(NEXT_DATA, 0); - end if; + switch_to(NEXT_DATA, 0); end if; when NEXT_DATA => csn_o <= '0'; - if selected_divisor = 2 then - sck_mask_o <= '0'; - else - sck_mask_o <= '1'; - end if; + sck_mask_o <= '0'; - if tx_got_data = '1' then + if pulse_csn_i = '1' then + switch_to(IDLE, CSN_PULSE_CYCLES - 1); + elsif tx_got_data = '1' then sck_mask_o <= '1'; - if selected_divisor = 2 then - switch_to(SHIFTING, shifting_length - 1); - else - switch_to(SHIFTING, shifting_length); - end if; + switch_to_shifting(true); ack_tx_got_data <= '1'; else switch_to(IDLE, 0); end if; - when CSN_PULSE => - csn_o <= '1'; - - if zero = '1' then - switch_to(NEXT_DATA, 0); - end if; when others => next_state <= RESET; end case; diff --git a/hdl_spi/src/spi_masterslave.vhd b/hdl_spi/src/spi_masterslave.vhd index b79a138..7a51012 100644 --- a/hdl_spi/src/spi_masterslave.vhd +++ b/hdl_spi/src/spi_masterslave.vhd @@ -98,7 +98,7 @@ architecture a1 of spi_masterslave is signal slave_latch_change_data : std_logic; signal master_sck : std_logic; - signal master_clock_rising : std_logic; + signal master_counter_overflow : std_logic; signal master_rx_valid : std_logic; signal master_tx_ready : std_logic; signal master_busy : std_logic; @@ -138,27 +138,27 @@ begin -- architecture a1 size_sel_i => size_sel_i, div_sel_i => div_sel_i, pulse_csn_i => pulse_csn_i, + clock_phase_i => clock_phase_i, rx_block_on_full_i => rx_block_on_full_i, rx_en_i => rx_en_i, rx_ready_i => rx_ready_i, tx_en_i => tx_en_i, tx_valid_i => tx_valid_i, clear_lost_rx_data_i => clear_lost_rx_data_i, - - clock_rising_i => master_clock_rising, - rx_valid_o => master_rx_valid, - tx_ready_o => master_tx_ready, - busy_o => master_busy, - err_lost_rx_data_o => master_err_lost_rx_data, - rst_on => master_ctrl_rst_n, - csn_o => master_csn, - csn_en_o => master_csn_en, - mosi_en_o => master_mosi_en, - miso_en_o => master_miso_en, - sck_mask_o => master_sck_mask, - sck_en_o => master_sck_en, - gen_clk_en_o => master_start_clock, - latch_tx_data_o => master_latch_new_tx_data); + counter_overflow_i => master_counter_overflow, + rx_valid_o => master_rx_valid, + tx_ready_o => master_tx_ready, + busy_o => master_busy, + err_lost_rx_data_o => master_err_lost_rx_data, + rst_on => master_ctrl_rst_n, + csn_o => master_csn, + csn_en_o => master_csn_en, + mosi_en_o => master_mosi_en, + miso_en_o => master_miso_en, + sck_mask_o => master_sck_mask, + sck_en_o => master_sck_en, + gen_clk_en_o => master_start_clock, + latch_tx_data_o => master_latch_new_tx_data); slave_ctrl : entity work.spi_slave_ctrl generic map ( @@ -197,17 +197,17 @@ begin -- architecture a1 DIVISORS => DIVISORS, DIVISORS_LOG2 => DIVISORS_LOG2) port map ( - clk_i => clk_i, - rst_in => master_ctrl_rst_n, - start_i => master_start_clock, - div_sel_i => div_sel_i, - clock_polarity_i => clock_polarity_i, - clock_phase_i => clock_phase_i, - sck_o => master_sck, - sck_mask_i => master_sck_mask, - clock_rising_o => master_clock_rising, - sample_data_o => master_latch_sample_data, - change_data_o => master_latch_change_data); + clk_i => clk_i, + rst_in => master_ctrl_rst_n, + start_i => master_start_clock, + div_sel_i => div_sel_i, + clock_polarity_i => clock_polarity_i, + clock_phase_i => clock_phase_i, + sck_o => master_sck, + sck_mask_i => master_sck_mask, + counter_overflow_o => master_counter_overflow, + sample_data_o => master_latch_sample_data, + change_data_o => master_latch_change_data); clkmon : entity work.spi_clkmon port map ( diff --git a/hdl_spi/tests/test_spi_masterslave.py b/hdl_spi/tests/test_spi_masterslave.py index 3b3af39..a29b77c 100644 --- a/hdl_spi/tests/test_spi_masterslave.py +++ b/hdl_spi/tests/test_spi_masterslave.py @@ -138,6 +138,8 @@ class DutDriver: self.dut.rx_ready_i.value = 1 await FallingEdge(self.dut.clk_i) self.dut.rx_ready_i.value = 0 + else: + await FallingEdge(self.dut.clk_i) should_lose_data = not self._set_ready and self.dut.tx_valid_i.value == 1 await RisingEdge(self.dut.clk_i) @@ -587,10 +589,15 @@ def spi_tests_runner(): hdl_toplevel_lang = "vhdl" sim = os.getenv("SIM", "questa") + test_filter = os.getenv("TESTCASE", None) + gui = True if os.getenv("GUI", "0") == "1" else False + proj_path = Path(__file__).resolve().parent.parent # equivalent to setting the PYTHONPATH environment variable sys.path.append(str(proj_path / "models")) + run = True if os.getenv("RUN", "1") == "1" else False + sources = [ proj_path / "src" / "spi_pkg.vhd", proj_path / "src" / "rs_latch.vhd", @@ -622,11 +629,12 @@ def spi_tests_runner(): always=True, build_args=build_args + extra_args, ) - runner.test( - hdl_toplevel="spi_masterslave", hdl_toplevel_lang=hdl_toplevel_lang, - test_module="test_spi_masterslave", test_args = extra_args - ) - + if run: + runner.test( + hdl_toplevel="spi_masterslave", hdl_toplevel_lang=hdl_toplevel_lang, + test_module="test_spi_masterslave", test_args = extra_args, + gui = gui, test_filter = test_filter + ) if __name__ == "__main__": spi_tests_runner() diff --git a/hdl_spi/tests/test_spi_peripheral.py b/hdl_spi/tests/test_spi_peripheral.py index 8b51087..c1d4820 100644 --- a/hdl_spi/tests/test_spi_peripheral.py +++ b/hdl_spi/tests/test_spi_peripheral.py @@ -126,7 +126,7 @@ class DutDriver: await irq_handler(self) @cocotb.test() -async def single_transission(dut): +async def single_transmission(dut): clk = Clock(dut.clk_i, 5, "ns") interface = SpiInterface(dut.csn_o, dut.sck_o, dut.miso_i, dut.mosi_o) config = SpiConfig(16, RisingEdge, FallingEdge, 40, "ns") @@ -363,6 +363,9 @@ def spi_peripheral_tests_runner(): hdl_toplevel_lang = "vhdl" sim = os.getenv("SIM", "questa") + test_filter = os.getenv("TESTCASE", None) + gui = True if os.getenv("GUI", "0") == "1" else False + proj_path = Path(__file__).resolve().parent.parent # equivalent to setting the PYTHONPATH environment variable sys.path.append(str(proj_path / "models")) @@ -399,9 +402,9 @@ def spi_peripheral_tests_runner(): build_args=build_args + extra_args, ) runner.test( - testcase = "interrupt", + test_filter = test_filter, hdl_toplevel="spi_peripheral", hdl_toplevel_lang=hdl_toplevel_lang, - test_module="test_spi_peripheral", test_args = extra_args, gui = True + test_module="test_spi_peripheral", test_args = extra_args, gui = gui ) -- 2.48.1