diff --git a/components/esp_driver_twai/esp_twai_onchip.c b/components/esp_driver_twai/esp_twai_onchip.c index 11157e53126..ae528af92a4 100644 --- a/components/esp_driver_twai/esp_twai_onchip.c +++ b/components/esp_driver_twai/esp_twai_onchip.c @@ -8,6 +8,8 @@ #include "esp_twai.h" #include "esp_twai_onchip.h" #include "twai_private.h" +#include "freertos/semphr.h" +#include "freertos/timers.h" #include "esp_private/twai_interface.h" #include "esp_private/twai_utils.h" #include "esp_private/twai_frame_queue.h" @@ -322,6 +324,26 @@ static void _node_isr_main(void *arg) } } +static void _node_pended_flush_marker(void *sem, uint32_t unused) +{ + (void)unused; + xSemaphoreGive((SemaphoreHandle_t)sem); +} + +static void _node_flush_pended_set_bits(void) +{ + StaticSemaphore_t sem_storage; + SemaphoreHandle_t sem = xSemaphoreCreateBinaryStatic(&sem_storage); + assert(sem); + + // The `xEventGroupSetBitsFromISR` which used in ISR is not done immediately but just called `xTimerPendFunctionCall`, + // wait a semaphore from xTimerPendFunctionCall again is able to ensure the timer task fifo is all done. + if (xTimerPendFunctionCall(_node_pended_flush_marker, sem, 0, portMAX_DELAY) == pdPASS) { + xSemaphoreTake(sem, portMAX_DELAY); + } + vSemaphoreDelete(sem); +} + static void _node_destroy(twai_onchip_ctx_t *twai_ctx) { #ifdef CONFIG_PM_ENABLE @@ -348,6 +370,8 @@ static void _node_destroy(twai_onchip_ctx_t *twai_ctx) } twai_frame_queue_del(twai_ctx->tx_queue); if (twai_ctx->event_group) { + // xEventGroupSetBitsFromISR is not done immediately, need flush it before deleting + _node_flush_pended_set_bits(); vEventGroupDeleteWithCaps(twai_ctx->event_group); } if (twai_ctx->ctrlr_id != -1) { @@ -767,7 +791,7 @@ esp_err_t twai_new_node_onchip(const twai_onchip_node_config_t *node_config, twa #endif //CONFIG_PM_ENABLE // Set clock source, enable bus clock and reset controller - ESP_RETURN_ON_ERROR(esp_clk_tree_enable_src(node->curr_clk_src, true), TAG, "enable clock source failed"); + ESP_GOTO_ON_ERROR(esp_clk_tree_enable_src(node->curr_clk_src, true), err, TAG, "enable clock source failed"); ESP_LOGD(TAG, "set clock source to %d, freq: %ld Hz", node->curr_clk_src, node->src_freq_hz); _twai_rcc_clock_sel(node->ctrlr_id, node->curr_clk_src); _twai_rcc_clock_ctrl(ctrlr_id, true); diff --git a/components/esp_driver_twai/include/esp_twai.h b/components/esp_driver_twai/include/esp_twai.h index 8a99ee75b22..8ad74731ec9 100644 --- a/components/esp_driver_twai/include/esp_twai.h +++ b/components/esp_driver_twai/include/esp_twai.h @@ -44,6 +44,7 @@ esp_err_t twai_node_recover(twai_node_handle_t node); /** * @brief Delete the TWAI node and release resources + * @note Do NOT call this function in the FreeRTOS timer task. * * @param node Handle to the TWAI node * @return - ESP_OK: Success diff --git a/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.cpp b/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.cpp index 2ec91804c6c..54fe2602c30 100644 --- a/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.cpp +++ b/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.cpp @@ -999,3 +999,65 @@ TEST_CASE("twai schedule transmit", "[twai]") TEST_ESP_OK(twai_node_disable(node_hdl)); TEST_ESP_OK(twai_node_delete(node_hdl)); } + +static IRAM_ATTR bool test_event_tx_done_cb(twai_node_handle_t handle, const twai_tx_done_event_data_t *edata, void *user_ctx) +{ + *((bool *)user_ctx) = true; + return false; +} + +static void test_driver_event_group(void *args) +{ + twai_node_handle_t node_hdl; + twai_onchip_node_config_t node_config = {}; + node_config.io_cfg.tx = TEST_TX_GPIO; + node_config.io_cfg.rx = TEST_TX_GPIO; // Using same pin for test without transceiver + node_config.io_cfg.quanta_clk_out = GPIO_NUM_NC; + node_config.io_cfg.bus_off_indicator = GPIO_NUM_NC; + node_config.bit_timing.bitrate = 500000; + node_config.tx_queue_depth = 1; + node_config.flags.enable_loopback = true; + node_config.flags.enable_self_test = true; + TEST_ESP_OK(twai_new_node_onchip(&node_config, &node_hdl)); + + bool tx_done = false; + twai_event_callbacks_t user_cbs = {}; + user_cbs.on_tx_done = test_event_tx_done_cb; + TEST_ESP_OK(twai_node_register_event_callbacks(node_hdl, &user_cbs, &tx_done)); + TEST_ESP_OK(twai_node_enable(node_hdl)); + + twai_frame_t tx_frame = {}; + TEST_ESP_OK(twai_node_transmit(node_hdl, &tx_frame, 100)); + + // TX done ISR has pended xEventGroupSetBitsFromISR now, but this high-priority + // task get run before lower-priority timer task, so xEventGroupSetBitsFromISR not actually finish now. + while (!tx_done); + TEST_ESP_OK(twai_node_disable(node_hdl)); + TEST_ESP_OK(twai_node_delete(node_hdl)); + + // alloc some memory to let deleted node's memory got polluted + void *blocks[20] = {}; + for (int i = 1; i < 20; i++) { + blocks[i] = heap_caps_calloc(i, 8, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); + } + for (int i = 0; i < 20; i++) { + free(blocks[i]); + } + vTaskDelete(NULL); +} + +// twai_task twai_isr timer task +// tx | | +// | tx done | +// | trigger timer task | +// | to set event | +// delete node (low priority not run) +// (start set event +// but node deleted) +//=============================================================== +TEST_CASE("twai delete event group before setbits", "[twai]") +{ + // let twai task higher than timer task so it can run first + TEST_ASSERT_EQUAL(pdPASS, xTaskCreate(test_driver_event_group, "twai_del", 4096, NULL, configTIMER_TASK_PRIORITY + 1, NULL)); + vTaskDelay(pdMS_TO_TICKS(500)); +} diff --git a/docs/en/api-reference/peripherals/twai.rst b/docs/en/api-reference/peripherals/twai.rst index 96ae54f463c..81c9ece0267 100644 --- a/docs/en/api-reference/peripherals/twai.rst +++ b/docs/en/api-reference/peripherals/twai.rst @@ -210,6 +210,8 @@ Stopping and Deleting the Node When the TWAI node is no longer needed, you should call :cpp:func:`twai_node_delete` to release software and hardware resources. Make sure the TWAI controller is stopped before deleting the node. +Note: Due to the design of the driver, please do not call the delete function in the FreeRTOS timer task. + Advanced Features ================= diff --git a/docs/zh_CN/api-reference/peripherals/twai.rst b/docs/zh_CN/api-reference/peripherals/twai.rst index a04dd2bc94a..aa70dce09d7 100644 --- a/docs/zh_CN/api-reference/peripherals/twai.rst +++ b/docs/zh_CN/api-reference/peripherals/twai.rst @@ -210,6 +210,8 @@ TWAI 驱动支持为每个成功接收的报文创建一个 64 位的时间戳 当不再需要使用 TWAI 时,应该调用 :cpp:func:`twai_node_delete` 函数来释放软硬件资源。删除前请确保 TWAI 已经处于停止状态。 +注意:由于驱动设计的原因,请不要在 FreeRTOS 定时器任务 (Timer task) 中调用删除函数。 + 进阶功能 ========