Merge branch 'fix/i2c_race_condition_etc_v5.3' into 'release/v5.3'

fix(i2c_master): Fix i2c master race condition issue, etc. (backport v5.3)

See merge request espressif/esp-idf!38266
This commit is contained in:
morris
2025-04-15 21:44:29 +08:00
3 changed files with 16 additions and 11 deletions

View File

@ -313,7 +313,7 @@ static bool s_i2c_read_command(i2c_master_bus_handle_t i2c_master, i2c_operation
i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx); i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx);
i2c_ll_master_write_cmd_reg(hal->dev, hw_end_cmd, i2c_master->cmd_idx + 1); i2c_ll_master_write_cmd_reg(hal->dev, hw_end_cmd, i2c_master->cmd_idx + 1);
portEXIT_CRITICAL_SAFE(&handle->spinlock); portEXIT_CRITICAL_SAFE(&handle->spinlock);
i2c_master->status = I2C_STATUS_READ; atomic_store(&i2c_master->status, I2C_STATUS_READ);
portENTER_CRITICAL_SAFE(&handle->spinlock); portENTER_CRITICAL_SAFE(&handle->spinlock);
if (i2c_master->async_trans == false) { if (i2c_master->async_trans == false) {
i2c_hal_master_trans_start(hal); i2c_hal_master_trans_start(hal);
@ -448,15 +448,15 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t
while (i2c_master->i2c_trans.cmd_count) { while (i2c_master->i2c_trans.cmd_count) {
if (xSemaphoreTake(i2c_master->cmd_semphr, ticks_to_wait) != pdTRUE) { if (xSemaphoreTake(i2c_master->cmd_semphr, ticks_to_wait) != pdTRUE) {
// Software timeout, clear the command link and finish this transaction. // Software timeout, clear the command link and finish this transaction.
atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT);
i2c_master->cmd_idx = 0; i2c_master->cmd_idx = 0;
i2c_master->trans_idx = 0; i2c_master->trans_idx = 0;
atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT);
ESP_LOGE(TAG, "I2C software timeout"); ESP_LOGE(TAG, "I2C software timeout");
xSemaphoreGive(i2c_master->cmd_semphr); xSemaphoreGive(i2c_master->cmd_semphr);
return; return;
} }
if (i2c_master->status == I2C_STATUS_TIMEOUT) { if (atomic_load(&i2c_master->status) == I2C_STATUS_TIMEOUT) {
s_i2c_hw_fsm_reset(i2c_master); s_i2c_hw_fsm_reset(i2c_master);
i2c_master->cmd_idx = 0; i2c_master->cmd_idx = 0;
i2c_master->trans_idx = 0; i2c_master->trans_idx = 0;
@ -465,7 +465,7 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t
return; return;
} }
if (i2c_master->status == I2C_STATUS_ACK_ERROR) { if (atomic_load(&i2c_master->status) == I2C_STATUS_ACK_ERROR) {
ESP_LOGE(TAG, "I2C hardware NACK detected"); ESP_LOGE(TAG, "I2C hardware NACK detected");
const i2c_ll_hw_cmd_t hw_stop_cmd = { const i2c_ll_hw_cmd_t hw_stop_cmd = {
.op_code = I2C_LL_CMD_STOP, .op_code = I2C_LL_CMD_STOP,
@ -565,7 +565,7 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf
TickType_t ticks_to_wait = (xfer_timeout_ms == -1) ? portMAX_DELAY : pdMS_TO_TICKS(xfer_timeout_ms); TickType_t ticks_to_wait = (xfer_timeout_ms == -1) ? portMAX_DELAY : pdMS_TO_TICKS(xfer_timeout_ms);
// Sometimes when the FSM get stuck, the ACK_ERR interrupt will occur endlessly until we reset the FSM and clear bus. // Sometimes when the FSM get stuck, the ACK_ERR interrupt will occur endlessly until we reset the FSM and clear bus.
esp_err_t ret = ESP_OK; esp_err_t ret = ESP_OK;
if (i2c_master->status == I2C_STATUS_TIMEOUT || i2c_ll_is_bus_busy(hal->dev)) { if (atomic_load(&i2c_master->status) == I2C_STATUS_TIMEOUT || i2c_ll_is_bus_busy(hal->dev)) {
ESP_RETURN_ON_ERROR(s_i2c_hw_fsm_reset(i2c_master), TAG, "reset hardware failed"); ESP_RETURN_ON_ERROR(s_i2c_hw_fsm_reset(i2c_master), TAG, "reset hardware failed");
} }
@ -599,7 +599,7 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf
} else { } else {
s_i2c_send_commands(i2c_master, ticks_to_wait); s_i2c_send_commands(i2c_master, ticks_to_wait);
// Wait event bits // Wait event bits
if (i2c_master->status != I2C_STATUS_DONE) { if (atomic_load(&i2c_master->status) != I2C_STATUS_DONE) {
ret = ESP_ERR_INVALID_STATE; ret = ESP_ERR_INVALID_STATE;
} }
// Interrupt can be disabled when on transaction finishes. // Interrupt can be disabled when on transaction finishes.
@ -619,7 +619,7 @@ IRAM_ATTR static void i2c_isr_receive_handler(i2c_master_bus_t *i2c_master)
{ {
i2c_hal_context_t *hal = &i2c_master->base->hal; i2c_hal_context_t *hal = &i2c_master->base->hal;
if (i2c_master->status == I2C_STATUS_READ) { if (atomic_load(&i2c_master->status) == I2C_STATUS_READ) {
i2c_operation_t *i2c_operation = &i2c_master->i2c_trans.ops[i2c_master->trans_idx]; i2c_operation_t *i2c_operation = &i2c_master->i2c_trans.ops[i2c_master->trans_idx];
portENTER_CRITICAL_ISR(&i2c_master->base->spinlock); portENTER_CRITICAL_ISR(&i2c_master->base->spinlock);
i2c_ll_read_rxfifo(hal->dev, i2c_operation->data + i2c_operation->bytes_used, i2c_master->rx_cnt); i2c_ll_read_rxfifo(hal->dev, i2c_operation->data + i2c_operation->bytes_used, i2c_master->rx_cnt);
@ -1214,6 +1214,8 @@ esp_err_t i2c_master_probe(i2c_master_bus_handle_t bus_handle, uint16_t address,
} }
i2c_ll_master_set_fractional_divider(hal->dev, 0, 0); i2c_ll_master_set_fractional_divider(hal->dev, 0, 0);
i2c_ll_enable_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR); i2c_ll_enable_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR);
// 20ms is sufficient for stretch, since there is no device config on probe operation.
i2c_hal_master_set_scl_timeout_val(hal, 20 * 1000, bus_handle->base->clk_src_freq_hz);
i2c_ll_update(hal->dev); i2c_ll_update(hal->dev);
s_i2c_send_commands(bus_handle, ticks_to_wait); s_i2c_send_commands(bus_handle, ticks_to_wait);

View File

@ -57,7 +57,7 @@ extern "C" {
#define I2C_ALLOW_INTR_PRIORITY_MASK ESP_INTR_FLAG_LOWMED #define I2C_ALLOW_INTR_PRIORITY_MASK ESP_INTR_FLAG_LOWMED
#define I2C_PM_LOCK_NAME_LEN_MAX 16 #define I2C_PM_LOCK_NAME_LEN_MAX 16
#define I2C_STATIC_OPERATION_ARRAY_MAX 6 #define I2C_STATIC_OPERATION_ARRAY_MAX SOC_I2C_CMD_REG_NUM
#define ACK_VAL 0 #define ACK_VAL 0
#define NACK_VAL 1 #define NACK_VAL 1

View File

@ -1,5 +1,5 @@
/* /*
* SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
* *
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
@ -202,6 +202,7 @@ esp_err_t i2c_new_slave_device(const i2c_slave_config_t *slave_config, i2c_slave
ESP_RETURN_ON_FALSE(i2c_slave, ESP_ERR_NO_MEM, TAG, "no memory for i2c slave bus"); ESP_RETURN_ON_FALSE(i2c_slave, ESP_ERR_NO_MEM, TAG, "no memory for i2c slave bus");
ESP_GOTO_ON_ERROR(i2c_acquire_bus_handle(i2c_port_num, &i2c_slave->base, I2C_BUS_MODE_SLAVE), err, TAG, "I2C bus acquire failed"); ESP_GOTO_ON_ERROR(i2c_acquire_bus_handle(i2c_port_num, &i2c_slave->base, I2C_BUS_MODE_SLAVE), err, TAG, "I2C bus acquire failed");
i2c_port_num = i2c_slave->base->port_num;
i2c_hal_context_t *hal = &i2c_slave->base->hal; i2c_hal_context_t *hal = &i2c_slave->base->hal;
i2c_slave->base->scl_num = slave_config->scl_io_num; i2c_slave->base->scl_num = slave_config->scl_io_num;
@ -287,7 +288,10 @@ err:
static esp_err_t i2c_slave_bus_destroy(i2c_slave_dev_handle_t i2c_slave) static esp_err_t i2c_slave_bus_destroy(i2c_slave_dev_handle_t i2c_slave)
{ {
if (i2c_slave) { if (i2c_slave) {
i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR); if (i2c_slave->base) {
i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR);
i2c_release_bus_handle(i2c_slave->base);
}
if (i2c_slave->slv_rx_mux) { if (i2c_slave->slv_rx_mux) {
vSemaphoreDeleteWithCaps(i2c_slave->slv_rx_mux); vSemaphoreDeleteWithCaps(i2c_slave->slv_rx_mux);
i2c_slave->slv_rx_mux = NULL; i2c_slave->slv_rx_mux = NULL;
@ -303,7 +307,6 @@ static esp_err_t i2c_slave_bus_destroy(i2c_slave_dev_handle_t i2c_slave)
if (i2c_slave->slv_evt_queue) { if (i2c_slave->slv_evt_queue) {
vQueueDeleteWithCaps(i2c_slave->slv_evt_queue); vQueueDeleteWithCaps(i2c_slave->slv_evt_queue);
} }
i2c_release_bus_handle(i2c_slave->base);
} }
free(i2c_slave); free(i2c_slave);