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

wifi/bt coexistence: Fix disabled cache access race when writing to flash

Browse source 
Moves the ets_timer_arm() / ets_timer_disarm() code paths to RAM

Overhead is 740 bytes of IRAM, 0 bytes DRAM

(For comparison: If all of esp_timer.c is moved to RAM, overhead is 1068 bytes IRAM and 480 bytes DRAM.)
This commit is contained in:
Angus Gratton 2017-10-16 19:16:20 +08:00 committed by Angus Gratton
parent b013f5d490
commit 094cf4d79d
4 changed files with 60 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
components/esp32/esp_timer.c
View file

@ -108,7 +108,7 @@ esp_err_t esp_timer_create(const esp_timer_create_args_t* args,
return ESP_OK; return ESP_OK;
} }
esp_err_t esp_timer_start_once(esp_timer_handle_t timer, uint64_t timeout_us) esp_err_t IRAM_ATTR esp_timer_start_once(esp_timer_handle_t timer, uint64_t timeout_us)
{ {
if (!is_initialized() || timer_armed(timer)) { if (!is_initialized() || timer_armed(timer)) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
@ -121,7 +121,7 @@ esp_err_t esp_timer_start_once(esp_timer_handle_t timer, uint64_t timeout_us)
return timer_insert(timer); return timer_insert(timer);
} }
esp_err_t esp_timer_start_periodic(esp_timer_handle_t timer, uint64_t period_us) esp_err_t IRAM_ATTR esp_timer_start_periodic(esp_timer_handle_t timer, uint64_t period_us)
{ {
if (!is_initialized() || timer_armed(timer)) { if (!is_initialized() || timer_armed(timer)) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
@ -135,7 +135,7 @@ esp_err_t esp_timer_start_periodic(esp_timer_handle_t timer, uint64_t period_us)
return timer_insert(timer); return timer_insert(timer);
} }
esp_err_t esp_timer_stop(esp_timer_handle_t timer) esp_err_t IRAM_ATTR esp_timer_stop(esp_timer_handle_t timer)
{ {
if (!is_initialized() || !timer_armed(timer)) { if (!is_initialized() || !timer_armed(timer)) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
@ -158,7 +158,7 @@ esp_err_t esp_timer_delete(esp_timer_handle_t timer)
return ESP_OK; return ESP_OK;
} }
static esp_err_t timer_insert(esp_timer_handle_t timer) static IRAM_ATTR esp_err_t timer_insert(esp_timer_handle_t timer)
{ {
timer_list_lock(); timer_list_lock();
#if WITH_PROFILING #if WITH_PROFILING
@ -187,7 +187,7 @@ static esp_err_t timer_insert(esp_timer_handle_t timer)
return ESP_OK; return ESP_OK;
} }
static esp_err_t timer_remove(esp_timer_handle_t timer) static IRAM_ATTR esp_err_t timer_remove(esp_timer_handle_t timer)
{ {
timer_list_lock(); timer_list_lock();
LIST_REMOVE(timer, list_entry); LIST_REMOVE(timer, list_entry);
@ -202,7 +202,7 @@ static esp_err_t timer_remove(esp_timer_handle_t timer)
#if WITH_PROFILING #if WITH_PROFILING
static void timer_insert_inactive(esp_timer_handle_t timer) static IRAM_ATTR void timer_insert_inactive(esp_timer_handle_t timer)
{ {
/* May be locked or not, depending on where this is called from. /* May be locked or not, depending on where this is called from.
* Lock recursively. * Lock recursively.
@ -220,7 +220,7 @@ static void timer_insert_inactive(esp_timer_handle_t timer)
timer_list_unlock(); timer_list_unlock();
} }
static void timer_remove_inactive(esp_timer_handle_t timer) static IRAM_ATTR void timer_remove_inactive(esp_timer_handle_t timer)
{ {
timer_list_lock(); timer_list_lock();
LIST_REMOVE(timer, list_entry); LIST_REMOVE(timer, list_entry);
@ -229,17 +229,17 @@ static void timer_remove_inactive(esp_timer_handle_t timer)
#endif // WITH_PROFILING #endif // WITH_PROFILING
static bool timer_armed(esp_timer_handle_t timer) static IRAM_ATTR bool timer_armed(esp_timer_handle_t timer)
{ {
return timer->alarm > 0; return timer->alarm > 0;
} }
static void timer_list_lock() static IRAM_ATTR void timer_list_lock()
{ {
portENTER_CRITICAL(&s_timer_lock); portENTER_CRITICAL(&s_timer_lock);
} }
static void timer_list_unlock() static IRAM_ATTR void timer_list_unlock()
{ {
portEXIT_CRITICAL(&s_timer_lock); portEXIT_CRITICAL(&s_timer_lock);
} }
@ -305,7 +305,7 @@ static void IRAM_ATTR timer_alarm_handler(void* arg)
} }
} }
static bool is_initialized() static IRAM_ATTR bool is_initialized()
{ {
return s_timer_task != NULL; return s_timer_task != NULL;
} }

2
components/esp32/esp_timer_esp32.c
View file

@ -255,7 +255,7 @@ void esp_timer_impl_deinit()
// FIXME: This value is safe for 80MHz APB frequency. // FIXME: This value is safe for 80MHz APB frequency.
// Should be modified to depend on clock frequency. // Should be modified to depend on clock frequency.
uint64_t esp_timer_impl_get_min_period_us() uint64_t IRAM_ATTR esp_timer_impl_get_min_period_us()
{ {
return 50; return 50;
} }

8
components/esp32/ets_timer_legacy.c
View file

@ -43,7 +43,7 @@
#define TIMER_INITIALIZED_FIELD(p_ets_timer) ((p_ets_timer)->timer_expire) #define TIMER_INITIALIZED_FIELD(p_ets_timer) ((p_ets_timer)->timer_expire)
#define TIMER_INITIALIZED_VAL 0x12121212 #define TIMER_INITIALIZED_VAL 0x12121212
static bool timer_initialized(ETSTimer *ptimer) static IRAM_ATTR bool timer_initialized(ETSTimer *ptimer)
{ {
return TIMER_INITIALIZED_FIELD(ptimer) == TIMER_INITIALIZED_VAL; return TIMER_INITIALIZED_FIELD(ptimer) == TIMER_INITIALIZED_VAL;
} }
@ -68,7 +68,7 @@ void ets_timer_setfn(ETSTimer *ptimer, ETSTimerFunc *pfunction, void *parg)
} }
void ets_timer_arm_us(ETSTimer *ptimer, uint32_t time_us, bool repeat_flag) void IRAM_ATTR ets_timer_arm_us(ETSTimer *ptimer, uint32_t time_us, bool repeat_flag)
{ {
assert(timer_initialized(ptimer)); assert(timer_initialized(ptimer));
esp_timer_stop(ESP_TIMER(ptimer)); // no error check esp_timer_stop(ESP_TIMER(ptimer)); // no error check
@ -79,7 +79,7 @@ void ets_timer_arm_us(ETSTimer *ptimer, uint32_t time_us, bool repeat_flag)
} }
} }
void ets_timer_arm(ETSTimer *ptimer, uint32_t time_ms, bool repeat_flag) void IRAM_ATTR ets_timer_arm(ETSTimer *ptimer, uint32_t time_ms, bool repeat_flag)
{ {
uint64_t time_us = 1000LL * (uint64_t) time_ms; uint64_t time_us = 1000LL * (uint64_t) time_ms;
assert(timer_initialized(ptimer)); assert(timer_initialized(ptimer));
@ -100,7 +100,7 @@ void ets_timer_done(ETSTimer *ptimer)
} }
} }
void ets_timer_disarm(ETSTimer *ptimer) void IRAM_ATTR ets_timer_disarm(ETSTimer *ptimer)
{ {
if (timer_initialized(ptimer)) { if (timer_initialized(ptimer)) {
esp_timer_stop(ESP_TIMER(ptimer)); esp_timer_stop(ESP_TIMER(ptimer));

44
components/esp32/test/test_ets_timer.c
View file

@ -7,6 +7,7 @@
#include "freertos/FreeRTOS.h" #include "freertos/FreeRTOS.h"
#include "freertos/task.h" #include "freertos/task.h"
#include "freertos/semphr.h" #include "freertos/semphr.h"
#include "esp_spi_flash.h"
TEST_CASE("ets_timer produces correct delay", "[ets_timer]") TEST_CASE("ets_timer produces correct delay", "[ets_timer]")
{ {
@ -192,3 +193,46 @@ TEST_CASE("multiple ETSTimers are ordered correctly", "[ets_timer]")
#undef N #undef N
} }
/* WiFi/BT coexistence will sometimes arm/disarm
timers from an ISR where flash may be disabled. */
IRAM_ATTR TEST_CASE("ETSTimers arm & disarm run from IRAM", "[ets_timer]")
{
void timer_func(void* arg)
{
volatile bool *b = (volatile bool *)arg;
*b = true;
}
volatile bool flag = false;
ETSTimer timer1;
const int INTERVAL = 5;
ets_timer_setfn(&timer1, &timer_func, (void *)&flag);
/* arm a disabled timer, then disarm a live timer */
g_flash_guard_default_ops.start(); // Disables flash cache
ets_timer_arm(&timer1, INTERVAL, false);
// redundant call is deliberate (test code path if already armed)
ets_timer_arm(&timer1, INTERVAL, false);
ets_timer_disarm(&timer1);
g_flash_guard_default_ops.end(); // Re-enables flash cache
TEST_ASSERT_FALSE(flag); // didn't expire yet
/* do the same thing but wait for the timer to expire */
g_flash_guard_default_ops.start();
ets_timer_arm(&timer1, INTERVAL, false);
g_flash_guard_default_ops.end();
vTaskDelay(2 * INTERVAL / portTICK_PERIOD_MS);
TEST_ASSERT_TRUE(flag);
g_flash_guard_default_ops.start();
ets_timer_disarm(&timer1);
g_flash_guard_default_ops.end();
}