fix(driver_twai): fixed clock source enable/disable

Author: wanckl

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.

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);
     }
 }