Merge branch 'fix/twai_fix_clock_sel_and_c5_errata' into 'master'

fix(driver_twai): fixed clock sel and c5 listen only mode errata

Closes IDF-13144 and IDF-13059

See merge request espressif/esp-idf!40057

Author: suda-morris

components/driver/twai/twai.c

@@ -308,24 +308,24 @@ static void twai_configure_gpio(twai_obj_t *p_obj)
     //Set RX pin
     gpio_func_sel(p_obj->rx_io, PIN_FUNC_GPIO);
     gpio_input_enable(p_obj->rx_io);
-    esp_rom_gpio_connect_in_signal(p_obj->rx_io, twai_controller_periph_signals.controllers[controller_id].rx_sig, false);
+    esp_rom_gpio_connect_in_signal(p_obj->rx_io, twai_periph_signals[controller_id].rx_sig, false);
 
     //Set TX pin
     gpio_func_sel(p_obj->tx_io, PIN_FUNC_GPIO);
-    esp_rom_gpio_connect_out_signal(p_obj->tx_io, twai_controller_periph_signals.controllers[controller_id].tx_sig, false, false);
+    esp_rom_gpio_connect_out_signal(p_obj->tx_io, twai_periph_signals[controller_id].tx_sig, false, false);
 
     //Configure output clock pin (Optional)
     if (GPIO_IS_VALID_OUTPUT_GPIO(p_obj->clkout_io)) {
         gpio_mask |= BIT64(p_obj->clkout_io);
         gpio_func_sel(p_obj->clkout_io, PIN_FUNC_GPIO);
-        esp_rom_gpio_connect_out_signal(p_obj->clkout_io, twai_controller_periph_signals.controllers[controller_id].clk_out_sig, false, false);
+        esp_rom_gpio_connect_out_signal(p_obj->clkout_io, twai_periph_signals[controller_id].clk_out_sig, false, false);
     }
 
     //Configure bus status pin (Optional)
     if (GPIO_IS_VALID_OUTPUT_GPIO(p_obj->bus_off_io)) {
         gpio_mask |= BIT64(p_obj->bus_off_io);
         gpio_func_sel(p_obj->bus_off_io, PIN_FUNC_GPIO);
-        esp_rom_gpio_connect_out_signal(p_obj->bus_off_io, twai_controller_periph_signals.controllers[controller_id].bus_off_sig, false, false);
+        esp_rom_gpio_connect_out_signal(p_obj->bus_off_io, twai_periph_signals[controller_id].bus_off_sig, false, false);
     }
 
     uint64_t busy_mask = esp_gpio_reserve(gpio_mask);
@@ -343,7 +343,7 @@ static void twai_release_gpio(twai_obj_t *p_obj)
     assert(GPIO_IS_VALID_OUTPUT_GPIO(p_obj->tx_io));    //coverity check
     uint64_t gpio_mask = BIT64(p_obj->tx_io);
 
-    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, twai_controller_periph_signals.controllers[p_obj->controller_id].rx_sig, false);
+    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, twai_periph_signals[p_obj->controller_id].rx_sig, false);
     gpio_output_disable(p_obj->tx_io);
     if (GPIO_IS_VALID_OUTPUT_GPIO(p_obj->clkout_io)) {
         gpio_mask |= BIT64(p_obj->clkout_io);
@@ -413,7 +413,7 @@ static esp_err_t twai_alloc_driver_obj(const twai_general_config_t *g_config, tw
         goto err;
     }
     //Allocate interrupt
-    ret = esp_intr_alloc(twai_controller_periph_signals.controllers[controller_id].irq_id,
+    ret = esp_intr_alloc(twai_periph_signals[controller_id].irq_id,
                          g_config->intr_flags | ESP_INTR_FLAG_INTRDISABLED,
                          twai_intr_handler_main,
                          p_obj,

components/esp_driver_twai/esp_twai.c

@@ -145,6 +145,7 @@ esp_err_t twai_node_transmit(twai_node_handle_t node, const twai_frame_t *frame,
 esp_err_t twai_node_receive_from_isr(twai_node_handle_t node, twai_frame_t *rx_frame)
 {
     ESP_RETURN_ON_FALSE_ISR(node && rx_frame, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null");
+    ESP_RETURN_ON_FALSE_ISR((rx_frame->buffer_len == 0) || esp_ptr_in_dram(rx_frame->buffer) || esp_ptr_external_ram(rx_frame->buffer), ESP_ERR_INVALID_ARG, TAG, "invalid 'rx_frame->buffer' pointer or buffer_len");
     ESP_RETURN_ON_FALSE_ISR(node->receive_isr, ESP_ERR_NOT_SUPPORTED, TAG, "receive func null");
 
     return node->receive_isr(node, rx_frame);

components/esp_driver_twai/esp_twai_onchip.c

@@ -6,6 +6,7 @@
 
 #include "esp_twai.h"
 #include "esp_twai_onchip.h"
+#include "soc/gpio_sig_map.h"
 #include "esp_private/twai_interface.h"
 #include "esp_private/twai_utils.h"
 #include "twai_private.h"
@@ -127,27 +128,25 @@ static esp_err_t _node_config_io(twai_onchip_ctx_t *node, const twai_onchip_node
     uint64_t reserve_mask = BIT64(node_config->io_cfg.tx);
 
     // Set RX pin
-    gpio_input_enable(node_config->io_cfg.rx);
     gpio_set_pull_mode(node_config->io_cfg.rx, GPIO_PULLUP_ONLY);    // pullup to avoid noise if no connection to transceiver
-    gpio_func_sel(node_config->io_cfg.rx, PIN_FUNC_GPIO);
-    esp_rom_gpio_connect_in_signal(node_config->io_cfg.rx, twai_controller_periph_signals.controllers[node->ctrlr_id].rx_sig, false);
+    gpio_matrix_input(node_config->io_cfg.rx, twai_periph_signals[node->ctrlr_id].rx_sig, false);
 
     // Set TX pin
-    gpio_func_sel(node_config->io_cfg.tx, PIN_FUNC_GPIO);
-    esp_rom_gpio_connect_out_signal(node_config->io_cfg.tx, twai_controller_periph_signals.controllers[node->ctrlr_id].tx_sig, false, false);
+    // If enable_listen_only, disconnect twai signal, and output high to avoid any influence to bus
+    gpio_set_level(node_config->io_cfg.tx, 1);
+    int tx_sig = (node_config->flags.enable_listen_only) ? SIG_GPIO_OUT_IDX : twai_periph_signals[node->ctrlr_id].tx_sig;
+    gpio_matrix_output(node_config->io_cfg.tx, tx_sig, false, false);
 
     //Configure output clock pin (Optional)
     if (GPIO_IS_VALID_OUTPUT_GPIO(node_config->io_cfg.quanta_clk_out)) {
         reserve_mask |= BIT64(node_config->io_cfg.quanta_clk_out);
-        gpio_func_sel(node_config->io_cfg.quanta_clk_out, PIN_FUNC_GPIO);
-        esp_rom_gpio_connect_out_signal(node_config->io_cfg.quanta_clk_out, twai_controller_periph_signals.controllers[node->ctrlr_id].clk_out_sig, false, false);
+        gpio_matrix_output(node_config->io_cfg.quanta_clk_out, twai_periph_signals[node->ctrlr_id].clk_out_sig, false, false);
     }
 
     //Configure bus status pin (Optional)
     if (GPIO_IS_VALID_OUTPUT_GPIO(node_config->io_cfg.bus_off_indicator)) {
         reserve_mask |= BIT64(node_config->io_cfg.bus_off_indicator);
-        gpio_func_sel(node_config->io_cfg.bus_off_indicator, PIN_FUNC_GPIO);
-        esp_rom_gpio_connect_out_signal(node_config->io_cfg.bus_off_indicator, twai_controller_periph_signals.controllers[node->ctrlr_id].bus_off_sig, false, false);
+        gpio_matrix_output(node_config->io_cfg.bus_off_indicator, twai_periph_signals[node->ctrlr_id].bus_off_sig, false, false);
     }
 
     node->gpio_reserved = reserve_mask;
@@ -163,7 +162,7 @@ static esp_err_t _node_config_io(twai_onchip_ctx_t *node, const twai_onchip_node
 
 static void _node_release_io(twai_onchip_ctx_t *node)
 {
-    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, twai_controller_periph_signals.controllers[node->ctrlr_id].rx_sig, false);
+    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, twai_periph_signals[node->ctrlr_id].rx_sig, false);
     esp_gpio_revoke(node->gpio_reserved);
     for (; node->gpio_reserved > 0;) {
         uint8_t pos = __builtin_ctzll(node->gpio_reserved);
@@ -321,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;
 }
@@ -353,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);
@@ -384,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)
@@ -615,9 +634,11 @@ esp_err_t twai_new_node_onchip(const twai_onchip_node_config_t *node_config, twa
     ESP_GOTO_ON_FALSE(node->tx_mount_queue, ESP_ERR_NO_MEM, err, TAG, "no_mem");
     uint32_t intr_flags = TWAI_INTR_ALLOC_FLAGS;
     intr_flags |= (node_config->intr_priority > 0) ? BIT(node_config->intr_priority) : ESP_INTR_FLAG_LOWMED;
-    ESP_GOTO_ON_ERROR(esp_intr_alloc(twai_controller_periph_signals.controllers[ctrlr_id].irq_id, intr_flags, _node_isr_main, (void *)node, &node->intr_hdl),
+    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.
@@ -638,10 +659,10 @@ esp_err_t twai_new_node_onchip(const twai_onchip_node_config_t *node_config, twa
 #if CONFIG_PM_ENABLE
 #if SOC_TWAI_CLK_SUPPORT_APB
     // DFS can change APB frequency. So add lock to prevent sleep and APB freq from changing
-    ESP_GOTO_ON_ERROR(esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, twai_controller_periph_signals.controllers[ctrlr_id].module_name, &node->pm_lock), err, TAG, "init power manager failed");
+    ESP_GOTO_ON_ERROR(esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, twai_periph_signals[ctrlr_id].module_name, &node->pm_lock), err, TAG, "init power manager failed");
 #else // XTAL
     // XTAL freq can be closed in light sleep, so we need to create a lock to prevent light sleep
-    ESP_GOTO_ON_ERROR(esp_pm_lock_create(ESP_PM_NO_LIGHT_SLEEP, 0, twai_controller_periph_signals.controllers[ctrlr_id].module_name, &node->pm_lock), err, TAG, "init power manager failed");
+    ESP_GOTO_ON_ERROR(esp_pm_lock_create(ESP_PM_NO_LIGHT_SLEEP, 0, twai_periph_signals[ctrlr_id].module_name, &node->pm_lock), err, TAG, "init power manager failed");
 #endif //SOC_TWAI_CLK_SUPPORT_APB
 #endif //CONFIG_PM_ENABLE
 

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"
@@ -26,7 +27,7 @@
 
 static IRAM_ATTR bool test_driver_install_rx_cb(twai_node_handle_t handle, const twai_rx_done_event_data_t *edata, void *user_ctx)
 {
-    twai_frame_t rx_frame;
+    twai_frame_t rx_frame = {0};
     if (ESP_OK == twai_node_receive_from_isr(handle, &rx_frame)) {
         ESP_EARLY_LOGI("Recv ", "id 0x%lx rtr %d", rx_frame.header.id, rx_frame.header.rtr);
     }
@@ -89,7 +90,7 @@ TEST_CASE("twai install uninstall (loopback)", "[twai]")
         TEST_ESP_OK(twai_node_enable(node_hdl[SOC_TWAI_CONTROLLER_NUM]));
         tx_frame.header.id = 0x100;
         TEST_ESP_OK(twai_node_transmit(node_hdl[SOC_TWAI_CONTROLLER_NUM], &tx_frame, 0));
-        twai_frame_t rx_frame;
+        twai_frame_t rx_frame = {0};
         printf("Test receive from task\n");
         TEST_ESP_ERR(ESP_ERR_INVALID_STATE, twai_node_receive_from_isr(node_hdl[SOC_TWAI_CONTROLLER_NUM], &rx_frame));
 
@@ -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);
     }
 }
 

components/hal/esp32c5/include/hal/twaifd_ll.h

@@ -165,7 +165,7 @@ static inline void twaifd_ll_set_mode(twaifd_dev_t *hw, bool listen_only, bool s
 
     twaifd_mode_settings_reg_t opmode = {.val = hw->mode_settings.val};
     opmode.stm = self_test;
-    opmode.bmm = listen_only;
+    (void)listen_only; // listen only is not available in this chip, see "CTU FD 2v5 errata 0v2 issue 5"
     opmode.ilbp = loopback;
 
     hw->mode_settings.val = opmode.val;

components/hal/twai_hal_ctufd.c

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include <sys/param.h>
 #include "hal/twai_hal.h"
 #include "hal/twaifd_ll.h"
 
@@ -118,17 +119,24 @@ void twai_hal_format_frame(const twai_hal_trans_desc_t *trans_desc, twai_hal_fra
 {
     const twai_frame_header_t *header = trans_desc->frame.header;
     int final_dlc = (header->dlc) ? header->dlc : twaifd_len2dlc(trans_desc->frame.buffer_len);
-    int data_len = (header->dlc) ? twaifd_dlc2len(header->dlc) : trans_desc->frame.buffer_len;
     twaifd_ll_format_frame_header(header, final_dlc, frame);
-    twaifd_ll_format_frame_data(trans_desc->frame.buffer, data_len, frame);
+    if (!header->rtr) {
+        int data_len = (header->dlc) ? twaifd_dlc2len(header->dlc) : trans_desc->frame.buffer_len;
+        data_len = (header->fdf) ? MIN(data_len, TWAIFD_FRAME_MAX_LEN) : MIN(data_len, TWAI_FRAME_MAX_LEN);
+        twaifd_ll_format_frame_data(trans_desc->frame.buffer, data_len, frame);
+    }
 }
 
 void twai_hal_parse_frame(const twai_hal_frame_t *frame, twai_frame_header_t *header, uint8_t *buffer, uint8_t buffer_len)
 {
     twaifd_ll_parse_frame_header(frame, header);
-    int frame_data_len = twaifd_dlc2len(header->dlc);
-    uint8_t final_len = (frame_data_len < buffer_len) ? frame_data_len : buffer_len;
-    twaifd_ll_parse_frame_data(frame, buffer, final_len);
+    if (!header->rtr) {
+        int frame_data_len = twaifd_dlc2len(header->dlc);
+        // limit data_len for twai classic non-iso mode.
+        frame_data_len = (header->fdf) ? MIN(frame_data_len, TWAIFD_FRAME_MAX_LEN) : MIN(frame_data_len, TWAI_FRAME_MAX_LEN);
+        uint8_t final_len = MIN(frame_data_len, buffer_len);
+        twaifd_ll_parse_frame_data(frame, buffer, final_len);
+    }
 }
 
 void twai_hal_set_tx_buffer_and_transmit(twai_hal_context_t *hal_ctx, twai_hal_frame_t *tx_frame, uint8_t buffer_idx)

components/soc/esp32/twai_periph.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -8,16 +8,14 @@
 #include "soc/twai_periph.h"
 #include "soc/gpio_sig_map.h"
 
-const twai_controller_signal_conn_t twai_controller_periph_signals = {
-    .controllers = {
-        [0] = {
-            .module = PERIPH_TWAI_MODULE,
-            .irq_id = ETS_TWAI_INTR_SOURCE,
-            .tx_sig = TWAI_TX_IDX,
-            .rx_sig = TWAI_RX_IDX,
-            .bus_off_sig = TWAI_BUS_OFF_ON_IDX,
-            .clk_out_sig = TWAI_CLKOUT_IDX,
-            .stand_by_sig = -1,
-        },
-    }
+const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM] = {
+    [0] = {
+        .module_name = "TWAI0",
+        .irq_id = ETS_TWAI_INTR_SOURCE,
+        .tx_sig = TWAI_TX_IDX,
+        .rx_sig = TWAI_RX_IDX,
+        .bus_off_sig = TWAI_BUS_OFF_ON_IDX,
+        .clk_out_sig = TWAI_CLKOUT_IDX,
+        .stand_by_sig = -1,
+    },
 };

components/soc/esp32c3/twai_periph.c

@@ -1,22 +1,20 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #include "soc/twai_periph.h"
 #include "soc/gpio_sig_map.h"
 
-const twai_controller_signal_conn_t twai_controller_periph_signals = {
-    .controllers = {
-        [0] = {
-            .module = PERIPH_TWAI_MODULE,
-            .irq_id = ETS_TWAI_INTR_SOURCE,
-            .tx_sig = TWAI_TX_IDX,
-            .rx_sig = TWAI_RX_IDX,
-            .bus_off_sig = TWAI_BUS_OFF_ON_IDX,
-            .clk_out_sig = TWAI_CLKOUT_IDX,
-            .stand_by_sig = -1,
-        },
-    }
+const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM] = {
+    [0] = {
+        .module_name = "TWAI0",
+        .irq_id = ETS_TWAI_INTR_SOURCE,
+        .tx_sig = TWAI_TX_IDX,
+        .rx_sig = TWAI_RX_IDX,
+        .bus_off_sig = TWAI_BUS_OFF_ON_IDX,
+        .clk_out_sig = TWAI_CLKOUT_IDX,
+        .stand_by_sig = -1,
+    },
 };