Merge branch 'bugfix/spi_master_add_dummy_check' into 'master'

spi_master: add dummy check when both mosi and miso are set

Closes IDF-1872 and IDF-266

See merge request espressif/esp-idf!9406

Author: ginkgm

components/driver/spi_master.c

@@ -352,7 +352,7 @@ esp_err_t spi_bus_add_device(spi_host_device_t host_id, const spi_device_interfa
 
     spi_hal_timing_conf_t temp_timing_conf;
 
-    esp_err_t ret = spi_hal_get_clock_conf(hal, dev_config->clock_speed_hz, duty_cycle,
+    esp_err_t ret = spi_hal_cal_clock_conf(hal, dev_config->clock_speed_hz, duty_cycle,
                                         !(bus_attr->flags & SPICOMMON_BUSFLAG_IOMUX_PINS),
                                         dev_config->input_delay_ns, &freq,
                                         &temp_timing_conf);
@@ -663,32 +663,37 @@ static SPI_MASTER_ISR_ATTR esp_err_t check_trans_valid(spi_device_handle_t handl
     SPI_CHECK(handle!=NULL, "invalid dev handle", ESP_ERR_INVALID_ARG);
     spi_host_t *host = handle->host;
     const spi_bus_attr_t* bus_attr = host->bus_attr;
+    bool tx_enabled = (trans_desc->flags & SPI_TRANS_USE_TXDATA) || (trans_desc->tx_buffer);
+    bool rx_enabled = (trans_desc->flags & SPI_TRANS_USE_RXDATA) || (trans_desc->rx_buffer);
+    spi_transaction_ext_t *t_ext = (spi_transaction_ext_t *)trans_desc;
+    bool dummy_enabled = (((trans_desc->flags & SPI_TRANS_VARIABLE_DUMMY)? t_ext->dummy_bits: handle->cfg.dummy_bits) != 0);
+    bool extra_dummy_enabled = handle->timing_conf.timing_dummy;
+    bool is_half_duplex = ((handle->cfg.flags & SPI_DEVICE_HALFDUPLEX) != 0);
+
     //check transmission length
     SPI_CHECK((trans_desc->flags & SPI_TRANS_USE_RXDATA)==0 ||trans_desc->rxlength <= 32, "rxdata transfer > 32 bits without configured DMA", ESP_ERR_INVALID_ARG);
     SPI_CHECK((trans_desc->flags & SPI_TRANS_USE_TXDATA)==0 ||trans_desc->length <= 32, "txdata transfer > 32 bits without configured DMA", ESP_ERR_INVALID_ARG);
     SPI_CHECK(trans_desc->length <= bus_attr->max_transfer_sz*8, "txdata transfer > host maximum", ESP_ERR_INVALID_ARG);
     SPI_CHECK(trans_desc->rxlength <= bus_attr->max_transfer_sz*8, "rxdata transfer > host maximum", ESP_ERR_INVALID_ARG);
-    SPI_CHECK((handle->cfg.flags & SPI_DEVICE_HALFDUPLEX) || trans_desc->rxlength <= trans_desc->length, "rx length > tx length in full duplex mode", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(is_half_duplex || trans_desc->rxlength <= trans_desc->length, "rx length > tx length in full duplex mode", ESP_ERR_INVALID_ARG);
     //check working mode
     SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && (handle->cfg.flags & SPI_DEVICE_3WIRE)), "incompatible iface params", ESP_ERR_INVALID_ARG);
-    SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && (!(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX))), "incompatible iface params", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && !is_half_duplex), "incompatible iface params", ESP_ERR_INVALID_ARG);
 #ifdef CONFIG_IDF_TARGET_ESP32
-    SPI_CHECK( !(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX) || bus_attr->dma_chan == 0 || !(trans_desc->flags & SPI_TRANS_USE_RXDATA || trans_desc->rx_buffer != NULL)
-        || !(trans_desc->flags & SPI_TRANS_USE_TXDATA || trans_desc->tx_buffer!=NULL), "SPI half duplex mode does not support using DMA with both MOSI and MISO phases.", ESP_ERR_INVALID_ARG );
+    SPI_CHECK(!is_half_duplex || bus_attr->dma_chan == 0 || !rx_enabled || !tx_enabled, "SPI half duplex mode does not support using DMA with both MOSI and MISO phases.", ESP_ERR_INVALID_ARG );
 #endif
     //MOSI phase is skipped only when both tx_buffer and SPI_TRANS_USE_TXDATA are not set.
-    SPI_CHECK(trans_desc->length != 0 || (trans_desc->tx_buffer == NULL && !(trans_desc->flags & SPI_TRANS_USE_TXDATA)),
-        "trans tx_buffer should be NULL and SPI_TRANS_USE_TXDATA should be cleared to skip MOSI phase.", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(trans_desc->length != 0 || !tx_enabled, "trans tx_buffer should be NULL and SPI_TRANS_USE_TXDATA should be cleared to skip MOSI phase.", ESP_ERR_INVALID_ARG);
     //MISO phase is skipped only when both rx_buffer and SPI_TRANS_USE_RXDATA are not set.
     //If set rxlength=0 in full_duplex mode, it will be automatically set to length
-    SPI_CHECK(!(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX) || trans_desc->rxlength != 0 ||
-        (trans_desc->rx_buffer == NULL && ((trans_desc->flags & SPI_TRANS_USE_RXDATA)==0)),
-        "trans rx_buffer should be NULL and SPI_TRANS_USE_RXDATA should be cleared to skip MISO phase.", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(!is_half_duplex || trans_desc->rxlength != 0 || !rx_enabled, "trans rx_buffer should be NULL and SPI_TRANS_USE_RXDATA should be cleared to skip MISO phase.", ESP_ERR_INVALID_ARG);
     //In Full duplex mode, default rxlength to be the same as length, if not filled in.
     // set rxlength to length is ok, even when rx buffer=NULL
-    if (trans_desc->rxlength==0 && !(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX)) {
+    if (trans_desc->rxlength==0 && !is_half_duplex) {
         trans_desc->rxlength=trans_desc->length;
     }
+    //Dummy phase is not available when both data out and in are enabled, regardless of FD or HD mode.
+    SPI_CHECK(!tx_enabled || !rx_enabled || !dummy_enabled || !extra_dummy_enabled, "Dummy phase is not available when both data out and in are enabled", ESP_ERR_INVALID_ARG);
 
     return ESP_OK;
 }

components/soc/include/hal/spi_hal.h

@@ -169,7 +169,7 @@ void spi_hal_fetch_result(const spi_hal_context_t *hal);
  * Utils
  * ---------------------------------------------------------*/
 /**
- * Get the configuration of clock and timing. The configuration will be used when ``spi_hal_setup_device``.
+ * Calculate the configuration of clock and timing. The configuration will be used when ``spi_hal_setup_device``.
  *
  * It is highly suggested to do this at initialization, since it takes long time.
  *
@@ -185,7 +185,7 @@ void spi_hal_fetch_result(const spi_hal_context_t *hal);
  *
  * @return ESP_OK if desired is available, otherwise fail.
  */
-esp_err_t spi_hal_get_clock_conf(const spi_hal_context_t *hal, int speed_hz, int duty_cycle, bool use_gpio, int input_delay_ns, int *out_freq, spi_hal_timing_conf_t *timing_conf);
+esp_err_t spi_hal_cal_clock_conf(const spi_hal_context_t *hal, int speed_hz, int duty_cycle, bool use_gpio, int input_delay_ns, int *out_freq, spi_hal_timing_conf_t *timing_conf);
 
 /**
  * Get the frequency actual used.

components/soc/src/hal/spi_hal.c

@@ -49,7 +49,7 @@ void spi_hal_deinit(spi_hal_context_t *hal)
     }
 }
 
-esp_err_t spi_hal_get_clock_conf(const spi_hal_context_t *hal, int speed_hz, int duty_cycle, bool use_gpio, int input_delay_ns, int *out_freq, spi_hal_timing_conf_t *timing_conf)
+esp_err_t spi_hal_cal_clock_conf(const spi_hal_context_t *hal, int speed_hz, int duty_cycle, bool use_gpio, int input_delay_ns, int *out_freq, spi_hal_timing_conf_t *timing_conf)
 {
     spi_hal_timing_conf_t temp_conf;
 

docs/en/api-reference/peripherals/spi_master.rst

@@ -520,10 +520,11 @@ Known Issues
         This can prohibit you from transmitting and receiving data longer than 64 bytes.
         3. Try using the command and address fields to replace the write phase.
 
-    2. Full-duplex transactions are not compatible with the *dummy bit workaround*, hence the frequency is limited. See :ref:`dummy
-    bit speed-up workaround <dummy_bit_workaround>`.
+    2. Full-duplex transactions are not compatible with the *dummy bit workaround*, hence the frequency is limited. See :ref:`dummy bit speed-up workaround <dummy_bit_workaround>`.
 
-    3. ``cs_ena_pretrans`` is not compatible with the command and address phases of full-duplex transactions.
+    3. ``dummy_bits`` in :cpp:type:`spi_device_interface_config_t` and :cpp:type:`spi_transaction_ext_t` are not available when SPI read and write phases are both enabled (regardless of full duplex or half duplex mode). 
+
+    4. ``cs_ena_pretrans`` is not compatible with the command and address phases of full-duplex transactions.
 
 
 Application Example