Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

twai: Don't abort on interrupt allocation failure (IDFGH-10231) #11494

Closed
wants to merge 1 commit into from
Closed

twai: Don't abort on interrupt allocation failure (IDFGH-10231) #11494

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions components/driver/twai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "freertos/task.h"
#include "freertos/queue.h"
#include "freertos/semphr.h"
#include "esp_check.h"
#include "esp_types.h"
#include "esp_log.h"
#include "esp_intr_alloc.h"
Expand Down Expand Up @@ -99,6 +100,8 @@ static portMUX_TYPE twai_spinlock = portMUX_INITIALIZER_UNLOCKED;

static twai_hal_context_t twai_context;

static const char *TWAI_TAG = "twai";

/* -------------------- Interrupt and Alert Handlers ------------------------ */

TWAI_ISR_ATTR static void twai_alert_handler(uint32_t alert_code, int *alert_req)
Expand Down Expand Up @@ -446,7 +449,8 @@ esp_err_t twai_driver_install(const twai_general_config_t *g_config, const twai_

//Allocate GPIO and Interrupts
twai_configure_gpio(g_config->tx_io, g_config->rx_io, g_config->clkout_io, g_config->bus_off_io);
ESP_ERROR_CHECK(esp_intr_alloc(ETS_TWAI_INTR_SOURCE, g_config->intr_flags, twai_intr_handler_main, NULL, &p_twai_obj->isr_handle));
ret = esp_intr_alloc(ETS_TWAI_INTR_SOURCE, g_config->intr_flags, twai_intr_handler_main, NULL, &p_twai_obj->isr_handle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer if we

  • Allocated interrupts immediately after allocating the driver object. This way, if any resource allocation fails, we can return early
  • Added the ESP_INTR_FLAG_INTRDISABLED flag so that an allocated interrupt remains disabled until we explicitly enable it using esp_intr_enable()

So something like...

Suggested change
ret = esp_intr_alloc(ETS_TWAI_INTR_SOURCE, g_config->intr_flags, twai_intr_handler_main, NULL, &p_twai_obj->isr_handle);
//Create a TWAI object (including queues and semaphores)
p_twai_obj_dummy = twai_alloc_driver_obj(g_config->tx_queue_len, g_config->rx_queue_len);
TWAI_CHECK(p_twai_obj_dummy != NULL, ESP_ERR_NO_MEM);
//Allocate interrupt for TWAI driver
intr_handle_t isr_hdl;
ret = esp_intr_alloc(ETS_TWAI_INTR_SOURCE, g_config->intr_flags | ESP_INTR_FLAG_INTRDISABLED, twai_intr_handler_main, NULL, &isr_hdl);
if (ret != ESP_OK) {
ESP_LOGE(TWAI_TAG, "Interrupt allocation failed");
goto intr_alloc_err;
}

ESP_GOTO_ON_ERROR(ret, late_err, TWAI_TAG, "Could not allocate an interrupt for TWAI");

#ifdef CONFIG_PM_ENABLE
ESP_ERROR_CHECK(esp_pm_lock_acquire(p_twai_obj->pm_lock)); //Acquire pm_lock to keep APB clock at 80MHz
Expand All @@ -456,6 +460,9 @@ esp_err_t twai_driver_install(const twai_general_config_t *g_config, const twai_
err:
twai_free_driver_obj(p_twai_obj_dummy);
return ret;
late_err:
twai_driver_uninstall();
return ret;
}

esp_err_t twai_driver_uninstall(void)
Expand All @@ -473,7 +480,7 @@ esp_err_t twai_driver_uninstall(void)
p_twai_obj = NULL;
TWAI_EXIT_CRITICAL();

ESP_ERROR_CHECK(esp_intr_free(p_twai_obj_dummy->isr_handle)); //Free interrupt
esp_intr_free(p_twai_obj_dummy->isr_handle); //Free interrupt

#ifdef CONFIG_PM_ENABLE
//Release and delete power management lock
Expand Down