Merge branch 'bugfix/spi_bus_lock_missing_semphrstatic' into 'master'

spi: fix config break and reduce overhead of the bus lock on SPI1

Closes IDFGH-3017

See merge request espressif/esp-idf!8221
This commit is contained in:
Michael (XIAO Xufeng) 2020-04-27 12:57:19 +08:00
commit 9d98111652
19 changed files with 271 additions and 72 deletions

View file

@ -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.
*

View file

@ -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;
}
}
#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

View file

@ -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;

View file

@ -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)

View file

@ -673,9 +673,9 @@ static IRAM_ATTR __attribute__((noinline)) bool iram_ringbuf_test(void)
{
bool result = true;
spi_flash_guard_get()->start(); // Disables flash cache
RingbufHandle_t handle = xRingbufferCreate(CONT_DATA_TEST_BUFF_LEN, RINGBUF_TYPE_NOSPLIT);
result = result && (handle != NULL);
spi_flash_guard_get()->start(); // Disables flash cache
xRingbufferGetMaxItemSize(handle);
vRingbufferDelete(handle);
spi_flash_guard_get()->end(); // Re-enables flash cache

View file

@ -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

View file

@ -2,4 +2,5 @@
archive: libfreertos.a
entries:
* (noflash_text)
queue:xQueueGenericCreateStatic (default)

View file

@ -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

View file

@ -94,6 +94,8 @@ To avoid reading flash cache accidentally, when one CPU initiates a flash write
If one CPU initiates a flash write or erase operation, the other CPU is put into a blocked state to avoid reading flash cache accidentally. All interrupts not safe for IRAM are disabled on both CPUs until the flash operation completes.
Please also see :ref:`esp_flash_os_func`, :ref:`spi_bus_lock`.
.. _iram-safe-interrupt-handlers:
IRAM-Safe Interrupt Handlers
@ -219,21 +221,28 @@ chip.
The chip driver relies on the host driver.
.. _esp_flash_os_func:
OS functions
^^^^^^^^^^^^
Currently the OS function layer provides a lock and a delay entries.
Currently the OS function layer provides entries of a lock and delay.
The lock is used to resolve the conflicts between the SPI chip access and
other functions. E.g. the cache (used for the code and PSRAM data fetch)
should be disabled when the flash chip on the SPI0/1 is being accessed. Also,
some devices which don't have CS wire, or the wire is controlled by the
software (e.g. SD card via SPI interface), requires the bus to be monopolized
during a period.
The lock (see :ref:`spi_bus_lock`) is used to resolve the conflicts among the access of devices
on the same SPI bus, and the SPI Flash chip access. E.g.
1. On SPI1 bus, the cache (used to fetch the data (code) in the Flash and PSRAM) should be
disabled when the flash chip on the SPI0/1 is being accessed.
2. On the other buses, the flash driver needs to disable the ISR registered by SPI Master driver,
to avoid conflicts.
3. Some devices of SPI Master driver may requires to use the bus monopolized during a period.
(especially when the device doesn't have CS wire, or the wire is controlled by the software
like SDSPI driver).
The delay is used by some long operations which requires the master to wait
or polling periodically.
The top API wraps these the chip driver and OS functions into an entire
component, and also provides some argument checking.

View file

@ -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

View file

@ -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

View file

@ -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)

View file

@ -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;
}

View file

@ -0,0 +1,39 @@
SPI Features
============
.. _spi_master_features:
SPI Master
----------
.. _spi_bus_lock:
SPI Bus Lock
^^^^^^^^^^^^
To realize the multiplexing of different devices from different drivers (SPI Master, SPI Flash,
etc.), an SPI bus lock is applied on each SPI bus. Drivers can attach their devices onto the bus
with the arbitration of the lock.
Each bus lock are initialized with a BG (background) service registered, all devices request to
do transactions on the bus should wait until the BG to be successfully disabled.
- For SPI1 bus, the BG is the cache, the bus lock will help to disable the cache before device
operations starts, and enable it again after device releasing the lock. No devices on SPI1 is
allowed using ISR (it's meaningless for the task to yield to other tasks when the cache is
disabled).
.. only:: esp32
There are quite a few limitations when using SPI Master driver on the SPI1 bus, see
:ref:`spi_master_on_spi1_bus`.
.. only:: esp32s2
The SPI Master driver hasn't supported SPI1 bus. Only SPI Flash driver can attach to the bus.
- For other buses, the driver may register its ISR as the BG. The bus lock will block a device
task when it requests for exclusive use of the bus, try to disable the ISR, and unblock the
device task allowed to exclusively use the bus when the ISR is successfully disabled. When the
task releases the lock, the lock will also try to resume the ISR if there are pending
transactions to be done in the ISR.

View file

@ -9,7 +9,17 @@ Overview of {IDF_TARGET_NAME}'s SPI peripherals
ESP32 integrates four SPI peripherals.
- SPI0 and SPI1 are used internally to access the ESP32's attached flash memory and thus are currently not open to users. They share one signal bus via an arbiter.
- SPI0 and SPI1 are used internally to access the ESP32's attached flash memory and share an arbiter.
.. only:: esp32
There are quite a few limitations when using SPI Master driver on the SPI1 bus, see
:ref:`spi_master_on_spi1_bus`.
.. only:: esp32s2
Currently SPI Master driver hasn't supported SPI1 bus.
- SPI2 and SPI3 are general purpose SPI controllers, sometimes referred to as HSPI and VSPI, respectively. They are open to users. SPI2 and SPI3 have independent signal buses with the same respective names. Each bus has three CS lines to drive up to three SPI slaves.
@ -44,7 +54,7 @@ The SPI master driver governs communications of Hosts with Devices. The driver s
- Multi-threaded environments
- Transparent handling of DMA transfers while reading and writing data
- Automatic time-division multiplexing of data coming from different Devices on the same signal bus
- Automatic time-division multiplexing of data coming from different Devices on the same signal bus, see :ref:`spi_bus_lock`.
.. warning::
@ -53,6 +63,10 @@ The SPI master driver governs communications of Hosts with Devices. The driver s
- Refactor your application so that each SPI peripheral is only accessed by a single task at a time.
- Add a mutex lock around the shared Device using :c:macro:`xSemaphoreCreateMutex`.
.. toctree::
:hidden:
SPI Features <spi_features>
SPI Transactions
----------------
@ -224,6 +238,69 @@ To have better control of the calling sequence of functions, send mixed transact
.. only:: esp32
.. _spi_master_on_spi1_bus:
Notes on Using the SPI Master driver on SPI1 Bus
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
.. note::
Though the :ref:`spi_bus_lock` feature makes it possible to use SPI Master driver on the SPI1
bus, it's still tricky and needs a lot of special treatment. It's a feature for advanced
developers.
To use SPI Master driver on SPI1 bus, you have to take care of two problems:
1. The code and data, required at the meanwhile the driver is operating SPI1 bus, should be
in the internal memory.
SPI1 bus is shared among devices and the cache for data (code) in the Flash as well as the
PSRAM. The cache should be disabled during the other drivers are operating the SPI1 bus.
Hence the data (code) in the flash as well as the PSRAM cannot be fetched at the meanwhile
the driver acquires the SPI1 bus by:
- Explicit bus acquiring between :cpp:func:`spi_device_acquire_bus` and
:cpp:func:`spi_device_release_bus`.
- Implicit bus acquiring between :cpp:func:`spi_device_polling_start` and
:cpp:func:`spi_device_polling_end` (or inside :cpp:func:`spi_device_polling_transmit`).
During the time above, all other tasks and most ISRs will be disabled (see
:ref:`iram-safe-interrupt-handlers`). Application code and data used by current task
should be placed in internal memory (DRAM or IRAM), or already in the ROM. Access to
external memory (flash code, const data in the flash, and static/heap data in the PSRAM)
will cause a `Cache disabled but cached memory region accessed` exception. For differences
between IRAM, DRAM, and flash cache, please refer to the :ref:`application memory layout
<memory-layout>` documentation.
To place functions into the IRAM, you can either:
1. Add `IRAM_ATTR` (include "esp_attr.h") to the function like:
IRAM_ATTR void foo(void) { }
Please note that when a function is inlined, it will follow its caller's segment, and
the attribute will not take effect. You may need to use `NOLINE_ATTR` to avoid this.
2. Use the `noflash` placement in the `linker.lf`. See more in
:doc:`../../api-guides/linker-script-generation`. Please note that, some code may be
transformed into lookup table in the const data by the compiler, so `noflash_text` is not
safe.
Please do take care that the optimization level may affect the compiler behavior of inline,
or transforming some code into lookup table in the const data, etc.
To place data into the DRAM, you can either:
1. Add `DRAM_ATTR` (include "esp_attr.h") to the data definition like:
DRAM_ATTR int g_foo = 3;
2. Use the `noflash` placement in the linker.lf. See more in
:doc:`../../api-guides/linker-script-generation`.
Please also see the example :example:`peripherals/spi_master/hd_eeprom`.
GPIO Matrix and IO_MUX
----------------------

View file

@ -0,0 +1 @@
.. include:: ../../../en/api-reference/peripherals/spi_features.rst

View file

@ -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.

View file

@ -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);

View file

@ -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