Guard RCC registers access with critical section (DarkFlippers#854)

* Core: critical section macros. FuriHal: guard rcc registers access with critical section, fix condition race. * FuriHal: update documentation. Co-authored-by: SG <[email protected]>
niels-hubs · Nov 30, 2021 · 418c093 · 418c093
1 parent 6f7d93f
commit 418c093
Show file tree

Hide file tree

Showing 14 changed files with 148 additions and 78 deletions.
diff --git a/bootloader/targets/f6/furi-hal/furi-hal-i2c.h b/bootloader/targets/f6/furi-hal/furi-hal-i2c.h
@@ -31,11 +31,11 @@ void furi_hal_i2c_release(FuriHalI2cBusHandle* handle);
 
 /** Perform I2C tx transfer
  *
- * @param      instance  I2C_TypeDef instance
- * @param      address   I2C slave address
- * @param      data      pointer to data buffer
- * @param      size      size of data buffer
- * @param      timeout   timeout in ticks
+ * @param      handle   pointer to FuriHalI2cBusHandle instance
+ * @param      address  I2C slave address
+ * @param      data     pointer to data buffer
+ * @param      size     size of data buffer
+ * @param      timeout  timeout in ticks
  *
  * @return     true on successful transfer, false otherwise
  */
@@ -48,11 +48,11 @@ bool furi_hal_i2c_tx(
 
 /** Perform I2C rx transfer
  *
- * @param      instance  I2C_TypeDef instance
- * @param      address   I2C slave address
- * @param      data      pointer to data buffer
- * @param      size      size of data buffer
- * @param      timeout   timeout in ticks
+ * @param      handle   pointer to FuriHalI2cBusHandle instance
+ * @param      address  I2C slave address
+ * @param      data     pointer to data buffer
+ * @param      size     size of data buffer
+ * @param      timeout  timeout in ticks
  *
  * @return     true on successful transfer, false otherwise
  */
@@ -65,13 +65,13 @@ bool furi_hal_i2c_rx(
 
 /** Perform I2C tx and rx transfers
  *
- * @param      instance  I2C_TypeDef instance
- * @param      address   I2C slave address
- * @param      tx_data   pointer to tx data buffer
- * @param      tx_size   size of tx data buffer
- * @param      rx_data   pointer to rx data buffer
- * @param      rx_size   size of rx data buffer
- * @param      timeout   timeout in ticks
+ * @param      handle   pointer to FuriHalI2cBusHandle instance
+ * @param      address  I2C slave address
+ * @param      tx_data  pointer to tx data buffer
+ * @param      tx_size  size of tx data buffer
+ * @param      rx_data  pointer to rx data buffer
+ * @param      rx_size  size of rx data buffer
+ * @param      timeout  timeout in ticks
  *
  * @return     true on successful transfer, false otherwise
  */

diff --git a/bootloader/targets/f7/furi-hal/furi-hal-i2c.h b/bootloader/targets/f7/furi-hal/furi-hal-i2c.h
@@ -31,11 +31,11 @@ void furi_hal_i2c_release(FuriHalI2cBusHandle* handle);
 
 /** Perform I2C tx transfer
  *
- * @param      instance  I2C_TypeDef instance
- * @param      address   I2C slave address
- * @param      data      pointer to data buffer
- * @param      size      size of data buffer
- * @param      timeout   timeout in ticks
+ * @param      handle   pointer to FuriHalI2cBusHandle instance
+ * @param      address  I2C slave address
+ * @param      data     pointer to data buffer
+ * @param      size     size of data buffer
+ * @param      timeout  timeout in ticks
  *
  * @return     true on successful transfer, false otherwise
  */
@@ -48,11 +48,11 @@ bool furi_hal_i2c_tx(
 
 /** Perform I2C rx transfer
  *
- * @param      instance  I2C_TypeDef instance
- * @param      address   I2C slave address
- * @param      data      pointer to data buffer
- * @param      size      size of data buffer
- * @param      timeout   timeout in ticks
+ * @param      handle   pointer to FuriHalI2cBusHandle instance
+ * @param      address  I2C slave address
+ * @param      data     pointer to data buffer
+ * @param      size     size of data buffer
+ * @param      timeout  timeout in ticks
  *
  * @return     true on successful transfer, false otherwise
  */
@@ -65,13 +65,13 @@ bool furi_hal_i2c_rx(
 
 /** Perform I2C tx and rx transfers
  *
- * @param      instance  I2C_TypeDef instance
- * @param      address   I2C slave address
- * @param      tx_data   pointer to tx data buffer
- * @param      tx_size   size of tx data buffer
- * @param      rx_data   pointer to rx data buffer
- * @param      rx_size   size of rx data buffer
- * @param      timeout   timeout in ticks
+ * @param      handle   pointer to FuriHalI2cBusHandle instance
+ * @param      address  I2C slave address
+ * @param      tx_data  pointer to tx data buffer
+ * @param      tx_size  size of tx data buffer
+ * @param      rx_data  pointer to rx data buffer
+ * @param      rx_size  size of rx data buffer
+ * @param      timeout  timeout in ticks
  *
  * @return     true on successful transfer, false otherwise
  */

diff --git a/core/furi/common_defines.h b/core/furi/common_defines.h
@@ -70,4 +70,14 @@
 #define REVERSE_BYTES_U32(x)                                                        \
     ((((x)&0x000000FF) << 24) | (((x)&0x0000FF00) << 8) | (((x)&0x00FF0000) >> 8) | \
      (((x)&0xFF000000) >> 24))
+#endif
+
+#ifndef FURI_CRITICAL_ENTER
+#define FURI_CRITICAL_ENTER()               \
+    uint32_t primask_bit = __get_PRIMASK(); \
+    __disable_irq()
+#endif
+
+#ifndef FURI_CRITICAL_EXIT
+#define FURI_CRITICAL_EXIT() __set_PRIMASK(primask_bit)
 #endif
diff --git a/firmware/targets/f6/furi-hal/furi-hal-console.c b/firmware/targets/f6/furi-hal/furi-hal-console.c
@@ -39,26 +39,26 @@ void furi_hal_console_tx(const uint8_t* buffer, size_t buffer_size) {
     if (!furi_hal_console_alive)
         return;
 
-    UTILS_ENTER_CRITICAL_SECTION();
+    FURI_CRITICAL_ENTER();
     // Transmit data
     furi_hal_uart_tx(FuriHalUartIdUSART1, (uint8_t*)buffer, buffer_size);
     // Wait for TC flag to be raised for last char
     while (!LL_USART_IsActiveFlag_TC(USART1));
-    UTILS_EXIT_CRITICAL_SECTION();
+    FURI_CRITICAL_EXIT();
 }
 
 void furi_hal_console_tx_with_new_line(const uint8_t* buffer, size_t buffer_size) {
     if (!furi_hal_console_alive)
         return;
 
-    UTILS_ENTER_CRITICAL_SECTION();
+    FURI_CRITICAL_ENTER();
     // Transmit data
     furi_hal_uart_tx(FuriHalUartIdUSART1, (uint8_t*)buffer, buffer_size);
     // Transmit new line symbols
     furi_hal_uart_tx(FuriHalUartIdUSART1, (uint8_t*)"\r\n", 2);
     // Wait for TC flag to be raised for last char
     while (!LL_USART_IsActiveFlag_TC(USART1));
-    UTILS_EXIT_CRITICAL_SECTION();
+    FURI_CRITICAL_EXIT();
 }
 
 void furi_hal_console_printf(const char format[], ...) {

diff --git a/firmware/targets/f6/furi-hal/furi-hal-i2c-config.c b/firmware/targets/f6/furi-hal/furi-hal-i2c-config.c
@@ -19,9 +19,11 @@ osMutexId_t furi_hal_i2c_bus_power_mutex = NULL;
 static void furi_hal_i2c_bus_power_event(FuriHalI2cBus* bus, FuriHalI2cBusEvent event) {
     if (event == FuriHalI2cBusEventInit) {
         furi_hal_i2c_bus_power_mutex = osMutexNew(NULL);
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_I2C1);
         LL_RCC_SetI2CClockSource(LL_RCC_I2C1_CLKSOURCE_PCLK1);
         LL_APB1_GRP1_ForceReset(LL_APB1_GRP1_PERIPH_I2C1);
+        FURI_CRITICAL_EXIT();
         bus->current_handle = NULL;
     } else if (event == FuriHalI2cBusEventDeinit) {
         osMutexDelete(furi_hal_i2c_bus_power_mutex);
@@ -30,9 +32,13 @@ static void furi_hal_i2c_bus_power_event(FuriHalI2cBus* bus, FuriHalI2cBusEvent
     } else if (event == FuriHalI2cBusEventUnlock) {
         furi_check(osMutexRelease(furi_hal_i2c_bus_power_mutex) == osOK);
     } else if (event == FuriHalI2cBusEventActivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_ReleaseReset(LL_APB1_GRP1_PERIPH_I2C1);
+        FURI_CRITICAL_EXIT();
     } else if (event == FuriHalI2cBusEventDeactivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_ForceReset(LL_APB1_GRP1_PERIPH_I2C1);
+        FURI_CRITICAL_EXIT();
     }
 }
 
@@ -45,11 +51,15 @@ osMutexId_t furi_hal_i2c_bus_external_mutex = NULL;
 
 static void furi_hal_i2c_bus_external_event(FuriHalI2cBus* bus, FuriHalI2cBusEvent event) {
     if (event == FuriHalI2cBusEventActivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_I2C3);
         LL_RCC_SetI2CClockSource(LL_RCC_I2C3_CLKSOURCE_PCLK1);
         LL_APB1_GRP1_ReleaseReset(LL_APB1_GRP1_PERIPH_I2C3);
+        FURI_CRITICAL_EXIT();
     } else if (event == FuriHalI2cBusEventDeactivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_ForceReset(LL_APB1_GRP1_PERIPH_I2C3);
+        FURI_CRITICAL_EXIT();
     }
 }
 
@@ -76,13 +86,12 @@ void furi_hal_i2c_bus_handle_power_event(FuriHalI2cBusHandle* handle, FuriHalI2c
             I2C_InitStruct.Timing = FURI_HAL_I2C_CONFIG_POWER_I2C_TIMINGS_100;
         }
         LL_I2C_Init(handle->bus->i2c, &I2C_InitStruct);
-
+        // I2C is enabled at this point
         LL_I2C_EnableAutoEndMode(handle->bus->i2c);
         LL_I2C_SetOwnAddress2(handle->bus->i2c, 0, LL_I2C_OWNADDRESS2_NOMASK);
         LL_I2C_DisableOwnAddress2(handle->bus->i2c);
         LL_I2C_DisableGeneralCall(handle->bus->i2c);
         LL_I2C_EnableClockStretching(handle->bus->i2c);
-        LL_I2C_Enable(handle->bus->i2c);
     } else if (event == FuriHalI2cBusHandleEventDeactivate) {
         LL_I2C_Disable(handle->bus->i2c);
         hal_gpio_write(&gpio_i2c_power_sda, 1);
@@ -111,13 +120,12 @@ void furi_hal_i2c_bus_handle_external_event(FuriHalI2cBusHandle* handle, FuriHal
         I2C_InitStruct.OwnAddrSize = LL_I2C_OWNADDRESS1_7BIT;
         I2C_InitStruct.Timing = FURI_HAL_I2C_CONFIG_POWER_I2C_TIMINGS_100;
         LL_I2C_Init(handle->bus->i2c, &I2C_InitStruct);
-
+        // I2C is enabled at this point
         LL_I2C_EnableAutoEndMode(handle->bus->i2c);
         LL_I2C_SetOwnAddress2(handle->bus->i2c, 0, LL_I2C_OWNADDRESS2_NOMASK);
         LL_I2C_DisableOwnAddress2(handle->bus->i2c);
         LL_I2C_DisableGeneralCall(handle->bus->i2c);
         LL_I2C_EnableClockStretching(handle->bus->i2c);
-        LL_I2C_Enable(handle->bus->i2c);
     } else if (event == FuriHalI2cBusHandleEventDeactivate) {
         LL_I2C_Disable(handle->bus->i2c);
         hal_gpio_write(&gpio_ext_pc0, 1);

diff --git a/firmware/targets/f6/furi-hal/furi-hal-irda.c b/firmware/targets/f6/furi-hal/furi-hal-irda.c
@@ -136,8 +136,10 @@ static void furi_hal_irda_tim_rx_isr() {
 void furi_hal_irda_async_rx_start(void) {
     furi_assert(furi_hal_irda_state == IrdaStateIdle);
 
+    FURI_CRITICAL_ENTER();
     LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_TIM2);
     LL_AHB2_GRP1_EnableClock(LL_AHB2_GRP1_PERIPH_GPIOA);
+    FURI_CRITICAL_EXIT();
 
     hal_gpio_init_ex(&gpio_irda_rx, GpioModeAltFunctionPushPull, GpioPullNo, GpioSpeedLow, GpioAltFn1TIM2);
 

diff --git a/firmware/targets/f6/furi-hal/furi-hal-spi-config.c b/firmware/targets/f6/furi-hal/furi-hal-spi-config.c
@@ -75,8 +75,10 @@ osMutexId_t furi_hal_spi_bus_r_mutex = NULL;
 static void furi_hal_spi_bus_r_event_callback(FuriHalSpiBus* bus, FuriHalSpiBusEvent event) {
     if (event == FuriHalSpiBusEventInit) {
         furi_hal_spi_bus_r_mutex = osMutexNew(NULL);
+        FURI_CRITICAL_ENTER();
         LL_APB2_GRP1_EnableClock(LL_APB2_GRP1_PERIPH_SPI1);
         LL_APB2_GRP1_ForceReset(LL_APB2_GRP1_PERIPH_SPI1);
+        FURI_CRITICAL_EXIT();
         bus->current_handle = NULL;
     } else if (event == FuriHalSpiBusEventDeinit) {
         furi_check(osMutexDelete(furi_hal_spi_bus_r_mutex));
@@ -85,9 +87,13 @@ static void furi_hal_spi_bus_r_event_callback(FuriHalSpiBus* bus, FuriHalSpiBusE
     } else if (event == FuriHalSpiBusEventUnlock) {
         furi_check(osMutexRelease(furi_hal_spi_bus_r_mutex) == osOK);
     } else if (event == FuriHalSpiBusEventActivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB2_GRP1_ReleaseReset(LL_APB2_GRP1_PERIPH_SPI1);
+        FURI_CRITICAL_EXIT();
     } else if (event == FuriHalSpiBusEventDeactivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB2_GRP1_ForceReset(LL_APB2_GRP1_PERIPH_SPI1);
+        FURI_CRITICAL_EXIT();
     }
 }
 
@@ -101,8 +107,10 @@ osMutexId_t furi_hal_spi_bus_d_mutex = NULL;
 static void furi_hal_spi_bus_d_event_callback(FuriHalSpiBus* bus, FuriHalSpiBusEvent event) {
     if (event == FuriHalSpiBusEventInit) {
         furi_hal_spi_bus_d_mutex = osMutexNew(NULL);
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_SPI2);
         LL_APB1_GRP1_ForceReset(LL_APB1_GRP1_PERIPH_SPI2);
+        FURI_CRITICAL_EXIT();
         bus->current_handle = NULL;
     } else if (event == FuriHalSpiBusEventDeinit) {
         furi_check(osMutexDelete(furi_hal_spi_bus_d_mutex));
@@ -111,9 +119,13 @@ static void furi_hal_spi_bus_d_event_callback(FuriHalSpiBus* bus, FuriHalSpiBusE
     } else if (event == FuriHalSpiBusEventUnlock) {
         furi_check(osMutexRelease(furi_hal_spi_bus_d_mutex) == osOK);
     } else if (event == FuriHalSpiBusEventActivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_ReleaseReset(LL_APB1_GRP1_PERIPH_SPI2);
+        FURI_CRITICAL_EXIT();
     } else if (event == FuriHalSpiBusEventDeactivate) {
+        FURI_CRITICAL_ENTER();
         LL_APB1_GRP1_ForceReset(LL_APB1_GRP1_PERIPH_SPI2);
+        FURI_CRITICAL_EXIT();
     }
 }
 

diff --git a/firmware/targets/f6/furi-hal/furi-hal-subghz.c b/firmware/targets/f6/furi-hal/furi-hal-subghz.c
@@ -575,7 +575,10 @@ void furi_hal_subghz_start_async_rx(FuriHalSubGhzCaptureCallback callback, void*
         &gpio_cc1101_g0, GpioModeAltFunctionPushPull, GpioPullNo, GpioSpeedLow, GpioAltFn1TIM2);
 
     // Timer: base
+    FURI_CRITICAL_ENTER();
     LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_TIM2);
+    FURI_CRITICAL_EXIT();
+
     LL_TIM_InitTypeDef TIM_InitStruct = {0};
     TIM_InitStruct.Prescaler = 64 - 1;
     TIM_InitStruct.CounterMode = LL_TIM_COUNTERMODE_UP;
@@ -635,8 +638,10 @@ void furi_hal_subghz_stop_async_rx() {
     // Shutdown radio
     furi_hal_subghz_idle();
 
+    FURI_CRITICAL_ENTER();
     LL_TIM_DeInit(TIM2);
     LL_APB1_GRP1_DisableClock(LL_APB1_GRP1_PERIPH_TIM2);
+    FURI_CRITICAL_EXIT();
     furi_hal_interrupt_set_timer_isr(TIM2, NULL);
 
     hal_gpio_init(&gpio_cc1101_g0, GpioModeAnalog, GpioPullNo, GpioSpeedLow);
@@ -761,7 +766,9 @@ bool furi_hal_subghz_start_async_tx(FuriHalSubGhzAsyncTxCallback callback, void*
     LL_DMA_EnableChannel(DMA1, LL_DMA_CHANNEL_1);
 
     // Configure TIM2
+    FURI_CRITICAL_ENTER();
     LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_TIM2);
+    FURI_CRITICAL_EXIT();
     LL_TIM_InitTypeDef TIM_InitStruct = {0};
     TIM_InitStruct.Prescaler = 64 - 1;
     TIM_InitStruct.CounterMode = LL_TIM_COUNTERMODE_UP;
@@ -820,6 +827,7 @@ void furi_hal_subghz_stop_async_tx() {
 #endif
 
     // Deinitialize Timer
+    FURI_CRITICAL_ENTER();
     LL_TIM_DeInit(TIM2);
     LL_APB1_GRP1_DisableClock(LL_APB1_GRP1_PERIPH_TIM2);
     furi_hal_interrupt_set_timer_isr(TIM2, NULL);
@@ -830,6 +838,7 @@ void furi_hal_subghz_stop_async_tx() {
 
     // Deinitialize GPIO
     hal_gpio_init(&gpio_cc1101_g0, GpioModeAnalog, GpioPullNo, GpioSpeedLow);
+    FURI_CRITICAL_EXIT();
 
     free(furi_hal_subghz_async_tx.buffer);