From 122b57902411a5da8063686adba2c7bf16d74c55 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Tue, 19 Nov 2024 16:45:05 +0800 Subject: [PATCH 1/6] test(i2c): Enhance the stability for i2c tests --- .../test_apps/i2c_test_apps/main/test_i2c_slave_v2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_slave_v2.c b/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_slave_v2.c index e1282824d0..65a133b79c 100644 --- a/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_slave_v2.c +++ b/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_i2c_slave_v2.c @@ -74,6 +74,7 @@ static void i2c_slave_read_test_v2(void) .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)); @@ -200,6 +201,7 @@ static void slave_write_buffer_test_v2(void) .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)); From 2803aed082f83a6af829873f317fa00ca9810059 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Thu, 21 Nov 2024 11:23:15 +0800 Subject: [PATCH 2/6] fix(i2c): Enhance lp clock source and avoid deadlock, Closes https://github.com/espressif/esp-idf/issues/14908, Closes https://github.com/espressif/esp-idf/issues/14906 --- components/esp_driver_i2c/i2c_common.c | 6 +++- components/esp_driver_i2c/i2c_master.c | 13 +++++++ .../i2c_test_apps/main/test_i2c_common.c | 35 +++++++++++++++++++ .../i2c_test_apps/main/test_lp_i2c.c | 14 ++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/components/esp_driver_i2c/i2c_common.c b/components/esp_driver_i2c/i2c_common.c index 87042b404a..b45c64d650 100644 --- a/components/esp_driver_i2c/i2c_common.c +++ b/components/esp_driver_i2c/i2c_common.c @@ -169,7 +169,11 @@ esp_err_t i2c_acquire_bus_handle(i2c_port_num_t port_num, i2c_bus_handle_t *i2c_ break; } } - ESP_RETURN_ON_FALSE((bus_found == true), ESP_ERR_NOT_FOUND, TAG, "acquire bus failed, no free bus"); + if (bus_found == false) { + ESP_LOGE(TAG, "acquire bus failed, no free bus"); + _lock_release(&s_i2c_platform.mutex); + return ESP_ERR_NOT_FOUND; + } } else { ret = s_i2c_bus_handle_acquire(port_num, i2c_new_bus, mode); if (ret != ESP_OK) { diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index 0ab6c32979..6739fa0385 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -962,6 +962,19 @@ esp_err_t i2c_new_master_bus(const i2c_master_bus_config_t *bus_config, i2c_mast } #if SOC_LP_I2C_SUPPORTED else { + + soc_periph_lp_i2c_clk_src_t clk_srcs[] = SOC_LP_I2C_CLKS; + bool lp_clock_match = false; + for (int i = 0; i < sizeof(clk_srcs) / sizeof(clk_srcs[0]); i++) { + if ((int)clk_srcs[i] == (int)i2c_master->base->clk_src) { + /* Clock source matches. Override the source clock type with the user configured value */ + lp_clock_match = true; + break; + } + } + + ESP_GOTO_ON_FALSE(lp_clock_match, ESP_ERR_NOT_SUPPORTED, err, TAG, "the clock source does not support lp i2c, please check"); + LP_I2C_SRC_CLK_ATOMIC() { lp_i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); } 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 55d7064b86..53034af1bd 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 @@ -146,6 +146,41 @@ TEST_CASE("I2C device add & remove check", "[i2c]") TEST_ESP_OK(i2c_del_master_bus(bus_handle)); } +TEST_CASE("I2C peripheral allocate all", "[i2c]") +{ + i2c_master_bus_handle_t bus_handle[SOC_HP_I2C_NUM]; + for (int i = 0; i < SOC_HP_I2C_NUM; i++) { + i2c_master_bus_config_t i2c_mst_config_1 = { + .clk_source = I2C_CLK_SRC_DEFAULT, + .i2c_port = -1, + .scl_io_num = I2C_MASTER_SCL_IO, + .sda_io_num = I2C_MASTER_SDA_IO, + .flags.enable_internal_pullup = true, + }; + + TEST_ESP_OK(i2c_new_master_bus(&i2c_mst_config_1, &bus_handle[i])); + } + i2c_master_bus_config_t i2c_mst_config_1 = { + .clk_source = I2C_CLK_SRC_DEFAULT, + .i2c_port = -1, + .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_2; + + TEST_ESP_ERR(ESP_ERR_NOT_FOUND, i2c_new_master_bus(&i2c_mst_config_1, &bus_handle_2)); + + for (int i = 0; i < SOC_HP_I2C_NUM; i++) { + TEST_ESP_OK(i2c_del_master_bus(bus_handle[i])); + } + + // Get another one + + TEST_ESP_OK(i2c_new_master_bus(&i2c_mst_config_1, &bus_handle_2)); + TEST_ESP_OK(i2c_del_master_bus(bus_handle_2)); +} + TEST_CASE("I2C master probe device test", "[i2c]") { // 0x22,33,44,55 does not exist on the I2C bus, so it's expected to return `not found` error diff --git a/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_lp_i2c.c b/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_lp_i2c.c index c9701e41b7..e17b025993 100644 --- a/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_lp_i2c.c +++ b/components/esp_driver_i2c/test_apps/i2c_test_apps/main/test_lp_i2c.c @@ -64,6 +64,20 @@ TEST_CASE("LP I2C initialize with wrong IO", "[i2c]") #endif +TEST_CASE("LP I2C initialize with wrong clock source", "[i2c]") +{ + i2c_master_bus_config_t i2c_mst_config = { + .lp_source_clk = I2C_CLK_SRC_DEFAULT, + .i2c_port = LP_I2C_NUM_0, + .scl_io_num = LP_I2C_SCL_IO, + .sda_io_num = LP_I2C_SDA_IO, + .flags.enable_internal_pullup = true, + }; + i2c_master_bus_handle_t bus_handle; + + TEST_ESP_ERR(ESP_ERR_NOT_SUPPORTED, i2c_new_master_bus(&i2c_mst_config, &bus_handle)); +} + static IRAM_ATTR bool test_i2c_rx_done_callback(i2c_slave_dev_handle_t channel, const i2c_slave_rx_done_event_data_t *edata, void *user_data) { BaseType_t high_task_wakeup = pdFALSE; From 2ab0921a80125e37be7b4d1f75fc6f77a7500e68 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Fri, 5 Jul 2024 12:12:32 +0800 Subject: [PATCH 3/6] fix(i2c): Fix lose byte during data reading in i2c master on esp32, Closes https://github.com/espressif/esp-idf/issues/12860 --- components/hal/esp32/include/hal/i2c_ll.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/hal/esp32/include/hal/i2c_ll.h b/components/hal/esp32/include/hal/i2c_ll.h index b451af7867..dcc24d803f 100644 --- a/components/hal/esp32/include/hal/i2c_ll.h +++ b/components/hal/esp32/include/hal/i2c_ll.h @@ -575,7 +575,9 @@ __attribute__((always_inline)) static inline void i2c_ll_read_rxfifo(i2c_dev_t *hw, uint8_t *ptr, uint8_t len) { for(int i = 0; i < len; i++) { - ptr[i] = HAL_FORCE_READ_U32_REG_FIELD(hw->fifo_data, data); + // Known issue that hardware read fifo will cause data lose, (fifo pointer jump over a random address) + // use `DPORT_REG_READ` can avoid this issue. + ptr[i] = DPORT_REG_READ((uint32_t)&hw->fifo_data); } } From 36834f34cd2b18eb2d8d7fe43dba99809b76cecd Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Wed, 4 Dec 2024 11:33:55 +0800 Subject: [PATCH 4/6] fix(i2c_slave): Fix the wrong semaphore take in isr --- components/esp_driver_i2c/i2c_slave_v2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/esp_driver_i2c/i2c_slave_v2.c b/components/esp_driver_i2c/i2c_slave_v2.c index d35a74de8c..7a38f0425c 100644 --- a/components/esp_driver_i2c/i2c_slave_v2.c +++ b/components/esp_driver_i2c/i2c_slave_v2.c @@ -104,7 +104,7 @@ IRAM_ATTR static bool i2c_slave_handle_rx_fifo(i2c_slave_dev_t *i2c_slave, uint3 i2c_slave->rx_data_count += len; } } - xSemaphoreTakeFromISR(i2c_slave->operation_mux, &xTaskWoken); + xSemaphoreGiveFromISR(i2c_slave->operation_mux, &xTaskWoken); return xTaskWoken; } From 82903b0bc6b2528d6dea6330249b7c498f2569a3 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Wed, 6 Nov 2024 13:52:45 +0800 Subject: [PATCH 5/6] fix(i2c): Add bus handle check so that it will not be panic when there is no free bus, Closes https://github.com/espressif/esp-idf/issues/14819 --- components/esp_driver_i2c/i2c_master.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index 6739fa0385..a0fadc64d5 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -785,8 +785,12 @@ static esp_err_t i2c_master_bus_destroy(i2c_master_bus_handle_t bus_handle) { ESP_RETURN_ON_FALSE(bus_handle, ESP_ERR_INVALID_ARG, TAG, "no memory for i2c master bus"); i2c_master_bus_handle_t i2c_master = bus_handle; - i2c_common_deinit_pins(i2c_master->base); - if (i2c_release_bus_handle(i2c_master->base) == ESP_OK) { + esp_err_t err = ESP_OK; + if (i2c_master->base) { + i2c_common_deinit_pins(i2c_master->base); + err = i2c_release_bus_handle(i2c_master->base); + } + if (err == ESP_OK) { if (i2c_master) { if (i2c_master->bus_lock_mux) { vSemaphoreDeleteWithCaps(i2c_master->bus_lock_mux); From 37101f5e0f8f3e79751a9c4780e54130b266099d Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Wed, 6 Nov 2024 14:54:37 +0800 Subject: [PATCH 6/6] fix(i2c): Fix some issue in programming guide, Closes https://github.com/espressif/esp-idf/issues/14794 --- docs/en/api-reference/peripherals/i2c.rst | 2 +- docs/zh_CN/api-reference/peripherals/i2c.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/api-reference/peripherals/i2c.rst b/docs/en/api-reference/peripherals/i2c.rst index fd8a453d87..f008024809 100644 --- a/docs/en/api-reference/peripherals/i2c.rst +++ b/docs/en/api-reference/peripherals/i2c.rst @@ -400,7 +400,7 @@ Please note that no STOP condition bit is inserted between the write and read op }; i2c_master_dev_handle_t dev_handle; - ESP_ERROR_CHECK(i2c_master_bus_add_device(I2C_PORT_NUM_0, &dev_cfg, &dev_handle)); + ESP_ERROR_CHECK(i2c_master_bus_add_device(bus_handle, &dev_cfg, &dev_handle)); uint8_t buf[20] = {0x20}; uint8_t buffer[2]; ESP_ERROR_CHECK(i2c_master_transmit_receive(dev_handle, buf, sizeof(buf), buffer, 2, -1)); diff --git a/docs/zh_CN/api-reference/peripherals/i2c.rst b/docs/zh_CN/api-reference/peripherals/i2c.rst index 6ec69b28e9..845465560a 100644 --- a/docs/zh_CN/api-reference/peripherals/i2c.rst +++ b/docs/zh_CN/api-reference/peripherals/i2c.rst @@ -401,7 +401,7 @@ I2C 主机写入后读取 }; i2c_master_dev_handle_t dev_handle; - ESP_ERROR_CHECK(i2c_master_bus_add_device(I2C_PORT_NUM_0, &dev_cfg, &dev_handle)); + ESP_ERROR_CHECK(i2c_master_bus_add_device(bus_handle, &dev_cfg, &dev_handle)); uint8_t buf[20] = {0x20}; uint8_t buffer[2]; ESP_ERROR_CHECK(i2c_master_transmit_receive(dev_handle, buf, sizeof(buf), buffer, 2, -1));