From 6e3e923290eafd56869013a61d39234ec86b0ad0 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Thu, 31 Aug 2023 15:06:46 +0800 Subject: [PATCH] fix(i2c_master): Fix issue that i2c master probe device failed, Closes https://github.com/espressif/esp-idf/issues/12159 --- components/driver/i2c/i2c_master.c | 29 +++++++++++++++++-- .../test_apps/i2c_test_apps/main/test_i2c.c | 18 ++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index 0fca0de002..c18f9a00a9 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -306,8 +306,9 @@ static void s_i2c_start_end_command(i2c_master_bus_handle_t i2c_master, i2c_oper i2c_master->cmd_idx++; portEXIT_CRITICAL_SAFE(&i2c_master->base->spinlock); if (i2c_operation->hw_cmd.op_code == I2C_LL_CMD_RESTART) { + i2c_operation_t next_transaction = i2c_master->i2c_trans.ops[i2c_master->trans_idx + 1]; - if (i2c_master->i2c_trans.ops[i2c_master->trans_idx + 1].hw_cmd.op_code != I2C_LL_CMD_WRITE) { + if (next_transaction.hw_cmd.op_code == I2C_LL_CMD_READ) { #if SOC_I2C_SUPPORT_10BIT_ADDR if (i2c_master->addr_10bits_bus == I2C_ADDR_BIT_LEN_10) { i2c_ll_hw_cmd_t hw_write_cmd = { @@ -340,6 +341,17 @@ static void s_i2c_start_end_command(i2c_master_bus_handle_t i2c_master, i2c_oper i2c_ll_master_write_cmd_reg(hal->dev, hw_write_cmd, i2c_master->cmd_idx); i2c_master->cmd_idx++; portEXIT_CRITICAL_SAFE(&i2c_master->base->spinlock); + } else if (next_transaction.hw_cmd.op_code == I2C_LL_CMD_STOP) { + i2c_ll_hw_cmd_t hw_write_cmd = { + .ack_en = true, + .op_code = I2C_LL_CMD_WRITE, + .byte_num = 1, + }; + portENTER_CRITICAL_SAFE(&i2c_master->base->spinlock); + i2c_ll_write_txfifo(hal->dev, addr_write, sizeof(addr_write)); + i2c_ll_master_write_cmd_reg(hal->dev, hw_write_cmd, i2c_master->cmd_idx); + i2c_master->cmd_idx++; + portEXIT_CRITICAL_SAFE(&i2c_master->base->spinlock); } else { /* The next transaction is a WRITE, we can merge the device address byte * with the next write command. No need to write the `cmd_reg` as the next @@ -411,7 +423,9 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t // For blocking implementation, we must wait `done` interrupt to update the status. while (i2c_master->trans_done == false) {}; - atomic_store(&i2c_master->status, I2C_STATUS_DONE); + if (i2c_master->status != I2C_STATUS_ACK_ERROR && i2c_master->status != I2C_STATUS_TIMEOUT) { + atomic_store(&i2c_master->status, I2C_STATUS_DONE); + } xSemaphoreGive(i2c_master->cmd_semphr); } @@ -1035,7 +1049,7 @@ esp_err_t i2c_master_receive(i2c_master_dev_handle_t i2c_dev, uint8_t *read_buff esp_err_t i2c_master_probe(i2c_master_bus_handle_t i2c_master, uint16_t address, int xfer_timeout_ms) { ESP_RETURN_ON_FALSE(i2c_master != NULL, ESP_ERR_INVALID_ARG, TAG, "i2c handle not initialized"); - + i2c_hal_context_t *hal = &i2c_master->base->hal; i2c_operation_t i2c_ops[] = { {.hw_cmd = I2C_TRANS_START_COMMAND}, {.hw_cmd = I2C_TRANS_STOP_COMMAND}, @@ -1047,8 +1061,17 @@ esp_err_t i2c_master_probe(i2c_master_bus_handle_t i2c_master, uint16_t address, .ops = i2c_ops, .cmd_count = DIM(i2c_ops), }; + + // 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, 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); + s_i2c_send_commands(i2c_master, ticks_to_wait); if (i2c_master->status == I2C_STATUS_ACK_ERROR) { + // Reset the status to done, in order not influence next time transaction. + i2c_master->status = I2C_STATUS_DONE; return ESP_ERR_NOT_FOUND; } return ESP_OK; diff --git a/components/driver/test_apps/i2c_test_apps/main/test_i2c.c b/components/driver/test_apps/i2c_test_apps/main/test_i2c.c index 282b2526b8..5871c0d9a7 100644 --- a/components/driver/test_apps/i2c_test_apps/main/test_i2c.c +++ b/components/driver/test_apps/i2c_test_apps/main/test_i2c.c @@ -129,3 +129,21 @@ TEST_CASE("I2C device add & remove check", "[i2c]") TEST_ESP_OK(i2c_del_master_bus(bus_handle)); } + +TEST_CASE("I2C master probe device test", "[i2c]") +{ + // 0x22 does not exist on the I2C bus, so it's expected to return `not found` error + // TODO: Add exist slave for testing probe success after i2c slave is merged. + i2c_master_bus_config_t i2c_mst_config_1 = { + .clk_source = I2C_CLK_SRC_DEFAULT, + .i2c_port = TEST_I2C_PORT, + .scl_io_num = TEST_I2C_SCL_PIN, + .sda_io_num = TEST_I2C_SDA_PIN, + .flags.enable_internal_pullup = true, + }; + i2c_master_bus_handle_t bus_handle; + + TEST_ESP_OK(i2c_new_master_bus(&i2c_mst_config_1, &bus_handle)); + TEST_ESP_ERR(i2c_master_probe(bus_handle, 0x22, -1), ESP_ERR_NOT_FOUND); + TEST_ESP_OK(i2c_del_master_bus(bus_handle)); +}