From 9dc908d105093f7dab1b7459a5bab40087877704 Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Thu, 8 Dec 2016 12:04:26 +0800 Subject: [PATCH] Add test for local interrupts, fix int disable code --- components/esp32/include/esp_intr_alloc.h | 20 ++++---- components/esp32/intr_alloc.c | 3 +- components/esp32/test/test_intr_alloc.c | 56 +++++++++++++++++++++++ components/freertos/xtensa_intr_asm.S | 12 ++--- 4 files changed, 73 insertions(+), 18 deletions(-) diff --git a/components/esp32/include/esp_intr_alloc.h b/components/esp32/include/esp_intr_alloc.h index 02c841c3d..c1f91dd2e 100644 --- a/components/esp32/include/esp_intr_alloc.h +++ b/components/esp32/include/esp_intr_alloc.h @@ -93,7 +93,7 @@ typedef intr_handle_data_t* intr_handle_t ; * @param intno The number of the interrupt (0-31) * @param cpu CPU on which the interrupt should be marked as shared (0 or 1) * @param is_in_iram Shared interrupt is for handlers that reside in IRAM and - * the int can be left enabled while the flash cache is out. + * the int can be left enabled while the flash cache is disabled. * * @return ESP_ERR_INVALID_ARG if cpu or intno is invalid * ESP_OK otherwise @@ -131,7 +131,8 @@ esp_err_t esp_intr_reserve(int intno, int cpu); * choice of interrupts that this routine can choose from. If this value * is 0, it will default to allocating a non-shared interrupt of level * 1, 2 or 3. If this is ESP_INTR_FLAG_SHARED, it will allocate a shared - * interrupt of level 1. + * interrupt of level 1. Setting ESP_INTR_FLAG_INTRDISABLED will return + * from this function with the interrupt disabled. * @param handler The interrupt handler. Must be NULL when an interrupt of level >3 * is requested, because these types of interrupts aren't C-callable. * @param arg Optional argument for passed to the interrupt handler @@ -164,7 +165,8 @@ esp_err_t esp_intr_alloc(int source, int flags, intr_handler_t handler, void *ar * choice of interrupts that this routine can choose from. If this value * is 0, it will default to allocating a non-shared interrupt of level * 1, 2 or 3. If this is ESP_INTR_FLAG_SHARED, it will allocate a shared - * interrupt of level 1. + * interrupt of level 1. Setting ESP_INTR_FLAG_INTRDISABLED will return + * from this function with the interrupt disabled. * @param intrstatusreg The address of an interrupt status register * @param intrstatusmask A mask. If a read of address intrstatusreg has any of the bits * that are 1 in the mask set, the ISR will be called. If not, it will be @@ -186,7 +188,7 @@ esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusre /** * @brief Disable and free an interrupt. * - * Use an interrupt handle to disable the interrupt (if non-shared) and release the resources + * Use an interrupt handle to disable the interrupt and release the resources * associated with it. * * @param handle The handle, as obtained by esp_intr_alloc or esp_intr_alloc_intrstatus @@ -220,9 +222,8 @@ int esp_intr_get_intno(intr_handle_t handle); /** * @brief Disable the interrupt associated with the handle * - * @note This function can only disable non-shared inteerupts allocated on the CPU that runs this function. - * @warning Do not call this in a critical section; when the critical section ends the interrupt status - * on critical section enter may be restored. + * @note For local interrupts (ESP_INTERNAL_* sources), this function has to be called on the + * CPU the interrupt is allocated on. Other interrupts have no such restriction. * * @param handle The handle, as obtained by esp_intr_alloc or esp_intr_alloc_intrstatus * @@ -234,9 +235,8 @@ esp_err_t esp_intr_disable(intr_handle_t handle); /** * @brief Ensable the interrupt associated with the handle * - * @note This function can only enable non-shared inteerupts allocated on the CPU that runs this function. - * @warning Do not call this in a critical section; when the critical section ends the interrupt status - * on critical section enter may be restored. + * @note For local interrupts (ESP_INTERNAL_* sources), this function has to be called on the + * CPU the interrupt is allocated on. Other interrupts have no such restriction. * * @param handle The handle, as obtained by esp_intr_alloc or esp_intr_alloc_intrstatus * diff --git a/components/esp32/intr_alloc.c b/components/esp32/intr_alloc.c index 9eec38c42..f17c56bc2 100644 --- a/components/esp32/intr_alloc.c +++ b/components/esp32/intr_alloc.c @@ -437,7 +437,6 @@ esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusre { intr_handle_data_t *ret=NULL; int force=-1; -printf("Src %d reg/mask %x/%x\n", source, intrstatusreg, intrstatusmask); ESP_EARLY_LOGV(TAG, "esp_intr_alloc_intrstatus (cpu %d): checking args", xPortGetCoreID()); //Shared interrupts should be level-triggered. if ((flags&ESP_INTR_FLAG_SHARED) && (flags&ESP_INTR_FLAG_EDGE)) return ESP_ERR_INVALID_ARG; @@ -574,8 +573,8 @@ esp_err_t esp_intr_free(intr_handle_t handle) //This routine should be called from the interrupt the task is scheduled on. if (handle->vector_desc->cpu!=xPortGetCoreID()) return ESP_ERR_INVALID_ARG; - esp_intr_disable(handle); portENTER_CRITICAL(&spinlock); + esp_intr_disable(handle); if (handle->vector_desc->flags&VECDESC_FL_SHARED) { //Find and kill the shared int shared_vector_desc_t *svd=handle->vector_desc->shared_vec_info; diff --git a/components/esp32/test/test_intr_alloc.c b/components/esp32/test/test_intr_alloc.c index 30f7d4bab..390079aa4 100644 --- a/components/esp32/test/test_intr_alloc.c +++ b/components/esp32/test/test_intr_alloc.c @@ -138,6 +138,62 @@ static void timer_test(int flags) { esp_intr_free(inth[3]); } +static volatile int int_timer_ctr; + + +void int_timer_handler(void *arg) { + xthal_set_ccompare(1, xthal_get_ccount()+8000000); + int_timer_ctr++; +} + +void local_timer_test() +{ + intr_handle_t ih; + esp_err_t r; + r=esp_intr_alloc(ETS_INTERNAL_TIMER1_INTR_SOURCE, 0, int_timer_handler, NULL, &ih); + TEST_ASSERT(r==ESP_OK); + printf("Int timer 1 intno %d\n", esp_intr_get_intno(ih)); + xthal_set_ccompare(1, xthal_get_ccount()+8000000); + int_timer_ctr=0; + vTaskDelay(1000 / portTICK_RATE_MS); + printf("Timer val after 1 sec: %d\n", int_timer_ctr); + TEST_ASSERT(int_timer_ctr!=0); + printf("Disabling int\n"); + esp_intr_disable(ih); + int_timer_ctr=0; + vTaskDelay(1000 / portTICK_RATE_MS); + printf("Timer val after 1 sec: %d\n", int_timer_ctr); + TEST_ASSERT(int_timer_ctr==0); + printf("Re-enabling\n"); + esp_intr_enable(ih); + vTaskDelay(1000 / portTICK_RATE_MS); + printf("Timer val after 1 sec: %d\n", int_timer_ctr); + TEST_ASSERT(int_timer_ctr!=0); + + printf("Free int, re-alloc disabled\n"); + r=esp_intr_free(ih); + TEST_ASSERT(r==ESP_OK); + r=esp_intr_alloc(ETS_INTERNAL_TIMER1_INTR_SOURCE, ESP_INTR_FLAG_INTRDISABLED, int_timer_handler, NULL, &ih); + TEST_ASSERT(r==ESP_OK); + int_timer_ctr=0; + vTaskDelay(1000 / portTICK_RATE_MS); + printf("Timer val after 1 sec: %d\n", int_timer_ctr); + TEST_ASSERT(int_timer_ctr==0); + printf("Re-enabling\n"); + esp_intr_enable(ih); + vTaskDelay(1000 / portTICK_RATE_MS); + printf("Timer val after 1 sec: %d\n", int_timer_ctr); + TEST_ASSERT(int_timer_ctr!=0); + r=esp_intr_free(ih); + TEST_ASSERT(r==ESP_OK); + printf("Done.\n"); +} + + +TEST_CASE("Intr_alloc test, CPU-local int source", "[esp32]") +{ + local_timer_test(); +} TEST_CASE("Intr_alloc test, private ints", "[esp32]") { diff --git a/components/freertos/xtensa_intr_asm.S b/components/freertos/xtensa_intr_asm.S index aff4e3cbc..330b68f59 100644 --- a/components/freertos/xtensa_intr_asm.S +++ b/components/freertos/xtensa_intr_asm.S @@ -204,12 +204,12 @@ xt_ints_off: wsr a5, INTENABLE /* Reenable interrupts */ mov a2, a3 /* Previous mask */ #else - movi a3, 0 - xsr a3, INTENABLE /* Disables all interrupts */ - or a2, a3, a2 /* set bits in mask */ - xor a2, a3, a2 /* invert bits in mask set in mask, essentially clearing them */ - wsr a2, INTENABLE /* Re-enable ints */ - mov a2, a3 /* return prev mask */ + movi a4, 0 + xsr a4, INTENABLE /* Disables all interrupts */ + or a3, a4, a2 /* set bits in mask */ + xor a3, a3, a2 /* invert bits in mask set in mask, essentially clearing them */ + wsr a3, INTENABLE /* Re-enable ints */ + mov a2, a4 /* return prev mask */ #endif #else movi a2, 0 /* return zero */