Merge branch 'bugfix/fix_i2c_master_issue' into 'master'

fix(i2c_master): Fix some issues on new i2c_master

Closes IDF-8034

See merge request espressif/esp-idf!25320

components/driver/i2c/i2c_common.c

@@ -87,7 +87,7 @@ bool i2c_bus_occupied(i2c_port_num_t port_num)
     return bus_occupied;
 }
 
-uint8_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus)
+esp_err_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus)
 {
     int port_num = i2c_bus->port_num;
     i2c_clock_source_t clk_src = i2c_bus->clk_src;
@@ -98,7 +98,12 @@ uint8_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus)
         if (s_i2c_platform.count[port_num] == 0) {
             do_deinitialize = true;
             s_i2c_platform.buses[port_num] = NULL;
-
+            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");
+            }
             // Disable I2C module
             periph_module_disable(i2c_periph_signal[port_num].module);
             free(i2c_bus);
@@ -119,7 +124,9 @@ uint8_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus)
     if (do_deinitialize) {
         ESP_LOGD(TAG, "delete bus %d", port_num);
     }
-    return s_i2c_platform.count[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;
 }
 
 esp_err_t i2c_select_periph_clock(i2c_bus_handle_t handle, i2c_clock_source_t clk_src)

components/driver/i2c/i2c_master.c

@@ -640,14 +640,8 @@ 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;
-    if(i2c_release_bus_handle(i2c_master->base) == 0) {
+    if(i2c_release_bus_handle(i2c_master->base) == ESP_OK) {
         if (i2c_master) {
-            if (i2c_master->base->intr_handle) {
-                ESP_RETURN_ON_ERROR(esp_intr_free(i2c_master->base->intr_handle), TAG, "delete interrupt service failed");
-            }
-            if (i2c_master->base->pm_lock) {
-                ESP_RETURN_ON_ERROR(esp_pm_lock_delete(i2c_master->base->pm_lock), TAG, "delete pm_lock failed");
-            }
             if (i2c_master->bus_lock_mux) {
                 vSemaphoreDeleteWithCaps(i2c_master->bus_lock_mux);
                 i2c_master->bus_lock_mux = NULL;
@@ -727,7 +721,7 @@ static esp_err_t s_i2c_asynchronous_transaction(i2c_master_dev_handle_t i2c_dev,
             i2c_dev->master_bus->num_trans_inqueue++;
             if (i2c_dev->master_bus->sent_all == true) {
                 // Oh no, you cannot get the queue from ISR, so you get queue here.
-                xQueueReceive(i2c_dev->master_bus->trans_queues[I2C_TRANS_QUEUE_PROGRESS], &i2c_queue_pre, portMAX_DELAY);
+                ESP_RETURN_ON_FALSE(xQueueReceive(i2c_dev->master_bus->trans_queues[I2C_TRANS_QUEUE_PROGRESS], &i2c_queue_pre, portMAX_DELAY) == pdTRUE, ESP_FAIL, TAG, "get trans from progress queue failed");
                 i2c_dev->master_bus->num_trans_inflight--;
                 i2c_dev->master_bus->num_trans_inqueue--;
                 i2c_dev->master_bus->sent_all = false;
@@ -903,7 +897,7 @@ esp_err_t i2c_master_bus_add_device(i2c_master_bus_handle_t bus_handle, const i2
     i2c_dev->master_bus = i2c_master;
 
     i2c_master_device_list_t *device_item = (i2c_master_device_list_t *)calloc(1, sizeof(i2c_master_device_list_t));
-    ESP_RETURN_ON_FALSE((device_item != NULL), ESP_ERR_NO_MEM, TAG, "no memory for i2c device item`");
+    ESP_GOTO_ON_FALSE((device_item != NULL), ESP_ERR_NO_MEM, err, TAG, "no memory for i2c device item`");
     device_item->device = i2c_dev;
     xSemaphoreTake(bus_handle->bus_lock_mux, portMAX_DELAY);
     SLIST_INSERT_HEAD(&bus_handle->device_list, device_item, next);

components/driver/i2c/i2c_private.h

@@ -91,7 +91,7 @@ struct i2c_bus_t {
     uint32_t clk_src_freq_hz; // Record the clock source frequency
     int sda_num; // SDA pin number
     int scl_num; // SCL pin number
-    bool pull_up_enable; // Disable pull-ups
+    bool pull_up_enable; // Enable pull-ups
     intr_handle_t intr_handle; // I2C interrupt handle
     esp_pm_lock_handle_t pm_lock; // power manange lock
 #if CONFIG_PM_ENABLE
@@ -169,8 +169,10 @@ esp_err_t i2c_acquire_bus_handle(i2c_port_num_t port_num, i2c_bus_handle_t *i2c_
  *
  * @param i2c_bus I2C bus handle, returned from `i2c_acquire_bus_handle`
  * @return ESP_OK: If release successfully
+ *         ESP_ERR_INVALID_STATE: Release bus failed because same bus has been required several times.
+ *         Otherwise: Other reasons.
  */
-uint8_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus);
+esp_err_t i2c_release_bus_handle(i2c_bus_handle_t i2c_bus);
 
 /**
  * @brief Set clock source for I2C peripheral

components/driver/i2c/include/driver/i2c_master.h

@@ -27,7 +27,7 @@ typedef struct {
     int intr_priority;                    /*!< I2C interrupt priority, if set to 0, driver will select the default priority (1,2,3). */
     size_t trans_queue_depth;             /*!< Depth of internal transfer queue, increase this value can support more transfers pending in the background, only valid in asynchronous transaction. (Typically max_device_num * per_transaction)*/
     struct {
-        uint32_t enable_internal_pullup:1;   /*!< Enable internal pullups */
+        uint32_t enable_internal_pullup:1;   /*!< Enable internal pullups. Note: This is not strong enough to pullup buses under high-speed frequency. Recommend proper external pull-up if possible */
     } flags;
 } i2c_master_bus_config_t;
 
@@ -163,7 +163,6 @@ esp_err_t i2c_master_receive(i2c_master_dev_handle_t i2c_dev, uint8_t *read_buff
  *      - ESP_OK: I2C device probe successfully
  *      - ESP_ERR_TIMEOUT: Operation timeout(larger than xfer_timeout_ms) because the bus is busy or hardware crash.
  */
-// esp_err_t i2c_master_probe(i2c_master_dev_handle_t i2c_dev, int xfer_timeout_ms);
 esp_err_t i2c_master_probe(i2c_master_bus_handle_t i2c_master, uint16_t address, int xfer_timeout_ms);
 
 /**

components/hal/esp32p4/include/hal/clk_gate_ll.h

@@ -33,9 +33,9 @@ static inline uint32_t periph_ll_get_clk_en_mask(periph_module_t periph)
     case PERIPH_MIPI_CSI_MODULE:
         return 0;
     case PERIPH_I2C0_MODULE:
-        return HP_SYS_CLKRST_REG_I2C0_CLK_EN;
+        return HP_SYS_CLKRST_REG_I2C0_APB_CLK_EN;
     case PERIPH_I2C1_MODULE:
-        return HP_SYS_CLKRST_REG_I2C1_CLK_EN;
+        return HP_SYS_CLKRST_REG_I2C1_APB_CLK_EN;
     case PERIPH_I2S0_MODULE:
         return HP_SYS_CLKRST_REG_I2S0_TX_CLK_EN | HP_SYS_CLKRST_REG_I2S0_RX_CLK_EN;
     case PERIPH_I2S1_MODULE:
@@ -240,7 +240,7 @@ static inline uint32_t periph_ll_get_clk_en_reg(periph_module_t periph)
         return HP_SYS_CLKRST_PERI_CLK_CTRL03_REG;
     case PERIPH_I2C0_MODULE:
     case PERIPH_I2C1_MODULE:
-        return HP_SYS_CLKRST_PERI_CLK_CTRL10_REG;
+        return HP_SYS_CLKRST_SOC_CLK_CTRL2_REG;
     case PERIPH_LCD_MODULE:
         return HP_SYS_CLKRST_PERI_CLK_CTRL110_REG;
     case PERIPH_UART0_MODULE:
@@ -323,6 +323,7 @@ static inline uint32_t periph_ll_get_rst_en_reg(periph_module_t periph)
     case PERIPH_I3C_MODULE:
     case PERIPH_I2C0_MODULE:
     case PERIPH_I2C1_MODULE:
+        return HP_SYS_CLKRST_HP_RST_EN1_REG;
     case PERIPH_RMT_MODULE:
     case PERIPH_MCPWM0_MODULE:
     case PERIPH_MCPWM1_MODULE: