Merge branch 'refactor/power_management_v3.3' into 'release/v3.3'

power_management: Using port*_CRITICAL_ISR to be consistent with FreeRTOS (backport v3.3)

See merge request idf/esp-idf!5079
This commit is contained in:
Angus Gratton 2019-06-26 14:33:19 +08:00
commit 129ac11c31
16 changed files with 228 additions and 42 deletions

View File

@ -1491,6 +1491,12 @@ UT_006_03:
- UT_T1_GPIO
UT_006_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_GPIO
UT_006_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1516,6 +1522,12 @@ UT_007_03:
- UT_T1_PCNT
UT_007_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_PCNT
UT_007_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1541,6 +1553,12 @@ UT_008_03:
- UT_T1_LEDC
UT_008_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_LEDC
UT_008_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1566,6 +1584,12 @@ UT_009_03:
- UT_T2_RS485
UT_009_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T2_RS485
UT_009_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1591,6 +1615,12 @@ UT_010_03:
- UT_T1_RMT
UT_010_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_RMT
UT_010_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1669,6 +1699,12 @@ UT_013_03:
- Example_SPI_Multi_device
UT_013_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- Example_SPI_Multi_device
UT_013_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1694,6 +1730,12 @@ UT_014_03:
- UT_T2_I2C
UT_014_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T2_I2C
UT_014_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1719,6 +1761,12 @@ UT_015_03:
- UT_T1_MCPWM
UT_015_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_MCPWM
UT_015_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1744,6 +1792,12 @@ UT_016_03:
- UT_T1_I2S
UT_016_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_I2S
UT_016_05:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1773,9 +1827,15 @@ UT_017_04:
tags:
- ESP32_IDF
- UT_T2_1
- psram
UT_017_05:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T2_1
- psram
UT_017_06:
<<: *unit_test_template
tags:
- ESP32_IDF
@ -1788,6 +1848,42 @@ UT_601_01:
- ESP32_IDF
- UT_T1_1
UT_601_02:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_1
UT_601_03:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_1
UT_601_04:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_1
UT_601_05:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_1
UT_601_06:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_1
UT_601_07:
<<: *unit_test_template
tags:
- ESP32_IDF
- UT_T1_1
IT_001_01:
<<: *test_template
tags:

View File

@ -29,26 +29,26 @@ static uint32_t get_rst_en_reg(periph_module_t periph);
void periph_module_enable(periph_module_t periph)
{
portENTER_CRITICAL(&periph_spinlock);
portENTER_CRITICAL_SAFE(&periph_spinlock);
DPORT_SET_PERI_REG_MASK(get_clk_en_reg(periph), get_clk_en_mask(periph));
DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, true));
portEXIT_CRITICAL(&periph_spinlock);
portEXIT_CRITICAL_SAFE(&periph_spinlock);
}
void periph_module_disable(periph_module_t periph)
{
portENTER_CRITICAL(&periph_spinlock);
portENTER_CRITICAL_SAFE(&periph_spinlock);
DPORT_CLEAR_PERI_REG_MASK(get_clk_en_reg(periph), get_clk_en_mask(periph));
DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false));
portEXIT_CRITICAL(&periph_spinlock);
portEXIT_CRITICAL_SAFE(&periph_spinlock);
}
void periph_module_reset(periph_module_t periph)
{
portENTER_CRITICAL(&periph_spinlock);
portENTER_CRITICAL_SAFE(&periph_spinlock);
DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false));
DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false));
portEXIT_CRITICAL(&periph_spinlock);
portEXIT_CRITICAL_SAFE(&periph_spinlock);
}
static uint32_t get_clk_en_mask(periph_module_t periph)

View File

@ -501,9 +501,9 @@ esp_err_t rmt_config(const rmt_config_t* rmt_param)
static void IRAM_ATTR rmt_fill_memory(rmt_channel_t channel, const rmt_item32_t* item, uint16_t item_num, uint16_t mem_offset)
{
portENTER_CRITICAL(&rmt_spinlock);
portENTER_CRITICAL_SAFE(&rmt_spinlock);
RMT.apb_conf.fifo_mask = RMT_DATA_MODE_MEM;
portEXIT_CRITICAL(&rmt_spinlock);
portEXIT_CRITICAL_SAFE(&rmt_spinlock);
int i;
for(i = 0; i < item_num; i++) {
RMTMEM.chan[channel].data32[i + mem_offset].val = item[i].val;

View File

@ -1964,15 +1964,15 @@ static void rtc_isr(void* arg)
{
uint32_t status = REG_READ(RTC_CNTL_INT_ST_REG);
rtc_isr_handler_t* it;
portENTER_CRITICAL(&s_rtc_isr_handler_list_lock);
portENTER_CRITICAL_ISR(&s_rtc_isr_handler_list_lock);
SLIST_FOREACH(it, &s_rtc_isr_handler_list, next) {
if (it->mask & status) {
portEXIT_CRITICAL(&s_rtc_isr_handler_list_lock);
portEXIT_CRITICAL_ISR(&s_rtc_isr_handler_list_lock);
(*it->handler)(it->handler_arg);
portENTER_CRITICAL(&s_rtc_isr_handler_list_lock);
portENTER_CRITICAL_ISR(&s_rtc_isr_handler_list_lock);
}
}
portEXIT_CRITICAL(&s_rtc_isr_handler_list_lock);
portEXIT_CRITICAL_ISR(&s_rtc_isr_handler_list_lock);
REG_WRITE(RTC_CNTL_INT_CLR_REG, status);
}

View File

@ -909,6 +909,9 @@ static IRAM_ATTR void spi_transmit_polling_measure(spi_device_handle_t spi, spi_
TEST_CASE("spi_speed","[spi]")
{
#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE
return;
#endif
uint32_t t_flight;
//to get rid of the influence of randomly interrupts, we measured the performance by median value
uint32_t t_flight_sorted[TEST_TIMES];

View File

@ -39,19 +39,19 @@ static const char* TIMER_TAG = "timer_group";
static timg_dev_t *TG[2] = {&TIMERG0, &TIMERG1};
static portMUX_TYPE timer_spinlock[TIMER_GROUP_MAX] = {portMUX_INITIALIZER_UNLOCKED, portMUX_INITIALIZER_UNLOCKED};
#define TIMER_ENTER_CRITICAL(mux) portENTER_CRITICAL(mux);
#define TIMER_EXIT_CRITICAL(mux) portEXIT_CRITICAL(mux);
#define TIMER_ENTER_CRITICAL(mux) portENTER_CRITICAL_SAFE(mux);
#define TIMER_EXIT_CRITICAL(mux) portEXIT_CRITICAL_SAFE(mux);
esp_err_t timer_get_counter_value(timer_group_t group_num, timer_idx_t timer_num, uint64_t* timer_val)
{
TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG);
TIMER_CHECK(timer_num < TIMER_MAX, TIMER_NUM_ERROR, ESP_ERR_INVALID_ARG);
TIMER_CHECK(timer_val != NULL, TIMER_PARAM_ADDR_ERROR, ESP_ERR_INVALID_ARG);
portENTER_CRITICAL(&timer_spinlock[group_num]);
portENTER_CRITICAL_SAFE(&timer_spinlock[group_num]);
TG[group_num]->hw_timer[timer_num].update = 1;
*timer_val = ((uint64_t) TG[group_num]->hw_timer[timer_num].cnt_high << 32)
| (TG[group_num]->hw_timer[timer_num].cnt_low);
portEXIT_CRITICAL(&timer_spinlock[group_num]);
portEXIT_CRITICAL_SAFE(&timer_spinlock[group_num]);
return ESP_OK;
}
@ -154,10 +154,10 @@ esp_err_t timer_get_alarm_value(timer_group_t group_num, timer_idx_t timer_num,
TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG);
TIMER_CHECK(timer_num < TIMER_MAX, TIMER_NUM_ERROR, ESP_ERR_INVALID_ARG);
TIMER_CHECK(alarm_value != NULL, TIMER_PARAM_ADDR_ERROR, ESP_ERR_INVALID_ARG);
portENTER_CRITICAL(&timer_spinlock[group_num]);
portENTER_CRITICAL_SAFE(&timer_spinlock[group_num]);
*alarm_value = ((uint64_t) TG[group_num]->hw_timer[timer_num].alarm_high << 32)
| (TG[group_num]->hw_timer[timer_num].alarm_low);
portEXIT_CRITICAL(&timer_spinlock[group_num]);
portEXIT_CRITICAL_SAFE(&timer_spinlock[group_num]);
return ESP_OK;
}

View File

@ -96,9 +96,9 @@ void esp_crosscore_int_init() {
static void IRAM_ATTR esp_crosscore_int_send(int core_id, uint32_t reason_mask) {
assert(core_id<portNUM_PROCESSORS);
//Mark the reason we interrupt the other CPU
portENTER_CRITICAL(&reason_spinlock);
portENTER_CRITICAL_ISR(&reason_spinlock);
reason[core_id] |= reason_mask;
portEXIT_CRITICAL(&reason_spinlock);
portEXIT_CRITICAL_ISR(&reason_spinlock);
//Poke the other CPU.
if (core_id==0) {
DPORT_WRITE_PERI_REG(DPORT_CPU_INTR_FROM_CPU_0_REG, DPORT_CPU_INTR_FROM_CPU_0);

View File

@ -494,7 +494,7 @@ static void IRAM_ATTR shared_intr_isr(void *arg)
{
vector_desc_t *vd=(vector_desc_t*)arg;
shared_vector_desc_t *sh_vec=vd->shared_vec_info;
portENTER_CRITICAL(&spinlock);
portENTER_CRITICAL_ISR(&spinlock);
while(sh_vec) {
if (!sh_vec->disabled) {
if ((sh_vec->statusreg == NULL) || (*sh_vec->statusreg & sh_vec->statusmask)) {
@ -512,7 +512,7 @@ static void IRAM_ATTR shared_intr_isr(void *arg)
}
sh_vec=sh_vec->next;
}
portEXIT_CRITICAL(&spinlock);
portEXIT_CRITICAL_ISR(&spinlock);
}
#if CONFIG_SYSVIEW_ENABLE
@ -520,7 +520,7 @@ static void IRAM_ATTR shared_intr_isr(void *arg)
static void IRAM_ATTR non_shared_intr_isr(void *arg)
{
non_shared_isr_arg_t *ns_isr_arg=(non_shared_isr_arg_t*)arg;
portENTER_CRITICAL(&spinlock);
portENTER_CRITICAL_ISR(&spinlock);
traceISR_ENTER(ns_isr_arg->source+ETS_INTERNAL_INTR_SOURCE_OFF);
// FIXME: can we call ISR and check port_switch_flag after releasing spinlock?
// when CONFIG_SYSVIEW_ENABLE = 0 ISRs for non-shared IRQs are called without spinlock
@ -529,7 +529,7 @@ static void IRAM_ATTR non_shared_intr_isr(void *arg)
if (!port_switch_flag[xPortGetCoreID()]) {
traceISR_EXIT();
}
portEXIT_CRITICAL(&spinlock);
portEXIT_CRITICAL_ISR(&spinlock);
}
#endif
@ -794,7 +794,7 @@ int esp_intr_get_cpu(intr_handle_t handle)
esp_err_t IRAM_ATTR esp_intr_enable(intr_handle_t handle)
{
if (!handle) return ESP_ERR_INVALID_ARG;
portENTER_CRITICAL(&spinlock);
portENTER_CRITICAL_SAFE(&spinlock);
int source;
if (handle->shared_vector_desc) {
handle->shared_vector_desc->disabled=0;
@ -810,14 +810,14 @@ esp_err_t IRAM_ATTR esp_intr_enable(intr_handle_t handle)
if (handle->vector_desc->cpu!=xPortGetCoreID()) return ESP_ERR_INVALID_ARG; //Can only enable these ints on this cpu
ESP_INTR_ENABLE(handle->vector_desc->intno);
}
portEXIT_CRITICAL(&spinlock);
portEXIT_CRITICAL_SAFE(&spinlock);
return ESP_OK;
}
esp_err_t IRAM_ATTR esp_intr_disable(intr_handle_t handle)
{
if (!handle) return ESP_ERR_INVALID_ARG;
portENTER_CRITICAL(&spinlock);
portENTER_CRITICAL_SAFE(&spinlock);
int source;
bool disabled = 1;
if (handle->shared_vector_desc) {
@ -850,7 +850,7 @@ esp_err_t IRAM_ATTR esp_intr_disable(intr_handle_t handle)
}
ESP_INTR_DISABLE(handle->vector_desc->intno);
}
portEXIT_CRITICAL(&spinlock);
portEXIT_CRITICAL_SAFE(&spinlock);
return ESP_OK;
}

View File

@ -264,7 +264,7 @@ void IRAM_ATTR esp_pm_impl_switch_mode(pm_mode_t mode,
{
bool need_switch = false;
uint32_t mode_mask = BIT(mode);
portENTER_CRITICAL(&s_switch_lock);
portENTER_CRITICAL_SAFE(&s_switch_lock);
uint32_t count;
if (lock_or_unlock == MODE_LOCK) {
count = ++s_mode_lock_counts[mode];
@ -291,7 +291,7 @@ void IRAM_ATTR esp_pm_impl_switch_mode(pm_mode_t mode,
s_last_mode_change_time = now;
#endif // WITH_PROFILING
}
portEXIT_CRITICAL(&s_switch_lock);
portEXIT_CRITICAL_SAFE(&s_switch_lock);
if (need_switch && new_mode != s_mode) {
do_switch(new_mode);
}

View File

@ -111,7 +111,7 @@ esp_err_t IRAM_ATTR esp_pm_lock_acquire(esp_pm_lock_handle_t handle)
return ESP_ERR_INVALID_ARG;
}
portENTER_CRITICAL(&handle->spinlock);
portENTER_CRITICAL_SAFE(&handle->spinlock);
if (handle->count++ == 0) {
pm_time_t now = 0;
#ifdef WITH_PROFILING
@ -123,7 +123,7 @@ esp_err_t IRAM_ATTR esp_pm_lock_acquire(esp_pm_lock_handle_t handle)
handle->times_taken++;
#endif
}
portEXIT_CRITICAL(&handle->spinlock);
portEXIT_CRITICAL_SAFE(&handle->spinlock);
return ESP_OK;
}
@ -137,7 +137,7 @@ esp_err_t IRAM_ATTR esp_pm_lock_release(esp_pm_lock_handle_t handle)
return ESP_ERR_INVALID_ARG;
}
esp_err_t ret = ESP_OK;
portENTER_CRITICAL(&handle->spinlock);
portENTER_CRITICAL_SAFE(&handle->spinlock);
if (handle->count == 0) {
ret = ESP_ERR_INVALID_STATE;
goto out;
@ -151,11 +151,10 @@ esp_err_t IRAM_ATTR esp_pm_lock_release(esp_pm_lock_handle_t handle)
esp_pm_impl_switch_mode(handle->mode, MODE_UNLOCK, now);
}
out:
portEXIT_CRITICAL(&handle->spinlock);
portEXIT_CRITICAL_SAFE(&handle->spinlock);
return ret;
}
esp_err_t esp_pm_dump_locks(FILE* stream)
{
#ifndef CONFIG_PM_ENABLE
@ -201,5 +200,3 @@ esp_err_t esp_pm_dump_locks(FILE* stream)
#endif
return ESP_OK;
}

View File

@ -419,4 +419,19 @@ menu "FreeRTOS"
abort the application. This option is also required for GDB backtraces and C++
exceptions to work correctly inside top-level task functions.
config FREERTOS_CHECK_MUTEX_GIVEN_BY_OWNER
bool "Check that mutex semaphore is given by owner task"
default y
help
If enabled, assert that when a mutex semaphore is given, the task giving the
semaphore is the task which is currently holding the mutex.
config FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE
bool "Tests compliance with Vanilla FreeRTOS port*_CRITICAL calls"
default n
help
If enabled, context of port*_CRITICAL calls (ISR or Non-ISR)
would be checked to be in compliance with Vanilla FreeRTOS.
e.g Calling port*_CRITICAL from ISR context would cause assert failure
endmenu

View File

@ -209,8 +209,34 @@ void vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *function, int line);
void vTaskEnterCritical( portMUX_TYPE *mux, const char *function, int line );
void vTaskExitCritical( portMUX_TYPE *mux, const char *function, int line );
#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE
/* Calling port*_CRITICAL from ISR context would cause an assert failure.
* If the parent function is called from both ISR and Non-ISR context then call port*_CRITICAL_SAFE
*/
#define portENTER_CRITICAL(mux) do { \
if(!xPortInIsrContext()) { \
vTaskEnterCritical(mux, __FUNCTION__, __LINE__); \
} else { \
ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \
__FUNCTION__); \
abort(); \
} \
} while(0)
#define portEXIT_CRITICAL(mux) do { \
if(!xPortInIsrContext()) { \
vTaskExitCritical(mux, __FUNCTION__, __LINE__); \
} else { \
ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \
__FUNCTION__); \
abort(); \
} \
} while(0)
#else
#define portENTER_CRITICAL(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__)
#define portEXIT_CRITICAL(mux) vTaskExitCritical(mux, __FUNCTION__, __LINE__)
#endif
#define portENTER_CRITICAL_ISR(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__)
#define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux, __FUNCTION__, __LINE__)
#else
@ -229,12 +255,54 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux);
bool vPortCPUAcquireMutexTimeout(portMUX_TYPE *mux, int timeout_cycles);
void vPortCPUReleaseMutex(portMUX_TYPE *mux);
#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE
/* Calling port*_CRITICAL from ISR context would cause an assert failure.
* If the parent function is called from both ISR and Non-ISR context then call port*_CRITICAL_SAFE
*/
#define portENTER_CRITICAL(mux) do { \
if(!xPortInIsrContext()) { \
vTaskEnterCritical(mux); \
} else { \
ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \
__FUNCTION__); \
abort(); \
} \
} while(0)
#define portEXIT_CRITICAL(mux) do { \
if(!xPortInIsrContext()) { \
vTaskExitCritical(mux); \
} else { \
ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \
__FUNCTION__); \
abort(); \
} \
} while(0)
#else
#define portENTER_CRITICAL(mux) vTaskEnterCritical(mux)
#define portEXIT_CRITICAL(mux) vTaskExitCritical(mux)
#endif
#define portENTER_CRITICAL_ISR(mux) vTaskEnterCritical(mux)
#define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux)
#endif
#define portENTER_CRITICAL_SAFE(mux) do { \
if (xPortInIsrContext()) { \
portENTER_CRITICAL_ISR(mux); \
} else { \
portENTER_CRITICAL(mux); \
} \
} while(0)
#define portEXIT_CRITICAL_SAFE(mux) do { \
if (xPortInIsrContext()) { \
portEXIT_CRITICAL_ISR(mux); \
} else { \
portEXIT_CRITICAL(mux); \
} \
} while(0)
// Critical section management. NW-TODO: replace XTOS_SET_INTLEVEL with more efficient version, if any?
// These cannot be nested. They should be used with a lot of care and cannot be called from interrupt level.
//

View File

@ -40,8 +40,8 @@ TEST_CASE("portMUX spinlocks (no contention)", "[freertos]")
BENCHMARK_START();
for (int i = 0; i < REPEAT_OPS; i++) {
portENTER_CRITICAL(&mux);
portEXIT_CRITICAL(&mux);
portENTER_CRITICAL_ISR(&mux);
portEXIT_CRITICAL_ISR(&mux);
}
BENCHMARK_END("no contention lock");

View File

@ -377,6 +377,11 @@ The ESP-IDF FreeRTOS critical section functions have been modified as follows…
``portEXIT_CRITICAL(mux)``, ``portEXIT_CRITICAL_ISR(mux)`` are all macro
defined to call :cpp:func:`vTaskExitCritical`
- ``portENTER_CRITICAL_SAFE(mux)``, ``portEXIT_CRITICAL_SAFE(mux)`` macro identifies
the context of execution, i.e ISR or Non-ISR, and calls appropriate critical
section functions (``port*_CRITICAL`` in Non-ISR and ``port*_CRITICAL_ISR`` in ISR)
in order to be in compliance with Vanilla FreeRTOS.
For more details see :component_file:`freertos/include/freertos/portmacro.h`
and :component_file:`freertos/task.c`

View File

@ -130,10 +130,10 @@ void ref_clock_init()
static void IRAM_ATTR pcnt_isr(void* arg)
{
portENTER_CRITICAL(&s_lock);
portENTER_CRITICAL_ISR(&s_lock);
PCNT.int_clr.val = BIT(REF_CLOCK_PCNT_UNIT);
s_milliseconds += REF_CLOCK_PRESCALER_MS;
portEXIT_CRITICAL(&s_lock);
portEXIT_CRITICAL_ISR(&s_lock);
}
void ref_clock_deinit()

View File

@ -0,0 +1,2 @@
TEST_COMPONENTS=driver esp32 spi_flash
CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE=y