From 63995472b1cb851e3daa3a182529a7beb9db0d0b Mon Sep 17 00:00:00 2001 From: Laez Barbosa Date: Thu, 28 Nov 2024 15:43:30 -0300 Subject: [PATCH 1/3] SPI Engine: fix segmented transfers with echo_sclk Signed-off-by: Laez Barbosa --- .../spi_engine_execution.v | 22 +++++++-- .../spi_engine_execution_shiftreg.v | 47 +++++-------------- 2 files changed, 28 insertions(+), 41 deletions(-) diff --git a/library/spi_engine/spi_engine_execution/spi_engine_execution.v b/library/spi_engine/spi_engine_execution/spi_engine_execution.v index 4dd61f6732..d2201200c2 100644 --- a/library/spi_engine/spi_engine_execution/spi_engine_execution.v +++ b/library/spi_engine/spi_engine_execution/spi_engine_execution.v @@ -114,6 +114,7 @@ module spi_engine_execution #( reg trigger_next = 1'b0; reg wait_for_io = 1'b0; reg transfer_active = 1'b0; + reg transfer_done = 1'b0; reg last_transfer; reg [7:0] word_length = DATA_WIDTH; @@ -136,6 +137,7 @@ module spi_engine_execution #( wire sdo_int_s; wire last_bit; + wire echo_last_bit; wire first_bit; wire end_of_word; @@ -163,8 +165,6 @@ module spi_engine_execution #( wire io_ready1; wire io_ready2; - wire end_of_sdi_latch; - wire sample_sdo; (* direct_enable = "yes" *) wire cs_gen; @@ -196,12 +196,12 @@ module spi_engine_execution #( .word_length(word_length), .sample_sdo(sample_sdo), .sdo_io_ready(sdo_io_ready), + .echo_last_bit(echo_last_bit), .transfer_active(transfer_active), .trigger_tx(trigger_tx), .trigger_rx(trigger_rx), .first_bit(first_bit), - .cs_activate(cs_activate), - .end_of_sdi_latch(end_of_sdi_latch)); + .cs_activate(cs_activate)); assign sample_sdo = sdo_data_valid && ((trigger_tx && last_bit) || (wait_for_io || exec_transfer_cmd)); @@ -331,7 +331,7 @@ module spi_engine_execution #( end else begin case (inst_d1) CMD_TRANSFER: begin - if (transfer_active == 1'b0 && wait_for_io == 1'b0 && end_of_sdi_latch == 1'b1) + if (transfer_done) idle <= 1'b1; end CMD_CHIPSELECT: begin @@ -426,6 +426,18 @@ module spi_engine_execution #( end end + always @(posedge clk ) begin + if (resetn == 1'b0) begin + transfer_done <= 1'b0; + end else begin + if (ECHO_SCLK) begin + transfer_done <= echo_last_bit && last_transfer; + end else begin + transfer_done <= (wait_for_io && io_ready1 && last_transfer) || (!wait_for_io && transfer_active && end_of_word && (last_transfer || !io_ready2)); // same conditions that make (!transfer_active && !wait_for_io) + end + end + end + always @(posedge clk) begin if (transfer_active == 1'b1 || wait_for_io == 1'b1) begin diff --git a/library/spi_engine/spi_engine_execution/spi_engine_execution_shiftreg.v b/library/spi_engine/spi_engine_execution/spi_engine_execution_shiftreg.v index 2298a20861..eec0845938 100644 --- a/library/spi_engine/spi_engine_execution/spi_engine_execution_shiftreg.v +++ b/library/spi_engine/spi_engine_execution/spi_engine_execution_shiftreg.v @@ -72,12 +72,12 @@ module spi_engine_execution_shiftreg #( // timing from main fsm input sample_sdo, output reg sdo_io_ready, + output echo_last_bit, input transfer_active, input trigger_tx, input trigger_rx, input first_bit, - input cs_activate, - output end_of_sdi_latch + input cs_activate ); reg [ 7:0] sdi_counter = 8'b0; @@ -156,9 +156,7 @@ module spi_engine_execution_shiftreg #( generate if (ECHO_SCLK == 1) begin : g_echo_sclk_miso_latch - reg [7:0] sdi_counter_d = 8'b0; - reg [7:0] sdi_transfer_counter = 8'b0; - reg [7:0] num_of_transfers = 8'b0; + reg last_sdi_bit_r; reg [(NUM_OF_SDI * DATA_WIDTH)-1:0] sdi_data_latch = {(NUM_OF_SDI * DATA_WIDTH){1'b0}}; if ((DEFAULT_SPI_CFG[1:0] == 2'b01) || (DEFAULT_SPI_CFG[1:0] == 2'b10)) begin : g_echo_miso_nshift_reg @@ -185,10 +183,10 @@ module spi_engine_execution_shiftreg #( always @(posedge echo_sclk or posedge cs_activate) begin if (cs_activate) begin sdi_counter <= 8'b0; - sdi_counter_d <= 8'b0; + last_sdi_bit_r <= 1'b0; end else begin - sdi_counter <= (sdi_counter == word_length-1) ? 8'b0 : sdi_counter + 1'b1; - sdi_counter_d <= sdi_counter; + last_sdi_bit_r <= (sdi_counter == word_length - 1); // FIXME: potentially unsafe path: what are the guarantees of settling time between changing word_length and first echo_sclk edge? + sdi_counter <= (sdi_counter == word_length - 1) ? 8'b0 : sdi_counter + 1'b1; end end @@ -214,17 +212,18 @@ module spi_engine_execution_shiftreg #( always @(posedge echo_sclk or posedge cs_activate) begin if (cs_activate) begin sdi_counter <= 8'b0; - sdi_counter_d <= 8'b0; + last_sdi_bit_r <= 1'b0; end else begin - sdi_counter <= (sdi_counter == word_length-1) ? 8'b0 : sdi_counter + 1'b1; - sdi_counter_d <= sdi_counter; + last_sdi_bit_r <= (sdi_counter == word_length - 1); // FIXME: potentially unsafe path: what are the guarantees of settling time between changing word_length and first echo_sclk edge? + sdi_counter <= (sdi_counter == word_length - 1) ? 8'b0 : sdi_counter + 1'b1; end end end assign sdi_data = sdi_data_latch; - assign last_sdi_bit = (sdi_counter == 0) && (sdi_counter_d == word_length-1); + assign last_sdi_bit = last_sdi_bit_r; + assign echo_last_bit = !last_sdi_bit_m[3] && last_sdi_bit_m[2]; // sdi_data_valid is synchronous to SPI clock, so synchronize the // last_sdi_bit to SPI clock @@ -250,34 +249,10 @@ module spi_engine_execution_shiftreg #( end end - always @(posedge clk) begin - if (cs_activate) begin - num_of_transfers <= 8'b0; - end else begin - if (current_instr == CMD_TRANSFER) begin - // current_cmd contains the NUM_OF_TRANSFERS - 1 - num_of_transfers <= current_cmd[7:0] + 1'b1; - end - end - end - - always @(posedge clk) begin - if (cs_activate) begin - sdi_transfer_counter <= 0; - end else if (last_sdi_bit_m[2] == 1'b0 && - last_sdi_bit_m[1] == 1'b1) begin - sdi_transfer_counter <= sdi_transfer_counter + 1'b1; - end - end - - assign end_of_sdi_latch = last_sdi_bit_m[2] & (sdi_transfer_counter == num_of_transfers); - end /* g_echo_sclk_miso_latch */ else begin : g_sclk_miso_latch - assign end_of_sdi_latch = 1'b1; - for (i=0; i Date: Tue, 3 Dec 2024 10:24:10 -0300 Subject: [PATCH 2/3] SPI Engine: improve timing pre-compute the last bit counter value Signed-off-by: Laez Barbosa --- .../spi_engine/spi_engine_execution/spi_engine_execution.v | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/spi_engine/spi_engine_execution/spi_engine_execution.v b/library/spi_engine/spi_engine_execution/spi_engine_execution.v index d2201200c2..0533de8021 100644 --- a/library/spi_engine/spi_engine_execution/spi_engine_execution.v +++ b/library/spi_engine/spi_engine_execution/spi_engine_execution.v @@ -118,6 +118,7 @@ module spi_engine_execution #( reg last_transfer; reg [7:0] word_length = DATA_WIDTH; + reg [7:0] last_bit_count = DATA_WIDTH-1; reg [7:0] left_aligned = 8'b0; assign first_bit = ((bit_counter == 'h0) || (bit_counter == word_length)); @@ -244,6 +245,7 @@ module spi_engine_execution #( sdo_idle_state <= SDO_DEFAULT; clk_div <= DEFAULT_CLK_DIV; word_length <= DATA_WIDTH; + last_bit_count <= DATA_WIDTH-1; left_aligned <= 8'b0; end else if (exec_write_cmd == 1'b1) begin if (cmd[9:8] == REG_CONFIG) begin @@ -256,6 +258,7 @@ module spi_engine_execution #( end else if (cmd[9:8] == REG_WORD_LENGTH) begin // the max value of this reg must be DATA_WIDTH word_length <= cmd[7:0]; + last_bit_count <= cmd[7:0] - 1; left_aligned <= DATA_WIDTH - cmd[7:0]; end end @@ -458,7 +461,7 @@ module spi_engine_execution #( // end_of_word will signal the end of a transaction, pushing the command // stream execution to the next command. end_of_word in normal mode can be // generated using the global bit_counter - assign last_bit = bit_counter == word_length - 1; + assign last_bit = (bit_counter == last_bit_count); assign end_of_word = last_bit == 1'b1 && ntx_rx == 1'b1 && clk_div_last == 1'b1; always @(posedge clk) begin From d533c4ba01df1b44a90f5874eab0ee29a81f239d Mon Sep 17 00:00:00 2001 From: Laez Barbosa Date: Thu, 12 Dec 2024 17:29:38 -0300 Subject: [PATCH 3/3] fix transfer_done condition for non-echo_sclk cases Signed-off-by: Laez Barbosa --- library/spi_engine/spi_engine_execution/spi_engine_execution.v | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/spi_engine/spi_engine_execution/spi_engine_execution.v b/library/spi_engine/spi_engine_execution/spi_engine_execution.v index 0533de8021..a741ec209f 100644 --- a/library/spi_engine/spi_engine_execution/spi_engine_execution.v +++ b/library/spi_engine/spi_engine_execution/spi_engine_execution.v @@ -436,7 +436,7 @@ module spi_engine_execution #( if (ECHO_SCLK) begin transfer_done <= echo_last_bit && last_transfer; end else begin - transfer_done <= (wait_for_io && io_ready1 && last_transfer) || (!wait_for_io && transfer_active && end_of_word && (last_transfer || !io_ready2)); // same conditions that make (!transfer_active && !wait_for_io) + transfer_done <= (wait_for_io && io_ready1 && last_transfer) || (!wait_for_io && transfer_active && end_of_word && last_transfer ); // same conditions that make (!transfer_active && !wait_for_io) end end end