From a69c6e8f41b17562b01f75535dbccaebdb6c83d2 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Thu, 11 Apr 2024 15:52:54 +0800 Subject: [PATCH 1/2] 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/esp_driver_i2c/i2c_master.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index 4fe138d4eb..f0a325941b 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -431,7 +431,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]; @@ -584,7 +584,7 @@ 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); @@ -645,7 +645,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 4ba3a4e482adffe754c0581efb7c4749fcc3dfda Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Fri, 12 Apr 2024 11:46:36 +0800 Subject: [PATCH 2/2] 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/esp_driver_i2c/i2c_master.c | 19 ++++++++++ components/esp_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/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index f0a325941b..4ee7c2b409 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -114,6 +114,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. @@ -412,6 +424,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; } @@ -431,6 +444,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; } @@ -453,6 +468,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; @@ -1115,6 +1131,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}, @@ -1147,6 +1165,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/esp_driver_i2c/i2c_private.h b/components/esp_driver_i2c/i2c_private.h index 8cf6b3b680..ae33a26ea6 100644 --- a/components/esp_driver_i2c/i2c_private.h +++ b/components/esp_driver_i2c/i2c_private.h @@ -145,7 +145,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 // async trans members bool async_break; // break transaction loop flag. diff --git a/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_common.c b/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_common.c index d9991248bd..45ec47b06b 100644 --- a/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_common.c +++ b/components/esp_driver_i2c/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)