Merge branch 'bugfix/spi_lose_last_3_bytes' into 'master'

spi_master: fix an issue where master cannot correctly receive data when using DMA in halfduplex mode.

Closes IDFGH-612

See merge request espressif/esp-idf!11354

components/driver/test/include/test/test_common_spi.h

@@ -298,4 +298,11 @@ void master_free_device_bus(spi_device_handle_t spi);
 //use this function to fix the output source when assign multiple funcitons to a same pin
 void spitest_gpio_output_sel(uint32_t gpio_num, int func, uint32_t signal_idx);
 
+//use this function to fix the input source when assign multiple funcitons to a same pin
+void spitest_gpio_input_sel(uint32_t gpio_num, int func, uint32_t signal_idx);
+
+//Note this cs_num is the ID of the connected devices' ID, e.g. if 2 devices are connected to the bus,
+//then the cs_num of the 1st and 2nd devices are 0 and 1 respectively.
+void same_pin_func_sel(spi_bus_config_t bus, spi_device_interface_config_t dev, uint8_t cs_num);
+
 #endif  //_TEST_COMMON_SPI_H_

components/driver/test/test_common_spi.c

@@ -203,5 +203,32 @@ void master_free_device_bus(spi_device_handle_t spi)
 void spitest_gpio_output_sel(uint32_t gpio_num, int func, uint32_t signal_idx)
 {
     PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[gpio_num], func);
-    GPIO.func_out_sel_cfg[gpio_num].func_sel=signal_idx;
+    GPIO.func_out_sel_cfg[gpio_num].func_sel = signal_idx;
+}
+
+void spitest_gpio_input_sel(uint32_t gpio_num, int func, uint32_t signal_idx)
+{
+    PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[gpio_num], func);
+    GPIO.func_in_sel_cfg[signal_idx].func_sel = gpio_num;
+}
+
+//Note this cs_num is the ID of the connected devices' ID, e.g. if 2 devices are connected to the bus,
+//then the cs_num of the 1st and 2nd devices are 0 and 1 respectively.
+void same_pin_func_sel(spi_bus_config_t bus, spi_device_interface_config_t dev, uint8_t cs_num)
+{
+    spitest_gpio_output_sel(bus.mosi_io_num, FUNC_GPIO, spi_periph_signal[TEST_SPI_HOST].spid_out);
+    spitest_gpio_input_sel(bus.mosi_io_num, FUNC_GPIO, spi_periph_signal[TEST_SLAVE_HOST].spid_in);
+
+    spitest_gpio_output_sel(bus.miso_io_num, FUNC_GPIO, spi_periph_signal[TEST_SLAVE_HOST].spiq_out);
+    spitest_gpio_input_sel(bus.miso_io_num, FUNC_GPIO, spi_periph_signal[TEST_SPI_HOST].spiq_in);
+
+    spitest_gpio_output_sel(dev.spics_io_num, FUNC_GPIO, spi_periph_signal[TEST_SPI_HOST].spics_out[cs_num]);
+    spitest_gpio_input_sel(dev.spics_io_num, FUNC_GPIO, spi_periph_signal[TEST_SLAVE_HOST].spics_in);
+
+    spitest_gpio_output_sel(bus.sclk_io_num, FUNC_GPIO, spi_periph_signal[TEST_SPI_HOST].spiclk_out);
+    spitest_gpio_input_sel(bus.sclk_io_num, FUNC_GPIO, spi_periph_signal[TEST_SLAVE_HOST].spiclk_in);
+
+#if CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3
+    GPIO.func_in_sel_cfg[FSPIQ_IN_IDX].sig_in_sel = 1;
+#endif
 }

components/driver/test/test_spi_master.c

@@ -936,7 +936,7 @@ TEST_CASE("SPI master variable dummy test", "[spi]")
     TEST_ESP_OK(spi_bus_initialize(TEST_SPI_HOST, &bus_cfg, 0));
     TEST_ESP_OK(spi_bus_add_device(TEST_SPI_HOST, &dev_cfg, &spi));
 
-    spi_slave_interface_config_t slave_cfg =SPI_SLAVE_TEST_DEFAULT_CONFIG();
+    spi_slave_interface_config_t slave_cfg = SPI_SLAVE_TEST_DEFAULT_CONFIG();
     TEST_ESP_OK(spi_slave_initialize(TEST_SLAVE_HOST, &bus_cfg, &slave_cfg, 0));
 
     spitest_gpio_output_sel(bus_cfg.mosi_io_num, FUNC_GPIO, spi_periph_signal[TEST_SPI_HOST].spid_out);
@@ -959,9 +959,101 @@ TEST_CASE("SPI master variable dummy test", "[spi]")
     master_free_device_bus(spi);
 }
 
+#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S3)
+/**
+ * This test is to check when the first transaction of the HD master is to send data without receiving data via DMA,
+ * then if the master could receive data correctly.
+ *
+ * Because an old version ESP32 silicon issue, there is a workaround to enable and start the RX DMA in FD/HD mode in
+ * this condition (TX without RX). And if RX DMA is enabled and started in HD mode, because there is no correctly
+ * linked RX DMA descriptor, there will be an inlink_dscr_error interrupt emerging, which will influence the following
+ * RX transactions.
+ *
+ * This bug is fixed by triggering this workaround only in FD mode.
+ *
+ */
+TEST_CASE("SPI master hd dma TX without RX test", "[spi]")
+{
+    spi_bus_config_t bus_cfg = SPI_BUS_TEST_DEFAULT_CONFIG();
+    TEST_ESP_OK(spi_bus_initialize(TEST_SPI_HOST, &bus_cfg, TEST_SPI_HOST));
+
+    spi_device_handle_t spi;
+    spi_device_interface_config_t dev_cfg = SPI_DEVICE_TEST_DEFAULT_CONFIG();
+    dev_cfg.flags = SPI_DEVICE_HALFDUPLEX;
+    dev_cfg.clock_speed_hz = 4*1000*1000;
+    TEST_ESP_OK(spi_bus_add_device(TEST_SPI_HOST, &dev_cfg, &spi));
+
+    spi_slave_interface_config_t slave_cfg = SPI_SLAVE_TEST_DEFAULT_CONFIG();
+    TEST_ESP_OK(spi_slave_initialize(TEST_SLAVE_HOST, &bus_cfg, &slave_cfg, TEST_SLAVE_HOST));
+
+    same_pin_func_sel(bus_cfg, dev_cfg, 0);
+
+    uint32_t buf_size = 32;
+    uint8_t *mst_send_buf = heap_caps_malloc(buf_size, MALLOC_CAP_DMA);
+    uint8_t *mst_recv_buf = heap_caps_calloc(buf_size, 1, MALLOC_CAP_DMA);
+    uint8_t *slv_send_buf = heap_caps_malloc(buf_size, MALLOC_CAP_DMA);
+    uint8_t *slv_recv_buf = heap_caps_calloc(buf_size, 1, MALLOC_CAP_DMA);
+
+    srand(199);
+    for (int i = 0; i < buf_size; i++) {
+        mst_send_buf[i] = rand();
+    }
+
+    //1. Master sends without receiving, no rx_buffer is set
+    spi_slave_transaction_t slave_trans = {
+        .rx_buffer = slv_recv_buf,
+        .length = buf_size * 8,
+    };
+    TEST_ESP_OK(spi_slave_queue_trans(TEST_SLAVE_HOST, &slave_trans, portMAX_DELAY));
+
+    spi_transaction_t master_trans = {
+        .tx_buffer = mst_send_buf,
+        .length = buf_size * 8,
+    };
+    TEST_ESP_OK(spi_device_transmit(spi, &master_trans));
+
+    spi_slave_transaction_t *ret_slave;
+    TEST_ESP_OK(spi_slave_get_trans_result(TEST_SLAVE_HOST, &ret_slave, portMAX_DELAY));
+
+    spitest_cmp_or_dump(mst_send_buf, slv_recv_buf, buf_size);
+
+    //2. Master receives data
+    for (int i = 100; i < 110; i++) {
+
+        srand(i);
+        for (int j = 0; j < buf_size; j++) {
+            slv_send_buf[j] = rand();
+        }
+        slave_trans = (spi_slave_transaction_t) {};
+        slave_trans.tx_buffer = slv_send_buf;
+        slave_trans.length = buf_size * 8;
+        TEST_ESP_OK(spi_slave_queue_trans(TEST_SLAVE_HOST, &slave_trans, portMAX_DELAY));
+
+        vTaskDelay(50);
+
+        master_trans = (spi_transaction_t) {};
+        master_trans.rx_buffer = mst_recv_buf;
+        master_trans.rxlength = buf_size * 8;
+        TEST_ESP_OK(spi_device_transmit(spi, &master_trans));
+
+        TEST_ESP_OK(spi_slave_get_trans_result(TEST_SLAVE_HOST, &ret_slave, portMAX_DELAY));
+
+        spitest_cmp_or_dump(slv_send_buf, mst_recv_buf, buf_size);
+    }
+
+    free(mst_send_buf);
+    free(mst_recv_buf);
+    free(slv_send_buf);
+    free(slv_recv_buf);
+    spi_slave_free(TEST_SLAVE_HOST);
+    master_free_device_bus(spi);
+}
+#endif  // #if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S3)
+
 //There is only one GPSPI controller, so single-board test is disabled.
 #endif  //#if !DISABLED_FOR_TARGETS(ESP32C3)
 
+
 #if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32C3)
 /********************************************************************************
  *      Test SPI transaction interval

components/hal/spi_hal_iram.c

@@ -151,13 +151,16 @@ void spi_hal_prepare_data(spi_hal_context_t *hal, const spi_hal_dev_config_t *de
             spi_dma_ll_rx_start(hal->dma_in, hal->dma_config.dmadesc_rx);
         }
 
-    } else {
+    }
+#if CONFIG_IDF_TARGET_ESP32
+    else {
         //DMA temporary workaround: let RX DMA work somehow to avoid the issue in ESP32 v0/v1 silicon
-        if (hal->dma_enabled) {
+        if (hal->dma_enabled && !dev->half_duplex) {
             spi_ll_dma_rx_enable(hal->hw, 1);
             spi_dma_ll_rx_start(hal->dma_in, 0);
         }
     }
+#endif
 
     if (trans->send_buffer) {
         if (!hal->dma_enabled) {