From 179bba918998417fe60779fb8953278316fb9d60 Mon Sep 17 00:00:00 2001 From: Konstantin Kochin Date: Sat, 14 Aug 2021 12:57:10 +0300 Subject: [PATCH 1/4] Move common STM32 SPI operations to separate functions - move a code that waits readable SPI state from `spi_master_write` function to inline functions `msp_writable` and `msp_wait_writable` - move a code that waits writeable SPI state from `spi_master_write` function to inline functions `msp_readable` and `msp_wait_readable` - move a code that writes data to SPI from `spi_master_write` function to inline function `msp_write_data` - move a code that reads data from SPI from `spi_master_write` function to inline function `msp_read_data` --- targets/TARGET_STM/stm_spi_api.c | 119 ++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/targets/TARGET_STM/stm_spi_api.c b/targets/TARGET_STM/stm_spi_api.c index 3c10bdaf3d..4320810f9e 100644 --- a/targets/TARGET_STM/stm_spi_api.c +++ b/targets/TARGET_STM/stm_spi_api.c @@ -777,6 +777,84 @@ static inline int datasize_to_transfer_bitshift(uint32_t DataSize) } } +/** + * Check if SPI master interface is writable. + * + * @param obj + * @return 0 - SPI isn't writable, non-zero - SPI is writable + */ +static inline int msp_writable(spi_t *obj) +{ +#if TARGET_STM32H7 + return (int)LL_SPI_IsActiveFlag_TXP(SPI_INST(obj)); +#else /* TARGET_STM32H7 */ + return (int)LL_SPI_IsActiveFlag_TXE(SPI_INST(obj)); +#endif /* TARGET_STM32H7 */ +} + +/** + * Check if SPI master interface is readable. + * + * @param obj + * @return 0 - SPI isn't readable, non-zero - SPI is readable + */ +static inline int msp_readable(spi_t *obj) +{ +#if TARGET_STM32H7 + return (int)LL_SPI_IsActiveFlag_RXP(SPI_INST(obj)); +#else /* TARGET_STM32H7 */ + return (int)LL_SPI_IsActiveFlag_RXNE(SPI_INST(obj)); +#endif /* TARGET_STM32H7 */ +} + +/** + * Wait till SPI master interface is writable. + */ +static inline void msp_wait_writable(spi_t *obj) +{ + while (!msp_writable(obj)); +} + +/** + * Wait till SPI master interface is readable. + */ +static inline void msp_wait_readable(spi_t *obj) +{ + while (!msp_readable(obj)); +} + +/** + * Write data to SPI master interface. + */ +static inline void msp_write_data(spi_t *obj, int value, int bitshift) +{ + if (bitshift == 1) { + LL_SPI_TransmitData16(SPI_INST(obj), (uint16_t)value); +#ifdef HAS_32BIT_SPI_TRANSFERS + } else if (bitshift == 2) { + LL_SPI_TransmitData32(SPI_INST(obj), (uint32_t)value); +#endif /* HAS_32BIT_SPI_TRANSFERS */ + } else { + LL_SPI_TransmitData8(SPI_INST(obj), (uint8_t)value); + } +} + +/** + * Read data from SPI master interface. + */ +static inline int msp_read_data(spi_t *obj, int bitshift) +{ + if (bitshift == 1) { + return LL_SPI_ReceiveData16(SPI_INST(obj)); +#ifdef HAS_32BIT_SPI_TRANSFERS + } else if (bitshift == 2) { + return LL_SPI_ReceiveData32(SPI_INST(obj)); +#endif /* HAS_32BIT_SPI_TRANSFERS */ + } else { + return LL_SPI_ReceiveData8(SPI_INST(obj)); + } +} + int spi_master_write(spi_t *obj, int value) { struct spi_s *spiobj = SPI_S(obj); @@ -806,44 +884,15 @@ int spi_master_write(spi_t *obj, int value) #if TARGET_STM32H7 /* Master transfer start */ LL_SPI_StartMasterTransfer(SPI_INST(obj)); - - /* Wait TXP flag to transmit data */ - while (!LL_SPI_IsActiveFlag_TXP(SPI_INST(obj))); -#else - /* Wait TXE flag to transmit data */ - while (!LL_SPI_IsActiveFlag_TXE(SPI_INST(obj))); - -#endif /* TARGET_STM32H7 */ +#endif /* Transmit data */ - if (bitshift == 1) { - LL_SPI_TransmitData16(SPI_INST(obj), (uint16_t)value); -#ifdef HAS_32BIT_SPI_TRANSFERS - } else if (bitshift == 2) { - LL_SPI_TransmitData32(SPI_INST(obj), (uint32_t)value); -#endif - } else { - LL_SPI_TransmitData8(SPI_INST(obj), (uint8_t)value); - } + msp_wait_writable(obj); + msp_write_data(obj, value, bitshift); -#if TARGET_STM32H7 - /* Wait for RXP or end of Transfer */ - while (!LL_SPI_IsActiveFlag_RXP(SPI_INST(obj))); -#else /* TARGET_STM32H7 */ - /* Wait for RXNE flag before reading */ - while (!LL_SPI_IsActiveFlag_RXNE(SPI_INST(obj))); -#endif /* TARGET_STM32H7 */ - - /* Read received data */ - if (bitshift == 1) { - return LL_SPI_ReceiveData16(SPI_INST(obj)); -#ifdef HAS_32BIT_SPI_TRANSFERS - } else if (bitshift == 2) { - return LL_SPI_ReceiveData32(SPI_INST(obj)); -#endif - } else { - return LL_SPI_ReceiveData8(SPI_INST(obj)); - } + /* Receive data */ + msp_wait_readable(obj); + return msp_read_data(obj, bitshift); } int spi_master_block_write(spi_t *obj, const char *tx_buffer, int tx_length, From f1c4a7fe525edfc1edac994618174a8fd69420a0 Mon Sep 17 00:00:00 2001 From: Konstantin Kochin Date: Sat, 14 Aug 2021 18:45:15 +0300 Subject: [PATCH 2/4] Fix STM32 SPI 3-wire (synchronous API) All STM32 families except STM32H7 has the following 3-wire SPI peculiarity in master receive mode: SPI continuously generates clock signal till it's disabled by a software. It causes that a software must disable SPI in time. Otherwise, "dummy" reads will be generated. Current STM32 synchronous SPI 3-wire implementation relies on HAL library functions HAL_SPI_Receive/HAL_SPI_Transmit. It performs some SPI state checks to detect errors, but unfortunately it isn't fast enough to disable SPI in time. Additionally, a multithreading environment or interrupt events may cause extra delays. This commit contains the custom transmit/receive function for SPI 3-wire mode. It uses critical sections to prevents accidental interrupt event delays, disables SPI after each frame receiving and disables SPI during frame generation. It adds some delay between SPI frames (~700 ns), but gives reliable 3-wire SPI communications. --- targets/TARGET_STM/stm_spi_api.c | 201 +++++++++++++++++++++++++++++-- 1 file changed, 188 insertions(+), 13 deletions(-) diff --git a/targets/TARGET_STM/stm_spi_api.c b/targets/TARGET_STM/stm_spi_api.c index 4320810f9e..a72d61102c 100644 --- a/targets/TARGET_STM/stm_spi_api.c +++ b/targets/TARGET_STM/stm_spi_api.c @@ -30,6 +30,8 @@ #include "mbed_assert.h" #include "mbed_error.h" #include "mbed_debug.h" +#include "mbed_critical.h" +#include "mbed_wait_api.h" #include "spi_api.h" #if DEVICE_SPI @@ -602,6 +604,50 @@ static const uint32_t baudrate_prescaler_table[] = {SPI_BAUDRATEPRESCALER_2, SPI_BAUDRATEPRESCALER_256 }; +/** + * Convert SPI_BAUDRATEPRESCALER_ constant into numeric prescaler rank. + */ +static uint8_t spi_get_baudrate_prescaler_rank(uint32_t value) +{ + switch (value) { + case SPI_BAUDRATEPRESCALER_2: + return 0; + case SPI_BAUDRATEPRESCALER_4: + return 1; + case SPI_BAUDRATEPRESCALER_8: + return 2; + case SPI_BAUDRATEPRESCALER_16: + return 3; + case SPI_BAUDRATEPRESCALER_32: + return 4; + case SPI_BAUDRATEPRESCALER_64: + return 5; + case SPI_BAUDRATEPRESCALER_128: + return 6; + case SPI_BAUDRATEPRESCALER_256: + return 7; + default: + return 0xFF; + } +} + +/** + * Get actual SPI baudrate. + * + * It may differ from a value that is passed to the ::spi_frequency function. + */ +int spi_get_baudrate(spi_t *obj) +{ + struct spi_s *spiobj = SPI_S(obj); + SPI_HandleTypeDef *handle = &(spiobj->handle); + + int freq = spi_get_clock_freq(obj); + uint8_t baudrate_rank = spi_get_baudrate_prescaler_rank(handle->Init.BaudRatePrescaler); + MBED_ASSERT(baudrate_rank != 0xFF); + return freq >> (baudrate_rank + 1); +} + + void spi_frequency(spi_t *obj, int hz) { struct spi_s *spiobj = SPI_S(obj); @@ -823,6 +869,29 @@ static inline void msp_wait_readable(spi_t *obj) while (!msp_readable(obj)); } +/** + * Check if SPI master interface is busy. + * + * @param obj + * @return 0 - SPI isn't busy, non-zero - SPI is busy + */ +static inline int msp_busy(spi_t *obj) +{ +#if TARGET_STM32H7 + return !(int)LL_SPI_IsActiveFlag_TXC(SPI_INST(obj)); +#else /* TARGET_STM32H7 */ + return (int)LL_SPI_IsActiveFlag_BSY(SPI_INST(obj)); +#endif /* TARGET_STM32H7 */ +} + +/** + * Wait till SPI master interface isn't busy. + */ +static inline void msp_wait_not_busy(spi_t *obj) +{ + while (msp_busy(obj)); +} + /** * Write data to SPI master interface. */ @@ -855,13 +924,126 @@ static inline int msp_read_data(spi_t *obj, int bitshift) } } +/** + * Transmit and receive SPI data in bidirectional mode. + * + * @param obj spi object + * @param tx_buffer byte-array of data to write to the device + * @param tx_length number of bytes to write, may be zero + * @param rx_buffer byte-array of data to read from the device + * @param rx_length number of bytes to read, may be zero + * @return number of transmitted and received bytes or negative code in case of error. + */ +static int spi_master_one_wire_transfer(spi_t *obj, const char *tx_buffer, int tx_length, + char *rx_buffer, int rx_length) +{ + struct spi_s *spiobj = SPI_S(obj); + SPI_HandleTypeDef *handle = &(spiobj->handle); + const int bitshift = datasize_to_transfer_bitshift(handle->Init.DataSize); + MBED_ASSERT(bitshift >= 0); + + /* Ensure that spi is disabled */ + LL_SPI_Disable(SPI_INST(obj)); + + /* Transmit data */ + if (tx_length) { + LL_SPI_SetTransferDirection(SPI_INST(obj), LL_SPI_HALF_DUPLEX_TX); +#if TARGET_STM32H7 + /* Set transaction size */ + LL_SPI_SetTransferSize(SPI_INST(obj), tx_length); +#endif /* TARGET_STM32H7 */ + LL_SPI_Enable(SPI_INST(obj)); +#if TARGET_STM32H7 + /* Master transfer start */ + LL_SPI_StartMasterTransfer(SPI_INST(obj)); +#endif /* TARGET_STM32H7 */ + + for (int i = 0; i < tx_length; i++) { + msp_wait_writable(obj); + msp_write_data(obj, tx_buffer[i], bitshift); + } + + /* Wait end of transaction */ + msp_wait_not_busy(obj); + + LL_SPI_Disable(SPI_INST(obj)); + +#if TARGET_STM32H7 + /* Clear transaction flags */ + LL_SPI_ClearFlag_EOT(SPI_INST(obj)); + LL_SPI_ClearFlag_TXTF(SPI_INST(obj)); + /* Reset transaction size */ + LL_SPI_SetTransferSize(SPI_INST(obj), 0); +#endif /* TARGET_STM32H7 */ + } + + /* Receive data */ + if (rx_length) { + LL_SPI_SetTransferDirection(SPI_INST(obj), LL_SPI_HALF_DUPLEX_RX); +#if TARGET_STM32H7 + /* Set transaction size and run SPI */ + LL_SPI_SetTransferSize(SPI_INST(obj), rx_length); + LL_SPI_Enable(SPI_INST(obj)); + LL_SPI_StartMasterTransfer(SPI_INST(obj)); + + /* Receive data */ + for (int i = 0; i < rx_length; i++) { + msp_wait_readable(obj); + rx_buffer[i] = msp_read_data(obj, bitshift); + } + + /* Stop SPI */ + LL_SPI_Disable(SPI_INST(obj)); + /* Clear transaction flags */ + LL_SPI_ClearFlag_EOT(SPI_INST(obj)); + LL_SPI_ClearFlag_TXTF(SPI_INST(obj)); + /* Reset transaction size */ + LL_SPI_SetTransferSize(SPI_INST(obj), 0); + +#else /* TARGET_STM32H7 */ + /* Unlike STM32H7 other STM32 families generates SPI Clock signal continuously in half-duplex receive mode + * till SPI is enabled. To stop clock generation a SPI should be disabled during last frame receiving, + * after generation at least one SPI clock cycle. It causes necessity of critical section usage. + * So the following consequences of steps is used to receive each byte: + * 1. Enter into critical section. + * 2. Enable SPI. + * 3. Wait one SPI clock cycle. + * 4. Disable SPI. + * 5. Wait full byte receiving. + * 6. Read byte. + * It gives some overhead, but gives stable byte reception without dummy reads and + * short delay of critical section holding. + */ + + /* get estimation about one SPI clock cycle */ + uint32_t baudrate_period_ns = 1000000000 / spi_get_baudrate(obj); + + for (int i = 0; i < rx_length; i++) { + core_util_critical_section_enter(); + LL_SPI_Enable(SPI_INST(obj)); + /* Wait single SPI clock cycle. */ + wait_ns(baudrate_period_ns); + LL_SPI_Disable(SPI_INST(obj)); + core_util_critical_section_exit(); + + msp_wait_readable(obj); + rx_buffer[i] = msp_read_data(obj, bitshift); + } + +#endif /* TARGET_STM32H7 */ + } + + return rx_length + tx_length; +} + int spi_master_write(spi_t *obj, int value) { struct spi_s *spiobj = SPI_S(obj); SPI_HandleTypeDef *handle = &(spiobj->handle); if (handle->Init.Direction == SPI_DIRECTION_1LINE) { - return HAL_SPI_Transmit(handle, (uint8_t *)&value, 1, TIMEOUT_1_BYTE); + int result = spi_master_one_wire_transfer(obj, (const char *)&value, 1, NULL, 0); + return result == 1 ? HAL_OK : HAL_ERROR; } const int bitshift = datasize_to_transfer_bitshift(handle->Init.DataSize); MBED_ASSERT(bitshift >= 0); @@ -910,18 +1092,11 @@ int spi_master_block_write(spi_t *obj, const char *tx_buffer, int tx_length, } } } else { - /* In case of 1 WIRE only, first handle TX, then Rx */ - if (tx_length != 0) { - if (HAL_OK != HAL_SPI_Transmit(handle, (uint8_t *)tx_buffer, tx_length, tx_length * TIMEOUT_1_BYTE)) { - /* report an error */ - total = 0; - } - } - if (rx_length != 0) { - if (HAL_OK != HAL_SPI_Receive(handle, (uint8_t *)rx_buffer, rx_length, rx_length * TIMEOUT_1_BYTE)) { - /* report an error */ - total = 0; - } + /* 1 wire case */ + int result = spi_master_one_wire_transfer(obj, tx_buffer, tx_length, rx_buffer, rx_length); + if (result != tx_length + rx_length) { + /* report an error */ + total = 0; } } From c60f0cc11eeec6eeffc67ae4877fedcf292abfcf Mon Sep 17 00:00:00 2001 From: Konstantin Kochin Date: Sun, 15 Aug 2021 15:15:06 +0300 Subject: [PATCH 3/4] Fix STM32 spi_abort_asynch function - add RX cleanup after SPI re-initialization, as it isn't implemented in the `HAL_SPI_Init` - cancel SPI enabling for 3-wire mode --- targets/TARGET_STM/stm_spi_api.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/targets/TARGET_STM/stm_spi_api.c b/targets/TARGET_STM/stm_spi_api.c index a72d61102c..46cd192b6b 100644 --- a/targets/TARGET_STM/stm_spi_api.c +++ b/targets/TARGET_STM/stm_spi_api.c @@ -81,6 +81,17 @@ extern HAL_StatusTypeDef HAL_SPIEx_FlushRxFifo(SPI_HandleTypeDef *hspi); #define HAS_32BIT_SPI_TRANSFERS 1 #endif // SPI_DATASIZE_X +/** + * Flush RX FIFO/input register of SPI interface and clear overrun flag. + */ +static inline void spi_flush_rx(spi_t *obj) +{ +#if defined(SPI_FLAG_FRLVL) + HAL_SPIEx_FlushRxFifo(&(SPI_S(obj)->handle)); +#endif + LL_SPI_ClearFlag_OVR(SPI_INST(obj)); +} + void spi_get_capabilities(PinName ssel, bool slave, spi_capabilities_t *cap) { if (slave) { @@ -1373,10 +1384,15 @@ void spi_abort_asynch(spi_t *obj) NVIC_DisableIRQ(irq_n); // clean-up - __HAL_SPI_DISABLE(handle); + LL_SPI_Disable(SPI_INST(obj)); HAL_SPI_DeInit(handle); HAL_SPI_Init(handle); - __HAL_SPI_ENABLE(handle); + // cleanup input buffer + spi_flush_rx(obj); + // enable SPI back if it isn't 3-wire mode + if (handle->Init.Direction != SPI_DIRECTION_1LINE) { + LL_SPI_Enable(SPI_INST(obj)); + } } #endif //DEVICE_SPI_ASYNCH From 7bc773badd1d0f1d88c59f8ac007b10d5c343d9a Mon Sep 17 00:00:00 2001 From: Konstantin Kochin Date: Sun, 15 Aug 2021 15:23:38 +0300 Subject: [PATCH 4/4] Improve STM32 SPI asynchronous API stability `HAL_SPI_Receive_IT` HAL function causes dummy reads in 3-wire mode, that causes data corruption in RX FIFO/register. It isn't possible to fix it without signification refactoring, but we may prevent data corruption with the following fixes: - RX buffer/register cleanup after asynchronous transfer in 3-wire mode - Explicit RX buffer/register cleanup after SPI initialization (for cases if we re-create SPI object). --- targets/TARGET_STM/stm_spi_api.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/targets/TARGET_STM/stm_spi_api.c b/targets/TARGET_STM/stm_spi_api.c index 46cd192b6b..44b6d13d96 100644 --- a/targets/TARGET_STM/stm_spi_api.c +++ b/targets/TARGET_STM/stm_spi_api.c @@ -149,6 +149,9 @@ void init_spi(spi_t *obj) if (HAL_SPI_Init(handle) != HAL_OK) { error("Cannot initialize SPI"); } + /* In some cases after SPI object re-creation SPI overrun flag may not + * be cleared, so clear RX data explicitly to prevent any transmissions errors */ + spi_flush_rx(obj); /* In case of standard 4 wires SPI,PI can be kept enabled all time * and SCK will only be generated during the write operations. But in case * of 3 wires, it should be only enabled during rd/wr unitary operations, @@ -1329,11 +1332,12 @@ void spi_master_transfer(spi_t *obj, const void *tx, size_t tx_length, void *rx, inline uint32_t spi_irq_handler_asynch(spi_t *obj) { int event = 0; + SPI_HandleTypeDef *handle = &(SPI_S(obj)->handle); // call the CubeF4 handler, this will update the handle - HAL_SPI_IRQHandler(&obj->spi.handle); + HAL_SPI_IRQHandler(handle); - if (obj->spi.handle.State == HAL_SPI_STATE_READY) { + if (handle->State == HAL_SPI_STATE_READY) { // When HAL SPI is back to READY state, check if there was an error int error = obj->spi.handle.ErrorCode; if (error != HAL_SPI_ERROR_NONE) { @@ -1351,9 +1355,19 @@ inline uint32_t spi_irq_handler_asynch(spi_t *obj) // disable the interrupt NVIC_DisableIRQ(obj->spi.spiIRQ); NVIC_ClearPendingIRQ(obj->spi.spiIRQ); +#ifndef TARGET_STM32H7 + if (handle->Init.Direction == SPI_DIRECTION_1LINE && obj->rx_buff.buffer != NULL) { + /** + * In case of 3-wire SPI data receiving we usually get dummy reads. + * So we need to cleanup FIFO/input register before next transmission. + * Probably it's better to set SPI_EVENT_RX_OVERFLOW event flag, + * but let's left it as is for backward compatibility. + */ + spi_flush_rx(obj); + } +#endif } - return (event & (obj->spi.event | SPI_EVENT_INTERNAL_TRANSFER_COMPLETE)); }