From bf909303ffaf64140f69f2c4696fad82dec01d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20de=20Giessen?= Date: Tue, 22 Apr 2025 17:12:26 +0200 Subject: [PATCH] esp32/machine_timer: Do not free interrupt from ISR. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit esp_intr_free is not safe to call from the timer ISR because it requires the current task (the one the ISR interrupted) to be pinned to the same core as the interrupt was allocated on. Merely disabling the ISR however is safe since that only requires that we're currently running on the same core (which the ISR always is), regardless of the current task. This was causing deadlocks in machine_uart when the ISR happened to interrupt a task that was not pinned to a specific core. Signed-off-by: Daniël van de Giessen --- ports/esp32/machine_timer.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/ports/esp32/machine_timer.c b/ports/esp32/machine_timer.c index 34d49c79d2..6bd4fa7390 100644 --- a/ports/esp32/machine_timer.c +++ b/ports/esp32/machine_timer.c @@ -50,12 +50,13 @@ const mp_obj_type_t machine_timer_type; static mp_obj_t machine_timer_init_helper(machine_timer_obj_t *self, mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args); +static mp_obj_t machine_timer_deinit(mp_obj_t self_in); void machine_timer_deinit_all(void) { // Disable, deallocate and remove all timers from list machine_timer_obj_t **t = &MP_STATE_PORT(machine_timer_obj_head); while (*t != NULL) { - machine_timer_disable(*t); + machine_timer_deinit(*t); machine_timer_obj_t *next = (*t)->next; m_del_obj(machine_timer_obj_t, *t); *t = next; @@ -96,6 +97,7 @@ machine_timer_obj_t *machine_timer_create(mp_uint_t timer) { self = mp_obj_malloc(machine_timer_obj_t, &machine_timer_type); self->group = group; self->index = index; + self->handle = NULL; // Add the timer to the linked-list of timers self->next = MP_STATE_PORT(machine_timer_obj_head); @@ -131,9 +133,8 @@ void machine_timer_disable(machine_timer_obj_t *self) { } if (self->handle) { - // Free the interrupt handler. - esp_intr_free(self->handle); - self->handle = NULL; + // Disable the interrupt + ESP_ERROR_CHECK(esp_intr_disable(self->handle)); } // We let the disabled timer stay in the list, as it might be @@ -167,6 +168,10 @@ void machine_timer_enable(machine_timer_obj_t *self, void (*timer_isr)) { // Allocate and enable the alarm interrupt. timer_ll_enable_intr(self->hal_context.dev, TIMER_LL_EVENT_ALARM(self->index), false); timer_ll_clear_intr_status(self->hal_context.dev, TIMER_LL_EVENT_ALARM(self->index)); + if (self->handle) { + ESP_ERROR_CHECK(esp_intr_free(self->handle)); + self->handle = NULL; + } ESP_ERROR_CHECK( esp_intr_alloc(timer_group_periph_signals.groups[self->group].timer_irq_id[self->index], TIMER_FLAGS, timer_isr, self, &self->handle) @@ -233,7 +238,13 @@ static mp_obj_t machine_timer_init_helper(machine_timer_obj_t *self, mp_uint_t n } static mp_obj_t machine_timer_deinit(mp_obj_t self_in) { - machine_timer_disable(self_in); + machine_timer_obj_t *self = self_in; + + machine_timer_disable(self); + if (self->handle) { + ESP_ERROR_CHECK(esp_intr_free(self->handle)); + self->handle = NULL; + } return mp_const_none; }