From e6fcaf4e3439ffa2015b74c6248713d9ef0bb63e Mon Sep 17 00:00:00 2001 From: wanckl Date: Thu, 26 Jun 2025 12:25:43 +0800 Subject: [PATCH 1/2] fix(driver_twai): fixed c5 listenonly mode errata, add rx buffer check --- components/driver/twai/twai.c | 12 ++-- components/esp_driver_twai/esp_twai.c | 1 + components/esp_driver_twai/esp_twai_onchip.c | 25 ++++---- .../test_twai/main/test_twai_common.c | 4 +- .../hal/esp32c5/include/hal/twaifd_ll.h | 2 +- components/hal/twai_hal_ctufd.c | 18 ++++-- components/soc/esp32/twai_periph.c | 24 ++++---- components/soc/esp32c3/twai_periph.c | 24 ++++---- components/soc/esp32c5/twai_periph.c | 46 +++++++-------- components/soc/esp32c6/twai_periph.c | 40 ++++++------- components/soc/esp32h2/twai_periph.c | 22 ++++--- components/soc/esp32p4/twai_periph.c | 58 +++++++++---------- components/soc/esp32s2/twai_periph.c | 24 ++++---- components/soc/esp32s3/twai_periph.c | 24 ++++---- components/soc/include/soc/twai_periph.h | 25 ++++---- .../twai_alert_and_recovery_example_main.c | 8 +-- tools/ldgen/test/data/libsoc.a.txt | 2 +- 17 files changed, 174 insertions(+), 185 deletions(-) diff --git a/components/driver/twai/twai.c b/components/driver/twai/twai.c index 9b52101c26..9ccbc7510e 100644 --- a/components/driver/twai/twai.c +++ b/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, diff --git a/components/esp_driver_twai/esp_twai.c b/components/esp_driver_twai/esp_twai.c index e79b666c39..0cbc86e7ad 100644 --- a/components/esp_driver_twai/esp_twai.c +++ b/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); diff --git a/components/esp_driver_twai/esp_twai_onchip.c b/components/esp_driver_twai/esp_twai_onchip.c index 4cc1318f34..76b68c7972 100644 --- a/components/esp_driver_twai/esp_twai_onchip.c +++ b/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); @@ -615,7 +614,7 @@ 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"); // Enable bus clock and reset controller @@ -638,10 +637,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 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 4fa3d8db80..6039900085 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 @@ -26,7 +26,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 +89,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)); diff --git a/components/hal/esp32c5/include/hal/twaifd_ll.h b/components/hal/esp32c5/include/hal/twaifd_ll.h index 9f015cfffb..112c83324a 100644 --- a/components/hal/esp32c5/include/hal/twaifd_ll.h +++ b/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; diff --git a/components/hal/twai_hal_ctufd.c b/components/hal/twai_hal_ctufd.c index ecd12e58ba..e71cb923b5 100644 --- a/components/hal/twai_hal_ctufd.c +++ b/components/hal/twai_hal_ctufd.c @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include #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) diff --git a/components/soc/esp32/twai_periph.c b/components/soc/esp32/twai_periph.c index cf763269fe..805918142b 100644 --- a/components/soc/esp32/twai_periph.c +++ b/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, + }, }; diff --git a/components/soc/esp32c3/twai_periph.c b/components/soc/esp32c3/twai_periph.c index bae7c6ec5d..56b1167c4c 100644 --- a/components/soc/esp32c3/twai_periph.c +++ b/components/soc/esp32c3/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 */ @@ -7,16 +7,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, + }, }; diff --git a/components/soc/esp32c5/twai_periph.c b/components/soc/esp32c5/twai_periph.c index 7c3bbe6937..80e627a0b2 100644 --- a/components/soc/esp32c5/twai_periph.c +++ b/components/soc/esp32c5/twai_periph.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,27 +7,25 @@ #include "soc/twai_periph.h" #include "soc/gpio_sig_map.h" -const twai_controller_signal_conn_t twai_controller_periph_signals = { - .controllers = { - [0] = { - .module_name = "TWAI0", - .irq_id = ETS_TWAI0_INTR_SOURCE, - .timer_irq_id = ETS_TWAI0_TIMER_INTR_SOURCE, - .tx_sig = TWAI0_TX_IDX, - .rx_sig = TWAI0_RX_IDX, - .bus_off_sig = TWAI0_BUS_OFF_ON_IDX, - .clk_out_sig = TWAI0_CLKOUT_IDX, - .stand_by_sig = TWAI0_STANDBY_IDX, - }, - [1] = { - .module_name = "TWAI1", - .irq_id = ETS_TWAI1_INTR_SOURCE, - .timer_irq_id = ETS_TWAI1_TIMER_INTR_SOURCE, - .tx_sig = TWAI1_TX_IDX, - .rx_sig = TWAI1_RX_IDX, - .bus_off_sig = TWAI1_BUS_OFF_ON_IDX, - .clk_out_sig = TWAI1_CLKOUT_IDX, - .stand_by_sig = TWAI1_STANDBY_IDX, - }, - } +const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM] = { + [0] = { + .module_name = "TWAI0", + .irq_id = ETS_TWAI0_INTR_SOURCE, + .timer_irq_id = ETS_TWAI0_TIMER_INTR_SOURCE, + .tx_sig = TWAI0_TX_IDX, + .rx_sig = TWAI0_RX_IDX, + .bus_off_sig = TWAI0_BUS_OFF_ON_IDX, + .clk_out_sig = TWAI0_CLKOUT_IDX, + .stand_by_sig = TWAI0_STANDBY_IDX, + }, + [1] = { + .module_name = "TWAI1", + .irq_id = ETS_TWAI1_INTR_SOURCE, + .timer_irq_id = ETS_TWAI1_TIMER_INTR_SOURCE, + .tx_sig = TWAI1_TX_IDX, + .rx_sig = TWAI1_RX_IDX, + .bus_off_sig = TWAI1_BUS_OFF_ON_IDX, + .clk_out_sig = TWAI1_CLKOUT_IDX, + .stand_by_sig = TWAI1_STANDBY_IDX, + }, }; diff --git a/components/soc/esp32c6/twai_periph.c b/components/soc/esp32c6/twai_periph.c index 3066be7bf8..6bb1ba677d 100644 --- a/components/soc/esp32c6/twai_periph.c +++ b/components/soc/esp32c6/twai_periph.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,26 +8,24 @@ #include "soc/twai_reg.h" #include "soc/gpio_sig_map.h" -const twai_controller_signal_conn_t twai_controller_periph_signals = { - .controllers = { - [0] = { - .module = PERIPH_TWAI0_MODULE, - .irq_id = ETS_TWAI0_INTR_SOURCE, - .tx_sig = TWAI0_TX_IDX, - .rx_sig = TWAI0_RX_IDX, - .bus_off_sig = TWAI0_BUS_OFF_ON_IDX, - .clk_out_sig = TWAI0_CLKOUT_IDX, - .stand_by_sig = TWAI0_STANDBY_IDX, - }, - [1] = { - .module = PERIPH_TWAI1_MODULE, - .irq_id = ETS_TWAI1_INTR_SOURCE, - .tx_sig = TWAI1_TX_IDX, - .rx_sig = TWAI1_RX_IDX, - .bus_off_sig = TWAI1_BUS_OFF_ON_IDX, - .clk_out_sig = TWAI1_CLKOUT_IDX, - .stand_by_sig = TWAI1_STANDBY_IDX, - } +const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM] = { + [0] = { + .module_name = "TWAI0", + .irq_id = ETS_TWAI0_INTR_SOURCE, + .tx_sig = TWAI0_TX_IDX, + .rx_sig = TWAI0_RX_IDX, + .bus_off_sig = TWAI0_BUS_OFF_ON_IDX, + .clk_out_sig = TWAI0_CLKOUT_IDX, + .stand_by_sig = TWAI0_STANDBY_IDX, + }, + [1] = { + .module_name = "TWAI1", + .irq_id = ETS_TWAI1_INTR_SOURCE, + .tx_sig = TWAI1_TX_IDX, + .rx_sig = TWAI1_RX_IDX, + .bus_off_sig = TWAI1_BUS_OFF_ON_IDX, + .clk_out_sig = TWAI1_CLKOUT_IDX, + .stand_by_sig = TWAI1_STANDBY_IDX, } }; diff --git a/components/soc/esp32h2/twai_periph.c b/components/soc/esp32h2/twai_periph.c index b81202a741..becf1e7928 100644 --- a/components/soc/esp32h2/twai_periph.c +++ b/components/soc/esp32h2/twai_periph.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,17 +8,15 @@ #include "soc/twai_reg.h" #include "soc/gpio_sig_map.h" -const twai_controller_signal_conn_t twai_controller_periph_signals = { - .controllers = { - [0] = { - .module = PERIPH_TWAI0_MODULE, - .irq_id = ETS_TWAI0_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 = TWAI_STANDBY_IDX, - } +const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM] = { + [0] = { + .module_name = "TWAI0", + .irq_id = ETS_TWAI0_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 = TWAI_STANDBY_IDX, } }; diff --git a/components/soc/esp32p4/twai_periph.c b/components/soc/esp32p4/twai_periph.c index b9fde3d047..7ca5639817 100644 --- a/components/soc/esp32p4/twai_periph.c +++ b/components/soc/esp32p4/twai_periph.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,35 +8,33 @@ #include "soc/twai_reg.h" #include "soc/gpio_sig_map.h" -const twai_controller_signal_conn_t twai_controller_periph_signals = { - .controllers = { - [0] = { - .module = PERIPH_TWAI0_MODULE, - .irq_id = ETS_TWAI0_INTR_SOURCE, - .tx_sig = TWAI0_TX_PAD_OUT_IDX, - .rx_sig = TWAI0_RX_PAD_IN_IDX, - .bus_off_sig = TWAI0_BUS_OFF_ON_PAD_OUT_IDX, - .clk_out_sig = TWAI0_CLKOUT_PAD_OUT_IDX, - .stand_by_sig = TWAI0_STANDBY_PAD_OUT_IDX, - }, - [1] = { - .module = PERIPH_TWAI1_MODULE, - .irq_id = ETS_TWAI1_INTR_SOURCE, - .tx_sig = TWAI1_TX_PAD_OUT_IDX, - .rx_sig = TWAI1_RX_PAD_IN_IDX, - .bus_off_sig = TWAI1_BUS_OFF_ON_PAD_OUT_IDX, - .clk_out_sig = TWAI1_CLKOUT_PAD_OUT_IDX, - .stand_by_sig = TWAI1_STANDBY_PAD_OUT_IDX, - }, - [2] = { - .module = PERIPH_TWAI2_MODULE, - .irq_id = ETS_TWAI2_INTR_SOURCE, - .tx_sig = TWAI2_TX_PAD_OUT_IDX, - .rx_sig = TWAI2_RX_PAD_IN_IDX, - .bus_off_sig = TWAI2_BUS_OFF_ON_PAD_OUT_IDX, - .clk_out_sig = TWAI2_CLKOUT_PAD_OUT_IDX, - .stand_by_sig = TWAI2_STANDBY_PAD_OUT_IDX, - } +const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM] = { + [0] = { + .module_name = "TWAI0", + .irq_id = ETS_TWAI0_INTR_SOURCE, + .tx_sig = TWAI0_TX_PAD_OUT_IDX, + .rx_sig = TWAI0_RX_PAD_IN_IDX, + .bus_off_sig = TWAI0_BUS_OFF_ON_PAD_OUT_IDX, + .clk_out_sig = TWAI0_CLKOUT_PAD_OUT_IDX, + .stand_by_sig = TWAI0_STANDBY_PAD_OUT_IDX, + }, + [1] = { + .module_name = "TWAI1", + .irq_id = ETS_TWAI1_INTR_SOURCE, + .tx_sig = TWAI1_TX_PAD_OUT_IDX, + .rx_sig = TWAI1_RX_PAD_IN_IDX, + .bus_off_sig = TWAI1_BUS_OFF_ON_PAD_OUT_IDX, + .clk_out_sig = TWAI1_CLKOUT_PAD_OUT_IDX, + .stand_by_sig = TWAI1_STANDBY_PAD_OUT_IDX, + }, + [2] = { + .module_name = "TWAI2", + .irq_id = ETS_TWAI2_INTR_SOURCE, + .tx_sig = TWAI2_TX_PAD_OUT_IDX, + .rx_sig = TWAI2_RX_PAD_IN_IDX, + .bus_off_sig = TWAI2_BUS_OFF_ON_PAD_OUT_IDX, + .clk_out_sig = TWAI2_CLKOUT_PAD_OUT_IDX, + .stand_by_sig = TWAI2_STANDBY_PAD_OUT_IDX, } }; diff --git a/components/soc/esp32s2/twai_periph.c b/components/soc/esp32s2/twai_periph.c index bae7c6ec5d..56b1167c4c 100644 --- a/components/soc/esp32s2/twai_periph.c +++ b/components/soc/esp32s2/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 */ @@ -7,16 +7,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, + }, }; diff --git a/components/soc/esp32s3/twai_periph.c b/components/soc/esp32s3/twai_periph.c index bae7c6ec5d..56b1167c4c 100644 --- a/components/soc/esp32s3/twai_periph.c +++ b/components/soc/esp32s3/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 */ @@ -7,16 +7,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, + }, }; diff --git a/components/soc/include/soc/twai_periph.h b/components/soc/include/soc/twai_periph.h index eac51da93e..0e1f0225bd 100644 --- a/components/soc/include/soc/twai_periph.h +++ b/components/soc/include/soc/twai_periph.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -21,22 +21,19 @@ extern "C" { #if SOC_TWAI_SUPPORTED typedef struct { - struct { - const periph_module_t module; // peripheral module - const char *module_name; // peripheral name - const int irq_id; // interrupt source ID + const char *module_name; // peripheral name + const int irq_id; // interrupt source ID #if SOC_TWAI_SUPPORT_TIMESTAMP - const int timer_irq_id; + const int timer_irq_id; #endif - const int tx_sig; // TX signal ID in GPIO matrix - const int rx_sig; // RX signal ID in GPIO matrix - const int clk_out_sig; // CLK_OUT signal ID in GPIO matrix - const int bus_off_sig; // BUS_OFF status signal ID in GPIO matrix - const int stand_by_sig; // STAND_BY signal ID in GPIO matrix - } controllers[SOC_TWAI_CONTROLLER_NUM]; -} twai_controller_signal_conn_t; + const int tx_sig; // TX signal ID in GPIO matrix + const int rx_sig; // RX signal ID in GPIO matrix + const int clk_out_sig; // CLK_OUT signal ID in GPIO matrix + const int bus_off_sig; // BUS_OFF status signal ID in GPIO matrix + const int stand_by_sig; // STAND_BY signal ID in GPIO matrix +} twai_signal_conn_t; -extern const twai_controller_signal_conn_t twai_controller_periph_signals; +extern const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM]; #if SOC_PAU_SUPPORTED typedef struct { diff --git a/examples/peripherals/twai/twai_alert_and_recovery/main/twai_alert_and_recovery_example_main.c b/examples/peripherals/twai/twai_alert_and_recovery/main/twai_alert_and_recovery_example_main.c index 7812d10448..cfd6610a11 100644 --- a/examples/peripherals/twai/twai_alert_and_recovery/main/twai_alert_and_recovery_example_main.c +++ b/examples/peripherals/twai/twai_alert_and_recovery/main/twai_alert_and_recovery_example_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2010-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2010-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: CC0-1.0 */ @@ -60,15 +60,15 @@ static SemaphoreHandle_t ctrl_task_sem; static bool trigger_tx_error = false; /* --------------------------- Tasks and Functions -------------------------- */ -extern const twai_controller_signal_conn_t twai_controller_periph_signals; +extern const twai_signal_conn_t twai_periph_signals[SOC_TWAI_CONTROLLER_NUM]; static void invert_tx_bits(bool enable) { if (enable) { //Inverts output of TX to trigger errors - esp_rom_gpio_connect_out_signal(TX_GPIO_NUM, twai_controller_periph_signals.controllers[0].tx_sig, true, false); + esp_rom_gpio_connect_out_signal(TX_GPIO_NUM, twai_periph_signals[0].tx_sig, true, false); } else { //Returns TX to default settings - esp_rom_gpio_connect_out_signal(TX_GPIO_NUM, twai_controller_periph_signals.controllers[0].tx_sig, false, false); + esp_rom_gpio_connect_out_signal(TX_GPIO_NUM, twai_periph_signals[0].tx_sig, false, false); } } diff --git a/tools/ldgen/test/data/libsoc.a.txt b/tools/ldgen/test/data/libsoc.a.txt index eacdf1218d..14264b0974 100644 --- a/tools/ldgen/test/data/libsoc.a.txt +++ b/tools/ldgen/test/data/libsoc.a.txt @@ -523,7 +523,7 @@ Idx Name Size VMA LMA File off Algn CONTENTS, ALLOC, LOAD, DATA 2 .bss 00000000 00000000 00000000 00000034 2**0 ALLOC - 3 .rodata.twai_controller_periph_signals 0000001c 00000000 00000000 00000034 2**2 + 3 .rodata.twai_periph_signals 0000001c 00000000 00000000 00000034 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 4 .debug_info 00000396 00000000 00000000 00000050 2**0 CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS From af6f1dcaaa6f79fcea3cbbcc4d7128aaa6797cf7 Mon Sep 17 00:00:00 2001 From: wanckl Date: Mon, 23 Jun 2025 15:14:28 +0800 Subject: [PATCH 2/2] 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); } }