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

2025-08-03 20:54:32 +02:00 · 2025-06-05 20:09:09 +08:00
parent 77402d2fa6
commit 5583e8d95e
9 changed files with 42 additions and 58 deletions
--- a/components/driver/test_apps/components/test_driver_utils/include/test_spi_utils.h
+++ b/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
--- a/components/esp_driver_spi/src/gpspi/spi_master.c
+++ b/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,36 +437,21 @@ 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 = SPI_CLK_SRC_DEFAULT;
    uint32_t clock_source_hz = 0;
    uint32_t clock_source_div = 1;
-    if (dev_config->clock_source) {
-        clk_src = dev_config->clock_source;
-    }
+    spi_clock_source_t clk_src = dev_config->clock_source ? dev_config->clock_source : SPI_CLK_SRC_DEFAULT;
    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.
--- a/components/esp_driver_spi/test_apps/components/spi_bench_mark/include/spi_performance.h
+++ b/components/esp_driver_spi/test_apps/components/spi_bench_mark/include/spi_performance.h
@@ -68,7 +68,7 @@
 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_NO_POLLING_NO_DMA         54

 #elif CONFIG_IDF_TARGET_ESP32P4
-#define IDF_PERFORMANCE_MAX_SPI_CLK_FREQ                            26*1000*1000
+#define IDF_PERFORMANCE_MAX_SPI_CLK_FREQ                            40*1000*1000
 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_NO_POLLING                44
 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_POLLING                   28
 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_NO_POLLING_NO_DMA         26
--- a/components/esp_driver_spi/test_apps/master/main/test_spi_master.c
+++ b/components/esp_driver_spi/test_apps/master/main/test_spi_master.c
@@ -121,7 +121,7 @@ TEST_CASE("SPI Master clockdiv calculation routines", "[spi]")

 // Test All clock source
 #define TEST_CLK_BYTE_LEN           10000
-#define TEST_TRANS_TIME_BIAS_RATIO  (float)2.0/100   // think 2% transfer time bias as acceptable
+#define TEST_TRANS_TIME_BIAS_RATIO  (float)5.0/100   // think 5% transfer time bias as acceptable
 TEST_CASE("SPI Master clk_source and divider accuracy", "[spi]")
 {
    int64_t start = 0, end = 0;
--- a/components/esp_driver_spi/test_apps/param/main/test_spi_param.c
+++ b/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_PERFORMANCE_MAX_SPI_CLK_FREQ / 7,
    IDF_PERFORMANCE_MAX_SPI_CLK_FREQ / 4,
    IDF_PERFORMANCE_MAX_SPI_CLK_FREQ / 2,
-#if !CONFIG_IDF_TARGET_ESP32P4      //TODO: IDF-8313, update P4 defaulte clock source
    IDF_PERFORMANCE_MAX_SPI_CLK_FREQ,
-#endif
 };

 //------------------------------------------- Full Duplex with DMA Freq test --------------------------------------
--- a/components/hal/esp32c5/include/hal/spi_ll.h
+++ b/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
--- a/components/hal/esp32c61/include/hal/spi_ll.h
+++ b/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
--- a/components/hal/esp32p4/include/hal/spi_ll.h
+++ b/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

 /**
--- a/components/soc/esp32p4/include/soc/clk_tree_defs.h
+++ b/components/soc/esp32p4/include/soc/clk_tree_defs.h
@@ -485,8 +485,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////////////////////////////////////////////////////////////////////