spi_slave_hd: refactor the hal append api to remove the spinlock

components/driver/spi_slave_hd.c

@@ -87,13 +87,12 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
     spi_chan_claimed = spicommon_periph_claim(host_id, "slave_hd");
     SPIHD_CHECK(spi_chan_claimed, "host already in use", ESP_ERR_INVALID_STATE);
 
-    spi_slave_hd_slot_t* host = malloc(sizeof(spi_slave_hd_slot_t));
+    spi_slave_hd_slot_t* host = calloc(1, sizeof(spi_slave_hd_slot_t));
     if (host == NULL) {
         ret = ESP_ERR_NO_MEM;
         goto cleanup;
     }
     spihost[host_id] = host;
-    memset(host, 0, sizeof(spi_slave_hd_slot_t));
     host->int_spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
     host->dma_enabled = (config->dma_chan != SPI_DMA_DISABLED);
 
@@ -398,9 +397,7 @@ static IRAM_ATTR void spi_slave_hd_intr_append(void *arg)
         spi_slave_hd_data_t *trans_desc;
         while (1) {
             bool trans_finish = false;
-            portENTER_CRITICAL_ISR(&host->int_spinlock);
             trans_finish = spi_slave_hd_hal_get_tx_finished_trans(hal, (void **)&trans_desc);
-            portEXIT_CRITICAL_ISR(&host->int_spinlock);
             if (!trans_finish) {
                 break;
             }
@@ -431,9 +428,7 @@ static IRAM_ATTR void spi_slave_hd_intr_append(void *arg)
         size_t trans_len;
         while (1) {
             bool trans_finish = false;
-            portENTER_CRITICAL_ISR(&host->int_spinlock);
             trans_finish = spi_slave_hd_hal_get_rx_finished_trans(hal, (void **)&trans_desc, &trans_len);
-            portEXIT_CRITICAL_ISR(&host->int_spinlock);
             if (!trans_finish) {
                 break;
             }
@@ -552,17 +547,13 @@ esp_err_t spi_slave_hd_append_trans(spi_host_device_t host_id, spi_slave_chan_t
         if (ret == pdFALSE) {
             return ESP_ERR_TIMEOUT;
         }
-        portENTER_CRITICAL(&host->int_spinlock);
         err = spi_slave_hd_hal_txdma_append(hal, trans->data, trans->len, trans);
-        portEXIT_CRITICAL(&host->int_spinlock);
     } else {
         BaseType_t ret = xSemaphoreTake(host->rx_cnting_sem, timeout);
         if (ret == pdFALSE) {
             return ESP_ERR_TIMEOUT;
         }
-        portENTER_CRITICAL(&host->int_spinlock);
         err = spi_slave_hd_hal_rxdma_append(hal, trans->data, trans->len, trans);
-        portEXIT_CRITICAL(&host->int_spinlock);
     }
     if (err != ESP_OK) {
         ESP_LOGE(TAG, "Wait until the DMA finishes its transaction");

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

@@ -1077,6 +1077,7 @@ static inline uint32_t spi_ll_slave_hd_get_last_addr(spi_dev_t* hw)
  *      RX DMA (Peripherals->DMA->RAM)
  *      TX DMA (RAM->DMA->Peripherals)
  *----------------------------------------------------------------------------*/
+//---------------------------------------------------RX-------------------------------------------------//
 /**
  * Reset RX DMA which stores the data received from a peripheral into RAM.
  *
@@ -1119,7 +1120,7 @@ static inline void spi_dma_ll_rx_enable_burst_data(spi_dma_dev_t *dma_in, uint32
 }
 
 /**
- * Enable DMA TX channel burst for descriptor
+ * Enable DMA RX channel burst for descriptor
  *
  * @param dma_in  Beginning address of the DMA peripheral registers which stores the data received from a peripheral into RAM.
  * @param channel DMA channel, for chip version compatibility, not used.
@@ -1130,6 +1131,19 @@ static inline void spi_dma_ll_rx_enable_burst_desc(spi_dma_dev_t *dma_in, uint32
     dma_in->dma_conf.indscr_burst_en = enable;
 }
 
+/**
+ * Get the last inlink descriptor address when DMA produces in_suc_eof interrupt
+ *
+ * @param dma_in  Beginning address of the DMA peripheral registers which stores the data received from a peripheral into RAM.
+ * @param channel DMA channel, for chip version compatibility, not used.
+ * @return        The address
+ */
+static inline uint32_t spi_dma_ll_get_in_suc_eof_desc_addr(spi_dma_dev_t *dma_in, uint32_t channel)
+{
+    return dma_in->dma_in_suc_eof_des_addr;
+}
+
+//---------------------------------------------------TX-------------------------------------------------//
 /**
  * Reset TX DMA which transmits the data from RAM to a peripheral.
  *
@@ -1204,6 +1218,18 @@ static inline void spi_dma_ll_enable_out_auto_wrback(spi_dma_dev_t *dma_out, uin
     dma_out->dma_conf.out_auto_wrback = enable;
 }
 
+/**
+ * Get the last outlink descriptor address when DMA produces out_eof intrrupt
+ *
+ * @param dma_out Beginning address of the DMA peripheral registers which transmits the data from RAM to a peripheral.
+ * @param channel DMA channel, for chip version compatibility, not used.
+ * @return        The address
+ */
+static inline uint32_t spi_dma_ll_get_out_eof_desc_addr(spi_dma_dev_t *dma_out, uint32_t channel)
+{
+    return dma_out->dma_out_eof_des_addr;
+}
+
 static inline void spi_dma_ll_rx_restart(spi_dma_dev_t *dma_in, uint32_t channel)
 {
     dma_in->dma_in_link.restart = 1;

components/hal/include/hal/spi_slave_hd_hal.h

@@ -104,16 +104,20 @@ 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
 
     /* Internal status used by the HAL implementation, initialized as 0. */
     uint32_t                        intr_not_triggered;
+    bool                            tx_dma_started;
+    bool                            rx_dma_started;
 } spi_slave_hd_hal_context_t;
 
 /**

components/hal/spi_slave_hd_hal.c

@@ -30,13 +30,15 @@
 #include "hal/gdma_ll.h"
 
 #define spi_dma_ll_rx_reset(dev, chan)                             gdma_ll_rx_reset_channel(&GDMA, chan)
-#define spi_dma_ll_tx_reset(dev, chan)                             gdma_ll_tx_reset_channel(&GDMA, chan);
-#define spi_dma_ll_rx_enable_burst_data(dev, chan, enable)         gdma_ll_rx_enable_data_burst(&GDMA, chan, enable);
-#define spi_dma_ll_tx_enable_burst_data(dev, chan, enable)         gdma_ll_tx_enable_data_burst(&GDMA, chan, enable);
-#define spi_dma_ll_rx_enable_burst_desc(dev, chan, enable)         gdma_ll_rx_enable_descriptor_burst(&GDMA, chan, enable);
-#define spi_dma_ll_tx_enable_burst_desc(dev, chan, enable)         gdma_ll_tx_enable_descriptor_burst(&GDMA, chan, enable);
-#define spi_dma_ll_enable_out_auto_wrback(dev, chan, enable)          gdma_ll_tx_enable_auto_write_back(&GDMA, chan, enable);
-#define spi_dma_ll_set_out_eof_generation(dev, chan, enable)          gdma_ll_tx_set_eof_mode(&GDMA, chan, enable);
+#define spi_dma_ll_tx_reset(dev, chan)                             gdma_ll_tx_reset_channel(&GDMA, chan)
+#define spi_dma_ll_rx_enable_burst_data(dev, chan, enable)         gdma_ll_rx_enable_data_burst(&GDMA, chan, enable)
+#define spi_dma_ll_tx_enable_burst_data(dev, chan, enable)         gdma_ll_tx_enable_data_burst(&GDMA, chan, enable)
+#define spi_dma_ll_rx_enable_burst_desc(dev, chan, enable)         gdma_ll_rx_enable_descriptor_burst(&GDMA, chan, enable)
+#define spi_dma_ll_tx_enable_burst_desc(dev, chan, enable)         gdma_ll_tx_enable_descriptor_burst(&GDMA, chan, enable)
+#define spi_dma_ll_enable_out_auto_wrback(dev, chan, enable)       gdma_ll_tx_enable_auto_write_back(&GDMA, chan, enable)
+#define spi_dma_ll_set_out_eof_generation(dev, chan, enable)       gdma_ll_tx_set_eof_mode(&GDMA, chan, enable)
+#define spi_dma_ll_get_out_eof_desc_addr(dev, chan)                gdma_ll_tx_get_eof_desc_addr(&GDMA, chan)
+#define spi_dma_ll_get_in_suc_eof_desc_addr(dev, chan)             gdma_ll_rx_get_success_eof_desc_addr(&GDMA, chan)
 #define spi_dma_ll_rx_start(dev, chan, addr) do {\
             gdma_ll_rx_set_desc_addr(&GDMA, chan, (uint32_t)addr);\
             gdma_ll_rx_start(&GDMA, chan);\
@@ -54,6 +56,7 @@ static void s_spi_slave_hd_hal_dma_init_config(const spi_slave_hd_hal_context_t
     spi_dma_ll_rx_enable_burst_desc(hal->dma_in, hal->rx_dma_chan, 1);
     spi_dma_ll_tx_enable_burst_desc(hal->dma_out, hal->tx_dma_chan, 1);
     spi_dma_ll_enable_out_auto_wrback(hal->dma_out, hal->tx_dma_chan, 1);
+    spi_dma_ll_set_out_eof_generation(hal->dma_out, hal->tx_dma_chan, 1);
 }
 
 void spi_slave_hd_hal_init(spi_slave_hd_hal_context_t *hal, const spi_slave_hd_hal_config_t *hal_config)
@@ -68,6 +71,10 @@ void spi_slave_hd_hal_init(spi_slave_hd_hal_context_t *hal, const spi_slave_hd_h
     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;
 
     //Configure slave
     s_spi_slave_hd_hal_dma_init_config(hal);
@@ -261,27 +268,27 @@ int spi_slave_hd_hal_rxdma_seg_get_len(spi_slave_hd_hal_context_t *hal)
 
 bool spi_slave_hd_hal_get_tx_finished_trans(spi_slave_hd_hal_context_t *hal, void **out_trans)
 {
-    if (!hal->tx_dma_head || hal->tx_dma_head->desc.owner) {
+    if ((uint32_t)&hal->tx_dma_head->desc == spi_dma_ll_get_out_eof_desc_addr(hal->dma_out, hal->tx_dma_chan)) {
         return false;
     }
 
+    hal->tx_dma_head = (spi_slave_hd_hal_desc_append_t *)STAILQ_NEXT(&hal->tx_dma_head->desc, qe);
     *out_trans = hal->tx_dma_head->arg;
     hal->tx_recycled_desc_cnt++;
-    hal->tx_dma_head = (spi_slave_hd_hal_desc_append_t *)STAILQ_NEXT(&hal->tx_dma_head->desc, qe);
 
     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 (!hal->rx_dma_head || hal->rx_dma_head->desc.owner) {
+    if ((uint32_t)&hal->rx_dma_head->desc == spi_dma_ll_get_in_suc_eof_desc_addr(hal->dma_in, hal->rx_dma_chan)) {
         return false;
     }
 
+    hal->rx_dma_head = (spi_slave_hd_hal_desc_append_t *)STAILQ_NEXT(&hal->rx_dma_head->desc, qe);
     *out_trans = hal->rx_dma_head->arg;
     *out_len = hal->rx_dma_head->desc.length;
     hal->rx_recycled_desc_cnt++;
-    hal->rx_dma_head = (spi_slave_hd_hal_desc_append_t *)STAILQ_NEXT(&hal->rx_dma_head->desc, qe);
 
     return true;
 }
@@ -331,22 +338,20 @@ esp_err_t spi_slave_hd_hal_txdma_append(spi_slave_hd_hal_context_t *hal, uint8_t
 
     spi_slave_hd_hal_link_append_desc(hal->tx_cur_desc, data, len, false, arg);
 
-    if (!hal->tx_dma_head) {
-        //start a new link
-        hal->tx_dma_head = hal->tx_cur_desc;
+    if (!hal->tx_dma_started) {
+        hal->tx_dma_started = true;
+        //start a link
         hal->tx_dma_tail = hal->tx_cur_desc;
-
-        spi_dma_ll_tx_reset(hal->dma_out, hal->tx_dma_chan);
-        spi_ll_outfifo_empty_clr(hal->dev);
         spi_ll_clear_intr(hal->dev, SPI_LL_INTR_OUT_EOF);
-
+        spi_ll_dma_tx_fifo_reset(hal->dma_out);
+        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_dma_head->desc);
+        spi_dma_ll_tx_start(hal->dma_out, hal->tx_dma_chan, &hal->tx_cur_desc->desc);
     } else {
-        //there is already a link
+        //there is already a consecutive link
         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);
     }
 
@@ -374,22 +379,20 @@ esp_err_t spi_slave_hd_hal_rxdma_append(spi_slave_hd_hal_context_t *hal, uint8_t
 
     spi_slave_hd_hal_link_append_desc(hal->rx_cur_desc, data, len, false, arg);
 
-    if (!hal->rx_dma_head) {
-        //start a new link
-        hal->rx_dma_head = hal->rx_cur_desc;
+    if (!hal->rx_dma_started) {
+        hal->rx_dma_started = true;
+        //start a link
         hal->rx_dma_tail = hal->rx_cur_desc;
-
+        spi_ll_clear_intr(hal->dev, SPI_LL_INTR_CMD7);
         spi_dma_ll_rx_reset(hal->dma_in, hal->rx_dma_chan);
+        spi_ll_dma_rx_fifo_reset(hal->dma_in);
         spi_ll_infifo_full_clr(hal->dev);
-        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->rx_dma_head->desc);
+        spi_dma_ll_rx_start(hal->dma_in, hal->rx_dma_chan, &hal->rx_cur_desc->desc);
     } else {
-        //there is already a link
+        //there is already a consecutive link
         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);
     }