diff --git a/components/driver/include/driver/spi_common_internal.h b/components/driver/include/driver/spi_common_internal.h index 52fc786424..48b00166e6 100644 --- a/components/driver/include/driver/spi_common_internal.h +++ b/components/driver/include/driver/spi_common_internal.h @@ -767,12 +767,19 @@ bool spi_bus_lock_bg_req_exist(spi_bus_lock_handle_t lock); /******************************************************************************* * Variable and APIs for the OS to initialize the locks for the main chip ******************************************************************************/ -/// The lock for the main flash device -extern const spi_bus_lock_dev_handle_t g_spi_lock_main_flash_dev; - /// The lock for the main bus extern const spi_bus_lock_handle_t g_main_spi_bus_lock; +/** + * @brief Initialize the main SPI bus, called during chip startup. + * + * @return always ESP_OK + */ +esp_err_t spi_bus_lock_init_main_bus(void); + +/// The lock for the main flash device +extern const spi_bus_lock_dev_handle_t g_spi_lock_main_flash_dev; + /** * @brief Initialize the main flash device, called during chip startup. * diff --git a/components/driver/spi_bus_lock.c b/components/driver/spi_bus_lock.c index b50b1ec022..8c63c9b685 100644 --- a/components/driver/spi_bus_lock.c +++ b/components/driver/spi_bus_lock.c @@ -590,6 +590,7 @@ static int try_acquire_free_dev(spi_bus_lock_t *lock, bool cs_required) esp_err_t spi_bus_lock_register_dev(spi_bus_lock_handle_t lock, spi_bus_lock_dev_config_t *config, spi_bus_lock_dev_handle_t *out_dev_handle) { + if (lock == NULL) return ESP_ERR_INVALID_ARG; int id = try_acquire_free_dev(lock, config->flags & SPI_BUS_LOCK_DEV_FLAG_CS_REQUIRED); if (id == -1) return ESP_ERR_NOT_SUPPORTED; @@ -792,7 +793,7 @@ SPI_MASTER_ISR_ATTR bool spi_bus_lock_bg_req_exist(spi_bus_lock_t *lock) /******************************************************************************* * Static variables of the locks of the main flash ******************************************************************************/ -static StaticSemaphore_t main_flash_semphr; +#if CONFIG_SPI_FLASH_SHARE_SPI1_BUS static spi_bus_lock_dev_t lock_main_flash_dev; static spi_bus_lock_t main_spi_bus_lock = { @@ -806,23 +807,34 @@ static spi_bus_lock_t main_spi_bus_lock = { .new_req = 0, .periph_cs_num = SOC_SPI_PERIPH_CS_NUM(0), }; +const spi_bus_lock_handle_t g_main_spi_bus_lock = &main_spi_bus_lock; + +esp_err_t spi_bus_lock_init_main_bus(void) +{ + spi_bus_main_set_lock(g_main_spi_bus_lock); + return ESP_OK; +} + +static StaticSemaphore_t main_flash_semphr; static spi_bus_lock_dev_t lock_main_flash_dev = { .semphr = NULL, .parent = &main_spi_bus_lock, .mask = DEV_MASK(0), }; - -const spi_bus_lock_handle_t g_main_spi_bus_lock = &main_spi_bus_lock; const spi_bus_lock_dev_handle_t g_spi_lock_main_flash_dev = &lock_main_flash_dev; esp_err_t spi_bus_lock_init_main_dev(void) { - spi_bus_main_set_lock(g_main_spi_bus_lock); g_spi_lock_main_flash_dev->semphr = xSemaphoreCreateBinaryStatic(&main_flash_semphr); if (g_spi_lock_main_flash_dev->semphr == NULL) { return ESP_ERR_NO_MEM; } - return ESP_OK; -} \ No newline at end of file +} +#else //CONFIG_SPI_FLASH_SHARE_SPI1_BUS + +//when the dev lock is not initialized, point to NULL +const spi_bus_lock_dev_handle_t g_spi_lock_main_flash_dev = NULL; + +#endif diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 5b8c73b428..981d8b73de 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -204,6 +204,7 @@ static esp_err_t spi_master_init_driver(spi_host_device_t host_id) const spi_bus_attr_t* bus_attr = spi_bus_get_attr(host_id); SPI_CHECK(bus_attr != NULL, "host_id not initialized", ESP_ERR_INVALID_STATE); + SPI_CHECK(bus_attr->lock != NULL, "SPI Master cannot attach to bus. (Check CONFIG_SPI_FLASH_SHARE_SPI1_BUS)", ESP_ERR_INVALID_ARG); // spihost contains atomic variables, which should not be put in PSRAM spi_host_t* host = heap_caps_malloc(sizeof(spi_host_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); if (host == NULL) { @@ -307,7 +308,7 @@ esp_err_t spi_bus_add_device(spi_host_device_t host_id, const spi_device_interfa SPI_CHECK(is_valid_host(host_id), "invalid host", ESP_ERR_INVALID_ARG); if (bus_driver_ctx[host_id] == NULL) { - //lasy initialization the driver, get deinitialized by the bus is freed + //lazy initialization the driver, get deinitialized by the bus is freed err = spi_master_init_driver(host_id); if (err != ESP_OK) { return err; diff --git a/components/driver/test/test_spi_bus_lock.c b/components/driver/test/test_spi_bus_lock.c index acab33bcf7..0f13af1d9d 100644 --- a/components/driver/test/test_spi_bus_lock.c +++ b/components/driver/test/test_spi_bus_lock.c @@ -336,6 +336,9 @@ TEST_CASE("spi master can be used on SPI1", "[spi]") err = test_polling_send(handle); TEST_ESP_OK(err); test_acquire(handle); + + err = spi_bus_remove_device(handle); + TEST_ESP_OK(err); } #endif //!TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2) diff --git a/components/freertos/Kconfig b/components/freertos/Kconfig index e94690fd5c..a2cdb4867e 100644 --- a/components/freertos/Kconfig +++ b/components/freertos/Kconfig @@ -204,7 +204,7 @@ menu "FreeRTOS" config FREERTOS_SUPPORT_STATIC_ALLOCATION bool "Enable FreeRTOS static allocation API" - default y + default n help FreeRTOS gives the application writer the ability to instead provide the memory themselves, allowing the following objects to optionally be created without any diff --git a/components/spi_flash/Kconfig b/components/spi_flash/Kconfig index f72161172c..7135d07673 100644 --- a/components/spi_flash/Kconfig +++ b/components/spi_flash/Kconfig @@ -84,6 +84,22 @@ menu "SPI Flash driver" The implementation of SPI flash has been greatly changed in IDF v4.0. Enable this option to use the legacy implementation. + config SPI_FLASH_SHARE_SPI1_BUS + bool "Support other devices attached to SPI1 bus" + default n + # The bus lock on SPI1 is meaningless when the legacy implementation is used, or the SPI + # driver does not support SPI1. + depends on !SPI_FLASH_USE_LEGACY_IMPL && !IDF_TARGET_ESP32S2 + select FREERTOS_SUPPORT_STATIC_ALLOCATION + help + Each SPI bus needs a lock for arbitration among devices. This allows multiple + devices on a same bus, but may reduce the speed of esp_flash driver access to the + main flash chip. + + If you only need to use esp_flash driver to access the main flash chip, disable + this option, and the lock will be bypassed on SPI1 bus. Otherwise if extra devices + are needed to attach to SPI1 bus, enable this option. + menu "Auto-detect flash chips" config SPI_FLASH_SUPPORT_ISSI_CHIP diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 7073ab62e7..a5c04a47b7 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -687,7 +687,7 @@ esp_err_t esp_flash_app_disable_protect(bool disable) if (disable) { return esp_flash_app_disable_os_functions(esp_flash_default_chip); } else { - return esp_flash_app_init_os_functions(esp_flash_default_chip); + return esp_flash_app_enable_os_functions(esp_flash_default_chip); } } #endif diff --git a/components/spi_flash/esp_flash_spi_init.c b/components/spi_flash/esp_flash_spi_init.c index 9713a24ae7..d72a388662 100644 --- a/components/spi_flash/esp_flash_spi_init.c +++ b/components/spi_flash/esp_flash_spi_init.c @@ -147,6 +147,11 @@ esp_err_t spi_bus_add_flash_device(esp_flash_t **out_chip, const esp_flash_spi_d int dev_id; esp_err_t err = esp_flash_init_os_functions(chip, config->host_id, &dev_id); + if (err == ESP_ERR_NOT_SUPPORTED) { + ESP_LOGE(TAG, "Init os functions failed! No free CS."); + } else if (err == ESP_ERR_INVALID_ARG) { + ESP_LOGE(TAG, "Init os functions failed! Bus lock not initialized (check CONFIG_SPI_FLASH_SHARE_SPI1_BUS)."); + } if (err != ESP_OK) { ret = err; goto fail; @@ -241,7 +246,13 @@ esp_err_t esp_flash_init_default_chip(void) esp_err_t esp_flash_app_init(void) { - return esp_flash_app_init_os_functions(&default_chip); + esp_err_t err = ESP_OK; +#if CONFIG_SPI_FLASH_SHARE_SPI1_BUS + err = esp_flash_init_main_bus_lock(); + if (err != ESP_OK) return err; +#endif + err = esp_flash_app_enable_os_functions(&default_chip); + return err; } -#endif +#endif //!CONFIG_SPI_FLASH_USE_LEGACY_IMPL diff --git a/components/spi_flash/include/esp_flash_internal.h b/components/spi_flash/include/esp_flash_internal.h index 379e90eb63..1ef3ed1eaf 100644 --- a/components/spi_flash/include/esp_flash_internal.h +++ b/components/spi_flash/include/esp_flash_internal.h @@ -52,7 +52,7 @@ esp_err_t esp_flash_app_init(void); #endif /** - * Disable OS-level SPI flash protections in IDF + * Disable (or enable) OS-level SPI flash protections in IDF * * Called by the IDF internal code (e.g. coredump). You do not need to call this in your own applications. * @@ -85,6 +85,16 @@ esp_err_t esp_flash_init_os_functions(esp_flash_t *chip, int host_id, int *out_d */ esp_err_t esp_flash_deinit_os_functions(esp_flash_t* chip); +/** + * @brief Initialize the bus lock on the SPI1 bus. Should be called if drivers (including esp_flash) + * wants to use SPI1 bus. + * + * @note When using legacy spi flash API, the bus lock will not be available on SPI1 bus. + * + * @return esp_err_t always ESP_OK. + */ +esp_err_t esp_flash_init_main_bus_lock(void); + /** * Initialize OS-level functions for the main flash chip. * @@ -92,7 +102,7 @@ esp_err_t esp_flash_deinit_os_functions(esp_flash_t* chip); * * @return always ESP_OK */ -esp_err_t esp_flash_app_init_os_functions(esp_flash_t* chip); +esp_err_t esp_flash_app_enable_os_functions(esp_flash_t* chip); /** * Disable OS-level functions for the main flash chip during special phases (e.g. coredump) diff --git a/components/spi_flash/spi_flash_os_func_app.c b/components/spi_flash/spi_flash_os_func_app.c index cb1dab534b..07b8778aa5 100644 --- a/components/spi_flash/spi_flash_os_func_app.c +++ b/components/spi_flash/spi_flash_os_func_app.c @@ -18,7 +18,7 @@ #include "esp_flash.h" #include "esp_flash_partitions.h" #include "hal/spi_types.h" - +#include "sdkconfig.h" #if CONFIG_IDF_TARGET_ESP32 #include "esp32/rom/ets_sys.h" @@ -45,38 +45,56 @@ typedef struct { bool no_protect; //to decide whether to check protected region (for the main chip) or not. } spi1_app_func_arg_t; - -// in the future we will have arbitration among devices, including flash on the same flash bus -static IRAM_ATTR esp_err_t spi_bus_acquire(spi_bus_lock_dev_handle_t dev_lock) +IRAM_ATTR static void cache_enable(void* arg) { - // was in BG operation (cache). Disable it and schedule + g_flash_guard_default_ops.end(); +} + +IRAM_ATTR static void cache_disable(void* arg) +{ + g_flash_guard_default_ops.start(); +} + +static IRAM_ATTR esp_err_t spi_start(void *arg) +{ + spi_bus_lock_dev_handle_t dev_lock = ((app_func_arg_t *)arg)->dev_lock; + + // wait for other devices (or cache) to finish their operation esp_err_t ret = spi_bus_lock_acquire_start(dev_lock, portMAX_DELAY); if (ret != ESP_OK) { return ret; } - return ESP_OK; -} - -static IRAM_ATTR esp_err_t spi_bus_release(spi_bus_lock_dev_handle_t dev_lock) -{ - return spi_bus_lock_acquire_end(dev_lock); -} - -//for SPI1, we have to disable the cache and interrupts before using the SPI bus -static IRAM_ATTR esp_err_t spi_start(void *arg) -{ - spi_bus_lock_dev_handle_t dev_lock = ((app_func_arg_t *)arg)->dev_lock; - spi_bus_acquire(dev_lock); spi_bus_lock_touch(dev_lock); return ESP_OK; } static IRAM_ATTR esp_err_t spi_end(void *arg) { - spi_bus_release(((app_func_arg_t *)arg)->dev_lock); + return spi_bus_lock_acquire_end(((app_func_arg_t *)arg)->dev_lock); +} + +static IRAM_ATTR esp_err_t spi1_start(void *arg) +{ +#if CONFIG_SPI_FLASH_SHARE_SPI1_BUS + //use the lock to disable the cache and interrupts before using the SPI bus + return spi_start(arg); +#else + //directly disable the cache and interrupts when lock is not used + cache_disable(NULL); +#endif return ESP_OK; } +static IRAM_ATTR esp_err_t spi1_end(void *arg) +{ +#if CONFIG_SPI_FLASH_SHARE_SPI1_BUS + return spi_end(arg); +#else + cache_enable(NULL); + return ESP_OK; +#endif +} + static IRAM_ATTR esp_err_t delay_ms(void *arg, unsigned ms) { ets_delay_us(1000 * ms); @@ -96,14 +114,14 @@ static IRAM_ATTR esp_err_t main_flash_region_protected(void* arg, size_t start_a static DRAM_ATTR spi1_app_func_arg_t main_flash_arg = {}; //for SPI1, we have to disable the cache and interrupts before using the SPI bus -const DRAM_ATTR esp_flash_os_functions_t esp_flash_spi1_default_os_functions = { - .start = spi_start, - .end = spi_end, +static const DRAM_ATTR esp_flash_os_functions_t esp_flash_spi1_default_os_functions = { + .start = spi1_start, + .end = spi1_end, .delay_ms = delay_ms, .region_protected = main_flash_region_protected, }; -const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = { +static const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = { .start = spi_start, .end = spi_end, .delay_ms = delay_ms, @@ -164,36 +182,27 @@ esp_err_t esp_flash_deinit_os_functions(esp_flash_t* chip) return ESP_OK; } -IRAM_ATTR static void cache_enable(void* arg) +esp_err_t esp_flash_init_main_bus_lock(void) { - g_flash_guard_default_ops.end(); -} + spi_bus_lock_init_main_bus(); + spi_bus_lock_set_bg_control(g_main_spi_bus_lock, cache_enable, cache_disable, NULL); -IRAM_ATTR static void cache_disable(void* arg) -{ - g_flash_guard_default_ops.start(); -} - -esp_err_t esp_flash_app_init_os_functions(esp_flash_t* chip) -{ esp_err_t err = spi_bus_lock_init_main_dev(); if (err != ESP_OK) { return err; } + return ESP_OK; +} - spi_bus_lock_set_bg_control(g_main_spi_bus_lock, - cache_enable, cache_disable, NULL); - - chip->os_func = &esp_flash_spi1_default_os_functions; - chip->os_func_data = &main_flash_arg; +esp_err_t esp_flash_app_enable_os_functions(esp_flash_t* chip) +{ main_flash_arg = (spi1_app_func_arg_t) { .common_arg = { .dev_lock = g_spi_lock_main_flash_dev, //for SPI1, }, .no_protect = false, }; + chip->os_func = &esp_flash_spi1_default_os_functions; + chip->os_func_data = &main_flash_arg; return ESP_OK; } - - - diff --git a/examples/peripherals/spi_master/hd_eeprom/main/Kconfig.projbuild b/examples/peripherals/spi_master/hd_eeprom/main/Kconfig.projbuild index 3dcb4a7017..e48344e2fc 100644 --- a/examples/peripherals/spi_master/hd_eeprom/main/Kconfig.projbuild +++ b/examples/peripherals/spi_master/hd_eeprom/main/Kconfig.projbuild @@ -1,9 +1,9 @@ menu "Example Configuration" config EXAMPLE_USE_SPI1_PINS - bool "The example runs on SPI1 pins or some other pins" + bool "The example uses SPI1 shared pins for EEPROM connection" default n - depends on IDF_TARGET_ESP32 + depends on SPI_FLASH_SHARE_SPI1_BUS help Enable this option will make the EEPROM use SPI1 pins, which is shared with the main flash chip. diff --git a/examples/peripherals/spi_master/hd_eeprom/main/spi_eeprom_main.c b/examples/peripherals/spi_master/hd_eeprom/main/spi_eeprom_main.c index 3130c27c39..06e5282dfd 100644 --- a/examples/peripherals/spi_master/hd_eeprom/main/spi_eeprom_main.c +++ b/examples/peripherals/spi_master/hd_eeprom/main/spi_eeprom_main.c @@ -85,9 +85,11 @@ void app_main(void) eeprom_handle_t eeprom_handle; ESP_LOGI(TAG, "Initializing device..."); - spi_eeprom_init(&eeprom_config, &eeprom_handle); + ret = spi_eeprom_init(&eeprom_config, &eeprom_handle); + ESP_ERROR_CHECK(ret); - spi_eeprom_write_enable(eeprom_handle); + ret = spi_eeprom_write_enable(eeprom_handle); + ESP_ERROR_CHECK(ret); const char test_str[] = "Hello World!"; ESP_LOGI(TAG, "Write: %s", test_str); diff --git a/tools/unit-test-app/sdkconfig.defaults.esp32 b/tools/unit-test-app/sdkconfig.defaults.esp32 index 1087b70b0b..423406bb2d 100644 --- a/tools/unit-test-app/sdkconfig.defaults.esp32 +++ b/tools/unit-test-app/sdkconfig.defaults.esp32 @@ -1,3 +1,4 @@ CONFIG_ESP32_DEFAULT_CPU_FREQ_240=y CONFIG_ESP32_XTAL_FREQ_AUTO=y CONFIG_ESP32_ULP_COPROC_ENABLED=y +CONFIG_SPI_FLASH_SHARE_SPI1_BUS=y