From 34abdaea46ef44c585d1c66772cd336eac618df1 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Thu, 11 Apr 2024 15:52:54 +0800 Subject: [PATCH 1/4] fix(i2c_master): Modify the behavior from ISR WDT to return timeout when circut get shortcut, Closes https://github.com/espressif/esp-idf/issues/13587 --- components/driver/i2c/i2c_master.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index 13954c805d01..89036d172a7a 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -424,7 +424,7 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t }; i2c_ll_master_write_cmd_reg(hal->dev, hw_stop_cmd, 0); i2c_hal_master_trans_start(hal); - return; + break; } i2c_operation_t *i2c_operation = &i2c_master->i2c_trans.ops[i2c_master->trans_idx]; @@ -563,7 +563,6 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf IRAM_ATTR static void i2c_isr_receive_handler(i2c_master_bus_t *i2c_master) { i2c_hal_context_t *hal = &i2c_master->base->hal; - while(i2c_ll_is_bus_busy(hal->dev)){} if (i2c_master->status == I2C_STATUS_READ) { i2c_operation_t *i2c_operation = &i2c_master->i2c_trans.ops[i2c_master->trans_idx]; portENTER_CRITICAL_ISR(&i2c_master->base->spinlock); @@ -624,7 +623,9 @@ static void IRAM_ATTR i2c_master_isr_handler_default(void *arg) xQueueSendFromISR(i2c_master->event_queue, (void *)&i2c_master->event, &HPTaskAwoken); } if (i2c_master->contains_read == true) { - i2c_isr_receive_handler(i2c_master); + if (int_mask & I2C_LL_INTR_MST_COMPLETE || int_mask & I2C_LL_INTR_END_DETECT) { + i2c_isr_receive_handler(i2c_master); + } } if (i2c_master->async_trans) { From 5847ba0b9af96b0598aaa41406fc37e049ca0f28 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Fri, 12 Apr 2024 11:46:36 +0800 Subject: [PATCH 2/4] fix(i2c_master): Fix the issue that probe cannot work properly after a general call, Closes https://github.com/espressif/esp-idf/issues/13547 --- components/driver/i2c/i2c_master.c | 19 ++++++++++ components/driver/i2c/i2c_private.h | 3 +- .../i2c_test_apps/main/test_i2c_common.c | 35 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index 89036d172a7a..21060fa0fbb1 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -107,6 +107,18 @@ static esp_err_t s_i2c_hw_fsm_reset(i2c_master_bus_handle_t i2c_master) return ESP_OK; } +static void s_i2c_err_log_print(i2c_master_event_t event, bool bypass_nack_log) +{ + if (event == I2C_EVENT_TIMEOUT) { + ESP_LOGE(TAG, "I2C transaction timeout detected"); + } + if (bypass_nack_log != true) { + if (event == I2C_EVENT_NACK) { + ESP_LOGE(TAG, "I2C transaction unexpected nack detected"); + } + } +} + //////////////////////////////////////I2C operation functions//////////////////////////////////////////// /** * @brief This function is used to send I2C write command, which is divided in two parts. @@ -405,6 +417,7 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t // Software timeout, clear the command link and finish this transaction. i2c_master->cmd_idx = 0; i2c_master->trans_idx = 0; + atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT); return; } @@ -424,6 +437,8 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t }; i2c_ll_master_write_cmd_reg(hal->dev, hw_stop_cmd, 0); i2c_hal_master_trans_start(hal); + // The master trans start would start a transaction. + // Queue wait for the event instead of return directly. break; } @@ -446,6 +461,7 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t if (event == I2C_EVENT_DONE) { atomic_store(&i2c_master->status, I2C_STATUS_DONE); } + s_i2c_err_log_print(event, i2c_master->bypass_nack_log); } else { i2c_master->cmd_idx = 0; i2c_master->trans_idx = 0; @@ -1093,6 +1109,8 @@ esp_err_t i2c_master_probe(i2c_master_bus_handle_t bus_handle, uint16_t address, bus_handle->cmd_idx = 0; bus_handle->trans_idx = 0; bus_handle->trans_done = false; + bus_handle->status = I2C_STATUS_IDLE; + bus_handle->bypass_nack_log = true; i2c_hal_context_t *hal = &bus_handle->base->hal; i2c_operation_t i2c_ops[] = { {.hw_cmd = I2C_TRANS_START_COMMAND}, @@ -1122,6 +1140,7 @@ esp_err_t i2c_master_probe(i2c_master_bus_handle_t bus_handle, uint16_t address, // Reset the status to done, in order not influence next time transaction. bus_handle->status = I2C_STATUS_DONE; + bus_handle->bypass_nack_log = false; i2c_ll_disable_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR); xSemaphoreGive(bus_handle->bus_lock_mux); return ret; diff --git a/components/driver/i2c/i2c_private.h b/components/driver/i2c/i2c_private.h index fde194dacbf1..8d09ccdf7b4c 100644 --- a/components/driver/i2c/i2c_private.h +++ b/components/driver/i2c/i2c_private.h @@ -126,7 +126,8 @@ struct i2c_master_bus_t { bool trans_over_buffer; // Data length is more than hardware fifo length, needs interrupt. bool async_trans; // asynchronous transaction, true after callback is installed. bool ack_check_disable; // Disable ACK check - volatile bool trans_done; // transaction command finish + volatile bool trans_done; // transaction command finish + bool bypass_nack_log; // Bypass the error log. Sometimes the error is expected. SLIST_HEAD(i2c_master_device_list_head, i2c_master_device_list) device_list; // I2C device (instance) list // asnyc trans members bool async_break; // break transaction loop flag. diff --git a/components/driver/test_apps/i2c_test_apps/main/test_i2c_common.c b/components/driver/test_apps/i2c_test_apps/main/test_i2c_common.c index b3e5a15e6fe3..53ad7f4326fe 100644 --- a/components/driver/test_apps/i2c_test_apps/main/test_i2c_common.c +++ b/components/driver/test_apps/i2c_test_apps/main/test_i2c_common.c @@ -167,6 +167,41 @@ TEST_CASE("I2C master probe device test", "[i2c]") TEST_ESP_OK(i2c_del_master_bus(bus_handle)); } +TEST_CASE("probe test after general call (0x00 0x06)", "[i2c]") +{ + uint8_t data_wr[1] = { 0x06 }; + + i2c_master_bus_config_t i2c_mst_config = { + .clk_source = I2C_CLK_SRC_DEFAULT, + .i2c_port = TEST_I2C_PORT, + .scl_io_num = I2C_MASTER_SCL_IO, + .sda_io_num = I2C_MASTER_SDA_IO, + .flags.enable_internal_pullup = true, + }; + i2c_master_bus_handle_t bus_handle; + + TEST_ESP_OK(i2c_new_master_bus(&i2c_mst_config, &bus_handle)); + + i2c_device_config_t dev_cfg1 = { + .dev_addr_length = I2C_ADDR_BIT_LEN_7, + .device_address = 0x00, + .scl_speed_hz = 100000, + }; + + i2c_master_dev_handle_t dev_handle1; + TEST_ESP_OK(i2c_master_bus_add_device(bus_handle, &dev_cfg1, &dev_handle1)); + + TEST_ESP_ERR(ESP_ERR_INVALID_STATE, i2c_master_transmit(dev_handle1, data_wr, 1, 200)); + + for (int i = 1; i < 0x7f; i++) { + TEST_ESP_ERR(ESP_ERR_NOT_FOUND, i2c_master_probe(bus_handle, i, 800)); + } + + TEST_ESP_OK(i2c_master_bus_rm_device(dev_handle1)); + + TEST_ESP_OK(i2c_del_master_bus(bus_handle)); +} + #define LENGTH 48 static IRAM_ATTR bool test_master_tx_done_callback(i2c_master_dev_handle_t i2c_dev, const i2c_master_event_data_t *evt_data, void *arg) From a44f8179deceab455b8ae2f7002c9c8e295776ea Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Thu, 26 Oct 2023 14:04:00 +0800 Subject: [PATCH 3/4] refactor(i2c): Add reset and clock control to i2c ll layer --- components/driver/i2c/i2c.c | 43 +++++++++++++++---- components/driver/i2c/i2c_common.c | 16 ++++--- components/driver/i2c/i2c_master.c | 16 +++++-- components/driver/i2c/i2c_private.h | 13 ++++++ components/driver/i2c/i2c_slave.c | 4 +- components/hal/esp32/include/hal/i2c_ll.h | 46 +++++++++++++++++++++ components/hal/esp32c2/include/hal/i2c_ll.h | 33 +++++++++++++++ components/hal/esp32c3/include/hal/i2c_ll.h | 33 +++++++++++++++ components/hal/esp32c6/include/hal/i2c_ll.h | 24 +++++++++++ components/hal/esp32h2/include/hal/i2c_ll.h | 22 ++++++++++ components/hal/esp32p4/include/hal/i2c_ll.h | 45 +++++++++++++++++++- components/hal/esp32s2/include/hal/i2c_ll.h | 46 +++++++++++++++++++++ components/hal/esp32s3/include/hal/i2c_ll.h | 41 ++++++++++++++++++ components/hal/i2c_hal.c | 7 ++-- components/hal/include/hal/i2c_hal.h | 30 ++++++++++++-- 15 files changed, 392 insertions(+), 27 deletions(-) diff --git a/components/driver/i2c/i2c.c b/components/driver/i2c/i2c.c index 1177ebca8b1e..658abff1e9fc 100644 --- a/components/driver/i2c/i2c.c +++ b/components/driver/i2c/i2c.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -110,6 +110,18 @@ static const char *I2C_TAG = "i2c"; #endif #define I2C_MEM_ALLOC_CAPS_DEFAULT MALLOC_CAP_DEFAULT +#if SOC_PERIPH_CLK_CTRL_SHARED +#define I2C_CLOCK_SRC_ATOMIC() PERIPH_RCC_ATOMIC() +#else +#define I2C_CLOCK_SRC_ATOMIC() +#endif + +#if !SOC_RCC_IS_INDEPENDENT +#define I2C_RCC_ATOMIC() PERIPH_RCC_ATOMIC() +#else +#define I2C_RCC_ATOMIC() +#endif + /** * I2C bus are defined in the header files, let's check that the values are correct */ @@ -240,7 +252,9 @@ static void i2c_hw_disable(i2c_port_t i2c_num) { I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); if (i2c_context[i2c_num].hw_enabled != false) { - periph_module_disable(i2c_periph_signal[i2c_num].module); + I2C_RCC_ATOMIC() { + i2c_ll_enable_bus_clock(i2c_num, false); + } i2c_context[i2c_num].hw_enabled = false; } I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); @@ -250,7 +264,10 @@ static void i2c_hw_enable(i2c_port_t i2c_num) { I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); if (i2c_context[i2c_num].hw_enabled != true) { - periph_module_enable(i2c_periph_signal[i2c_num].module); + I2C_RCC_ATOMIC() { + i2c_ll_enable_bus_clock(i2c_num, true); + i2c_ll_reset_register(i2c_num); + } i2c_context[i2c_num].hw_enabled = true; } I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); @@ -375,7 +392,9 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_ return ESP_FAIL; } i2c_hw_enable(i2c_num); - i2c_hal_init(&i2c_context[i2c_num].hal, i2c_num); + I2C_CLOCK_SRC_ATOMIC() { + i2c_hal_init(&i2c_context[i2c_num].hal, i2c_num); + } //Disable I2C interrupt. i2c_ll_disable_intr_mask(i2c_context[i2c_num].hal.dev, I2C_LL_INTR_MASK); i2c_ll_clear_intr_mask(i2c_context[i2c_num].hal.dev, I2C_LL_INTR_MASK); @@ -478,7 +497,9 @@ esp_err_t i2c_driver_delete(i2c_port_t i2c_num) } #endif - i2c_hal_deinit(&i2c_context[i2c_num].hal); + I2C_CLOCK_SRC_ATOMIC() { + i2c_hal_deinit(&i2c_context[i2c_num].hal); + } free(p_i2c_obj[i2c_num]); p_i2c_obj[i2c_num] = NULL; @@ -746,7 +767,9 @@ esp_err_t i2c_param_config(i2c_port_t i2c_num, const i2c_config_t *i2c_conf) return ret; } i2c_hw_enable(i2c_num); - i2c_hal_init(&i2c_context[i2c_num].hal, i2c_num); + I2C_CLOCK_SRC_ATOMIC() { + i2c_hal_init(&i2c_context[i2c_num].hal, i2c_num); + } I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_ll_disable_intr_mask(i2c_context[i2c_num].hal.dev, I2C_LL_INTR_MASK); i2c_ll_clear_intr_mask(i2c_context[i2c_num].hal.dev, I2C_LL_INTR_MASK); @@ -754,7 +777,9 @@ esp_err_t i2c_param_config(i2c_port_t i2c_num, const i2c_config_t *i2c_conf) if (i2c_conf->mode == I2C_MODE_SLAVE) { //slave mode i2c_hal_slave_init(&(i2c_context[i2c_num].hal)); i2c_ll_slave_tx_auto_start_en(i2c_context[i2c_num].hal.dev, true); - i2c_ll_set_source_clk(i2c_context[i2c_num].hal.dev, src_clk); + I2C_CLOCK_SRC_ATOMIC() { + i2c_ll_set_source_clk(i2c_context[i2c_num].hal.dev, src_clk); + } i2c_ll_set_slave_addr(i2c_context[i2c_num].hal.dev, i2c_conf->slave.slave_addr, i2c_conf->slave.addr_10bit_en); i2c_ll_set_rxfifo_full_thr(i2c_context[i2c_num].hal.dev, I2C_FIFO_FULL_THRESH_VAL); i2c_ll_set_txfifo_empty_thr(i2c_context[i2c_num].hal.dev, I2C_FIFO_EMPTY_THRESH_VAL); @@ -768,7 +793,9 @@ esp_err_t i2c_param_config(i2c_port_t i2c_num, const i2c_config_t *i2c_conf) i2c_hal_master_init(&(i2c_context[i2c_num].hal)); //Default, we enable hardware filter i2c_ll_master_set_filter(i2c_context[i2c_num].hal.dev, I2C_FILTER_CYC_NUM_DEF); - i2c_hal_set_bus_timing(&(i2c_context[i2c_num].hal), i2c_conf->master.clk_speed, src_clk, s_get_src_clk_freq(src_clk)); + I2C_CLOCK_SRC_ATOMIC() { + i2c_hal_set_bus_timing(&(i2c_context[i2c_num].hal), i2c_conf->master.clk_speed, src_clk, s_get_src_clk_freq(src_clk)); + } } i2c_ll_update(i2c_context[i2c_num].hal.dev); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); diff --git a/components/driver/i2c/i2c_common.c b/components/driver/i2c/i2c_common.c index f9cb44da5944..70a3f8241462 100644 --- a/components/driver/i2c/i2c_common.c +++ b/components/driver/i2c/i2c_common.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -56,9 +56,13 @@ static esp_err_t s_i2c_bus_handle_aquire(i2c_port_num_t port_num, i2c_bus_handle bus->bus_mode = mode; // Enable the I2C module - periph_module_enable(i2c_periph_signal[port_num].module); - periph_module_reset(i2c_periph_signal[port_num].module); - i2c_hal_init(&bus->hal, port_num); + I2C_RCC_ATOMIC() { + i2c_ll_enable_bus_clock(bus->port_num, true); + i2c_ll_reset_register(bus->port_num); + } + I2C_CLOCK_SRC_ATOMIC() { + i2c_hal_init(&bus->hal, port_num); + } } } else { ESP_LOGE(TAG, "I2C bus id(%d) has already been acquired", port_num); @@ -131,7 +135,9 @@ esp_err_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus) ESP_RETURN_ON_ERROR(esp_pm_lock_delete(i2c_bus->pm_lock), TAG, "delete pm_lock failed"); } // Disable I2C module - periph_module_disable(i2c_periph_signal[port_num].module); + I2C_RCC_ATOMIC() { + i2c_ll_enable_bus_clock(port_num, false); + } free(i2c_bus); } } diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index 21060fa0fbb1..ac54ba5d0496 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -92,8 +92,9 @@ static esp_err_t s_i2c_hw_fsm_reset(i2c_master_bus_handle_t i2c_master) //to reset the I2C hw module, we need re-enable the hw s_i2c_master_clear_bus(i2c_master->base); - periph_module_disable(i2c_periph_signal[i2c_master->base->port_num].module); - periph_module_enable(i2c_periph_signal[i2c_master->base->port_num].module); + I2C_RCC_ATOMIC() { + i2c_ll_reset_register(i2c_master->base->port_num); + } i2c_hal_master_init(hal); i2c_ll_disable_intr_mask(hal->dev, I2C_LL_INTR_MASK); @@ -546,7 +547,11 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf i2c_master->read_len_static = 0; i2c_hal_master_set_scl_timeout_val(hal, i2c_dev->scl_wait_us, i2c_master->base->clk_src_freq_hz); - i2c_hal_set_bus_timing(hal, i2c_dev->scl_speed_hz, i2c_master->base->clk_src, i2c_master->base->clk_src_freq_hz); + + I2C_CLOCK_SRC_ATOMIC() { + i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); + i2c_hal_set_bus_timing(hal, i2c_dev->scl_speed_hz, i2c_master->base->clk_src, i2c_master->base->clk_src_freq_hz); + } i2c_ll_master_set_fractional_divider(hal->dev, 0, 0); i2c_ll_update(hal->dev); @@ -1125,7 +1130,10 @@ esp_err_t i2c_master_probe(i2c_master_bus_handle_t bus_handle, uint16_t address, // I2C probe does not have i2c device module. So set the clock parameter independently // This will not influence device transaction. - i2c_hal_set_bus_timing(hal, 100000, bus_handle->base->clk_src, bus_handle->base->clk_src_freq_hz); + I2C_CLOCK_SRC_ATOMIC() { + i2c_ll_set_source_clk(hal->dev, bus_handle->base->clk_src); + i2c_hal_set_bus_timing(hal, 100000, bus_handle->base->clk_src, bus_handle->base->clk_src_freq_hz); + } i2c_ll_master_set_fractional_divider(hal->dev, 0, 0); i2c_ll_enable_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR); i2c_ll_update(hal->dev); diff --git a/components/driver/i2c/i2c_private.h b/components/driver/i2c/i2c_private.h index 8d09ccdf7b4c..38776d3b9ad8 100644 --- a/components/driver/i2c/i2c_private.h +++ b/components/driver/i2c/i2c_private.h @@ -17,12 +17,25 @@ #include "freertos/task.h" #include "freertos/ringbuf.h" #include "driver/i2c_slave.h" +#include "esp_private/periph_ctrl.h" #include "esp_pm.h" #ifdef __cplusplus extern "C" { #endif +#if SOC_PERIPH_CLK_CTRL_SHARED +#define I2C_CLOCK_SRC_ATOMIC() PERIPH_RCC_ATOMIC() +#else +#define I2C_CLOCK_SRC_ATOMIC() +#endif + +#if !SOC_RCC_IS_INDEPENDENT +#define I2C_RCC_ATOMIC() PERIPH_RCC_ATOMIC() +#else +#define I2C_RCC_ATOMIC() +#endif + #if CONFIG_I2C_ISR_IRAM_SAFE #define I2C_MEM_ALLOC_CAPS (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) #else diff --git a/components/driver/i2c/i2c_slave.c b/components/driver/i2c/i2c_slave.c index 0376b6bbfddb..32070ebd78f1 100644 --- a/components/driver/i2c/i2c_slave.c +++ b/components/driver/i2c/i2c_slave.c @@ -247,7 +247,9 @@ esp_err_t i2c_new_slave_device(const i2c_slave_config_t *slave_config, i2c_slave #endif //Default, we enable hardware filter - i2c_ll_set_source_clk(hal->dev, slave_config->clk_source); + I2C_CLOCK_SRC_ATOMIC() { + i2c_ll_set_source_clk(hal->dev, slave_config->clk_source); + } bool addr_10bit_en = slave_config->addr_bit_len != I2C_ADDR_BIT_LEN_7; i2c_ll_set_slave_addr(hal->dev, slave_config->slave_addr, addr_10bit_en); #if SOC_I2C_SLAVE_SUPPORT_BROADCAST diff --git a/components/hal/esp32/include/hal/i2c_ll.h b/components/hal/esp32/include/hal/i2c_ll.h index e78a7ff32d9b..243f732756f0 100644 --- a/components/hal/esp32/include/hal/i2c_ll.h +++ b/components/hal/esp32/include/hal/i2c_ll.h @@ -13,6 +13,7 @@ #include "soc/i2c_periph.h" #include "soc/i2c_struct.h" #include "soc/clk_tree_defs.h" +#include "soc/dport_reg.h" #include "hal/i2c_types.h" #include "esp_attr.h" #include "hal/assert.h" @@ -709,6 +710,51 @@ static inline void i2c_ll_update(i2c_dev_t *hw) ;// ESP32 do not support } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + if (i2c_port == 0) { + uint32_t reg_val = DPORT_READ_PERI_REG(DPORT_PERIP_CLK_EN_REG); + reg_val &= ~DPORT_I2C_EXT0_CLK_EN; + reg_val |= enable << 7; + DPORT_WRITE_PERI_REG(DPORT_PERIP_CLK_EN_REG, reg_val); + } else if (i2c_port == 1) { + uint32_t reg_val = DPORT_READ_PERI_REG(DPORT_PERIP_CLK_EN_REG); + reg_val &= ~DPORT_I2C_EXT1_CLK_EN; + reg_val |= enable << 18; + DPORT_WRITE_PERI_REG(DPORT_PERIP_CLK_EN_REG, reg_val); + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_enable_bus_clock(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_enable_bus_clock(__VA_ARGS__);} while(0) + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + if (i2c_port == 0) { + DPORT_WRITE_PERI_REG(DPORT_PERIP_RST_EN_REG, DPORT_I2C_EXT0_RST); + DPORT_WRITE_PERI_REG(DPORT_PERIP_RST_EN_REG, 0); + } else if (i2c_port == 1) { + DPORT_WRITE_PERI_REG(DPORT_PERIP_RST_EN_REG, DPORT_I2C_EXT1_RST); + DPORT_WRITE_PERI_REG(DPORT_PERIP_RST_EN_REG, 0); + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_reset_register(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_reset_register(__VA_ARGS__);} while(0) + /** * @brief Set whether slave should auto start, or only start with start signal from master * diff --git a/components/hal/esp32c2/include/hal/i2c_ll.h b/components/hal/esp32c2/include/hal/i2c_ll.h index a7550cab6912..8fa30c225697 100644 --- a/components/hal/esp32c2/include/hal/i2c_ll.h +++ b/components/hal/esp32c2/include/hal/i2c_ll.h @@ -17,6 +17,7 @@ #include "hal/i2c_types.h" #include "soc/rtc_cntl_reg.h" #include "soc/clk_tree_defs.h" +#include "soc/system_struct.h" #include "esp_attr.h" #ifdef __cplusplus @@ -112,6 +113,38 @@ static inline void i2c_ll_update(i2c_dev_t *hw) hw->ctr.conf_upgate = 1; } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + (void)i2c_port; + SYSTEM.perip_clk_en0.i2c_ext0_clk_en = enable; +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_enable_bus_clock(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_enable_bus_clock(__VA_ARGS__);} while(0) + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + (void)i2c_port; + SYSTEM.perip_rst_en0.i2c_ext0_rst = 1; + SYSTEM.perip_rst_en0.i2c_ext0_rst = 0; +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_reset_register(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_reset_register(__VA_ARGS__);} while(0) + /** * @brief Configure the I2C bus timing related register. * diff --git a/components/hal/esp32c3/include/hal/i2c_ll.h b/components/hal/esp32c3/include/hal/i2c_ll.h index 11091748eed2..f22d15947f75 100644 --- a/components/hal/esp32c3/include/hal/i2c_ll.h +++ b/components/hal/esp32c3/include/hal/i2c_ll.h @@ -17,6 +17,7 @@ #include "hal/i2c_types.h" #include "soc/rtc_cntl_reg.h" #include "soc/clk_tree_defs.h" +#include "soc/system_struct.h" #include "esp_attr.h" #include "hal/misc.h" @@ -126,6 +127,38 @@ static inline void i2c_ll_update(i2c_dev_t *hw) hw->ctr.conf_upgate = 1; } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + (void)i2c_port; + SYSTEM.perip_clk_en0.reg_i2c_ext0_clk_en = enable; +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_enable_bus_clock(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_enable_bus_clock(__VA_ARGS__);} while(0) + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + (void)i2c_port; + SYSTEM.perip_rst_en0.reg_i2c_ext0_rst = 1; + SYSTEM.perip_rst_en0.reg_i2c_ext0_rst = 0; +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_reset_register(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_reset_register(__VA_ARGS__);} while(0) + /** * @brief Configure the I2C bus timing related register. * diff --git a/components/hal/esp32c6/include/hal/i2c_ll.h b/components/hal/esp32c6/include/hal/i2c_ll.h index 6b6aa120f154..2d465c20a433 100644 --- a/components/hal/esp32c6/include/hal/i2c_ll.h +++ b/components/hal/esp32c6/include/hal/i2c_ll.h @@ -128,6 +128,30 @@ static inline void i2c_ll_update(i2c_dev_t *hw) hw->ctr.conf_upgate = 1; } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + (void)i2c_port; + PCR.i2c_conf.i2c_clk_en = enable; +} + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + (void)i2c_port; + PCR.i2c_conf.i2c_rst_en = 1; + PCR.i2c_conf.i2c_rst_en = 0; +} + /** * @brief Configure the I2C bus timing related register. * diff --git a/components/hal/esp32h2/include/hal/i2c_ll.h b/components/hal/esp32h2/include/hal/i2c_ll.h index 26ff59a11aa3..1f21cae77c9f 100644 --- a/components/hal/esp32h2/include/hal/i2c_ll.h +++ b/components/hal/esp32h2/include/hal/i2c_ll.h @@ -127,6 +127,28 @@ static inline void i2c_ll_update(i2c_dev_t *hw) hw->ctr.conf_upgate = 1; } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + PCR.i2c[i2c_port].i2c_conf.i2c_clk_en = enable; +} + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + PCR.i2c[i2c_port].i2c_conf.i2c_rst_en = 1; + PCR.i2c[i2c_port].i2c_conf.i2c_rst_en = 0; +} + /** * @brief Configure the I2C bus timing related register. * diff --git a/components/hal/esp32p4/include/hal/i2c_ll.h b/components/hal/esp32p4/include/hal/i2c_ll.h index 88940be813dc..cb2a0af0cd5e 100644 --- a/components/hal/esp32p4/include/hal/i2c_ll.h +++ b/components/hal/esp32p4/include/hal/i2c_ll.h @@ -132,6 +132,45 @@ static inline void i2c_ll_update(i2c_dev_t *hw) hw->ctr.conf_upgate = 1; } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + if (i2c_port == 0) { + HP_SYS_CLKRST.soc_clk_ctrl2.reg_i2c0_apb_clk_en = enable; + } else if (i2c_port == 1) { + HP_SYS_CLKRST.soc_clk_ctrl2.reg_i2c1_apb_clk_en = enable; + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_enable_bus_clock(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_enable_bus_clock(__VA_ARGS__);} while(0) + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + if (i2c_port == 0) { + HP_SYS_CLKRST.hp_rst_en1.reg_rst_en_i2c0 = 1; + HP_SYS_CLKRST.hp_rst_en1.reg_rst_en_i2c0 = 0; + } else if (i2c_port == 1) { + HP_SYS_CLKRST.hp_rst_en1.reg_rst_en_i2c1 = 1; + HP_SYS_CLKRST.hp_rst_en1.reg_rst_en_i2c1 = 0; + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_reset_register(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_reset_register(__VA_ARGS__);} while(0) + /** * @brief Configure the I2C bus timing related register. * @@ -754,6 +793,10 @@ static inline void i2c_ll_set_source_clk(i2c_dev_t *hw, i2c_clock_source_t src_c } +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_set_source_clk(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_set_source_clk(__VA_ARGS__);} while(0) + /** * @brief Enable I2C peripheral controller clock * @@ -763,10 +806,8 @@ static inline void i2c_ll_set_source_clk(i2c_dev_t *hw, i2c_clock_source_t src_c static inline void i2c_ll_enable_controller_clock(i2c_dev_t *hw, bool en) { if (hw == &I2C0) { - HP_SYS_CLKRST.soc_clk_ctrl2.reg_i2c0_apb_clk_en = en; HP_SYS_CLKRST.peri_clk_ctrl10.reg_i2c0_clk_en = en; } else if (hw == &I2C1) { - HP_SYS_CLKRST.soc_clk_ctrl2.reg_i2c1_apb_clk_en = en; HP_SYS_CLKRST.peri_clk_ctrl10.reg_i2c1_clk_en = en; } else if (hw == &LP_I2C) { // Do nothing diff --git a/components/hal/esp32s2/include/hal/i2c_ll.h b/components/hal/esp32s2/include/hal/i2c_ll.h index f06851299684..f22accc1847f 100644 --- a/components/hal/esp32s2/include/hal/i2c_ll.h +++ b/components/hal/esp32s2/include/hal/i2c_ll.h @@ -11,6 +11,7 @@ #include "soc/i2c_periph.h" #include "soc/i2c_struct.h" #include "soc/clk_tree_defs.h" +#include "soc/system_reg.h" #include "hal/i2c_types.h" #include "esp_attr.h" #include "hal/misc.h" @@ -758,6 +759,51 @@ static inline void i2c_ll_update(i2c_dev_t *hw) ;// ESP32S2 do not support } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + if (i2c_port == 0) { + uint32_t reg_val = READ_PERI_REG(DPORT_PERIP_CLK_EN0_REG); + reg_val &= ~DPORT_I2C_EXT0_CLK_EN_M; + reg_val |= enable << DPORT_I2C_EXT0_CLK_EN_S; + WRITE_PERI_REG(DPORT_PERIP_CLK_EN0_REG, reg_val); + } else if (i2c_port == 1) { + uint32_t reg_val = READ_PERI_REG(DPORT_PERIP_CLK_EN0_REG); + reg_val &= ~DPORT_I2C_EXT1_CLK_EN_M; + reg_val |= enable << DPORT_I2C_EXT1_CLK_EN_S; + WRITE_PERI_REG(DPORT_PERIP_CLK_EN0_REG, reg_val); + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_enable_bus_clock(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_enable_bus_clock(__VA_ARGS__);} while(0) + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + if (i2c_port == 0) { + WRITE_PERI_REG(DPORT_PERIP_RST_EN0_REG, DPORT_I2C_EXT0_RST_M); + WRITE_PERI_REG(DPORT_PERIP_RST_EN0_REG, 0); + } else if (i2c_port == 1) { + WRITE_PERI_REG(DPORT_PERIP_RST_EN0_REG, DPORT_I2C_EXT1_RST_M); + WRITE_PERI_REG(DPORT_PERIP_RST_EN0_REG, 0); + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_reset_register(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_reset_register(__VA_ARGS__);} while(0) + /** * @brief Set whether slave should auto start, or only start with start signal from master * diff --git a/components/hal/esp32s3/include/hal/i2c_ll.h b/components/hal/esp32s3/include/hal/i2c_ll.h index f833cf9aa539..97131a888b66 100644 --- a/components/hal/esp32s3/include/hal/i2c_ll.h +++ b/components/hal/esp32s3/include/hal/i2c_ll.h @@ -15,6 +15,7 @@ #include "soc/soc_caps.h" #include "soc/i2c_struct.h" #include "soc/clk_tree_defs.h" +#include "soc/system_struct.h" #include "hal/i2c_types.h" #include "esp_attr.h" #include "esp_assert.h" @@ -126,6 +127,46 @@ static inline void i2c_ll_update(i2c_dev_t *hw) hw->ctr.conf_upgate = 1; } +/** + * @brief Enable the bus clock for I2C module + * + * @param i2c_port I2C port id + * @param enable true to enable, false to disable + */ +static inline void i2c_ll_enable_bus_clock(int i2c_port, bool enable) +{ + if (i2c_port == 0) { + SYSTEM.perip_clk_en0.i2c_ext0_clk_en = enable; + } else if (i2c_port == 1) { + SYSTEM.perip_clk_en0.i2c_ext1_clk_en = enable; + } +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_enable_bus_clock(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_enable_bus_clock(__VA_ARGS__);} while(0) + +/** + * @brief Reset the I2C module + * + * @param i2c_port Group ID + */ +static inline void i2c_ll_reset_register(int i2c_port) +{ + if (i2c_port == 0) { + SYSTEM.perip_rst_en0.i2c_ext0_rst = 1; + SYSTEM.perip_rst_en0.i2c_ext0_rst = 0; + } else if (i2c_port == 1) { + SYSTEM.perip_rst_en0.i2c_ext1_rst = 1; + SYSTEM.perip_rst_en0.i2c_ext1_rst = 0; + } + +} + +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_ll_reset_register(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; i2c_ll_reset_register(__VA_ARGS__);} while(0) + /** * @brief Configure the I2C bus timing related register. * diff --git a/components/hal/i2c_hal.c b/components/hal/i2c_hal.c index 3e43f5602785..2daa902bb721 100644 --- a/components/hal/i2c_hal.c +++ b/components/hal/i2c_hal.c @@ -22,9 +22,8 @@ void i2c_hal_slave_init(i2c_hal_context_t *hal) } #endif -void i2c_hal_set_bus_timing(i2c_hal_context_t *hal, int scl_freq, i2c_clock_source_t src_clk, int source_freq) +void _i2c_hal_set_bus_timing(i2c_hal_context_t *hal, int scl_freq, i2c_clock_source_t src_clk, int source_freq) { - i2c_ll_set_source_clk(hal->dev, src_clk); i2c_hal_clk_config_t clk_cal = {0}; i2c_ll_master_cal_bus_clk(source_freq, scl_freq, &clk_cal); i2c_ll_master_set_bus_timing(hal->dev, &clk_cal); @@ -45,7 +44,7 @@ void i2c_hal_master_init(i2c_hal_context_t *hal) i2c_ll_rxfifo_rst(hal->dev); } -void i2c_hal_init(i2c_hal_context_t *hal, int i2c_port) +void _i2c_hal_init(i2c_hal_context_t *hal, int i2c_port) { if (hal->dev == NULL) { hal->dev = I2C_LL_GET_HW(i2c_port); @@ -53,7 +52,7 @@ void i2c_hal_init(i2c_hal_context_t *hal, int i2c_port) i2c_ll_enable_controller_clock(hal->dev, true); } -void i2c_hal_deinit(i2c_hal_context_t *hal) +void _i2c_hal_deinit(i2c_hal_context_t *hal) { i2c_ll_enable_controller_clock(hal->dev, false); hal->dev = NULL; diff --git a/components/hal/include/hal/i2c_hal.h b/components/hal/include/hal/i2c_hal.h index 831c7b6f528a..b7f7518e1f4b 100644 --- a/components/hal/include/hal/i2c_hal.h +++ b/components/hal/include/hal/i2c_hal.h @@ -93,7 +93,15 @@ void i2c_hal_master_init(i2c_hal_context_t *hal); * * @return None */ -void i2c_hal_set_bus_timing(i2c_hal_context_t *hal, int scl_freq, i2c_clock_source_t src_clk, int source_freq); +void _i2c_hal_set_bus_timing(i2c_hal_context_t *hal, int scl_freq, i2c_clock_source_t src_clk, int source_freq); + +#if SOC_PERIPH_CLK_CTRL_SHARED +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_hal_set_bus_timing(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; _i2c_hal_set_bus_timing(__VA_ARGS__);} while(0) +#else +#define i2c_hal_set_bus_timing(...) _i2c_hal_set_bus_timing(__VA_ARGS__) +#endif /** * @brief I2C hardware FSM reset @@ -139,14 +147,30 @@ void i2c_hal_master_set_scl_timeout_val(i2c_hal_context_t *hal, uint32_t timeout * @param hal Context of the HAL * @param i2c_port I2C port number. */ -void i2c_hal_init(i2c_hal_context_t *hal, int i2c_port); +void _i2c_hal_init(i2c_hal_context_t *hal, int i2c_port); + +#if SOC_PERIPH_CLK_CTRL_SHARED +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_hal_init(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; _i2c_hal_init(__VA_ARGS__);} while(0) +#else +#define i2c_hal_init(...) _i2c_hal_init(__VA_ARGS__) +#endif /** * @brief Deinit I2C hal layer * * @param hal Context of the HAL */ -void i2c_hal_deinit(i2c_hal_context_t *hal); +void _i2c_hal_deinit(i2c_hal_context_t *hal); + +#if SOC_PERIPH_CLK_CTRL_SHARED +/// use a macro to wrap the function, force the caller to use it in a critical section +/// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance +#define i2c_hal_deinit(...) do {(void)__DECLARE_RCC_ATOMIC_ENV; _i2c_hal_deinit(__VA_ARGS__);} while(0) +#else +#define i2c_hal_deinit(...) _i2c_hal_deinit(__VA_ARGS__) +#endif /** * @brief Start I2C master transaction From 4ce9b783f3c658d1752d836505c0fd46d67a3d52 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Tue, 18 Jun 2024 15:01:18 +0800 Subject: [PATCH 4/4] fix(i2c): Fix i2c not release semaphore in command send loop --- components/driver/i2c/i2c_master.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index ac54ba5d0496..fbd5d4c5bc76 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -419,6 +419,8 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t i2c_master->cmd_idx = 0; i2c_master->trans_idx = 0; atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT); + ESP_LOGE(TAG, "I2C software timeout"); + xSemaphoreGive(i2c_master->cmd_semphr); return; }