From 8f84ab348ff1cabefed98c846188bc558c1f390e Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Wed, 6 Mar 2024 19:04:12 +0800 Subject: [PATCH] fix(i2c): Fix I2C synchronous transaction cost so much CPU source, Closes https://github.com/espressif/esp-idf/issues/13137, Closes https://github.com/espressif/esp-idf/pull/13322 --- components/driver/i2c/i2c_master.c | 34 +++++++++++++++---- components/driver/i2c/i2c_private.h | 1 + .../driver/i2c/include/driver/i2c_types.h | 5 ++- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index 342fe2c63fb0..c29e94477d5a 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -392,6 +392,9 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t uint8_t fifo_fill = 0; uint8_t address_fill = 0; + // Initialise event queue. + xQueueReset(i2c_master->event_queue); + i2c_master->event = I2C_EVENT_ALIVE; while (i2c_master->i2c_trans.cmd_count) { if (xSemaphoreTake(i2c_master->cmd_semphr, ticks_to_wait) != pdTRUE) { // Software timeout, clear the command link and finish this transaction. @@ -429,10 +432,17 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t } i2c_hal_master_trans_start(hal); - // For blocking implementation, we must wait `done` interrupt to update the status. - while (i2c_master->trans_done == false) {}; - if (i2c_master->status != I2C_STATUS_ACK_ERROR && i2c_master->status != I2C_STATUS_TIMEOUT) { - atomic_store(&i2c_master->status, I2C_STATUS_DONE); + // For blocking implementation, we must wait event interrupt to update the status. + // Otherwise, update status to timeout. + i2c_master_event_t event; + if (xQueueReceive(i2c_master->event_queue, &event, ticks_to_wait) == pdTRUE) { + if (event == I2C_EVENT_DONE) { + atomic_store(&i2c_master->status, I2C_STATUS_DONE); + } + } else { + i2c_master->cmd_idx = 0; + i2c_master->trans_idx = 0; + atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT); } xSemaphoreGive(i2c_master->cmd_semphr); } @@ -573,15 +583,19 @@ static void IRAM_ATTR i2c_master_isr_handler_default(void *arg) return; } - if (int_mask == I2C_LL_INTR_NACK) { + if (int_mask & I2C_LL_INTR_NACK) { atomic_store(&i2c_master->status, I2C_STATUS_ACK_ERROR); i2c_master->event = I2C_EVENT_NACK; - } else if (int_mask == I2C_LL_INTR_TIMEOUT || int_mask == I2C_LL_INTR_ARBITRATION) { + } else if (int_mask & I2C_LL_INTR_TIMEOUT || int_mask & I2C_LL_INTR_ARBITRATION) { atomic_store(&i2c_master->status, I2C_STATUS_TIMEOUT); - } else if (int_mask == I2C_LL_INTR_MST_COMPLETE) { + i2c_master->event = I2C_EVENT_TIMEOUT; + } else if (int_mask & I2C_LL_INTR_MST_COMPLETE) { i2c_master->trans_done = true; i2c_master->event = I2C_EVENT_DONE; } + if (i2c_master->event != I2C_EVENT_ALIVE) { + xQueueSendFromISR(i2c_master->event_queue, (void *)&i2c_master->event, &HPTaskAwoken); + } if (i2c_master->contains_read == true) { i2c_isr_receive_handler(i2c_master); } @@ -672,6 +686,9 @@ static esp_err_t i2c_master_bus_destroy(i2c_master_bus_handle_t bus_handle) vSemaphoreDeleteWithCaps(i2c_master->cmd_semphr); i2c_master->cmd_semphr = NULL; } + if (i2c_master->event_queue) { + vQueueDeleteWithCaps(i2c_master->event_queue); + } if (i2c_master->queues_storage) { free(i2c_master->queues_storage); } @@ -814,6 +831,9 @@ esp_err_t i2c_new_master_bus(const i2c_master_bus_config_t *bus_config, i2c_mast i2c_master->cmd_semphr = xSemaphoreCreateBinaryWithCaps(I2C_MEM_ALLOC_CAPS); ESP_GOTO_ON_FALSE(i2c_master->cmd_semphr, ESP_ERR_NO_MEM, err, TAG, "no memory for i2c semaphore struct"); + i2c_master->event_queue = xQueueCreateWithCaps(1, sizeof(i2c_master_event_t), I2C_MEM_ALLOC_CAPS); + ESP_GOTO_ON_FALSE(i2c_master->event_queue, ESP_ERR_NO_MEM, err, TAG, "no memory for i2c queue struct"); + portENTER_CRITICAL(&i2c_master->base->spinlock); i2c_ll_clear_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR); portEXIT_CRITICAL(&i2c_master->base->spinlock); diff --git a/components/driver/i2c/i2c_private.h b/components/driver/i2c/i2c_private.h index 8d4688562d81..7ceec24da28d 100644 --- a/components/driver/i2c/i2c_private.h +++ b/components/driver/i2c/i2c_private.h @@ -118,6 +118,7 @@ struct i2c_master_bus_t { i2c_operation_t i2c_ops[I2C_STATIC_OPERATION_ARRAY_MAX]; // I2C operation array _Atomic uint16_t trans_idx; // Index of I2C transaction command. SemaphoreHandle_t cmd_semphr; // Semaphore between task and interrupt, using for synchronizing ISR and I2C task. + QueueHandle_t event_queue; // I2C event queue uint32_t read_buf_pos; // Read buffer position bool contains_read; // Whether command array includes read operation, true: yes, otherwise, false. uint32_t read_len_static; // Read static buffer length diff --git a/components/driver/i2c/include/driver/i2c_types.h b/components/driver/i2c/include/driver/i2c_types.h index 81b3c239c164..9010309e8faf 100644 --- a/components/driver/i2c/include/driver/i2c_types.h +++ b/components/driver/i2c/include/driver/i2c_types.h @@ -34,11 +34,14 @@ typedef enum { I2C_STATUS_TIMEOUT, /*!< I2C bus status error, and operation timeout */ } i2c_master_status_t; - +/** + * @brief Enumeration for I2C event. + */ typedef enum { I2C_EVENT_ALIVE, /*!< i2c bus in alive status.*/ I2C_EVENT_DONE, /*!< i2c bus transaction done */ I2C_EVENT_NACK, /*!< i2c bus nack */ + I2C_EVENT_TIMEOUT, /*!< i2c bus timeout */ } i2c_master_event_t; /**