From af6f1dcaaa6f79fcea3cbbcc4d7128aaa6797cf7 Mon Sep 17 00:00:00 2001 From: wanckl Date: Mon, 23 Jun 2025 15:14:28 +0800 Subject: [PATCH] fix(driver_twai): fixed clock source enable/disable --- components/esp_driver_twai/esp_twai_onchip.c | 36 +++++++++++++++---- .../test_twai/main/test_twai_common.c | 7 ++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/components/esp_driver_twai/esp_twai_onchip.c b/components/esp_driver_twai/esp_twai_onchip.c index 76b68c7972..a804a00946 100644 --- a/components/esp_driver_twai/esp_twai_onchip.c +++ b/components/esp_driver_twai/esp_twai_onchip.c @@ -320,6 +320,8 @@ static esp_err_t _node_delete(twai_node_handle_t node) _node_release_io(twai_ctx); twai_hal_deinit(twai_ctx->hal); _twai_rcc_clock_ctrl(twai_ctx->ctrlr_id, false); + // curr_clk_src must not NULL as we already set to Default in twai_new_node_onchip + ESP_RETURN_ON_ERROR(esp_clk_tree_enable_src(twai_ctx->curr_clk_src, false), TAG, "disable clock source failed"); _node_destroy(twai_ctx); return ESP_OK; } @@ -352,6 +354,30 @@ static esp_err_t _node_check_timing_valid(twai_onchip_ctx_t *twai_ctx, const twa return ESP_OK; } +static esp_err_t _node_set_clock_source(twai_node_handle_t node, twai_clock_source_t clock_src) +{ + twai_onchip_ctx_t *twai_ctx = __containerof(node, twai_onchip_ctx_t, api_base); + if (clock_src != twai_ctx->curr_clk_src) { + // Order of operations is important here. + // First enable and switch to the new clock source, then disable the old one. + // To ensure the clock to controller is continuous. + ESP_RETURN_ON_ERROR(esp_clk_tree_enable_src(clock_src, true), TAG, "enable clock source failed"); + _twai_rcc_clock_sel(twai_ctx->ctrlr_id, clock_src); + if (twai_ctx->curr_clk_src) { + // Disable previous clock source + esp_err_t err = esp_clk_tree_enable_src(twai_ctx->curr_clk_src, false); + if (err != ESP_OK) { + ESP_LOGE(TAG, "disable previous clock source failed, err: %d", err); + esp_clk_tree_enable_src(clock_src, false); + return err; + } + } + twai_ctx->curr_clk_src = clock_src; + ESP_LOGD(TAG, "set clock source to %d", clock_src); + } + return ESP_OK; +} + static esp_err_t _node_set_bit_timing(twai_node_handle_t node, const twai_timing_advanced_config_t *timing, const twai_timing_advanced_config_t *timing_fd) { twai_onchip_ctx_t *twai_ctx = __containerof(node, twai_onchip_ctx_t, api_base); @@ -383,13 +409,7 @@ static esp_err_t _node_set_bit_timing(twai_node_handle_t node, const twai_timing } #endif - if (new_clock_src != twai_ctx->curr_clk_src) { - // TODO: IDF-13144 - ESP_ERROR_CHECK(esp_clk_tree_enable_src((soc_module_clk_t)(new_clock_src), true)); - twai_ctx->curr_clk_src = new_clock_src; - _twai_rcc_clock_sel(twai_ctx->ctrlr_id, new_clock_src); - } - return ESP_OK; + return _node_set_clock_source(node, new_clock_src); } static esp_err_t _node_calc_set_bit_timing(twai_node_handle_t node, twai_clock_source_t clk_src, const twai_timing_basic_config_t *timing, const twai_timing_basic_config_t *timing_fd) @@ -617,6 +637,8 @@ esp_err_t twai_new_node_onchip(const twai_onchip_node_config_t *node_config, twa ESP_GOTO_ON_ERROR(esp_intr_alloc(twai_periph_signals[ctrlr_id].irq_id, intr_flags, _node_isr_main, (void *)node, &node->intr_hdl), err, TAG, "Alloc interrupt failed"); + // Set default clock source first + ESP_RETURN_ON_ERROR(_node_set_clock_source(&node->api_base, TWAI_CLK_SRC_DEFAULT), TAG, "enable default clock source failed"); // Enable bus clock and reset controller _twai_rcc_clock_ctrl(ctrlr_id, true); // Initialize HAL and configure register defaults. diff --git a/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.c b/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.c index 6039900085..5ab6719a54 100644 --- a/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.c +++ b/components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.c @@ -12,6 +12,7 @@ #include "esp_attr.h" #include "esp_log.h" #include "esp_heap_caps.h" +#include "esp_clk_tree.h" #include "freertos/FreeRTOS.h" #include "esp_twai.h" #include "esp_twai_onchip.h" @@ -115,6 +116,7 @@ static void test_twai_baudrate_correctness(twai_clock_source_t clk_src, uint32_t }; TEST_ESP_OK(twai_new_node_onchip(&node_config, &twai_node)); TEST_ESP_OK(twai_node_enable(twai_node)); + printf("TWAI driver installed @ %ld Hz\n", test_bitrate); // We use the UART baudrate detection submodule to measure the TWAI baudrate uart_bitrate_detect_config_t detect_config = { @@ -148,8 +150,13 @@ static void test_twai_baudrate_correctness(twai_clock_source_t clk_src, uint32_t TEST_CASE("twai baudrate measurement", "[twai]") { twai_clock_source_t twai_available_clk_srcs[] = SOC_TWAI_CLKS; + uint32_t source_freq = 0; for (size_t i = 0; i < sizeof(twai_available_clk_srcs) / sizeof(twai_available_clk_srcs[0]); i++) { + TEST_ESP_OK(esp_clk_tree_src_get_freq_hz(twai_available_clk_srcs[i], ESP_CLK_TREE_SRC_FREQ_PRECISION_APPROX, &source_freq)); + printf("Test clock source %d frequency: %ld Hz\n", twai_available_clk_srcs[i], source_freq); test_twai_baudrate_correctness(twai_available_clk_srcs[i], 200000); + + test_twai_baudrate_correctness(twai_available_clk_srcs[i], 1000000); } }