Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'fix/spi_callback_in_iram_v3.2' into 'release/v3.2'

Browse source 
spi: fix the crash when callbacks are not in the IRAM (Backports v3.2)

See merge request idf/esp-idf!3884
This commit is contained in:
Jiang Jiang Jian 2018-12-07 10:37:07 +08:00
commit bb47146710
7 changed files with 81 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
components/driver/Kconfig
View file

@ -41,8 +41,10 @@ config SPI_MASTER_ISR_IN_IRAM
bool "Place SPI master ISR function into IRAM"
default y
help
Place the SPI master ISR in to IRAM to avoid possibly cache miss, or
being disabled during flash writing access.
Place the SPI master ISR in to IRAM to avoid possible cache miss.
Also you can forbid the ISR being disabled during flash writing
access, by add ESP_INTR_FLAG_IRAM when initializing the driver.
config SPI_SLAVE_IN_IRAM
bool "Place transmitting functions of SPI slave into IRAM"
@ -61,8 +63,10 @@ config SPI_SLAVE_ISR_IN_IRAM
bool "Place SPI slave ISR function into IRAM"
default y
help
Place the SPI slave ISR in to IRAM to avoid possibly cache miss, or
being disabled during flash writing access.
Place the SPI slave ISR in to IRAM to avoid possible cache miss.
Also you can forbid the ISR being disabled during flash writing
access, by add ESP_INTR_FLAG_IRAM when initializing the driver.
endmenu # SPI Configuration

5
components/driver/include/driver/spi_common.h
View file

@ -87,6 +87,11 @@ typedef struct {
int quadhd_io_num; ///< GPIO pin for HD (HolD) signal which is used as D3 in 4-bit communication modes, or -1 if not used.
int max_transfer_sz; ///< Maximum transfer size, in bytes. Defaults to 4094 if 0.
uint32_t flags; ///< Abilities of bus to be checked by the driver. Or-ed value of ``SPICOMMON_BUSFLAG_*`` flags.
int intr_flags; /**< Interrupt flag for the bus to set the priority, and IRAM attribute, see
* ``esp_intr_alloc.h``. Note that the EDGE, INTRDISABLED attribute are ignored
* by the driver. Note that if ESP_INTR_FLAG_IRAM is set, ALL the callbacks of
* the driver, and their callee functions, should be put in the IRAM.
*/
} spi_bus_config_t;

22
components/driver/include/driver/spi_master.h
View file

@ -79,8 +79,26 @@ typedef struct {
int spics_io_num; ///< CS GPIO pin for this device, or -1 if not used
uint32_t flags; ///< Bitwise OR of SPI_DEVICE_* flags
int queue_size; ///< Transaction queue size. This sets how many transactions can be 'in the air' (queued using spi_device_queue_trans but not yet finished using spi_device_get_trans_result) at the same time
transaction_cb_t pre_cb; ///< Callback to be called before a transmission is started. This callback is called within interrupt context.
transaction_cb_t post_cb; ///< Callback to be called after a transmission has completed. This callback is called within interrupt context.
transaction_cb_t pre_cb; /**< Callback to be called before a transmission is started.
*
* This callback is called within interrupt
* context should be in IRAM for best
* performance, see "Transferring Speed"
* section in the SPI Master documentation for
* full details. If not, the callback may crash
* during flash operation when the driver is
* initialized with ESP_INTR_FLAG_IRAM.
*/
transaction_cb_t post_cb; /**< Callback to be called after a transmission has completed.
*
* This callback is called within interrupt
* context should be in IRAM for best
* performance, see "Transferring Speed"
* section in the SPI Master documentation for
* full details. If not, the callback may crash
* during flash operation when the driver is
* initialized with ESP_INTR_FLAG_IRAM.
*/
} spi_device_interface_config_t;

22
components/driver/include/driver/spi_slave.h
View file

@ -44,8 +44,26 @@ typedef struct {
uint32_t flags; ///< Bitwise OR of SPI_SLAVE_* flags
int queue_size; ///< Transaction queue size. This sets how many transactions can be 'in the air' (queued using spi_slave_queue_trans but not yet finished using spi_slave_get_trans_result) at the same time
uint8_t mode; ///< SPI mode (0-3)
slave_transaction_cb_t post_setup_cb; ///< Callback called after the SPI registers are loaded with new data
slave_transaction_cb_t post_trans_cb; ///< Callback called after a transaction is done
slave_transaction_cb_t post_setup_cb; /**< Callback called after the SPI registers are loaded with new data.
*
* This callback is called within interrupt
* context should be in IRAM for best
* performance, see "Transferring Speed"
* section in the SPI Master documentation for
* full details. If not, the callback may crash
* during flash operation when the driver is
* initialized with ESP_INTR_FLAG_IRAM.
*/
slave_transaction_cb_t post_trans_cb; /**< Callback called after a transaction is done.
*
* This callback is called within interrupt
* context should be in IRAM for best
* performance, see "Transferring Speed"
* section in the SPI Master documentation for
* full details. If not, the callback may crash
* during flash operation when the driver is
* initialized with ESP_INTR_FLAG_IRAM.
*/
} spi_slave_interface_config_t;
/**

9
components/driver/spi_master.c
View file

@ -233,6 +233,10 @@ esp_err_t spi_bus_initialize(spi_host_device_t host, const spi_bus_config_t *bus
SPI_CHECK(host>=SPI_HOST && host<=VSPI_HOST, "invalid host", ESP_ERR_INVALID_ARG);
SPI_CHECK( dma_chan >= 0 && dma_chan <= 2, "invalid dma channel", ESP_ERR_INVALID_ARG );
SPI_CHECK((bus_config->intr_flags & (ESP_INTR_FLAG_HIGH|ESP_INTR_FLAG_EDGE|ESP_INTR_FLAG_INTRDISABLED))==0, "intr flag not allowed", ESP_ERR_INVALID_ARG);
#ifndef CONFIG_SPI_MASTER_ISR_IN_IRAM
SPI_CHECK((bus_config->intr_flags & ESP_INTR_FLAG_IRAM)==0, "ESP_INTR_FLAG_IRAM should be disabled when CONFIG_SPI_MASTER_ISR_IN_IRAM is not set.", ESP_ERR_INVALID_ARG);
#endif
spi_chan_claimed=spicommon_periph_claim(host);
SPI_CHECK(spi_chan_claimed, "host already in use", ESP_ERR_INVALID_STATE);
@ -284,10 +288,7 @@ esp_err_t spi_bus_initialize(spi_host_device_t host, const spi_bus_config_t *bus
}
}
int flags = ESP_INTR_FLAG_INTRDISABLED;
#ifdef CONFIG_SPI_MASTER_ISR_IN_IRAM
flags |= ESP_INTR_FLAG_IRAM;
#endif
int flags = bus_config->intr_flags | ESP_INTR_FLAG_INTRDISABLED;
err = esp_intr_alloc(spicommon_irqsource_for_host(host), flags, spi_intr, (void*)spihost[host], &spihost[host]->intr);
if (err != ESP_OK) {
ret = err;

9
components/driver/spi_slave.c
View file

@ -109,6 +109,10 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b
//We only support HSPI/VSPI, period.
SPI_CHECK(VALID_HOST(host), "invalid host", ESP_ERR_INVALID_ARG);
SPI_CHECK( dma_chan >= 0 && dma_chan <= 2, "invalid dma channel", ESP_ERR_INVALID_ARG );
SPI_CHECK((bus_config->intr_flags & (ESP_INTR_FLAG_HIGH|ESP_INTR_FLAG_EDGE|ESP_INTR_FLAG_INTRDISABLED))==0, "intr flag not allowed", ESP_ERR_INVALID_ARG);
#ifndef CONFIG_SPI_SLAVE_ISR_IN_IRAM
SPI_CHECK((bus_config->intr_flags & ESP_INTR_FLAG_IRAM)==0, "ESP_INTR_FLAG_IRAM should be disabled when CONFIG_SPI_SLAVE_ISR_IN_IRAM is not set.", ESP_ERR_INVALID_ARG);
#endif
spi_chan_claimed=spicommon_periph_claim(host);
SPI_CHECK(spi_chan_claimed, "host already in use", ESP_ERR_INVALID_STATE);
@ -174,10 +178,7 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b
goto cleanup;
}
int flags = ESP_INTR_FLAG_INTRDISABLED;
#ifdef CONFIG_SPI_SLAVE_ISR_IN_IRAM
flags |= ESP_INTR_FLAG_IRAM;
#endif
int flags = bus_config->intr_flags | ESP_INTR_FLAG_INTRDISABLED;
err = esp_intr_alloc(spicommon_irqsource_for_host(host), flags, spi_intr, (void *)spihost[host], &spihost[host]->intr);
if (err != ESP_OK) {
ret = err;

19
docs/en/api-reference/peripherals/spi_master.rst
View file

@ -305,7 +305,8 @@ Speed and Timing Considerations
Transferring speed
^^^^^^^^^^^^^^^^^^
There're two factors limiting the transferring speed: (1) The transaction interval, (2) The SPI clock frequency used.
There're three factors limiting the transferring speed: (1) The transaction interval, (2) The SPI clock frequency used.
(3) The cache miss of SPI functions including callbacks.
When large transactions are used, the clock frequency determines the transferring speed; while the interval effects the
speed a lot if small transactions are used.
@ -343,6 +344,13 @@ speed a lot if small transactions are used.
2. SPI clock frequency: Each byte transferred takes 8 times of the clock period *8/fspi*. If the clock frequency is
too high, some functions may be limited to use. See :ref:`timing_considerations`.
3. The cache miss: the default config puts only the ISR into the IRAM.
Other SPI related functions including the driver itself and the callback
may suffer from the cache miss and wait for some time while reading code
from the flash. Select :ref:`CONFIG_SPI_MASTER_IN_IRAM` to put the whole
SPI driver into IRAM, and put the entire callback(s) and its callee
functions into IRAM to prevent this.
For an interrupt transaction, the overall cost is *20+8n/Fspi[MHz]* [us] for n bytes tranferred
in one transaction. Hence the transferring speed is : *n/(20+8n/Fspi)*. Example of transferring speed under 8MHz
clock speed:
@ -366,6 +374,15 @@ clock speed:
When the length of transaction is short, the cost of transaction interval is really high. Please try to squash data
into one transaction if possible to get higher transfer speed.
BTW, the ISR is disabled during flash operation by default. To keep sending
transactions during flash operations, enable
:ref:`CONFIG_SPI_MASTER_ISR_IN_IRAM` and set :cpp:class:`ESP_INTR_FLAG_IRAM`
in the ``intr_flags`` member of :cpp:class:`spi_bus_config_t`. Then all the
transactions queued before the flash operations will be handled by the ISR
continuously during flash operation. Note that the callback of each devices,
and their callee functions, should be in the IRAM in this case, or your
callback will crash due to cache miss.
.. _timing_considerations:
Timing considerations