From 85f97d0167f3a30cad8eddd2d89dd634b5343169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franti=C5=A1ek=20Boh=C3=A1=C4=8Dek?= Date: Mon, 28 Aug 2023 20:14:45 +0200 Subject: [PATCH] feat: simplify spi_transmit, send data without delay The transmit entity did contain a lot of states that were dependent on each other, it was simply too chaotic. The entity is rewritten to have only one important state variable with few different states. There were also changes to change the bit output on falling edge rather than rising edge to comply with t_su and t_hold of the device on the other end. The entity now also sends data right away on the first clock cycle, that did not work before. It may also align to send only every WIDTH bits, to be in sync with the spi_recv if used together. --- src/spi_transmit.vhd | 124 +++++++++++++++++++--------------- testbench/tb_spi_transmit.vhd | 32 +++------ 2 files changed, 82 insertions(+), 74 deletions(-) diff --git a/src/spi_transmit.vhd b/src/spi_transmit.vhd index f39940d..ea977cf 100644 --- a/src/spi_transmit.vhd +++ b/src/spi_transmit.vhd @@ -4,7 +4,8 @@ use ieee.std_logic_1164.all; entity spi_transmit is generic ( - WIDTH : integer := 8); + WIDTH : integer := 8; + ALIGN_START : std_logic := '1'); port ( clk_i : in std_logic; -- Clock @@ -14,7 +15,7 @@ entity spi_transmit is -- Change only -- if ready_o -- is '1'. - transmit_i : in std_logic; -- Pulse to signal new data are present + valid_i : in std_logic; -- Pulse to signal new data are present -- in transmit_data. Data has to change -- only if ready_o is '1' ready_o : out std_logic; -- Signals that the transmitter is @@ -26,80 +27,97 @@ entity spi_transmit is end entity spi_transmit; architecture a1 of spi_transmit is - signal ready_reg : std_logic; -- is the transmitter ready for new data? - signal ready_next : std_logic; - - signal data_prepared_reg : std_logic; -- is there data prepared for next transmission? - signal data_prepared_next : std_logic; - signal store_data_in_sr_reg : std_logic; -- should data be stored in the + signal store_data_in_sr : std_logic; -- should data be stored in the -- shift register next clock cycle? - signal store_data_in_sr_next : std_logic; + + signal transmit_bit_falling : std_logic; + signal sr_q_o : std_logic; signal data_bit_index_reg : integer range 0 to WIDTH; signal data_bit_index_next : integer range 0 to WIDTH; - signal transmitting_reg : std_logic; -- is there an ongoing transmission? - signal transmitting_next : std_logic; - - signal continue_in_ongoing_transmission : std_logic; -- at the end of - -- transmission, are - -- next data ready and - -- should the transmission continue? -begin -- architecture a1 - ready_o <= ready_reg; - transmitting_o <= transmitting_reg; - - ready_next <= '0' when ready_reg = '1' and transmit_i = '1' and transmitting_reg = '1' else - '0' when ready_reg = '0' and data_prepared_next = '1' else - '1'; - - data_bit_index_next <= (data_bit_index_reg + 1) mod WIDTH when transmitting_reg = '1' else - 0; + type state is (IDLE, WAITING, SINGLE_TRANSMISSION, CONTINUOUS_TRANSMISSION); + -- IDLE - not sending anything + -- WAITING - waiting for the right moment to send - data_bit_index = 0 + -- SINGLE_TRANSMISSION - just one data currently loaded into the shift register + -- CONTINUOUS_TRANSMISSION - one data currently being sent, and another data + -- loaded into the shift register, and another ready to be sent - continue_in_ongoing_transmission <= '1' when transmitting_reg = '1' and data_bit_index_next = 0 and store_data_in_sr_next = '1' else - '0'; + signal state_reg : state; + signal state_next : state; - transmitting_next <= '1' when continue_in_ongoing_transmission = '1' else - '0' when transmitting_reg = '1' and data_bit_index_next = 0 else - '1' when transmitting_reg = '1' else - '1' when store_data_in_sr_next = '1' else - '0'; - - store_data_in_sr_next <= '0' when store_data_in_sr_reg = '1' else - '1' when transmitting_reg = '0' and transmit_i = '1' else - '1' when transmitting_reg = '0' and data_prepared_reg = '1' else - '1' when transmitting_reg = '1' and data_bit_index_next = 0 and data_prepared_reg = '1' else - '0'; - - data_prepared_next <= '1' when transmit_i = '1' and ready_reg = '1' and transmitting_reg = '1' else - '1' when data_prepared_reg = '1' and store_data_in_sr_next = '0' else - '0'; + signal can_begin_transmission_next : std_logic; +begin -- architecture a1 + transmit_bit_o <= sr_q_o when state_reg = SINGLE_TRANSMISSION and data_bit_index_reg = 1 else + transmit_bit_falling when state_reg /= IDLE else + sr_q_o; + + can_begin_transmission_next <= '1' when data_bit_index_reg = 0 else '0'; + + -- in IDLE, go to + -- -- SINGLE_TRANSMISSION if data valid, and can start transmission + -- -- WAITING if data valid, and cannot start transmission + -- -- IDLE + -- in WAITING, go to + -- -- SINGLE_TRANSMISSION if can start transmission + -- -- WAITING + -- in SINGLE_TRANSMISSION, go to + -- -- CONTINUOUS_TRANSMISSION if data valid and cannot begin new transmission + -- -- SINGLE_TRANSMISSION if data valid and can begin new transmission + -- -- IDLE if transmission done + -- -- SINGLE_TRANSMISSION if transmission ongoing + -- in CONTINUOUS_TRANSMISSION, go to + -- -- CONTINUOUS_TRANSMISSION if cannot begin new transmission + -- -- SINGLE_TRANSMISSION if can begin new transmission + + state_next <= SINGLE_TRANSMISSION when state_reg = IDLE and valid_i = '1' and can_begin_transmission_next = '1' else + WAITING when state_reg = IDLE and valid_i = '1' else + SINGLE_TRANSMISSION when state_reg = WAITING and can_begin_transmission_next = '1' else + WAITING when state_reg = WAITING else + SINGLE_TRANSMISSION when state_reg = SINGLE_TRANSMISSION and valid_i = '1' and can_begin_transmission_next = '1' else + CONTINUOUS_TRANSMISSION when state_reg = SINGLE_TRANSMISSION and valid_i = '1' else + SINGLE_TRANSMISSION when state_reg = SINGLE_TRANSMISSION and data_bit_index_reg /= 0 else + CONTINUOUS_TRANSMISSION when state_reg = CONTINUOUS_TRANSMISSION and can_begin_transmission_next = '0' else + SINGLE_TRANSMISSION when state_reg = CONTINUOUS_TRANSMISSION and can_begin_transmission_next = '1' else + IDLE; + + store_data_in_sr <= '1' when data_bit_index_reg = 0 and state_next /= IDLE else '0'; + ready_o <= '1' when state_reg /= CONTINUOUS_TRANSMISSION else '0'; + transmitting_o <= '1' when state_next = SINGLE_TRANSMISSION or state_next = CONTINUOUS_TRANSMISSION else '0'; + + data_bit: if ALIGN_START = '1' generate + data_bit_index_next <= (data_bit_index_reg + 1) mod WIDTH; + else generate + data_bit_index_next <= (data_bit_index_reg + 1) mod WIDTH when state_reg /= IDLE else + 0; + end generate data_bit; store_next: process (clk_i) is begin -- process store_next if rst_in = '0' then -- synchronous reset (active low) - ready_reg <= '0'; - data_prepared_reg <= '0'; - store_data_in_sr_reg <= '0'; data_bit_index_reg <= 0; - transmitting_reg <= '0'; + state_reg <= IDLE; elsif rising_edge(clk_i) then -- rising clock edge - ready_reg <= ready_next; - data_prepared_reg <= data_prepared_next; - store_data_in_sr_reg <= store_data_in_sr_next; data_bit_index_reg <= data_bit_index_next; - transmitting_reg <= transmitting_next; + state_reg <= state_next; end if; end process store_next; + store_falling: process (clk_i) is + begin -- process store_falling + if falling_edge(clk_i) then + transmit_bit_falling <= sr_q_o; + end if; + end process store_falling; + sr: entity work.piso_shift_register generic map ( WIDTH => WIDTH) port map ( clk_i => clk_i, data_i => transmit_data_i, - store_i => store_data_in_sr_next, - q_o => transmit_bit_o + store_i => store_data_in_sr, + q_o => sr_q_o ); end architecture a1; diff --git a/testbench/tb_spi_transmit.vhd b/testbench/tb_spi_transmit.vhd index c0efc69..168e6f5 100644 --- a/testbench/tb_spi_transmit.vhd +++ b/testbench/tb_spi_transmit.vhd @@ -18,7 +18,7 @@ architecture a1 of tb_spi_transmit is signal rst : std_logic := '0'; signal transmit_data : std_logic_vector(7 downto 0); - signal transmit_pulse : std_logic; + signal valid_pulse : std_logic; signal ready : std_logic; signal transmitting : std_logic; @@ -31,7 +31,7 @@ begin -- architecture a1 clk_i => clk, rst_in => rst, transmit_data_i => transmit_data, - transmit_i => transmit_pulse, + valid_i => valid_pulse, ready_o => ready, transmitting_o => transmitting, transmit_bit_o => mosi); @@ -45,13 +45,12 @@ begin -- architecture a1 test_runner_setup(runner, runner_cfg); while test_suite loop - if run("transmit_one_byte") then - wait until falling_edge(clk); + if run("one_byte") then check_equal(ready, '1'); - transmit_pulse <= '1'; + valid_pulse <= '1'; transmit_data <= "11100010"; wait until falling_edge(clk); - transmit_pulse <= '0'; + valid_pulse <= '0'; for i in 0 to 2 loop check_equal(ready, '1'); @@ -72,27 +71,21 @@ begin -- architecture a1 check_equal(mosi, '1'); wait until falling_edge(clk); check_equal(ready, '1'); - check_equal(transmitting, '1'); + check_equal(transmitting, '0'); check_equal(mosi, '0'); wait until falling_edge(clk); check_equal(ready, '1'); check_equal(transmitting, '0'); - elsif run("transmit_more_bytes") then - wait until falling_edge(clk); - check_equal(ready, '1'); - transmit_pulse <= '1'; + elsif run("more_bytes") then + valid_pulse <= '1'; transmit_data <= "11100010"; wait until falling_edge(clk); check_equal(ready, '1'); - transmit_pulse <= '1'; + valid_pulse <= '1'; transmit_data <= "00011101"; - - check_equal(ready, '1'); - check_equal(transmitting, '1'); - check_equal(mosi, '1'); wait until falling_edge(clk); check_equal(ready, '0'); - transmit_pulse <= '0'; + valid_pulse <= '0'; for i in 0 to 1 loop check_equal(ready, '0'); @@ -139,15 +132,12 @@ begin -- architecture a1 wait until falling_edge(clk); check_equal(ready, '1'); - check_equal(transmitting, '1'); + check_equal(transmitting, '0'); check_equal(mosi, '1'); wait until falling_edge(clk); check_equal(ready, '1'); check_equal(transmitting, '0'); - wait until falling_edge(clk); - check_equal(ready, '1'); - check_equal(transmitting, '0'); end if; end loop; -- 2.48.1