From b55b7188047fe457d9f35fd69b9629b4f1c61658 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Mon, 30 Jun 2025 14:38:27 +0800 Subject: [PATCH 1/2] fix(i2c): Make i2c nack log as debug level --- components/esp_driver_i2c/i2c_master.c | 50 ++++++++++++++++---------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index 8beb655502..4cfb10fb28 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -14,7 +14,7 @@ #if CONFIG_I2C_ENABLE_DEBUG_LOG // The local log level must be defined before including esp_log.h // Set the maximum log level for this source file -#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#define LOG_LOCAL_LEVEL ESP_LOG_VERBOSE #endif #include "esp_log.h" #include "esp_intr_alloc.h" @@ -157,7 +157,7 @@ static void s_i2c_err_log_print(i2c_master_event_t event, bool bypass_nack_log) } if (bypass_nack_log != true) { if (event == I2C_EVENT_NACK) { - ESP_LOGE(TAG, "I2C transaction unexpected nack detected"); + ESP_LOGD(TAG, "I2C transaction unexpected nack detected"); } } } @@ -507,7 +507,7 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t } if (atomic_load(&i2c_master->status) == I2C_STATUS_ACK_ERROR) { - ESP_LOGE(TAG, "I2C hardware NACK detected"); + ESP_LOGD(TAG, "I2C hardware NACK detected"); const i2c_ll_hw_cmd_t hw_stop_cmd = { .op_code = I2C_LL_CMD_STOP, }; @@ -942,7 +942,10 @@ static esp_err_t s_i2c_synchronous_transaction(i2c_master_dev_handle_t i2c_dev, i2c_dev->master_bus->trans_finish = false; i2c_dev->master_bus->queue_trans = false; i2c_dev->master_bus->ack_check_disable = i2c_dev->ack_check_disable; - ESP_GOTO_ON_ERROR(s_i2c_transaction_start(i2c_dev, timeout_ms), err, TAG, "I2C transaction failed"); + ret = s_i2c_transaction_start(i2c_dev, timeout_ms); + if (ret != ESP_OK) { + goto err; + } xSemaphoreGive(i2c_dev->master_bus->bus_lock_mux); return ret; @@ -955,9 +958,6 @@ err: esp_err_t i2c_new_master_bus(const i2c_master_bus_config_t *bus_config, i2c_master_bus_handle_t *ret_bus_handle) { -#if CONFIG_I2C_ENABLE_DEBUG_LOG - esp_log_level_set(TAG, ESP_LOG_DEBUG); -#endif esp_err_t ret = ESP_OK; i2c_master_bus_t *i2c_master = NULL; i2c_port_num_t i2c_port_num = bus_config->i2c_port; @@ -1187,6 +1187,7 @@ esp_err_t i2c_master_multi_buffer_transmit(i2c_master_dev_handle_t i2c_dev, i2c_ ESP_RETURN_ON_FALSE(array_size <= (SOC_I2C_CMD_REG_NUM - 2), ESP_ERR_INVALID_ARG, TAG, "i2c command list cannot contain so many commands"); ESP_RETURN_ON_FALSE(buffer_info_array != NULL, ESP_ERR_INVALID_ARG, TAG, "buffer info array is empty"); + esp_err_t ret = ESP_OK; size_t op_index = 0; i2c_operation_t i2c_ops[SOC_I2C_CMD_REG_NUM] = {}; i2c_ops[op_index++].hw_cmd.op_code = I2C_LL_CMD_RESTART; @@ -1204,11 +1205,11 @@ esp_err_t i2c_master_multi_buffer_transmit(i2c_master_dev_handle_t i2c_dev, i2c_ i2c_ops[op_index++].hw_cmd.op_code = I2C_LL_CMD_STOP; if (i2c_dev->master_bus->async_trans == false) { - ESP_RETURN_ON_ERROR(s_i2c_synchronous_transaction(i2c_dev, i2c_ops, op_index, xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_synchronous_transaction(i2c_dev, i2c_ops, op_index, xfer_timeout_ms); } else { - ESP_RETURN_ON_ERROR(s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, op_index, xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, op_index, xfer_timeout_ms); } - return ESP_OK; + return ret; } esp_err_t i2c_master_transmit(i2c_master_dev_handle_t i2c_dev, const uint8_t *write_buffer, size_t write_size, int xfer_timeout_ms) @@ -1228,6 +1229,7 @@ esp_err_t i2c_master_transmit_receive(i2c_master_dev_handle_t i2c_dev, const uin ESP_RETURN_ON_FALSE((write_buffer != NULL) && (write_size > 0), ESP_ERR_INVALID_ARG, TAG, "i2c transmit buffer or size invalid"); ESP_RETURN_ON_FALSE((read_buffer != NULL) && (read_size > 0), ESP_ERR_INVALID_ARG, TAG, "i2c receive buffer or size invalid"); + esp_err_t ret = ESP_OK; i2c_operation_t i2c_ops[] = { {.hw_cmd = I2C_TRANS_START_COMMAND}, {.hw_cmd = I2C_TRANS_WRITE_COMMAND(i2c_dev->ack_check_disable ? false : true), .data = (uint8_t *)write_buffer, .total_bytes = write_size}, @@ -1238,17 +1240,18 @@ esp_err_t i2c_master_transmit_receive(i2c_master_dev_handle_t i2c_dev, const uin }; if (i2c_dev->master_bus->async_trans == false) { - ESP_RETURN_ON_ERROR(s_i2c_synchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_synchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms); } else { - ESP_RETURN_ON_ERROR(s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms); } - return ESP_OK; + return ret; } esp_err_t i2c_master_receive(i2c_master_dev_handle_t i2c_dev, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms) { ESP_RETURN_ON_FALSE(i2c_dev != NULL, ESP_ERR_INVALID_ARG, TAG, "i2c handle not initialized"); ESP_RETURN_ON_FALSE((read_buffer != NULL) && (read_size > 0), ESP_ERR_INVALID_ARG, TAG, "i2c receive buffer or size invalid"); + esp_err_t ret = ESP_OK; i2c_operation_t i2c_ops[] = { {.hw_cmd = I2C_TRANS_START_COMMAND}, @@ -1258,11 +1261,11 @@ esp_err_t i2c_master_receive(i2c_master_dev_handle_t i2c_dev, uint8_t *read_buff }; if (i2c_dev->master_bus->async_trans == false) { - ESP_RETURN_ON_ERROR(s_i2c_synchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_synchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms); } else { - ESP_RETURN_ON_ERROR(s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, DIM(i2c_ops), xfer_timeout_ms); } - return ESP_OK; + return ret; } esp_err_t i2c_master_probe(i2c_master_bus_handle_t bus_handle, uint16_t address, int xfer_timeout_ms) @@ -1327,6 +1330,7 @@ esp_err_t i2c_master_execute_defined_operations(i2c_master_dev_handle_t i2c_dev, ESP_RETURN_ON_FALSE(i2c_operation != NULL, ESP_ERR_INVALID_ARG, TAG, "i2c operation pointer is invalid"); ESP_RETURN_ON_FALSE(operation_list_num <= (SOC_I2C_CMD_REG_NUM), ESP_ERR_INVALID_ARG, TAG, "i2c command list cannot contain so many commands"); + esp_err_t ret = ESP_OK; i2c_operation_t i2c_ops[operation_list_num]; memset(i2c_ops, 0, sizeof(i2c_ops)); for (int i = 0; i < operation_list_num; i++) { @@ -1363,11 +1367,11 @@ esp_err_t i2c_master_execute_defined_operations(i2c_master_dev_handle_t i2c_dev, } if (i2c_dev->master_bus->async_trans == false) { - ESP_RETURN_ON_ERROR(s_i2c_synchronous_transaction(i2c_dev, i2c_ops, operation_list_num, xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_synchronous_transaction(i2c_dev, i2c_ops, operation_list_num, xfer_timeout_ms); } else { - ESP_RETURN_ON_ERROR(s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, operation_list_num, xfer_timeout_ms), TAG, "I2C transaction failed"); + ret = s_i2c_asynchronous_transaction(i2c_dev, i2c_ops, operation_list_num, xfer_timeout_ms); } - return ESP_OK; + return ret; } esp_err_t i2c_master_register_event_callbacks(i2c_master_dev_handle_t i2c_dev, const i2c_master_event_callbacks_t *cbs, void *user_data) @@ -1410,3 +1414,11 @@ esp_err_t i2c_master_bus_wait_all_done(i2c_master_bus_handle_t bus_handle, int t return ESP_OK; } + +#if CONFIG_I2C_ENABLE_DEBUG_LOG +__attribute__((constructor)) +static void i2c_master_override_default_log_level(void) +{ + esp_log_level_set(TAG, ESP_LOG_VERBOSE); +} +#endif From d77dc39bb22f0285a273e31a9a642c327a3e8c69 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Wed, 21 May 2025 12:04:23 +0800 Subject: [PATCH 2/2] fix(i2c): Add gpio reserve check on i2c driver, Closes https://github.com/espressif/esp-idf/issues/15995 --- components/esp_driver_i2c/i2c_common.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/components/esp_driver_i2c/i2c_common.c b/components/esp_driver_i2c/i2c_common.c index 12a452d550..01c506c55e 100644 --- a/components/esp_driver_i2c/i2c_common.c +++ b/components/esp_driver_i2c/i2c_common.c @@ -27,6 +27,7 @@ #include "esp_clk_tree.h" #include "clk_ctrl_os.h" #include "esp_private/gpio.h" +#include "esp_private/esp_gpio_reserve.h" #if SOC_LP_I2C_SUPPORTED #include "hal/rtc_io_ll.h" #include "driver/rtc_io.h" @@ -318,6 +319,17 @@ static esp_err_t s_hp_i2c_pins_config(i2c_bus_handle_t handle) { int port_id = handle->port_num; + // reserve the GPIO output path, because we don't expect another peripheral to signal to the same GPIO + uint64_t old_gpio_rsv_mask = esp_gpio_reserve(BIT64(handle->sda_num) | BIT64(handle->scl_num)); + // check if the GPIO is already used by others + if (old_gpio_rsv_mask & BIT64(handle->sda_num)) { + ESP_LOGW(TAG, "GPIO %d is not usable, maybe conflict with others", handle->sda_num); + } + // check if the GPIO is already used by others + if (old_gpio_rsv_mask & BIT64(handle->scl_num)) { + ESP_LOGW(TAG, "GPIO %d is not usable, maybe conflict with others", handle->scl_num); + } + // SDA pin configurations ESP_RETURN_ON_ERROR(gpio_set_level(handle->sda_num, 1), TAG, "i2c sda pin set level failed"); gpio_input_enable(handle->sda_num); @@ -356,6 +368,17 @@ static esp_err_t s_lp_i2c_pins_config(i2c_bus_handle_t handle) ESP_RETURN_ON_ERROR(!rtc_gpio_is_valid_gpio(handle->sda_num), TAG, "LP I2C SDA GPIO invalid"); ESP_RETURN_ON_ERROR(!rtc_gpio_is_valid_gpio(handle->scl_num), TAG, "LP I2C SCL GPIO invalid"); + // reserve the GPIO output path, because we don't expect another peripheral to signal to the same GPIO + uint64_t old_gpio_rsv_mask = esp_gpio_reserve(BIT64(handle->sda_num) | BIT64(handle->scl_num)); + // check if the GPIO is already used by others + if (old_gpio_rsv_mask & BIT64(handle->sda_num)) { + ESP_LOGW(TAG, "GPIO %d is not usable, maybe conflict with others", handle->sda_num); + } + // check if the GPIO is already used by others + if (old_gpio_rsv_mask & BIT64(handle->scl_num)) { + ESP_LOGW(TAG, "GPIO %d is not usable, maybe conflict with others", handle->scl_num); + } + #if !SOC_LP_GPIO_MATRIX_SUPPORTED /* Verify that the SDA and SCL line belong to the LP IO Mux I2C function group */ ESP_RETURN_ON_FALSE((handle->sda_num == LP_I2C_SDA_IOMUX_PAD), ESP_ERR_INVALID_ARG, TAG, LP_I2C_SDA_PIN_ERR_LOG); @@ -418,6 +441,9 @@ esp_err_t i2c_common_deinit_pins(i2c_bus_handle_t handle) { int port_id = handle->port_num; + esp_gpio_revoke(BIT64(handle->sda_num)); + esp_gpio_revoke(BIT64(handle->scl_num)); + if (handle->is_lp_i2c == false) { ESP_RETURN_ON_ERROR(gpio_output_disable(handle->sda_num), TAG, "disable i2c pins failed"); esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ZERO_INPUT, i2c_periph_signal[port_id].sda_in_sig, 0);