refactor(spi_slave_hd): refactor append mode dma_desc struct

components/driver/spi/gpspi/spi_slave_hd.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2010-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2010-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -89,12 +89,40 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
     spihost[host_id] = host;
     host->int_spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
     host->dma_enabled = (config->dma_chan != SPI_DMA_DISABLED);
+    host->append_mode = append_mode;
 
     if (host->dma_enabled) {
         ret = spicommon_dma_chan_alloc(host_id, config->dma_chan, &actual_tx_dma_chan, &actual_rx_dma_chan);
         if (ret != ESP_OK) {
             goto cleanup;
         }
+
+        //Malloc for all the DMA descriptors
+        int dma_desc_ct = (bus_config->max_transfer_sz + DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED - 1) / DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED;
+        if (dma_desc_ct == 0) {
+            dma_desc_ct = 1; //default to 4k when max is not given
+        }
+        host->hal.dma_desc_num = dma_desc_ct;
+
+        lldesc_t *orig_dmadesc_tx = heap_caps_malloc(sizeof(lldesc_t) * dma_desc_ct, MALLOC_CAP_DMA);
+        lldesc_t *orig_dmadesc_rx = heap_caps_malloc(sizeof(lldesc_t) * dma_desc_ct, MALLOC_CAP_DMA);
+        host->hal.dmadesc_tx = heap_caps_malloc(sizeof(spi_slave_hd_hal_desc_append_t) * dma_desc_ct, MALLOC_CAP_DEFAULT);
+        host->hal.dmadesc_rx = heap_caps_malloc(sizeof(spi_slave_hd_hal_desc_append_t) * dma_desc_ct, MALLOC_CAP_DEFAULT);
+        if (!(host->hal.dmadesc_tx && host->hal.dmadesc_rx && orig_dmadesc_tx && orig_dmadesc_rx)) {
+            ret = ESP_ERR_NO_MEM;
+            goto cleanup;
+        }
+        //Pair each desc to each possible trans
+        for (int i = 0; i < dma_desc_ct; i ++) {
+            host->hal.dmadesc_tx[i].desc = &orig_dmadesc_tx[i];
+            host->hal.dmadesc_rx[i].desc = &orig_dmadesc_rx[i];
+        }
+
+        //Get the actual SPI bus transaction size in bytes.
+        host->max_transfer_sz = dma_desc_ct * DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED;
+    } else {
+        //We're limited to non-DMA transfers: the SPI work registers can hold 64 bytes at most.
+        host->max_transfer_sz = 0;
     }
 
     ret = spicommon_bus_initialize_io(host_id, bus_config, SPICOMMON_BUSFLAG_SLAVE | bus_config->flags, &host->flags);
@@ -104,7 +132,6 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
     gpio_set_direction(config->spics_io_num, GPIO_MODE_INPUT);
     spicommon_cs_initialize(host_id, config->spics_io_num, 0,
                             !(bus_config->flags & SPICOMMON_BUSFLAG_NATIVE_PINS));
-    host->append_mode = append_mode;
 
     spi_slave_hd_hal_config_t hal_config = {
         .host_id = host_id,
@@ -119,23 +146,6 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
         .rx_lsbfirst = (config->flags & SPI_SLAVE_HD_TXBIT_LSBFIRST),
     };
 
-    if (host->dma_enabled) {
-        //Malloc for all the DMA descriptors
-        uint32_t total_desc_size = spi_slave_hd_hal_get_total_desc_size(&host->hal, bus_config->max_transfer_sz);
-        host->hal.dmadesc_tx = heap_caps_malloc(total_desc_size, MALLOC_CAP_DMA);
-        host->hal.dmadesc_rx = heap_caps_malloc(total_desc_size, MALLOC_CAP_DMA);
-        if (!host->hal.dmadesc_tx || !host->hal.dmadesc_rx) {
-            ret = ESP_ERR_NO_MEM;
-            goto cleanup;
-        }
-
-        //Get the actual SPI bus transaction size in bytes.
-        host->max_transfer_sz = spi_salve_hd_hal_get_max_bus_size(&host->hal);
-    } else {
-        //We're limited to non-DMA transfers: the SPI work registers can hold 64 bytes at most.
-        host->max_transfer_sz = 0;
-    }
-
     //Init the hal according to the hal_config set above
     spi_slave_hd_hal_init(&host->hal, &hal_config);
 
@@ -232,21 +242,21 @@ esp_err_t spi_slave_hd_deinit(spi_host_device_t host_id)
     if (host->rx_ret_queue) vQueueDelete(host->rx_ret_queue);
     if (host->tx_cnting_sem) vSemaphoreDelete(host->tx_cnting_sem);
     if (host->rx_cnting_sem) vSemaphoreDelete(host->rx_cnting_sem);
-    if (host) {
-        free(host->hal.dmadesc_tx);
-        free(host->hal.dmadesc_rx);
-        esp_intr_free(host->intr);
-        esp_intr_free(host->intr_dma);
+    esp_intr_free(host->intr);
+    esp_intr_free(host->intr_dma);
 #ifdef CONFIG_PM_ENABLE
-        if (host->pm_lock) {
-            esp_pm_lock_release(host->pm_lock);
-            esp_pm_lock_delete(host->pm_lock);
-        }
-#endif
+    if (host->pm_lock) {
+        esp_pm_lock_release(host->pm_lock);
+        esp_pm_lock_delete(host->pm_lock);
     }
+#endif
 
     spicommon_periph_free(host_id);
     if (host->dma_enabled) {
+        free(host->hal.dmadesc_tx->desc);
+        free(host->hal.dmadesc_rx->desc);
+        free(host->hal.dmadesc_tx);
+        free(host->hal.dmadesc_rx);
         spicommon_dma_chan_free(host_id);
     }
     free(host);

components/hal/include/hal/spi_slave_hd_hal.h

@@ -62,7 +62,7 @@ extern "C" {
  *        this structure inherits DMA descriptor, with a pointer to the transaction descriptor passed from users.
  */
 typedef struct {
-    lldesc_t      desc;                             ///< DMA descriptor
+    lldesc_t      *desc;                            ///< DMA descriptor
     void          *arg;                             ///< This points to the transaction descriptor user passed in
 } spi_slave_hd_hal_desc_append_t;
 
@@ -105,13 +105,11 @@ typedef struct {
     spi_slave_hd_hal_desc_append_t  *tx_cur_desc;           ///< Current TX DMA descriptor that could be linked (set up).
     spi_slave_hd_hal_desc_append_t  *tx_dma_head;           ///< Head of the linked TX DMA descriptors which are not used by hardware
     spi_slave_hd_hal_desc_append_t  *tx_dma_tail;           ///< Tail of the linked TX DMA descriptors which are not used by hardware
-    spi_slave_hd_hal_desc_append_t  tx_dummy_head;          ///< Dummy descriptor for ``tx_dma_head`` to start
     uint32_t                        tx_used_desc_cnt;       ///< Number of the TX descriptors that have been setup
     uint32_t                        tx_recycled_desc_cnt;   ///< Number of the TX descriptors that could be recycled
     spi_slave_hd_hal_desc_append_t  *rx_cur_desc;           ///< Current RX DMA descriptor that could be linked (set up).
     spi_slave_hd_hal_desc_append_t  *rx_dma_head;           ///< Head of the linked RX DMA descriptors which are not used by hardware
     spi_slave_hd_hal_desc_append_t  *rx_dma_tail;           ///< Tail of the linked RX DMA descriptors which are not used by hardware
-    spi_slave_hd_hal_desc_append_t  rx_dummy_head;          ///< Dummy descriptor for ``rx_dma_head`` to start
     uint32_t                        rx_used_desc_cnt;       ///< Number of the RX descriptors that have been setup
     uint32_t                        rx_recycled_desc_cnt;   ///< Number of the RX descriptors that could be recycled
 
@@ -129,23 +127,6 @@ typedef struct {
  */
 void spi_slave_hd_hal_init(spi_slave_hd_hal_context_t *hal, const spi_slave_hd_hal_config_t *hal_config);
 
-/**
- * @brief Get the size of one DMA descriptor
- *
- * @param hal       Context of the HAL layer
- * @param bus_size  SPI bus maximum transfer size, in bytes.
- * @return          Total size needed for all the DMA descriptors
- */
-uint32_t spi_slave_hd_hal_get_total_desc_size(spi_slave_hd_hal_context_t *hal, uint32_t bus_size);
-
-/**
- * @brief Get the actual bus size
- *
- * @param hal       Context of the HAL layer
- * @return          Actual bus transaction size
- */
-uint32_t spi_salve_hd_hal_get_max_bus_size(spi_slave_hd_hal_context_t *hal);
-
 /**
  * @brief Check and clear signal of one event
  *

components/hal/spi_slave_hd_hal.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -63,12 +63,10 @@ void spi_slave_hd_hal_init(spi_slave_hd_hal_context_t *hal, const spi_slave_hd_h
     hal->tx_dma_chan = hal_config->tx_dma_chan;
     hal->rx_dma_chan = hal_config->rx_dma_chan;
     hal->append_mode = hal_config->append_mode;
-    hal->rx_cur_desc = hal->dmadesc_rx;
     hal->tx_cur_desc = hal->dmadesc_tx;
-    STAILQ_NEXT(&hal->tx_dummy_head.desc, qe) = &hal->dmadesc_tx->desc;
-    hal->tx_dma_head = &hal->tx_dummy_head;
-    STAILQ_NEXT(&hal->rx_dummy_head.desc, qe) = &hal->dmadesc_rx->desc;
-    hal->rx_dma_head = &hal->rx_dummy_head;
+    hal->rx_cur_desc = hal->dmadesc_rx;
+    hal->tx_dma_head = hal->dmadesc_tx + hal->dma_desc_num -1;
+    hal->rx_dma_head = hal->dmadesc_rx + hal->dma_desc_num -1;
 
     //Configure slave
     if (hal_config->dma_enabled) {
@@ -119,26 +117,9 @@ void spi_slave_hd_hal_init(spi_slave_hd_hal_context_t *hal, const spi_slave_hd_h
     spi_ll_slave_set_seg_mode(hal->dev, true);
 }
 
-uint32_t spi_salve_hd_hal_get_max_bus_size(spi_slave_hd_hal_context_t *hal)
-{
-    return hal->dma_desc_num * LLDESC_MAX_NUM_PER_DESC;
-}
-
-uint32_t spi_slave_hd_hal_get_total_desc_size(spi_slave_hd_hal_context_t *hal, uint32_t bus_size)
-{
-    //See how many dma descriptors we need
-    int dma_desc_ct = (bus_size + LLDESC_MAX_NUM_PER_DESC - 1) / LLDESC_MAX_NUM_PER_DESC;
-    if (dma_desc_ct == 0) {
-        dma_desc_ct = 1; //default to 4k when max is not given
-    }
-    hal->dma_desc_num = dma_desc_ct;
-
-    return hal->dma_desc_num * sizeof(spi_slave_hd_hal_desc_append_t);
-}
-
 void spi_slave_hd_hal_rxdma(spi_slave_hd_hal_context_t *hal, uint8_t *out_buf, size_t len)
 {
-    lldesc_setup_link(&hal->dmadesc_rx->desc, out_buf, len, true);
+    lldesc_setup_link(hal->dmadesc_rx->desc, out_buf, len, true);
 
     spi_ll_dma_rx_fifo_reset(hal->dev);
     spi_dma_ll_rx_reset(hal->dma_in, hal->rx_dma_chan);
@@ -147,12 +128,12 @@ void spi_slave_hd_hal_rxdma(spi_slave_hd_hal_context_t *hal, uint8_t *out_buf, s
     spi_ll_clear_intr(hal->dev, SPI_LL_INTR_CMD7);
 
     spi_ll_dma_rx_enable(hal->dev, 1);
-    spi_dma_ll_rx_start(hal->dma_in, hal->rx_dma_chan, &hal->dmadesc_rx->desc);
+    spi_dma_ll_rx_start(hal->dma_in, hal->rx_dma_chan, hal->dmadesc_rx->desc);
 }
 
 void spi_slave_hd_hal_txdma(spi_slave_hd_hal_context_t *hal, uint8_t *data, size_t len)
 {
-    lldesc_setup_link(&hal->dmadesc_tx->desc, data, len, false);
+    lldesc_setup_link(hal->dmadesc_tx->desc, data, len, false);
 
     spi_ll_dma_tx_fifo_reset(hal->dev);
     spi_dma_ll_tx_reset(hal->dma_out, hal->tx_dma_chan);
@@ -161,7 +142,7 @@ void spi_slave_hd_hal_txdma(spi_slave_hd_hal_context_t *hal, uint8_t *data, size
     spi_ll_clear_intr(hal->dev, SPI_LL_INTR_CMD8);
 
     spi_ll_dma_tx_enable(hal->dev, 1);
-    spi_dma_ll_tx_start(hal->dma_out, hal->tx_dma_chan, &hal->dmadesc_tx->desc);
+    spi_dma_ll_tx_start(hal->dma_out, hal->tx_dma_chan, hal->dmadesc_tx->desc);
 }
 
 static spi_ll_intr_t get_event_intr(spi_slave_hd_hal_context_t *hal, spi_event_t ev)
@@ -257,68 +238,45 @@ int spi_slave_hd_hal_get_rxlen(spi_slave_hd_hal_context_t *hal)
 
 int spi_slave_hd_hal_rxdma_seg_get_len(spi_slave_hd_hal_context_t *hal)
 {
-    lldesc_t *desc = &hal->dmadesc_rx->desc;
+    lldesc_t *desc = hal->dmadesc_rx->desc;
     return lldesc_get_received_len(desc, NULL);
 }
 
 bool spi_slave_hd_hal_get_tx_finished_trans(spi_slave_hd_hal_context_t *hal, void **out_trans)
 {
-    if ((uint32_t)&hal->tx_dma_head->desc == spi_dma_ll_get_out_eof_desc_addr(hal->dma_out, hal->tx_dma_chan)) {
+    uint32_t desc_now = spi_dma_ll_get_out_eof_desc_addr(hal->dma_out, hal->tx_dma_chan);
+    if ((uint32_t)hal->tx_dma_head->desc == desc_now) {
         return false;
     }
 
-    hal->tx_dma_head = (spi_slave_hd_hal_desc_append_t *)STAILQ_NEXT(&hal->tx_dma_head->desc, qe);
+    //find used paired desc-trans by desc addr
+    hal->tx_dma_head++;
+    if (hal->tx_dma_head >= hal->dmadesc_tx + hal->dma_desc_num) {
+        hal->tx_dma_head = hal->dmadesc_tx;
+    }
     *out_trans = hal->tx_dma_head->arg;
     hal->tx_recycled_desc_cnt++;
-
     return true;
 }
 
 bool spi_slave_hd_hal_get_rx_finished_trans(spi_slave_hd_hal_context_t *hal, void **out_trans, size_t *out_len)
 {
-    if ((uint32_t)&hal->rx_dma_head->desc == spi_dma_ll_get_in_suc_eof_desc_addr(hal->dma_in, hal->rx_dma_chan)) {
+    uint32_t desc_now = spi_dma_ll_get_in_suc_eof_desc_addr(hal->dma_in, hal->rx_dma_chan);
+    if ((uint32_t)hal->rx_dma_head->desc == desc_now) {
         return false;
     }
 
-    hal->rx_dma_head = (spi_slave_hd_hal_desc_append_t *)STAILQ_NEXT(&hal->rx_dma_head->desc, qe);
+    //find used paired desc-trans by desc addr
+    hal->rx_dma_head++;
+    if (hal->rx_dma_head >= hal->dmadesc_rx + hal->dma_desc_num) {
+        hal->rx_dma_head = hal->dmadesc_rx;
+    }
     *out_trans = hal->rx_dma_head->arg;
-    *out_len = hal->rx_dma_head->desc.length;
+    *out_len = hal->rx_dma_head->desc->length;
     hal->rx_recycled_desc_cnt++;
-
     return true;
 }
 
-static void spi_slave_hd_hal_link_append_desc(spi_slave_hd_hal_desc_append_t *dmadesc, const void *data, int len, bool isrx, void *arg)
-{
-    HAL_ASSERT(len <= LLDESC_MAX_NUM_PER_DESC);     //TODO: Add support for transaction with length larger than 4092, IDF-2660
-    int n = 0;
-    while (len) {
-        int dmachunklen = len;
-        if (dmachunklen > LLDESC_MAX_NUM_PER_DESC) {
-            dmachunklen = LLDESC_MAX_NUM_PER_DESC;
-        }
-        if (isrx) {
-            //Receive needs DMA length rounded to next 32-bit boundary
-            dmadesc[n].desc.size = (dmachunklen + 3) & (~3);
-            dmadesc[n].desc.length = (dmachunklen + 3) & (~3);
-        } else {
-            dmadesc[n].desc.size = dmachunklen;
-            dmadesc[n].desc.length = dmachunklen;
-        }
-        dmadesc[n].desc.buf = (uint8_t *)data;
-        dmadesc[n].desc.eof = 0;
-        dmadesc[n].desc.sosf = 0;
-        dmadesc[n].desc.owner = 1;
-        dmadesc[n].desc.qe.stqe_next = &dmadesc[n + 1].desc;
-        dmadesc[n].arg = arg;
-        len -= dmachunklen;
-        data += dmachunklen;
-        n++;
-    }
-    dmadesc[n - 1].desc.eof = 1; //Mark last DMA desc as end of stream.
-    dmadesc[n - 1].desc.qe.stqe_next = NULL;
-}
-
 esp_err_t spi_slave_hd_hal_txdma_append(spi_slave_hd_hal_context_t *hal, uint8_t *data, size_t len, void *arg)
 {
     //Check if there are enough available DMA descriptors for software to use
@@ -329,7 +287,8 @@ esp_err_t spi_slave_hd_hal_txdma_append(spi_slave_hd_hal_context_t *hal, uint8_t
         return ESP_ERR_INVALID_STATE;
     }
 
-    spi_slave_hd_hal_link_append_desc(hal->tx_cur_desc, data, len, false, arg);
+    lldesc_setup_link(hal->tx_cur_desc->desc, data, len, false);
+    hal->tx_cur_desc->arg = arg;
 
     if (!hal->tx_dma_started) {
         hal->tx_dma_started = true;
@@ -339,10 +298,10 @@ esp_err_t spi_slave_hd_hal_txdma_append(spi_slave_hd_hal_context_t *hal, uint8_t
         spi_ll_outfifo_empty_clr(hal->dev);
         spi_dma_ll_tx_reset(hal->dma_out, hal->tx_dma_chan);
         spi_ll_dma_tx_enable(hal->dev, 1);
-        spi_dma_ll_tx_start(hal->dma_out, hal->tx_dma_chan, &hal->tx_cur_desc->desc);
+        spi_dma_ll_tx_start(hal->dma_out, hal->tx_dma_chan, hal->tx_cur_desc->desc);
     } else {
         //there is already a consecutive link
-        STAILQ_NEXT(&hal->tx_dma_tail->desc, qe) = &hal->tx_cur_desc->desc;
+        STAILQ_NEXT(hal->tx_dma_tail->desc, qe) = hal->tx_cur_desc->desc;
         hal->tx_dma_tail = hal->tx_cur_desc;
         spi_dma_ll_tx_restart(hal->dma_out, hal->tx_dma_chan);
     }
@@ -369,7 +328,8 @@ esp_err_t spi_slave_hd_hal_rxdma_append(spi_slave_hd_hal_context_t *hal, uint8_t
         return ESP_ERR_INVALID_STATE;
     }
 
-    spi_slave_hd_hal_link_append_desc(hal->rx_cur_desc, data, len, false, arg);
+    lldesc_setup_link(hal->rx_cur_desc->desc, data, len, false);
+    hal->rx_cur_desc->arg = arg;
 
     if (!hal->rx_dma_started) {
         hal->rx_dma_started = true;
@@ -379,10 +339,10 @@ esp_err_t spi_slave_hd_hal_rxdma_append(spi_slave_hd_hal_context_t *hal, uint8_t
         spi_ll_dma_rx_fifo_reset(hal->dma_in);
         spi_ll_infifo_full_clr(hal->dev);
         spi_ll_dma_rx_enable(hal->dev, 1);
-        spi_dma_ll_rx_start(hal->dma_in, hal->rx_dma_chan, &hal->rx_cur_desc->desc);
+        spi_dma_ll_rx_start(hal->dma_in, hal->rx_dma_chan, hal->rx_cur_desc->desc);
     } else {
         //there is already a consecutive link
-        STAILQ_NEXT(&hal->rx_dma_tail->desc, qe) = &hal->rx_cur_desc->desc;
+        STAILQ_NEXT(hal->rx_dma_tail->desc, qe) = hal->rx_cur_desc->desc;
         hal->rx_dma_tail = hal->rx_cur_desc;
         spi_dma_ll_rx_restart(hal->dma_in, hal->rx_dma_chan);
     }