Merge branch 'bugfix/spi_slave_wrong_miso_mosi' into 'master'

spi_slave: Fix MOSI/MISO inconsistent references on the SPI Slave drivers.

See merge request espressif/esp-idf!13645

components/driver/test/test_spi_slave.c

@@ -23,9 +23,14 @@
 #ifndef CONFIG_SPIRAM
 //This test should be removed once the timing test is merged.
 
-#define MASTER_SEND {0x93, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf0, 0xaa, 0xcc, 0xff, 0xee, 0x55, 0x77, 0x88, 0x43}
-#define SLAVE_SEND { 0xaa, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0x13, 0x57, 0x9b, 0xdf, 0x24, 0x68, 0xac, 0xe0 }
+static spi_device_handle_t spi;
+static WORD_ALIGNED_ATTR uint8_t master_txbuf[320];
+static WORD_ALIGNED_ATTR uint8_t master_rxbuf[320];
+static WORD_ALIGNED_ATTR uint8_t slave_txbuf[320];
+static WORD_ALIGNED_ATTR uint8_t slave_rxbuf[320];
 
+static const uint8_t master_send[] = { 0x93, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf0, 0xaa, 0xcc, 0xff, 0xee, 0x55, 0x77, 0x88, 0x43 };
+static const uint8_t slave_send[] = { 0xaa, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0x13, 0x57, 0x9b, 0xdf, 0x24, 0x68, 0xac, 0xe0 };
 
 static inline void int_connect( uint32_t gpio, uint32_t sigo, uint32_t sigi )
 {
@@ -33,7 +38,7 @@ static inline void int_connect( uint32_t gpio, uint32_t sigo, uint32_t sigi )
     esp_rom_gpio_connect_in_signal( gpio, sigi, false );
 }
 
-static void master_init_nodma( spi_device_handle_t* spi)
+static void master_init( spi_device_handle_t* spi)
 {
     esp_err_t ret;
     spi_bus_config_t buscfg={
@@ -83,24 +88,129 @@ static void slave_init(void)
     TEST_ESP_OK(spi_slave_initialize(TEST_SLAVE_HOST, &buscfg, &slvcfg, SPI_DMA_CH_AUTO));
 }
 
-TEST_CASE("test slave send unaligned","[spi]")
-{
-    WORD_ALIGNED_ATTR uint8_t master_txbuf[320]=MASTER_SEND;
-    WORD_ALIGNED_ATTR uint8_t master_rxbuf[320];
-    WORD_ALIGNED_ATTR uint8_t slave_txbuf[320]=SLAVE_SEND;
-    WORD_ALIGNED_ATTR uint8_t slave_rxbuf[320];
+static void custom_setup(void) {
+    //Initialize buffers
+    memset(master_txbuf, 0, sizeof(master_txbuf));
+    memset(master_rxbuf, 0, sizeof(master_rxbuf));
+    memset(slave_txbuf, 0, sizeof(slave_txbuf));
+    memset(slave_rxbuf, 0, sizeof(slave_rxbuf));
 
-    spi_device_handle_t spi;
-    //initial master
-    master_init_nodma( &spi );
-    //initial slave
+    //Initialize SPI Master
+    master_init( &spi );
+    //Initialize SPI Slave
     slave_init();
 
-    //do internal connection
+    //Do internal connections
     int_connect( PIN_NUM_MOSI,  spi_periph_signal[TEST_SPI_HOST].spid_out,      spi_periph_signal[TEST_SLAVE_HOST].spiq_in );
     int_connect( PIN_NUM_MISO,  spi_periph_signal[TEST_SLAVE_HOST].spiq_out,    spi_periph_signal[TEST_SPI_HOST].spid_in );
     int_connect( PIN_NUM_CS,    spi_periph_signal[TEST_SPI_HOST].spics_out[0],  spi_periph_signal[TEST_SLAVE_HOST].spics_in );
     int_connect( PIN_NUM_CLK,   spi_periph_signal[TEST_SPI_HOST].spiclk_out,    spi_periph_signal[TEST_SLAVE_HOST].spiclk_in );
+}
+
+static void custom_teardown(void) {
+    TEST_ASSERT(spi_slave_free(TEST_SLAVE_HOST) == ESP_OK);
+    TEST_ASSERT(spi_bus_remove_device(spi) == ESP_OK);
+    TEST_ASSERT(spi_bus_free(TEST_SPI_HOST) == ESP_OK);
+}
+
+TEST_CASE("test fullduplex slave with only RX direction","[spi]")
+{
+    custom_setup();
+
+    memcpy(master_txbuf, master_send, sizeof(master_send));
+
+    for ( int i = 0; i < 4; i ++ ) {
+        //slave send
+        spi_slave_transaction_t slave_t;
+        spi_slave_transaction_t* out;
+        memset(&slave_t, 0, sizeof(spi_slave_transaction_t));
+        slave_t.length=8*32;
+        slave_t.tx_buffer=NULL;
+        slave_t.rx_buffer=slave_rxbuf;
+
+        // Colorize RX buffer with known pattern
+        memset( slave_rxbuf, 0x66, sizeof(slave_rxbuf));
+
+        TEST_ESP_OK(spi_slave_queue_trans(TEST_SLAVE_HOST, &slave_t, portMAX_DELAY));
+
+        //send
+        spi_transaction_t t = {};
+        t.length = 32*(i+1);
+        if ( t.length != 0 ) {
+            t.tx_buffer = master_txbuf;
+            t.rx_buffer = NULL;
+        }
+        spi_device_transmit( spi, (spi_transaction_t*)&t );
+
+        //wait for end
+        TEST_ESP_OK(spi_slave_get_trans_result(TEST_SLAVE_HOST, &out, portMAX_DELAY));
+
+        //show result
+        ESP_LOGI(SLAVE_TAG, "trans_len: %d", slave_t.trans_len);
+        ESP_LOG_BUFFER_HEX( "master tx", t.tx_buffer, t.length/8 );
+        ESP_LOG_BUFFER_HEX( "slave rx", slave_t.rx_buffer, (slave_t.trans_len+7)/8);
+
+        TEST_ASSERT_EQUAL_HEX8_ARRAY( t.tx_buffer, slave_t.rx_buffer, t.length/8 );
+        TEST_ASSERT_EQUAL( t.length, slave_t.trans_len );
+    }
+
+    custom_teardown();
+
+    ESP_LOGI(SLAVE_TAG, "test passed.");
+}
+
+TEST_CASE("test fullduplex slave with only TX direction","[spi]")
+{
+    custom_setup();
+
+    memcpy(slave_txbuf, slave_send, sizeof(slave_send));
+
+    for ( int i = 0; i < 4; i ++ ) {
+        //slave send
+        spi_slave_transaction_t slave_t;
+        spi_slave_transaction_t* out;
+        memset(&slave_t, 0, sizeof(spi_slave_transaction_t));
+        slave_t.length=8*32;
+        slave_t.tx_buffer=slave_txbuf;
+        slave_t.rx_buffer=NULL;
+
+        // Colorize RX buffer with known pattern
+        memset( master_rxbuf, 0x66, sizeof(master_rxbuf));
+
+        TEST_ESP_OK(spi_slave_queue_trans(TEST_SLAVE_HOST, &slave_t, portMAX_DELAY));
+
+        //send
+        spi_transaction_t t = {};
+        t.length = 32*(i+1);
+        if ( t.length != 0 ) {
+            t.tx_buffer = NULL;
+            t.rx_buffer = master_rxbuf;
+        }
+        spi_device_transmit( spi, (spi_transaction_t*)&t );
+
+        //wait for end
+        TEST_ESP_OK(spi_slave_get_trans_result(TEST_SLAVE_HOST, &out, portMAX_DELAY));
+
+        //show result
+        ESP_LOGI(SLAVE_TAG, "trans_len: %d", slave_t.trans_len);
+        ESP_LOG_BUFFER_HEX( "master rx", t.rx_buffer, t.length/8 );
+        ESP_LOG_BUFFER_HEX( "slave tx", slave_t.tx_buffer, (slave_t.trans_len+7)/8);
+
+        TEST_ASSERT_EQUAL_HEX8_ARRAY( slave_t.tx_buffer, t.rx_buffer, t.length/8 );
+        TEST_ASSERT_EQUAL( t.length, slave_t.trans_len );
+    }
+
+    custom_teardown();
+
+    ESP_LOGI(SLAVE_TAG, "test passed.");
+}
+
+TEST_CASE("test slave send unaligned","[spi]")
+{
+    custom_setup();
+
+    memcpy(master_txbuf, master_send, sizeof(master_send));
+    memcpy(slave_txbuf, slave_send, sizeof(slave_send));
 
     for ( int i = 0; i < 4; i ++ ) {
         //slave send
@@ -111,6 +221,10 @@ TEST_CASE("test slave send unaligned","[spi]")
         slave_t.tx_buffer=slave_txbuf+i;
         slave_t.rx_buffer=slave_rxbuf;
 
+        // Colorize RX buffers with known pattern
+        memset( master_rxbuf, 0x66, sizeof(master_rxbuf));
+        memset( slave_rxbuf, 0x66, sizeof(slave_rxbuf));
+
         TEST_ESP_OK(spi_slave_queue_trans(TEST_SLAVE_HOST, &slave_t, portMAX_DELAY));
 
         //send
@@ -135,18 +249,11 @@ TEST_CASE("test slave send unaligned","[spi]")
         TEST_ASSERT_EQUAL_HEX8_ARRAY( t.tx_buffer, slave_t.rx_buffer, t.length/8 );
         TEST_ASSERT_EQUAL_HEX8_ARRAY( slave_t.tx_buffer, t.rx_buffer, t.length/8 );
         TEST_ASSERT_EQUAL( t.length, slave_t.trans_len );
-
-        //clean
-        memset( master_rxbuf, 0x66, sizeof(master_rxbuf));
-        memset( slave_rxbuf, 0x66, sizeof(slave_rxbuf));
     }
 
-    TEST_ASSERT(spi_slave_free(TEST_SLAVE_HOST) == ESP_OK);
-
-    TEST_ASSERT(spi_bus_remove_device(spi) == ESP_OK);
-    TEST_ASSERT(spi_bus_free(TEST_SPI_HOST) == ESP_OK);
+    custom_teardown();
 
-    ESP_LOGI(MASTER_TAG, "test passed.");
+    ESP_LOGI(SLAVE_TAG, "test passed.");
 }
 
 #endif // !CONFIG_SPIRAM

components/hal/esp32/include/hal/spi_ll.h

@@ -744,7 +744,7 @@ static inline void spi_ll_set_mosi_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    hw->slv_wrbuf_dlen.bit_len = bitlen - 1;
+    hw->slv_rdbuf_dlen.bit_len = bitlen - 1;
 }
 
 /**
@@ -755,7 +755,7 @@ static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_tx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    hw->slv_rdbuf_dlen.bit_len = bitlen - 1;
+    hw->slv_wrbuf_dlen.bit_len = bitlen - 1;
 }
 
 /**

components/hal/esp32c3/include/hal/spi_ll.h

@@ -136,6 +136,8 @@ static inline void spi_ll_slave_init(spi_dev_t *hw)
     hw->user.usr_miso_highpart = 0;
     hw->user.usr_mosi_highpart = 0;
 
+    // Configure DMA In-Link to not be terminated when transaction bit counter exceeds
+    hw->dma_conf.rx_eof_en = 0;
     hw->dma_conf.dma_seg_trans_en = 0;
 
     //Disable unneeded ints
@@ -584,7 +586,6 @@ static inline void spi_ll_master_set_io_mode(spi_dev_t *hw, spi_ll_io_mode_t io_
 static inline void spi_ll_slave_set_seg_mode(spi_dev_t *hw, bool seg_trans)
 {
     hw->dma_conf.dma_seg_trans_en = seg_trans;
-    hw->dma_conf.rx_eof_en = seg_trans;
 }
 
 /**
@@ -819,7 +820,7 @@ static inline void spi_ll_set_miso_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    spi_ll_set_mosi_bitlen(hw, bitlen);
+    //This is not used in esp32c3
 }
 
 /**
@@ -830,7 +831,7 @@ static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_tx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    spi_ll_set_mosi_bitlen(hw, bitlen);
+    //This is not used in esp32c3
 }
 
 /**

components/hal/esp32s2/include/hal/spi_ll.h

@@ -137,6 +137,9 @@ static inline void spi_ll_slave_init(spi_dev_t *hw)
     hw->user.usr_miso_highpart = 0;
     hw->user.usr_mosi_highpart = 0;
 
+    // Configure DMA In-Link to not be terminated when transaction bit counter exceeds
+    hw->dma_conf.rx_eof_en = 0;
+
     //Disable unneeded ints
     hw->slave.val &= ~SPI_LL_UNUSED_INT_MASK;
 }
@@ -545,7 +548,6 @@ static inline void spi_ll_master_set_io_mode(spi_dev_t *hw, spi_ll_io_mode_t io_
 static inline void spi_ll_slave_set_seg_mode(spi_dev_t *hw, bool seg_trans)
 {
     hw->dma_conf.dma_seg_trans_en = seg_trans;
-    hw->dma_conf.rx_eof_en = seg_trans;
 }
 
 /**
@@ -807,7 +809,7 @@ static inline void spi_ll_set_mosi_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    spi_ll_set_miso_bitlen(hw, bitlen);
+    //This is not used in esp32s2
 }
 
 /**
@@ -818,7 +820,7 @@ static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_tx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    spi_ll_set_miso_bitlen(hw, bitlen);
+    //This is not used in esp32s2
 }
 
 /**

components/hal/esp32s3/include/hal/spi_ll.h

@@ -136,6 +136,8 @@ static inline void spi_ll_slave_init(spi_dev_t *hw)
     hw->user.usr_miso_highpart = 0;
     hw->user.usr_mosi_highpart = 0;
 
+    // Configure DMA In-Link to not be terminated when transaction bit counter exceeds
+    hw->dma_conf.rx_eof_en = 0;
     hw->dma_conf.dma_seg_trans_en = 0;
 
     //Disable unneeded ints
@@ -584,7 +586,6 @@ static inline void spi_ll_master_set_io_mode(spi_dev_t *hw, spi_ll_io_mode_t io_
 static inline void spi_ll_slave_set_seg_mode(spi_dev_t *hw, bool seg_trans)
 {
     hw->dma_conf.dma_seg_trans_en = seg_trans;
-    hw->dma_conf.rx_eof_en = seg_trans;
 }
 
 /**
@@ -827,7 +828,7 @@ static inline void spi_ll_set_miso_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    spi_ll_set_mosi_bitlen(hw, bitlen);
+    //This is not used in esp32s3
 }
 
 /**
@@ -838,7 +839,7 @@ static inline void spi_ll_slave_set_rx_bitlen(spi_dev_t *hw, size_t bitlen)
  */
 static inline void spi_ll_slave_set_tx_bitlen(spi_dev_t *hw, size_t bitlen)
 {
-    spi_ll_set_mosi_bitlen(hw, bitlen);
+    //This is not used in esp32s3
 }
 
 /**

components/hal/spi_slave_hal_iram.c

@@ -71,8 +71,11 @@ void spi_slave_hal_prepare_data(const spi_slave_hal_context_t *hal)
     spi_ll_slave_set_rx_bitlen(hal->hw, hal->bitlen);
     spi_ll_slave_set_tx_bitlen(hal->hw, hal->bitlen);
 
-    spi_ll_enable_mosi(hal->hw, (hal->tx_buffer == NULL) ? 0 : 1);
-    spi_ll_enable_miso(hal->hw, (hal->rx_buffer == NULL) ? 0 : 1);
+#ifdef CONFIG_IDF_TARGET_ESP32
+    //SPI Slave mode on ESP32 requires MOSI/MISO enable
+    spi_ll_enable_mosi(hal->hw, (hal->rx_buffer == NULL) ? 0 : 1);
+    spi_ll_enable_miso(hal->hw, (hal->tx_buffer == NULL) ? 0 : 1);
+#endif
 }
 
 void spi_slave_hal_store_result(spi_slave_hal_context_t *hal)

components/hal/spi_slave_hd_hal.c

@@ -150,7 +150,6 @@ void spi_slave_hd_hal_rxdma(spi_slave_hd_hal_context_t *hal, uint8_t *out_buf, s
     spi_ll_infifo_full_clr(hal->dev);
     spi_ll_clear_intr(hal->dev, SPI_LL_INTR_CMD7);
 
-    spi_ll_slave_set_rx_bitlen(hal->dev, len * 8);
     spi_ll_dma_rx_enable(hal->dev, 1);
     spi_dma_ll_rx_start(hal->dma_in, hal->rx_dma_chan, &hal->dmadesc_rx->desc);
 }