From 1428638fd6d1db5f6553b2d2bcb701cf3d0278be Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 30 Jun 2026 16:52:21 +0800 Subject: [PATCH] fix(i2c): release platform mutex on intr/pm_lock delete failure ESP_RETURN_ON_ERROR inside the s_i2c_platform.mutex critical section returns without releasing the mutex, permanently blocking all I2C bus operations. Replace with ESP_GOTO_ON_ERROR that jumps to a cleanup label releasing the mutex before return. --- components/esp_driver_i2c/i2c_common.c | 29 +++++++++++++++--------- components/esp_driver_i2c/i2c_master.c | 7 +++--- components/esp_driver_i2c/i2c_slave_v2.c | 5 ++++ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/components/esp_driver_i2c/i2c_common.c b/components/esp_driver_i2c/i2c_common.c index ac0c38bf460..88346206242 100644 --- a/components/esp_driver_i2c/i2c_common.c +++ b/components/esp_driver_i2c/i2c_common.c @@ -190,15 +190,24 @@ esp_err_t i2c_acquire_bus_handle(i2c_port_num_t port_num, i2c_bus_handle_t *i2c_ esp_err_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus) { + esp_err_t ret = ESP_OK; int port_num = i2c_bus->port_num; i2c_clock_source_t clk_src = i2c_bus->clk_src; bool do_deinitialize = false; _lock_acquire(&s_i2c_platform.mutex); if (s_i2c_platform.buses[port_num]) { - s_i2c_platform.count[port_num]--; - if (s_i2c_platform.count[port_num] == 0) { + if (s_i2c_platform.count[port_num] > 1) { + s_i2c_platform.count[port_num]--; + } else { do_deinitialize = true; - s_i2c_platform.buses[port_num] = NULL; + if (i2c_bus->intr_handle) { + ESP_GOTO_ON_ERROR(esp_intr_free(i2c_bus->intr_handle), err, TAG, "delete interrupt service failed"); + i2c_bus->intr_handle = NULL; + } + if (i2c_bus->pm_lock) { + esp_pm_lock_delete(i2c_bus->pm_lock); + i2c_bus->pm_lock = NULL; + } #if I2C_USE_RETENTION_LINK if (i2c_bus->is_lp_i2c == false) { if (i2c_bus->retention_link_created) { @@ -207,12 +216,8 @@ esp_err_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus) sleep_retention_module_deinit(i2c_regs_retention[port_num].module_id); } #endif - if (i2c_bus->intr_handle) { - ESP_RETURN_ON_ERROR(esp_intr_free(i2c_bus->intr_handle), TAG, "delete interrupt service failed"); - } - if (i2c_bus->pm_lock) { - ESP_RETURN_ON_ERROR(esp_pm_lock_delete(i2c_bus->pm_lock), TAG, "delete pm_lock failed"); - } + s_i2c_platform.count[port_num] = 0; + s_i2c_platform.buses[port_num] = NULL; // Disable I2C module if (!i2c_bus->is_lp_i2c) { I2C_RCC_ATOMIC() { @@ -244,9 +249,11 @@ esp_err_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus) if (do_deinitialize) { ESP_LOGD(TAG, "delete bus %d", port_num); } - - ESP_RETURN_ON_FALSE(s_i2c_platform.count[port_num] == 0, ESP_ERR_INVALID_STATE, TAG, "Bus not freed entirely"); return ESP_OK; + +err: + _lock_release(&s_i2c_platform.mutex); + return ret; } esp_err_t i2c_select_periph_clock(i2c_bus_handle_t handle, soc_module_clk_t clk_src) diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index 13f45339e6f..b6349269312 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -863,11 +863,12 @@ static esp_err_t i2c_master_bus_destroy(i2c_master_bus_handle_t bus_handle) } bus_handle = NULL; } - - free(i2c_master); } else { - free(i2c_master); + // Non-OK here means interrupt teardown did not complete, so the ISR + // may still reference i2c_slave and its wrapper-owned resources. + return err; } + free(i2c_master); return ESP_OK; } diff --git a/components/esp_driver_i2c/i2c_slave_v2.c b/components/esp_driver_i2c/i2c_slave_v2.c index 49504276549..b0ce6697846 100644 --- a/components/esp_driver_i2c/i2c_slave_v2.c +++ b/components/esp_driver_i2c/i2c_slave_v2.c @@ -212,6 +212,11 @@ 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); i2c_common_deinit_pins(i2c_slave->base); ret = i2c_release_bus_handle(i2c_slave->base); + if (ret != ESP_OK) { + // Non-OK here means interrupt teardown did not complete, so the ISR + // may still reference i2c_slave and its wrapper-owned resources. + return ret; + } } if (i2c_slave->rx_ring_buf) { vRingbufferDeleteWithCaps(i2c_slave->rx_ring_buf);