espressif/esp-idf
1
0
Fork 1
mirror of https://github.com/espressif/esp-idf.git synced 2025-07-30 18:57:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

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

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

See merge request espressif/esp-idf!38265
This commit is contained in:
morris
2025-04-09 10:55:54 +08:00
parent 0cb892d6a2 c29d6575a1
commit 89e81dfdc9
5 changed files with 172 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
components/esp_driver_i2c/i2c_master.c
View File

@ -294,6 +294,13 @@ static bool s_i2c_read_command(i2c_master_bus_handle_t i2c_master, i2c_operation
i2c_master->read_buf_pos = i2c_master->trans_idx;
} else {
i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx);
// If the read position has not been marked, that means the transaction doesn't contain read-ack
// operation, then mark the read position in read-nack operation.
// i2c_master->read_buf_pos will never be 0.
if (i2c_master->read_buf_pos == 0) {
i2c_master->read_buf_pos = i2c_master->trans_idx;
i2c_master->read_len_static = i2c_master->rx_cnt;
}
i2c_master->cmd_idx++;
}
i2c_master->trans_idx++;
@ -331,7 +338,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_end_cmd, i2c_master->cmd_idx + 1);
portEXIT_CRITICAL_SAFE(&handle->spinlock);
i2c_master->status = I2C_STATUS_READ;
atomic_store(&i2c_master->status, I2C_STATUS_READ);
portENTER_CRITICAL_SAFE(&handle->spinlock);
if (i2c_master->async_trans == false) {
i2c_hal_master_trans_start(hal);
@ -472,15 +479,15 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t
while (i2c_master->i2c_trans.cmd_count) {
if (xSemaphoreTake(i2c_master->cmd_semphr, ticks_to_wait) != pdTRUE) {
// 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->trans_idx = 0;
atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT);
ESP_LOGE(TAG, "I2C software timeout");
xSemaphoreGive(i2c_master->cmd_semphr);
return;
}
if (i2c_master->status == I2C_STATUS_TIMEOUT) {
if (atomic_load(&i2c_master->status) == I2C_STATUS_TIMEOUT) {
s_i2c_hw_fsm_reset(i2c_master);
i2c_master->cmd_idx = 0;
i2c_master->trans_idx = 0;
@ -489,7 +496,7 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t
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");
const i2c_ll_hw_cmd_t hw_stop_cmd = {
.op_code = I2C_LL_CMD_STOP,
@ -596,7 +603,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);
// 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;
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");
}
@ -610,6 +617,7 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf
i2c_master->cmd_idx = 0;
i2c_master->rx_cnt = 0;
i2c_master->read_len_static = 0;
i2c_master->read_buf_pos = 0;
I2C_CLOCK_SRC_ATOMIC() {
i2c_hal_set_bus_timing(hal, i2c_dev->scl_speed_hz, i2c_master->base->clk_src, i2c_master->base->clk_src_freq_hz);
@ -630,7 +638,7 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf
} else {
s_i2c_send_commands(i2c_master, ticks_to_wait);
// Wait event bits
if (i2c_master->status != I2C_STATUS_DONE) {
if (atomic_load(&i2c_master->status) != I2C_STATUS_DONE) {
ret = ESP_ERR_INVALID_STATE;
}
// Interrupt can be disabled when on transaction finishes.
@ -650,7 +658,7 @@ IRAM_ATTR static void i2c_isr_receive_handler(i2c_master_bus_t *i2c_master)
{
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];
portENTER_CRITICAL_ISR(&i2c_master->base->spinlock);
i2c_ll_read_rxfifo(hal->dev, i2c_operation->data + i2c_operation->bytes_used, i2c_master->rx_cnt);
@ -673,7 +681,10 @@ IRAM_ATTR static void i2c_isr_receive_handler(i2c_master_bus_t *i2c_master)
i2c_operation_t *i2c_operation = &i2c_master->i2c_trans.ops[i2c_master->read_buf_pos];
portENTER_CRITICAL_ISR(&i2c_master->base->spinlock);
i2c_ll_read_rxfifo(hal->dev, i2c_operation->data + i2c_operation->bytes_used, i2c_master->read_len_static);
i2c_ll_read_rxfifo(hal->dev, i2c_master->i2c_trans.ops[i2c_master->read_buf_pos + 1].data, 1);
// If the read command only contain nack marker, no read it for the second time.
if (i2c_master->i2c_trans.ops[i2c_master->read_buf_pos + 1].data) {
i2c_ll_read_rxfifo(hal->dev, i2c_master->i2c_trans.ops[i2c_master->read_buf_pos + 1].data, 1);
}
i2c_master->w_r_size = i2c_master->read_len_static + 1;
i2c_master->contains_read = false;
portEXIT_CRITICAL_ISR(&i2c_master->base->spinlock);
@ -1278,6 +1289,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_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);
s_i2c_send_commands(bus_handle, ticks_to_wait);

2
components/esp_driver_i2c/i2c_private.h
View File

@ -61,7 +61,7 @@ extern "C" {
#define I2C_ALLOW_INTR_PRIORITY_MASK ESP_INTR_FLAG_LOWMED
#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 I2C_TRANS_READ_COMMAND(ack_value) {.ack_val = (ack_value), .op_code = I2C_LL_CMD_READ}
#define I2C_TRANS_WRITE_COMMAND(ack_check) {.ack_en = (ack_check), .op_code = I2C_LL_CMD_WRITE}

11
components/esp_driver_i2c/i2c_slave.c
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
*/
@ -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_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_slave->base->scl_num = slave_config->scl_io_num;
@ -293,8 +294,11 @@ err:
static esp_err_t i2c_slave_bus_destroy(i2c_slave_dev_handle_t i2c_slave)
{
if (i2c_slave) {
i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR);
i2c_common_deinit_pins(i2c_slave->base);
if (i2c_slave->base) {
i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR);
i2c_common_deinit_pins(i2c_slave->base);
i2c_release_bus_handle(i2c_slave->base);
}
if (i2c_slave->slv_rx_mux) {
vSemaphoreDeleteWithCaps(i2c_slave->slv_rx_mux);
i2c_slave->slv_rx_mux = NULL;
@ -310,7 +314,6 @@ static esp_err_t i2c_slave_bus_destroy(i2c_slave_dev_handle_t i2c_slave)
if (i2c_slave->slv_evt_queue) {
vQueueDeleteWithCaps(i2c_slave->slv_evt_queue);
}
i2c_release_bus_handle(i2c_slave->base);
}
free(i2c_slave);

12
components/esp_driver_i2c/i2c_slave_v2.c
View File

@ -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
*/
@ -207,7 +207,12 @@ IRAM_ATTR static void i2c_slave_isr_handler(void *arg)
static esp_err_t i2c_slave_device_destroy(i2c_slave_dev_handle_t i2c_slave)
{
i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR);
esp_err_t ret = ESP_OK;
if (i2c_slave->base) {
i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR);
i2c_common_deinit_pins(i2c_slave->base);
ret = i2c_release_bus_handle(i2c_slave->base);
}
if (i2c_slave->rx_ring_buf) {
vRingbufferDeleteWithCaps(i2c_slave->rx_ring_buf);
i2c_slave->rx_ring_buf = NULL;
@ -223,7 +228,6 @@ static esp_err_t i2c_slave_device_destroy(i2c_slave_dev_handle_t i2c_slave)
if (i2c_slave->receive_desc.buffer) {
free(i2c_slave->receive_desc.buffer);
}
esp_err_t ret = i2c_release_bus_handle(i2c_slave->base);
free(i2c_slave);
return ret;
@ -256,7 +260,7 @@ esp_err_t i2c_new_slave_device(const i2c_slave_config_t *slave_config, i2c_slave
i2c_slave->base->sda_num = slave_config->sda_io_num;
i2c_slave->base->pull_up_enable = slave_config->flags.enable_internal_pullup;
i2c_slave->rx_data_count = 0;
int i2c_port_num = slave_config->i2c_port;
i2c_port_num_t i2c_port_num = i2c_slave->base->port_num;
ESP_GOTO_ON_ERROR(i2c_common_set_pins(i2c_slave->base), err, TAG, "i2c slave set pins failed");
i2c_slave->rx_ring_buf = xRingbufferCreateWithCaps(slave_config->receive_buf_depth, RINGBUF_TYPE_BYTEBUF, I2C_MEM_ALLOC_CAPS);

138
components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_slave_v2.c
View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Unlicense OR CC0-1.0
*/
@ -19,6 +19,47 @@
#if SOC_I2C_SLAVE_CAN_GET_STRETCH_CAUSE
TEST_CASE("I2C peripheral allocate slave all", "[i2c]")
{
i2c_slave_dev_handle_t dev_handle[SOC_HP_I2C_NUM];
for (int i = 0; i < SOC_HP_I2C_NUM; i++) {
i2c_slave_config_t i2c_slv_config_1 = {
.clk_source = I2C_CLK_SRC_DEFAULT,
.i2c_port = -1,
.scl_io_num = I2C_SLAVE_SCL_IO,
.sda_io_num = I2C_SLAVE_SDA_IO,
.slave_addr = ESP_SLAVE_ADDR,
.send_buf_depth = DATA_LENGTH,
.receive_buf_depth = DATA_LENGTH,
.flags.enable_internal_pullup = true,
};
TEST_ESP_OK(i2c_new_slave_device(&i2c_slv_config_1, &dev_handle[i]));
}
i2c_slave_config_t i2c_slv_config_1 = {
.clk_source = I2C_CLK_SRC_DEFAULT,
.i2c_port = -1,
.scl_io_num = I2C_SLAVE_SCL_IO,
.sda_io_num = I2C_SLAVE_SDA_IO,
.slave_addr = ESP_SLAVE_ADDR,
.send_buf_depth = DATA_LENGTH,
.receive_buf_depth = DATA_LENGTH,
.flags.enable_internal_pullup = true,
};
i2c_slave_dev_handle_t dev_handle_2;
TEST_ESP_ERR(ESP_ERR_NOT_FOUND, i2c_new_slave_device(&i2c_slv_config_1, &dev_handle_2));
for (int i = 0; i < SOC_HP_I2C_NUM; i++) {
TEST_ESP_OK(i2c_del_slave_device(dev_handle[i]));
}
// Get another one
TEST_ESP_OK(i2c_new_slave_device(&i2c_slv_config_1, &dev_handle_2));
TEST_ESP_OK(i2c_del_slave_device(dev_handle_2));
}
static QueueHandle_t event_queue;
static uint8_t *temp_data;
static size_t temp_len = 0;
@ -91,7 +132,7 @@ static void i2c_slave_read_test_v2(void)
unity_wait_for_signal("master write");
i2c_slave_event_t evt;
if (xQueueReceive(event_queue, &evt, 1) == pdTRUE) {
if (xQueueReceive(event_queue, &evt, portMAX_DELAY) == pdTRUE) {
if (evt == I2C_SLAVE_EVT_RX) {
disp_buf(temp_data, temp_len);
printf("length is %x\n", temp_len);
@ -286,7 +327,98 @@ static void i2c_master_write_test_with_customize_api(void)
TEST_ESP_OK(i2c_del_master_bus(bus_handle));
}
TEST_CASE_MULTIPLE_DEVICES("I2C master write slave with customize api", "[i2c][test_env=generic_multi_device][timeout=150]", i2c_master_write_test_with_customize_api, i2c_slave_read_test_v2);
static void master_read_slave_test_v2_single_byte(void)
{
uint8_t data_rd[DATA_LENGTH] = {0};
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_cfg = {
.dev_addr_length = I2C_ADDR_BIT_LEN_7,
.device_address = I2C_DEVICE_ADDRESS_NOT_USED,
.scl_speed_hz = 100000,
.scl_wait_us = 20000,
};
i2c_master_dev_handle_t dev_handle;
TEST_ESP_OK(i2c_master_bus_add_device(bus_handle, &dev_cfg, &dev_handle));
unity_wait_for_signal("i2c slave init finish");
uint8_t read_address = (ESP_SLAVE_ADDR << 1 | 1);
i2c_operation_job_t i2c_ops[] = {
{ .command = I2C_MASTER_CMD_START },
{ .command = I2C_MASTER_CMD_WRITE, .write = { .ack_check = true, .data = (uint8_t *) &read_address, .total_bytes = 1 } },
{ .command = I2C_MASTER_CMD_READ, .read = { .ack_value = I2C_NACK_VAL, .data = data_rd, .total_bytes = 1 }},
{ .command = I2C_MASTER_CMD_STOP },
};
i2c_master_execute_defined_operations(dev_handle, i2c_ops, sizeof(i2c_ops) / sizeof(i2c_operation_job_t), 1000);
vTaskDelay(100 / portTICK_PERIOD_MS);
TEST_ASSERT(data_rd[0] == 6);
unity_send_signal("ready to delete master read test");
TEST_ESP_OK(i2c_master_bus_rm_device(dev_handle));
TEST_ESP_OK(i2c_del_master_bus(bus_handle));
}
static void slave_write_buffer_test_v2_single_byte(void)
{
i2c_slave_dev_handle_t handle;
uint8_t data_wr[DATA_LENGTH];
event_queue = xQueueCreate(2, sizeof(i2c_slave_event_t));
assert(event_queue);
i2c_slave_config_t i2c_slv_config = {
.i2c_port = TEST_I2C_PORT,
.clk_source = I2C_CLK_SRC_DEFAULT,
.scl_io_num = I2C_SLAVE_SCL_IO,
.sda_io_num = I2C_SLAVE_SDA_IO,
.slave_addr = ESP_SLAVE_ADDR,
.send_buf_depth = DATA_LENGTH,
.receive_buf_depth = DATA_LENGTH,
.flags.enable_internal_pullup = true,
};
TEST_ESP_OK(i2c_new_slave_device(&i2c_slv_config, &handle));
i2c_slave_event_callbacks_t cbs = {
.on_receive = i2c_slave_receive_cb,
.on_request = i2c_slave_request_cb,
};
TEST_ESP_OK(i2c_slave_register_event_callbacks(handle, &cbs, NULL));
unity_send_signal("i2c slave init finish");
data_wr[0] = 6;
i2c_slave_event_t evt;
uint32_t write_len;
while (true) {
if (xQueueReceive(event_queue, &evt, portMAX_DELAY) == pdTRUE) {
if (evt == I2C_SLAVE_EVT_TX) {
TEST_ESP_OK(i2c_slave_write(handle, data_wr, 1, &write_len, 1000));
break;
}
}
}
unity_wait_for_signal("ready to delete master read test");
vQueueDelete(event_queue);
TEST_ESP_OK(i2c_del_slave_device(handle));
}
TEST_CASE_MULTIPLE_DEVICES("I2C master read slave test single byte", "[i2c][test_env=generic_multi_device][timeout=150]", master_read_slave_test_v2_single_byte, slave_write_buffer_test_v2_single_byte);
#endif // SOC_I2C_SLAVE_CAN_GET_STRETCH_CAUSE