Merge branch 'feat/spi_slave_pm_add_api' into 'master'

feat(spi_slave): add api for pm lock control

See merge request espressif/esp-idf!30766

Author: wanckl

components/driver/test_apps/components/test_driver_utils/test_spi_utils.c

@@ -252,6 +252,7 @@ void same_pin_func_sel(spi_bus_config_t bus, spi_device_interface_config_t dev,
 void spi_master_trans_impl_gpio(spi_bus_config_t bus, uint8_t cs_pin, uint8_t speed_hz, void *tx, void *rx, uint32_t len)
 {
     uint8_t *u8_tx = tx, *u8_rx = rx;
+    gpio_set_level(cs_pin, 1);  //ensure CS is inactive before transaction start
     esp_rom_gpio_connect_out_signal(cs_pin, SIG_GPIO_OUT_IDX, 0, 0);
     esp_rom_gpio_connect_out_signal(bus.sclk_io_num, SIG_GPIO_OUT_IDX, 0, 0);
     esp_rom_gpio_connect_out_signal(bus.mosi_io_num, SIG_GPIO_OUT_IDX, 0, 0);

components/esp_driver_spi/include/driver/spi_slave.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2010-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2010-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -80,7 +80,7 @@ struct spi_slave_transaction_t {
 };
 
 /**
- * @brief Initialize a SPI bus as a slave interface
+ * @brief Initialize a SPI bus as a slave interface and enable it by default
  *
  * @warning SPI0/1 is not supported
  *
@@ -119,6 +119,30 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b
  */
 esp_err_t spi_slave_free(spi_host_device_t host);
 
+/**
+ * @brief Enable the spi slave function for an initialized spi host
+ * @note No need to call this function additionally after `spi_slave_initialize`,
+ *       because it has been enabled already during the initialization.
+ *
+ * @param host SPI peripheral to be enabled
+ * @return
+ *         - ESP_OK                 On success
+ *         - ESP_ERR_INVALID_ARG    Unsupported host
+ *         - ESP_ERR_INVALID_STATE  Peripheral already enabled
+ */
+esp_err_t spi_slave_enable(spi_host_device_t host);
+
+/**
+ * @brief Disable the spi slave function for an initialized spi host
+ *
+ * @param host SPI peripheral to be disabled
+ * @return
+ *         - ESP_OK                 On success
+ *         - ESP_ERR_INVALID_ARG    Unsupported host
+ *         - ESP_ERR_INVALID_STATE  Peripheral already disabled
+ */
+esp_err_t spi_slave_disable(spi_host_device_t host);
+
 /**
  * @brief Queue a SPI transaction for execution
  *

components/esp_driver_spi/include/driver/spi_slave_hd.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2010-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2010-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -87,7 +87,7 @@ typedef struct {
 } spi_slave_hd_slot_config_t;
 
 /**
- * @brief Initialize the SPI Slave HD driver.
+ * @brief Initialize the SPI Slave HD driver and enable it by default.
  *
  * @param host_id       The host to use
  * @param bus_config    Bus configuration for the bus used
@@ -113,6 +113,30 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
  */
 esp_err_t spi_slave_hd_deinit(spi_host_device_t host_id);
 
+/**
+ * @brief Enable the spi slave HD function for an initialized spi host
+ * @note No need to call this function additionally after `spi_slave_hd_init`,
+ *       because it has been enabled already during the initialization.
+ *
+ * @param host_id SPI peripheral to be enabled
+ * @return
+ *         - ESP_OK                 On success
+ *         - ESP_ERR_INVALID_ARG    Unsupported host_id
+ *         - ESP_ERR_INVALID_STATE  Peripheral already enabled
+ */
+esp_err_t spi_slave_hd_enable(spi_host_device_t host_id);
+
+/**
+ * @brief Disable the spi slave HD function for an initialized spi host
+ *
+ * @param host_id SPI peripheral to be disabled
+ * @return
+ *         - ESP_OK                 On success
+ *         - ESP_ERR_INVALID_ARG    Unsupported host_id
+ *         - ESP_ERR_INVALID_STATE  Peripheral already disabled
+ */
+esp_err_t spi_slave_hd_disable(spi_host_device_t host_id);
+
 /**
  * @brief Queue transactions (segment mode)
  *

components/esp_driver_spi/include/esp_private/spi_common_internal.h

@@ -44,6 +44,12 @@ typedef dma_descriptor_align4_t spi_dma_desc_t;
 #define ADDR_CPU_2_DMA(addr)   (addr)
 #endif
 
+// Status of a spi bus
+typedef enum {
+    SPI_BUS_FSM_DISABLED,               ///< Bus is disabled, clock and power is allowed to be closed.
+    SPI_BUS_FSM_ENABLED,                ///< Bus is ready to be used
+} spi_bus_fsm_t;
+
 /// Attributes of an SPI bus
 typedef struct {
     spi_bus_config_t bus_cfg;           ///< Config used to initialize the bus

components/esp_driver_spi/src/gpspi/spi_slave.c

@@ -5,6 +5,7 @@
  */
 
 #include <string.h>
+#include <stdatomic.h>
 #include "esp_types.h"
 #include "esp_attr.h"
 #include "esp_check.h"
@@ -59,6 +60,7 @@ typedef struct {
 
 typedef struct {
     int id;
+    _Atomic spi_bus_fsm_t fsm;
     spi_bus_config_t bus_config;
     spi_dma_ctx_t   *dma_ctx;
     spi_slave_interface_config_t cfg;
@@ -177,6 +179,7 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b
     memcpy(&spihost[host]->cfg, slave_config, sizeof(spi_slave_interface_config_t));
     memcpy(&spihost[host]->bus_config, bus_config, sizeof(spi_bus_config_t));
     spihost[host]->id = host;
+    atomic_store(&spihost[host]->fsm, SPI_BUS_FSM_ENABLED);
     spi_slave_hal_context_t *hal = &spihost[host]->hal;
 
     spihost[host]->dma_enabled = (dma_chan != SPI_DMA_DISABLED);
@@ -224,8 +227,7 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b
     }
 
 #ifdef CONFIG_PM_ENABLE
-    err = esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, "spi_slave",
-                             &spihost[host]->pm_lock);
+    err = esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, "spi_slave", &spihost[host]->pm_lock);
     if (err != ESP_OK) {
         ret = err;
         goto cleanup;
@@ -351,6 +353,46 @@ esp_err_t spi_slave_free(spi_host_device_t host)
     return ESP_OK;
 }
 
+esp_err_t spi_slave_enable(spi_host_device_t host)
+{
+    SPI_CHECK(is_valid_host(host), "invalid host", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(spihost[host], "host not slave or not initialized", ESP_ERR_INVALID_ARG);
+    spi_bus_fsm_t curr_sta = SPI_BUS_FSM_DISABLED;
+    SPI_CHECK(atomic_compare_exchange_strong(&spihost[host]->fsm, &curr_sta, SPI_BUS_FSM_ENABLED), "host already enabled", ESP_ERR_INVALID_STATE);
+
+#ifdef CONFIG_PM_ENABLE
+    esp_pm_lock_acquire(spihost[host]->pm_lock);
+#endif //CONFIG_PM_ENABLE
+
+// If going to TOP_PD power down, the bus_clock is required during reg_dma, and will be disabled by sleep flow then
+#if !CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP
+    SPI_COMMON_RCC_CLOCK_ATOMIC() {
+        spi_ll_enable_bus_clock(host, true);
+    }
+#endif
+    return ESP_OK;
+}
+
+esp_err_t spi_slave_disable(spi_host_device_t host)
+{
+    SPI_CHECK(is_valid_host(host), "invalid host", ESP_ERR_INVALID_ARG);
+    SPI_CHECK(spihost[host], "host not slave or not initialized", ESP_ERR_INVALID_ARG);
+    spi_bus_fsm_t curr_sta = SPI_BUS_FSM_ENABLED;
+    SPI_CHECK(atomic_compare_exchange_strong(&spihost[host]->fsm, &curr_sta, SPI_BUS_FSM_DISABLED), "host already disabled", ESP_ERR_INVALID_STATE);
+
+#ifdef CONFIG_PM_ENABLE
+    esp_pm_lock_release(spihost[host]->pm_lock);
+#endif //CONFIG_PM_ENABLE
+
+// same as above
+#if !CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP
+    SPI_COMMON_RCC_CLOCK_ATOMIC() {
+        spi_ll_enable_bus_clock(host, false);
+    }
+#endif
+    return ESP_OK;
+}
+
 static void SPI_SLAVE_ISR_ATTR spi_slave_uninstall_priv_trans(spi_host_device_t host, spi_slave_trans_priv_t *priv_trans)
 {
 #if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE

components/esp_driver_spi/src/gpspi/spi_slave_hd.c

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include <stdatomic.h>
 #include "esp_compiler.h"
 #include "esp_log.h"
 #include "esp_check.h"
@@ -38,6 +39,7 @@ typedef struct {
 
 typedef struct {
     spi_host_device_t host_id;
+    _Atomic spi_bus_fsm_t fsm;
     spi_dma_ctx_t   *dma_ctx;
     uint16_t internal_mem_align_size;
     int max_transfer_sz;
@@ -117,6 +119,7 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
     host->host_id = host_id;
     host->int_spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
     host->append_mode = append_mode;
+    atomic_store(&host->fsm, SPI_BUS_FSM_ENABLED);
 
     ret = spicommon_dma_chan_alloc(host_id, config->dma_chan, &host->dma_ctx);
     if (ret != ESP_OK) {
@@ -178,7 +181,7 @@ esp_err_t spi_slave_hd_init(spi_host_device_t host_id, const spi_bus_config_t *b
     spi_slave_hd_hal_init(&host->hal, &hal_config);
 
 #ifdef CONFIG_PM_ENABLE
-    ret = esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, "spi_slave", &host->pm_lock);
+    ret = esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, "spi_slave_hd", &host->pm_lock);
     if (ret != ESP_OK) {
         goto cleanup;
     }
@@ -343,6 +346,46 @@ esp_err_t spi_slave_hd_deinit(spi_host_device_t host_id)
     return ESP_OK;
 }
 
+esp_err_t spi_slave_hd_enable(spi_host_device_t host_id)
+{
+    SPIHD_CHECK(VALID_HOST(host_id), "invalid host", ESP_ERR_INVALID_ARG);
+    SPIHD_CHECK(spihost[host_id], "host not slave or not initialized", ESP_ERR_INVALID_ARG);
+    spi_bus_fsm_t curr_sta = SPI_BUS_FSM_DISABLED;
+    SPIHD_CHECK(atomic_compare_exchange_strong(&spihost[host_id]->fsm, &curr_sta, SPI_BUS_FSM_ENABLED), "host already enabled", ESP_ERR_INVALID_STATE);
+
+#ifdef CONFIG_PM_ENABLE
+    esp_pm_lock_acquire(spihost[host_id]->pm_lock);
+#endif //CONFIG_PM_ENABLE
+
+// If going to TOP_PD power down, the bus_clock is required during reg_dma, and will be disabled by sleep flow then
+#if !CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP
+    SPI_COMMON_RCC_CLOCK_ATOMIC() {
+        spi_ll_enable_bus_clock(host_id, true);
+    }
+#endif
+    return ESP_OK;
+}
+
+esp_err_t spi_slave_hd_disable(spi_host_device_t host_id)
+{
+    SPIHD_CHECK(VALID_HOST(host_id), "invalid host", ESP_ERR_INVALID_ARG);
+    SPIHD_CHECK(spihost[host_id], "host not slave or not initialized", ESP_ERR_INVALID_ARG);
+    spi_bus_fsm_t curr_sta = SPI_BUS_FSM_ENABLED;
+    SPIHD_CHECK(atomic_compare_exchange_strong(&spihost[host_id]->fsm, &curr_sta, SPI_BUS_FSM_DISABLED), "host already disabled", ESP_ERR_INVALID_STATE);
+
+#ifdef CONFIG_PM_ENABLE
+    esp_pm_lock_release(spihost[host_id]->pm_lock);
+#endif //CONFIG_PM_ENABLE
+
+// same as above
+#if !CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP
+    SPI_COMMON_RCC_CLOCK_ATOMIC() {
+        spi_ll_enable_bus_clock(host_id, false);
+    }
+#endif
+    return ESP_OK;
+}
+
 static void tx_invoke(spi_slave_hd_slot_t *host)
 {
     portENTER_CRITICAL(&host->int_spinlock);

components/esp_driver_spi/test_apps/master/pytest_spi_master.py

@@ -6,7 +6,7 @@
 # If `test_env` is define, should not run on generic runner
 @pytest.mark.supported_targets
 @pytest.mark.generic
-@pytest.mark.parametrize('config', ['defaults', 'release', 'freertos_compliance', 'freertos_flash',], indirect=True)
+@pytest.mark.parametrize('config', ['release', 'freertos_compliance', 'freertos_flash',], indirect=True)
 def test_master_single_dev(case_tester) -> None:       # type: ignore
     for case in case_tester.test_menu:
         if 'test_env' in case.attributes:
@@ -17,7 +17,7 @@ def test_master_single_dev(case_tester) -> None:       # type: ignore
 # Job for test_env `external_flash` just for esp32 only
 @pytest.mark.esp32
 @pytest.mark.flash_multi
-@pytest.mark.parametrize('config', ['defaults',], indirect=True)
+@pytest.mark.parametrize('config', ['release',], indirect=True)
 def test_master_esp_flash(case_tester) -> None:        # type: ignore
     for case in case_tester.test_menu:
         # test case `spi_bus_lock_with_flash` use difference test env
@@ -33,7 +33,6 @@ def test_master_esp_flash(case_tester) -> None:        # type: ignore
 @pytest.mark.parametrize(
     'count, config',
     [
-        (2, 'defaults',),
         (2, 'release',),
         (2, 'freertos_compliance',),
         (2, 'freertos_flash',),

components/esp_driver_spi/test_apps/slave/main/test_spi_slave.c

@@ -725,6 +725,7 @@ static IRAM_ATTR void spi_queue_reset_in_isr(void)
 
     free(slave_isr_send);
     free(slave_isr_recv);
+    free(dummy_data);
     free(slave_isr_exp);
     spi_slave_free(TEST_SPI_HOST);
 }
@@ -733,7 +734,6 @@ TEST_CASE_MULTIPLE_DEVICES("SPI_Slave: Test_Queue_Reset_in_ISR", "[spi_ms]", tes
 #endif  // CONFIG_SPI_SLAVE_ISR_IN_IRAM
 
 #if (SOC_CPU_CORES_NUM > 1) && (!CONFIG_FREERTOS_UNICORE)
-
 #define TEST_ISR_CNT    100
 static void test_slave_isr_core_setup_cbk(spi_slave_transaction_t *curr_trans)
 {
@@ -782,7 +782,7 @@ TEST_CASE("test_slave_isr_pin_to_core", "[spi]")
 }
 #endif
 
-TEST_CASE("test spi slave sleep retention", "[spi]")
+TEST_CASE("test_spi_slave_sleep_retention", "[spi]")
 {
     // Prepare a TOP PD sleep
     TEST_ESP_OK(esp_sleep_enable_timer_wakeup(1 * 1000 * 1000));
@@ -811,12 +811,14 @@ TEST_CASE("test spi slave sleep retention", "[spi]")
 
         for (uint8_t cnt = 0; cnt < 3; cnt ++) {
             printf("Going into sleep with power %s ...\n", (buscfg.flags & SPICOMMON_BUSFLAG_SLP_ALLOW_PD) ? "down" : "hold");
+            TEST_ESP_OK(spi_slave_disable(TEST_SPI_HOST));
             TEST_ESP_OK(esp_light_sleep_start());
+            TEST_ESP_OK(spi_slave_enable(TEST_SPI_HOST));
             printf("Waked up!\n");
 
             // check if the sleep happened as expected
             TEST_ASSERT_EQUAL(0, sleep_ctx.sleep_request_result);
-#if SOC_SPI_SUPPORT_SLEEP_RETENTION
+#if SOC_SPI_SUPPORT_SLEEP_RETENTION && CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP
             // check if the power domain also is powered down
             TEST_ASSERT_EQUAL((buscfg.flags & SPICOMMON_BUSFLAG_SLP_ALLOW_PD) ? PMU_SLEEP_PD_TOP : 0, (sleep_ctx.sleep_flags) & PMU_SLEEP_PD_TOP);
 #endif

components/esp_driver_spi/test_apps/slave/pytest_spi_slave.py

@@ -6,7 +6,7 @@
 # If `test_env` is define, should not run on generic runner
 @pytest.mark.supported_targets
 @pytest.mark.generic
-@pytest.mark.parametrize('config', ['defaults',], indirect=True)
+@pytest.mark.parametrize('config', ['release', 'iram_safe'], indirect=True)
 def test_slave_single_dev(case_tester) -> None:       # type: ignore
     for case in case_tester.test_menu:
         if 'test_env' in case.attributes:
@@ -19,7 +19,7 @@ def test_slave_single_dev(case_tester) -> None:       # type: ignore
 @pytest.mark.temp_skip_ci(targets=['esp32c61'], reason='no multi-dev runner')
 @pytest.mark.supported_targets
 @pytest.mark.generic_multi_device
-@pytest.mark.parametrize('count, config', [(2, 'defaults'), (2, 'iram_safe')], indirect=True)
+@pytest.mark.parametrize('count, config', [(2, 'release'), (2, 'iram_safe')], indirect=True)
 def test_slave_multi_dev(case_tester) -> None:        # type: ignore
     for case in case_tester.test_menu:
         if case.attributes.get('test_env', 'generic_multi_device') == 'generic_multi_device':

components/esp_driver_spi/test_apps/slave/sdkconfig.ci.iram_safe

@@ -3,5 +3,6 @@ CONFIG_SPI_MASTER_ISR_IN_IRAM=n
 CONFIG_SPI_SLAVE_ISR_IN_IRAM=y
 CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y
 CONFIG_COMPILER_OPTIMIZATION_NONE=y
+CONFIG_ESP_IPC_TASK_STACK_SIZE=2048
 # silent the error check, as the error string are stored in rodata, causing RTL check failure
 CONFIG_COMPILER_OPTIMIZATION_CHECKS_SILENT=y