fix(driver_spi): master driver change esp32p4 default src to pll

Author: wanckl

components/driver/test_apps/components/test_driver_utils/include/test_spi_utils.h

@@ -89,6 +89,7 @@
 #define WIRE_DELAY              12.5
 
 #else
+#define UNCONNECTED_PIN         8
 #define GPIO_DELAY              0
 #define ESP_SPI_SLAVE_TV        0
 #define WIRE_DELAY              12.5

components/esp_driver_spi/src/gpspi/spi_common.c

@@ -515,6 +515,7 @@ static void bus_iomux_pins_set_quad(spi_host_device_t host, const spi_bus_config
 // check if the GPIO is already used by others
 static void s_spi_common_gpio_check_reserve(gpio_num_t gpio_num)
 {
+    assert(GPIO_IS_VALID_GPIO(gpio_num));  //coverity check
     uint64_t orig_occupied_map = esp_gpio_reserve(BIT64(gpio_num));
     if (orig_occupied_map & BIT64(gpio_num)) {
         ESP_LOGW(SPI_TAG, "GPIO %d is conflict with others and be overwritten", gpio_num);
@@ -523,6 +524,7 @@ static void s_spi_common_gpio_check_reserve(gpio_num_t gpio_num)
 
 static void s_spi_common_bus_via_gpio(gpio_num_t gpio_num, int in_sig, int out_sig, uint64_t *io_mask)
 {
+    assert(GPIO_IS_VALID_GPIO(gpio_num));  //coverity check
     if (in_sig != -1) {
         gpio_input_enable(gpio_num);
         esp_rom_gpio_connect_in_signal(gpio_num, in_sig, false);
@@ -807,6 +809,7 @@ esp_err_t spi_bus_initialize(spi_host_device_t host_id, const spi_bus_config_t *
     ESP_RETURN_ON_ERROR(spicommon_bus_alloc(host_id, "spi master"), SPI_TAG, "alloc host failed");
     spi_bus_attr_t *bus_attr = (spi_bus_attr_t *)spi_bus_get_attr(host_id);
     spicommon_bus_context_t *ctx = __containerof(bus_attr, spicommon_bus_context_t, bus_attr);
+    assert(bus_attr && ctx);  //coverity check
     bus_attr->bus_cfg = *bus_config;
 
     if (dma_chan != SPI_DMA_DISABLED) {

components/esp_driver_spi/src/gpspi/spi_master.c

@@ -157,6 +157,8 @@ We have two bits to control the interrupt:
 #define SPI_MASTER_PERI_CLOCK_ATOMIC()
 #endif
 
+#define SPI_PERIPH_SRC_FREQ_MAX     (80*1000*1000)    //peripheral hardware limitation for clock source into peripheral
+
 static const char *SPI_TAG = "spi_master";
 #define SPI_CHECK(a, str, ret_val, ...)  ESP_RETURN_ON_FALSE_ISR(a, ret_val, SPI_TAG, str, ##__VA_ARGS__)
 
@@ -395,6 +397,25 @@ int spi_get_freq_limit(bool gpio_is_used, int input_delay_ns)
 #endif
 }
 
+#if SPI_LL_SUPPORT_CLK_SRC_PRE_DIV
+static uint32_t s_spi_find_clock_src_pre_div(uint32_t src_freq, uint32_t target_freq)
+{
+    // pre division must be even and at least 2
+    uint32_t min_div = ((src_freq / SPI_PERIPH_SRC_FREQ_MAX) + 1) & (~0x01UL);
+    min_div = min_div < 2 ? 2 : min_div;
+
+    uint32_t total_div = src_freq / target_freq;
+    // Loop the `div` to find a divisible value of `total_div`
+    for (uint32_t pre_div = min_div; pre_div <= total_div; pre_div += 2) {
+        if ((total_div % pre_div) || (total_div / pre_div) > SPI_LL_PERIPH_CLK_DIV_MAX) {
+            continue;
+        }
+        return pre_div;
+    }
+    return min_div;
+}
+#endif //SPI_LL_SUPPORT_CLK_SRC_PRE_DIV
+
 /*
  Add a device. This allocates a CS line for the device, allocates memory for the device structure and hooks
  up the CS pin to whatever is specified.
@@ -416,34 +437,22 @@ esp_err_t spi_bus_add_device(spi_host_device_t host_id, const spi_device_interfa
     spi_host_t *host = bus_driver_ctx[host_id];
     const spi_bus_attr_t* bus_attr = host->bus_attr;
     SPI_CHECK(dev_config->spics_io_num < 0 || GPIO_IS_VALID_OUTPUT_GPIO(dev_config->spics_io_num), "spics pin invalid", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(dev_config->clock_speed_hz > 0, "invalid sclk speed", ESP_ERR_INVALID_ARG);
 #if SOC_SPI_SUPPORT_CLK_RC_FAST
     if (dev_config->clock_source == SPI_CLK_SRC_RC_FAST) {
         SPI_CHECK(periph_rtc_dig_clk8m_enable(), "the selected clock not available", ESP_ERR_INVALID_STATE);
     }
 #endif
-    spi_clock_source_t clk_src = dev_config->clock_source ? dev_config->clock_source : SPI_CLK_SRC_DEFAULT;
     uint32_t clock_source_hz = 0;
     uint32_t clock_source_div = 1;
+    spi_clock_source_t clk_src = dev_config->clock_source ? dev_config->clock_source : SPI_CLK_SRC_DEFAULT;
     SPI_CHECK(esp_clk_tree_enable_src(clk_src, true) == ESP_OK, "clock source enable failed", ESP_ERR_INVALID_STATE);
     esp_clk_tree_src_get_freq_hz(clk_src, ESP_CLK_TREE_SRC_FREQ_PRECISION_CACHED, &clock_source_hz);
 #if SPI_LL_SUPPORT_CLK_SRC_PRE_DIV
-    SPI_CHECK((dev_config->clock_speed_hz > 0) && (dev_config->clock_speed_hz <= MIN(clock_source_hz / 2, (80 * 1000000))), "invalid sclk speed", ESP_ERR_INVALID_ARG);
-
-    if (clock_source_hz / 2 > (80 * 1000000)) {    //clock_source_hz beyond peripheral HW limitation, calc pre-divider
-        hal_utils_clk_info_t clk_cfg = {
-            .src_freq_hz = clock_source_hz,
-            .exp_freq_hz = dev_config->clock_speed_hz * 2,  //we have (hs_clk = 2*mst_clk), calc hs_clk first
-            .round_opt = HAL_DIV_ROUND,
-            .min_integ = 1,
-            .max_integ = SPI_LL_CLK_SRC_PRE_DIV_MAX / 2,
-        };
-        hal_utils_calc_clk_div_integer(&clk_cfg, &clock_source_div);
-    }
-    clock_source_div *= 2; //convert to mst_clk function divider
-    clock_source_hz /= clock_source_div;    //actual freq enter to SPI peripheral
-#else
-    SPI_CHECK((dev_config->clock_speed_hz > 0) && (dev_config->clock_speed_hz <= clock_source_hz), "invalid sclk speed", ESP_ERR_INVALID_ARG);
+    clock_source_div = s_spi_find_clock_src_pre_div(clock_source_hz, dev_config->clock_speed_hz);
+    clock_source_hz /= clock_source_div; //actual freq enter to SPI peripheral
 #endif
+    SPI_CHECK(dev_config->clock_speed_hz <= clock_source_hz, "invalid sclk speed", ESP_ERR_INVALID_ARG);
 #ifdef CONFIG_IDF_TARGET_ESP32
     //The hardware looks like it would support this, but actually setting cs_ena_pretrans when transferring in full
     //duplex mode does absolutely nothing on the ESP32.

components/esp_driver_spi/test_apps/components/spi_bench_mark/include/spi_performance.h

@@ -68,7 +68,7 @@
 #define IDF_TARGET_MAX_TRANS_TIME_INTR_CPU          54
 
 #elif CONFIG_IDF_TARGET_ESP32P4
-#define IDF_TARGET_MAX_SPI_CLK_FREQ                 20*1000*1000
+#define IDF_TARGET_MAX_SPI_CLK_FREQ                 40*1000*1000
 #define IDF_TARGET_MAX_TRANS_TIME_INTR_DMA          44
 #define IDF_TARGET_MAX_TRANS_TIME_POLL_DMA          28
 #define IDF_TARGET_MAX_TRANS_TIME_INTR_CPU          26

components/esp_driver_spi/test_apps/master/main/test_spi_master.c

@@ -175,7 +175,7 @@ TEST_CASE("SPI Master clk_source and divider accuracy", "[spi]")
 #if !SOC_CLK_TREE_SUPPORTED
             time_tolerance *= 2;    //cpu is executing too slow before clock supported
 #endif
-            printf("real_freq %dk predict_cost %d real_cost_us %d diff %d tolerance %d us\n", real_freq_khz, trans_cost_us_predict, trans_cost, (trans_cost - trans_cost_us_predict), time_tolerance);
+            printf("exp_freq %dk real_freq %dk predict_cost %d real_cost_us %d diff %d tolerance %d us\n", devcfg.clock_speed_hz / 1000, real_freq_khz, trans_cost_us_predict, trans_cost, (trans_cost - trans_cost_us_predict), time_tolerance);
 
             TEST_ASSERT_LESS_THAN_UINT32(time_tolerance, abs(trans_cost - trans_cost_us_predict));
             TEST_ESP_OK(spi_bus_remove_device(handle));

components/esp_driver_spi/test_apps/param/main/test_spi_param.c

@@ -3,6 +3,7 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  */
+#include <sys/param.h>
 #include "esp_log.h"
 #include "esp_attr.h"
 #include "soc/spi_periph.h"
@@ -21,10 +22,6 @@
 #if (TEST_SPI_PERIPH_NUM >= 2)
 //These will only be enabled on chips with 2 or more SPI peripherals
 
-#ifndef MIN
-#define MIN(a, b)((a) > (b)? (b): (a))
-#endif
-
 /********************************************************************************
  *      Test By Internal Connections
  ********************************************************************************/
@@ -109,10 +106,6 @@ static void local_test_start(spi_device_handle_t *spi, int freq, const spitest_p
         devcfg.flags |= SPI_DEVICE_NO_DUMMY;
     }
 
-#if CONFIG_IDF_TARGET_ESP32P4      //TODO: IDF-8313, update P4 defaulte clock source
-    devcfg.clock_source = SPI_CLK_SRC_SPLL;
-#endif
-
     //slave config
     slvcfg.mode = pset->mode;
     slave_pull_up(&buscfg, slvcfg.spics_io_num);
@@ -660,20 +653,10 @@ TEST_CASE("Slave receive correct data", "[spi]")
     }
 }
 
-#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2, ESP32S3, ESP32C3, ESP32C2)
-//These tests are ESP32 only due to lack of runners
+#else // #if (TEST_SPI_PERIPH_NUM >= 2)
+
 /********************************************************************************
- *      Test By Master & Slave (2 boards)
- *
- *  Wiring:
- * | Master | Slave |
- * | ------ | ----- |
- * | 12     | 19    |
- * | 13     | 23    |
- * | 14     | 18    |
- * | 15     | 5     |
- * | GND    | GND   |
- *
+ *      Test By Master & Slave (2 boards) using burger runner
  ********************************************************************************/
 static void test_master_init(void **context);
 static void test_master_deinit(void *context);
@@ -732,7 +715,7 @@ static void test_master_start(spi_device_handle_t *spi, int freq, const spitest_
         buspset.quadhd_io_num = UNCONNECTED_PIN;
     }
     spi_device_interface_config_t devpset = SPI_DEVICE_TEST_DEFAULT_CONFIG();
-    devpset.spics_io_num = SPI2_IOMUX_PIN_NUM_CS;
+    devpset.spics_io_num = PIN_NUM_CS;
     devpset.mode = pset->mode;
     const int cs_pretrans_max = 15;
     if (pset->dup == HALF_DUPLEX_MISO) {
@@ -876,7 +859,7 @@ static void timing_slave_start(int speed, const spitest_param_set_t *pset, spite
         slv_buscfg.quadhd_io_num = UNCONNECTED_PIN;
     }
     spi_slave_interface_config_t slvcfg = SPI_SLAVE_TEST_DEFAULT_CONFIG();
-    slvcfg.spics_io_num = SPI2_IOMUX_PIN_NUM_CS;
+    slvcfg.spics_io_num = PIN_NUM_CS;
     slvcfg.mode = pset->mode;
     //Enable pull-ups on SPI lines so we don't detect rogue pulses when no master is connected.
     slave_pull_up(&slv_buscfg, slvcfg.spics_io_num);
@@ -1261,9 +1244,6 @@ spitest_param_set_t mode_conf[] = {
     },
 };
 TEST_SPI_MASTER_SLAVE(MODE, mode_conf, "")
-
-#endif // !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2, ESP32S3, ESP32C3, ESP32C2)
-
 #endif // #if (TEST_SPI_PERIPH_NUM >= 2)
 
 #define TEST_STEP_LEN         96
@@ -1273,9 +1253,7 @@ static int s_spi_bus_freq[] = {
     IDF_TARGET_MAX_SPI_CLK_FREQ / 7,
     IDF_TARGET_MAX_SPI_CLK_FREQ / 4,
     IDF_TARGET_MAX_SPI_CLK_FREQ / 2,
-#if !CONFIG_IDF_TARGET_ESP32P4      //TODO: IDF-8313, update P4 defaulte clock source
     IDF_TARGET_MAX_SPI_CLK_FREQ,
-#endif
 };
 
 //------------------------------------------- Full Duplex with DMA Freq test --------------------------------------

components/hal/esp32c5/include/hal/spi_ll.h

@@ -39,11 +39,11 @@ extern "C" {
 #define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
 #define SPI_LL_GET_HW(ID) (((ID)==1) ? &GPSPI2 : NULL)
 
-#define SPI_LL_DMA_MAX_BIT_LEN    (1 << 18)    //reg len: 18 bits
+#define SPI_LL_DMA_MAX_BIT_LEN    SPI_MS_DATA_BITLEN
 #define SPI_LL_CPU_MAX_BIT_LEN    (16 * 32)    //Fifo len: 16 words
 #define SPI_LL_MOSI_FREE_LEVEL    1            //Default level after bus initialized
 #define SPI_LL_SUPPORT_CLK_SRC_PRE_DIV      1  //clock source have divider before peripheral
-#define SPI_LL_CLK_SRC_PRE_DIV_MAX          256//div1(8bit)
+#define SPI_LL_PERIPH_CLK_DIV_MAX   ((SPI_CLKCNT_N + 1) * (SPI_CLKDIV_PRE + 1)) //peripheral internal maxmum clock divider
 
 /**
  * The data structure holding calculated clock configuration. Since the

components/hal/esp32c61/include/hal/spi_ll.h

@@ -39,11 +39,11 @@ extern "C" {
 #define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
 #define SPI_LL_GET_HW(ID) (((ID)==1) ? &GPSPI2 : NULL)
 
-#define SPI_LL_DMA_MAX_BIT_LEN    (1 << 18)    //reg len: 18 bits
+#define SPI_LL_DMA_MAX_BIT_LEN    SPI_MS_DATA_BITLEN
 #define SPI_LL_CPU_MAX_BIT_LEN    (16 * 32)    //Fifo len: 16 words
 #define SPI_LL_MOSI_FREE_LEVEL    1            //Default level after bus initialized
 #define SPI_LL_SUPPORT_CLK_SRC_PRE_DIV      1  //clock source have divider before peripheral
-#define SPI_LL_CLK_SRC_PRE_DIV_MAX          256//div1(8bit)
+#define SPI_LL_PERIPH_CLK_DIV_MAX   ((SPI_CLKCNT_N + 1) * (SPI_CLKDIV_PRE + 1)) //peripheral internal maxmum clock divider
 
 /**
  * The data structure holding calculated clock configuration. Since the

components/hal/esp32p4/include/hal/spi_ll.h

@@ -38,10 +38,10 @@ extern "C" {
 #define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
 #define SPI_LL_GET_HW(ID) (((ID)==1) ? &GPSPI2 : (((ID)==2) ? &GPSPI3 : NULL))
 
-#define SPI_LL_DMA_MAX_BIT_LEN    (1 << 18)    //reg len: 18 bits
+#define SPI_LL_DMA_MAX_BIT_LEN    SPI_MS_DATA_BITLEN
 #define SPI_LL_CPU_MAX_BIT_LEN    (16 * 32)    //Fifo len: 16 words
 #define SPI_LL_SUPPORT_CLK_SRC_PRE_DIV      1  //clock source have divider before peripheral
-#define SPI_LL_CLK_SRC_PRE_DIV_MAX          512//div1(8bit) * div2(8bit but set const 2)
+#define SPI_LL_PERIPH_CLK_DIV_MAX ((SPI_CLKCNT_N + 1) * (SPI_CLKDIV_PRE + 1)) //peripheral internal maxmum clock divider
 #define SPI_LL_MOSI_FREE_LEVEL    1            //Default level after bus initialized
 
 /**

components/soc/esp32p4/include/soc/clk_tree_defs.h

@@ -500,8 +500,7 @@ typedef enum {
     SPI_CLK_SRC_XTAL     = SOC_MOD_CLK_XTAL,        /*!< Select XTAL as SPI source clock */
     SPI_CLK_SRC_RC_FAST  = SOC_MOD_CLK_RC_FAST,     /*!< Select RC_FAST_20M as SPI source clock */
     SPI_CLK_SRC_SPLL     = SOC_MOD_CLK_SPLL,        /*!< Select SPLL as SPI source clock */
-    // TODO: IDF-8313, use PLL as default
-    SPI_CLK_SRC_DEFAULT  = SOC_MOD_CLK_XTAL,        /*!< Select XTAL as default source clock */
+    SPI_CLK_SRC_DEFAULT  = SOC_MOD_CLK_SPLL,        /*!< Select SPLL as default source clock */
 } soc_periph_spi_clk_src_t;
 
 /////////////////////////////////////////////////PSRAM////////////////////////////////////////////////////////////////////