Merge branch 'feature/i2c_add_static_buffer_transfers' into 'master'

i2c: add `i2c_cmd_link_create_static()` to create commands from a given buffer

Closes IDFGH-3087

See merge request espressif/esp-idf!13013

components/driver/i2c.c

@@ -24,6 +24,7 @@
 #include "driver/periph_ctrl.h"
 #include "esp_rom_gpio.h"
 #include "esp_rom_sys.h"
+#include <sys/param.h>
 
 static const char *I2C_TAG = "i2c";
 #define I2C_CHECK(a, str, ret)  if(!(a)) {                                             \
@@ -51,6 +52,7 @@ static const char *I2C_TAG = "i2c";
 #define I2C_MASTER_MODE_ERR_STR        "Only allowed in master mode"
 #define I2C_MODE_SLAVE_ERR_STR         "Only allowed in slave mode"
 #define I2C_CMD_MALLOC_ERR_STR         "i2c command link malloc error"
+#define I2C_CMD_USER_ALLOC_ERR_STR     "i2c command link allocation error: the buffer provided is too small."
 #define I2C_TRANS_MODE_ERR_STR         "i2c trans mode error"
 #define I2C_MODE_ERR_STR               "i2c mode error"
 #define I2C_SDA_IO_ERR_STR             "sda gpio number error"
@@ -78,6 +80,11 @@ static const char *I2C_TAG = "i2c";
 #define I2C_FILTER_CYC_NUM_DEF         (7)         /* The number of apb cycles filtered by default*/
 #define I2C_CLR_BUS_SCL_NUM            (9)
 #define I2C_CLR_BUS_HALF_PERIOD_US     (5)
+#define I2C_TRANS_BUF_MINIMUM_SIZE     (sizeof(i2c_cmd_desc_t) + \
+                                        sizeof(i2c_cmd_link_t) * 8) /* It is required to have allocate one i2c_cmd_desc_t per command:
+                                                                     * start + write (device address) + write buffer +
+                                                                     * start + write (device address) + read buffer + read buffer for NACK +
+                                                                     * stop */
 
 #define I2C_CONTEX_INIT_DEF(uart_num) {\
     .hal.dev = I2C_LL_GET_HW(uart_num),\
@@ -93,8 +100,12 @@ static const char *I2C_TAG = "i2c";
 
 typedef struct {
     i2c_hw_cmd_t hw_cmd;
-    uint8_t *data;     /*!< data address */
-    uint8_t byte_cmd;  /*!< to save cmd for one byte command mode */
+    union {
+        uint8_t* data;      // When total_bytes > 1
+        uint8_t data_byte;  //when total_byte == 1
+    };
+    size_t bytes_used;
+    size_t total_bytes;
 } i2c_cmd_t;
 
 typedef struct i2c_cmd_link {
@@ -106,8 +117,15 @@ typedef struct {
     i2c_cmd_link_t *head;     /*!< head of the command link */
     i2c_cmd_link_t *cur;      /*!< last node of the command link */
     i2c_cmd_link_t *free;     /*!< the first node to free of the command link */
+
+    void     *free_buffer;    /*!< pointer to the next free data in user's buffer */
+    uint32_t  free_size;      /*!< remaining size of the user's buffer */
 } i2c_cmd_desc_t;
 
+/* INTERNAL_STRUCT_SIZE must be at least sizeof(i2c_cmd_link_t) */
+_Static_assert(I2C_INTERNAL_STRUCT_SIZE >= sizeof(i2c_cmd_link_t),
+                "I2C_INTERNAL_STRUCT_SIZE must be at least sizeof(i2c_cmd_link_t), please adjust this value.");
+
 typedef enum {
     I2C_STATUS_READ,      /*!< read status for current master command */
     I2C_STATUS_WRITE,     /*!< write status for current master command */
@@ -857,6 +875,144 @@ esp_err_t i2c_set_pin(i2c_port_t i2c_num, int sda_io_num, int scl_io_num, bool s
     return ESP_OK;
 }
 
+esp_err_t i2c_master_write_to_device(i2c_port_t i2c_num, uint8_t device_address,
+                                     const uint8_t* write_buffer, size_t write_size,
+                                     TickType_t ticks_to_wait)
+{
+    esp_err_t err = ESP_OK;
+    uint8_t buffer[I2C_TRANS_BUF_MINIMUM_SIZE] = { 0 };
+
+    i2c_cmd_handle_t handle = i2c_cmd_link_create_static(buffer, sizeof(buffer));
+    assert (handle != NULL);
+
+    err = i2c_master_start(handle);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_write_byte(handle, device_address << 1 | I2C_MASTER_WRITE, true);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_write(handle, write_buffer, write_size, true);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    i2c_master_stop(handle);
+    err = i2c_master_cmd_begin(i2c_num, handle, ticks_to_wait);
+
+end:
+    i2c_cmd_link_delete_static(handle);
+    return err;
+}
+
+
+esp_err_t i2c_master_read_from_device(i2c_port_t i2c_num, uint8_t device_address,
+                                      uint8_t* read_buffer, size_t read_size,
+                                      TickType_t ticks_to_wait)
+{
+    esp_err_t err = ESP_OK;
+    uint8_t buffer[I2C_TRANS_BUF_MINIMUM_SIZE] = { 0 };
+
+    i2c_cmd_handle_t handle = i2c_cmd_link_create_static(buffer, sizeof(buffer));
+    assert (handle != NULL);
+
+    err = i2c_master_start(handle);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_write_byte(handle, device_address << 1 | I2C_MASTER_READ, true);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_read(handle, read_buffer, read_size, I2C_MASTER_LAST_NACK);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    i2c_master_stop(handle);
+    err = i2c_master_cmd_begin(i2c_num, handle, ticks_to_wait);
+
+end:
+    i2c_cmd_link_delete_static(handle);
+    return err;
+}
+
+
+esp_err_t i2c_master_write_read_device(i2c_port_t i2c_num, uint8_t device_address,
+                                       const uint8_t* write_buffer, size_t write_size,
+                                       uint8_t* read_buffer, size_t read_size,
+                                       TickType_t ticks_to_wait)
+{
+    esp_err_t err = ESP_OK;
+    uint8_t buffer[I2C_TRANS_BUF_MINIMUM_SIZE] = { 0 };
+
+    i2c_cmd_handle_t handle = i2c_cmd_link_create_static(buffer, sizeof(buffer));
+    assert (handle != NULL);
+
+    err = i2c_master_start(handle);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_write_byte(handle, device_address << 1 | I2C_MASTER_WRITE, true);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_write(handle, write_buffer, write_size, true);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_start(handle);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_write_byte(handle, device_address << 1 | I2C_MASTER_READ, true);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    err = i2c_master_read(handle, read_buffer, read_size, I2C_MASTER_LAST_NACK);
+    if (err != ESP_OK) {
+        goto end;
+    }
+
+    i2c_master_stop(handle);
+    err = i2c_master_cmd_begin(i2c_num, handle, ticks_to_wait);
+
+end:
+    i2c_cmd_link_delete_static(handle);
+    return err;
+}
+
+static inline bool i2c_cmd_link_is_static(i2c_cmd_desc_t *cmd_desc)
+{
+    return (cmd_desc->free_buffer != NULL);
+}
+
+i2c_cmd_handle_t i2c_cmd_link_create_static(uint8_t* buffer, uint32_t size)
+{
+    if (buffer == NULL || size <= sizeof(i2c_cmd_desc_t)) {
+        return NULL;
+    }
+
+    i2c_cmd_desc_t *cmd_desc = (i2c_cmd_desc_t *) buffer;
+    cmd_desc->head = NULL;
+    cmd_desc->cur = NULL;
+    cmd_desc->free = NULL;
+    cmd_desc->free_buffer = cmd_desc + 1;
+    cmd_desc->free_size = size - sizeof(i2c_cmd_desc_t);
+
+    return (i2c_cmd_handle_t) cmd_desc;
+}
+
 i2c_cmd_handle_t i2c_cmd_link_create(void)
 {
 #if !CONFIG_SPIRAM_USE_MALLOC
@@ -867,12 +1023,27 @@ i2c_cmd_handle_t i2c_cmd_link_create(void)
     return (i2c_cmd_handle_t) cmd_desc;
 }
 
-void i2c_cmd_link_delete(i2c_cmd_handle_t cmd_handle)
+void i2c_cmd_link_delete_static(i2c_cmd_handle_t cmd_handle)
 {
-    if (cmd_handle == NULL) {
+    i2c_cmd_desc_t *cmd = (i2c_cmd_desc_t *) cmd_handle;
+    if (cmd == NULL || !i2c_cmd_link_is_static(cmd)) {
         return;
     }
+    /* Currently, this function does nothing, but it is not impossible
+     * that it will change in a near future. */
+}
+
+void i2c_cmd_link_delete(i2c_cmd_handle_t cmd_handle)
+{
     i2c_cmd_desc_t *cmd = (i2c_cmd_desc_t *) cmd_handle;
+
+    /* Memory should be freed only if allocated dynamically.
+     * If the user gave the buffer for a static allocation, do
+     * nothing. */
+    if (cmd == NULL || i2c_cmd_link_is_static(cmd)) {
+        return;
+    }
+
     while (cmd->free) {
         i2c_cmd_link_t *ptmp = cmd->free;
         cmd->free = cmd->free->next;
@@ -885,64 +1056,89 @@ void i2c_cmd_link_delete(i2c_cmd_handle_t cmd_handle)
     return;
 }
 
-static esp_err_t i2c_cmd_link_append(i2c_cmd_handle_t cmd_handle, i2c_cmd_t *cmd)
+static esp_err_t i2c_cmd_allocate(i2c_cmd_desc_t *cmd_desc, size_t n, size_t size, void** outptr)
 {
-    i2c_cmd_desc_t *cmd_desc = (i2c_cmd_desc_t *) cmd_handle;
-    if (cmd_desc->head == NULL) {
+    esp_err_t err = ESP_OK;
+
+    if (i2c_cmd_link_is_static(cmd_desc)) {
+        const size_t required = n * size;
+        /* User defined buffer.
+         * Check whether there is enough space in the buffer. */
+        if (cmd_desc->free_size < required) {
+            err = ESP_ERR_NO_MEM;
+        } else {
+            /* Allocate the pointer. */
+            *outptr = cmd_desc->free_buffer;
+
+            /* Decrement the free size from the user's bufffer. */
+            cmd_desc->free_buffer += required;
+            cmd_desc->free_size -= required;
+        }
+    } else {
 #if !CONFIG_SPIRAM_USE_MALLOC
-        cmd_desc->head = (i2c_cmd_link_t *) calloc(1, sizeof(i2c_cmd_link_t));
+        *outptr = calloc(n, size);
 #else
-        cmd_desc->head = (i2c_cmd_link_t *) heap_caps_calloc(1, sizeof(i2c_cmd_link_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+        *outptr = heap_caps_calloc(n, size, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
 #endif
-        if (cmd_desc->head == NULL) {
-            ESP_LOGE(I2C_TAG, I2C_CMD_MALLOC_ERR_STR);
-            goto err;
+        if (*outptr == NULL) {
+            err = ESP_FAIL;
+        }
+    }
+
+    return err;
+}
+
+static inline void i2c_cmd_log_alloc_error(i2c_cmd_desc_t *cmd_desc)
+{
+    if (i2c_cmd_link_is_static(cmd_desc)) {
+        ESP_LOGE(I2C_TAG, I2C_CMD_USER_ALLOC_ERR_STR);
+    } else {
+        ESP_LOGE(I2C_TAG, I2C_CMD_MALLOC_ERR_STR);
+    }
+}
+
+static esp_err_t i2c_cmd_link_append(i2c_cmd_handle_t cmd_handle, i2c_cmd_t *cmd)
+{
+    esp_err_t err = ESP_OK;
+    i2c_cmd_desc_t *cmd_desc = (i2c_cmd_desc_t *) cmd_handle;
+
+    assert(cmd_desc != NULL);
+
+    if (cmd_desc->head == NULL) {
+        err = i2c_cmd_allocate(cmd_desc, 1, sizeof(i2c_cmd_link_t), (void**) &cmd_desc->head);
+        if (err != ESP_OK) {
+            i2c_cmd_log_alloc_error(cmd_desc);
+            return err;
         }
         cmd_desc->cur = cmd_desc->head;
         cmd_desc->free = cmd_desc->head;
     } else {
-#if !CONFIG_SPIRAM_USE_MALLOC
-        cmd_desc->cur->next = (i2c_cmd_link_t *) calloc(1, sizeof(i2c_cmd_link_t));
-#else
-        cmd_desc->cur->next = (i2c_cmd_link_t *) heap_caps_calloc(1, sizeof(i2c_cmd_link_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
-#endif
-        if (cmd_desc->cur->next == NULL) {
-            ESP_LOGE(I2C_TAG, I2C_CMD_MALLOC_ERR_STR);
-            goto err;
+        assert(cmd_desc->cur != NULL);
+        err = i2c_cmd_allocate(cmd_desc, 1, sizeof(i2c_cmd_link_t), (void**) &cmd_desc->cur->next);
+        if (err != ESP_OK) {
+            i2c_cmd_log_alloc_error(cmd_desc);
+            return err;
         }
         cmd_desc->cur = cmd_desc->cur->next;
     }
     memcpy((uint8_t *) &cmd_desc->cur->cmd, (uint8_t *) cmd, sizeof(i2c_cmd_t));
     cmd_desc->cur->next = NULL;
-    return ESP_OK;
-
-err:
-    return ESP_FAIL;
+    return err;
 }
 
 esp_err_t i2c_master_start(i2c_cmd_handle_t cmd_handle)
 {
     I2C_CHECK(cmd_handle != NULL, I2C_CMD_LINK_INIT_ERR_STR, ESP_ERR_INVALID_ARG);
-    i2c_cmd_t cmd;
-    cmd.hw_cmd.ack_en = 0;
-    cmd.hw_cmd.ack_exp = 0;
-    cmd.hw_cmd.ack_val = 0;
+    i2c_cmd_t cmd = { 0 };
     cmd.hw_cmd.op_code = I2C_LL_CMD_RESTART;
-    cmd.hw_cmd.byte_num = 0;
-    cmd.data = NULL;
     return i2c_cmd_link_append(cmd_handle, &cmd);
 }
 
 esp_err_t i2c_master_stop(i2c_cmd_handle_t cmd_handle)
 {
     I2C_CHECK(cmd_handle != NULL, I2C_CMD_LINK_INIT_ERR_STR, ESP_ERR_INVALID_ARG);
-    i2c_cmd_t cmd;
-    cmd.hw_cmd.ack_en = 0;
-    cmd.hw_cmd.ack_exp = 0;
-    cmd.hw_cmd.ack_val = 0;
+    i2c_cmd_t cmd = { 0 };
     cmd.hw_cmd.op_code = I2C_LL_CMD_STOP;
-    cmd.hw_cmd.byte_num = 0;
-    cmd.data = NULL;
     return i2c_cmd_link_append(cmd_handle, &cmd);
 }
 
@@ -951,64 +1147,46 @@ esp_err_t i2c_master_write(i2c_cmd_handle_t cmd_handle, const uint8_t *data, siz
     I2C_CHECK((data != NULL), I2C_ADDR_ERROR_STR, ESP_ERR_INVALID_ARG);
     I2C_CHECK(cmd_handle != NULL, I2C_CMD_LINK_INIT_ERR_STR, ESP_ERR_INVALID_ARG);
 
-    uint8_t len_tmp;
-    int data_offset = 0;
-    esp_err_t ret;
-    while (data_len > 0) {
-        len_tmp = data_len > 0xff ? 0xff : data_len;
-        data_len -= len_tmp;
-        i2c_cmd_t cmd;
-        cmd.hw_cmd.ack_en = ack_en;
-        cmd.hw_cmd.ack_exp = 0;
-        cmd.hw_cmd.ack_val = 0;
-        cmd.hw_cmd.op_code = I2C_LL_CMD_WRITE;
-        cmd.hw_cmd.byte_num = len_tmp;
-        cmd.data = (uint8_t*) data + data_offset;
-        ret = i2c_cmd_link_append(cmd_handle, &cmd);
-        data_offset += len_tmp;
-        if (ret != ESP_OK) {
-            return ret;
-        }
-    }
-    return ESP_OK;
+    i2c_cmd_t cmd = {
+        .hw_cmd = {
+            .ack_en = ack_en,
+            .op_code = I2C_LL_CMD_WRITE,
+        },
+        .data = (uint8_t*) data,
+        .total_bytes = data_len,
+    };
+
+    return i2c_cmd_link_append(cmd_handle, &cmd);
 }
 
 esp_err_t i2c_master_write_byte(i2c_cmd_handle_t cmd_handle, uint8_t data, bool ack_en)
 {
     I2C_CHECK(cmd_handle != NULL, I2C_CMD_LINK_INIT_ERR_STR, ESP_ERR_INVALID_ARG);
-    i2c_cmd_t cmd;
-    cmd.hw_cmd.ack_en = ack_en;
-    cmd.hw_cmd.ack_exp = 0;
-    cmd.hw_cmd.ack_val = 0;
-    cmd.hw_cmd.op_code = I2C_LL_CMD_WRITE;
-    cmd.hw_cmd.byte_num = 1;
-    cmd.data = NULL;
-    cmd.byte_cmd = data;
+
+    i2c_cmd_t cmd = {
+        .hw_cmd = {
+            .ack_en = ack_en,
+            .op_code = I2C_LL_CMD_WRITE,
+        },
+        .data_byte = data,
+        .total_bytes = 1,
+    };
+
     return i2c_cmd_link_append(cmd_handle, &cmd);
 }
 
 static esp_err_t i2c_master_read_static(i2c_cmd_handle_t cmd_handle, uint8_t *data, size_t data_len, i2c_ack_type_t ack)
 {
-    int len_tmp;
-    int data_offset = 0;
-    esp_err_t ret;
-    while (data_len > 0) {
-        len_tmp = data_len > 0xff ? 0xff : data_len;
-        data_len -= len_tmp;
-        i2c_cmd_t cmd;
-        cmd.hw_cmd.ack_en = 0;
-        cmd.hw_cmd.ack_exp = 0;
-        cmd.hw_cmd.ack_val = ack & 0x1;
-        cmd.hw_cmd.byte_num = len_tmp;
-        cmd.hw_cmd.op_code = I2C_LL_CMD_READ;
-        cmd.data = data + data_offset;
-        ret = i2c_cmd_link_append(cmd_handle, &cmd);
-        data_offset += len_tmp;
-        if (ret != ESP_OK) {
-            return ret;
-        }
-    }
-    return ESP_OK;
+    i2c_cmd_t cmd = {
+        .hw_cmd = {
+            .ack_val = ack & 0x1,
+            .op_code = I2C_LL_CMD_READ,
+        },
+        .data = data,
+        .total_bytes = data_len,
+    };
+
+    return i2c_cmd_link_append(cmd_handle, &cmd);
 }
 
 esp_err_t i2c_master_read_byte(i2c_cmd_handle_t cmd_handle, uint8_t *data, i2c_ack_type_t ack)
@@ -1017,13 +1195,15 @@ esp_err_t i2c_master_read_byte(i2c_cmd_handle_t cmd_handle, uint8_t *data, i2c_a
     I2C_CHECK(cmd_handle != NULL, I2C_CMD_LINK_INIT_ERR_STR, ESP_ERR_INVALID_ARG);
     I2C_CHECK(ack < I2C_MASTER_ACK_MAX, I2C_ACK_TYPE_ERR_STR, ESP_ERR_INVALID_ARG);
 
-    i2c_cmd_t cmd;
-    cmd.hw_cmd.ack_en = 0;
-    cmd.hw_cmd.ack_exp = 0;
-    cmd.hw_cmd.ack_val = ((ack == I2C_MASTER_LAST_NACK) ? I2C_MASTER_NACK : (ack & 0x1));
-    cmd.hw_cmd.byte_num = 1;
-    cmd.hw_cmd.op_code = I2C_LL_CMD_READ;
-    cmd.data = data;
+    i2c_cmd_t cmd = {
+        .hw_cmd = {
+            .ack_val = ((ack == I2C_MASTER_LAST_NACK) ? I2C_MASTER_NACK : (ack & 0x1)),
+            .op_code = I2C_LL_CMD_READ,
+        },
+        .data = data,
+        .total_bytes = 1,
+    };
+
     return i2c_cmd_link_append(cmd_handle, &cmd);
 }
 
@@ -1034,34 +1214,52 @@ esp_err_t i2c_master_read(i2c_cmd_handle_t cmd_handle, uint8_t *data, size_t dat
     I2C_CHECK(ack < I2C_MASTER_ACK_MAX, I2C_ACK_TYPE_ERR_STR, ESP_ERR_INVALID_ARG);
     I2C_CHECK(data_len > 0, I2C_DATA_LEN_ERR_STR, ESP_ERR_INVALID_ARG);
 
+    esp_err_t ret = ESP_OK;
+
+    /* Check if we can perform a single transfer.
+     * This is the case if a NACK is NOT required at the end of the last transferred byte
+     * (i.e. ACK is required at the end), or if a single byte has to be read.
+     */
     if (ack != I2C_MASTER_LAST_NACK) {
-        return i2c_master_read_static(cmd_handle, data, data_len, ack);
+        ret = i2c_master_read_static(cmd_handle, data, data_len, ack);
+    } else if (data_len == 1) {
+        ret = i2c_master_read_byte(cmd_handle, data, I2C_MASTER_NACK);
     } else {
-        if (data_len == 1) {
-            return i2c_master_read_byte(cmd_handle, data, I2C_MASTER_NACK);
-        } else {
-            esp_err_t ret;
-            if ((ret =  i2c_master_read_static(cmd_handle, data, data_len - 1, I2C_MASTER_ACK)) != ESP_OK) {
-                return ret;
-            }
-            return i2c_master_read_byte(cmd_handle, data + data_len - 1, I2C_MASTER_NACK);
+        /* In this case, we have to read data_len-1 bytes sending an ACK at the end
+         * of each one.
+         */
+        ret = i2c_master_read_static(cmd_handle, data, data_len - 1, I2C_MASTER_ACK);
+
+        /* Last byte has to be NACKed. */
+        if (ret == ESP_OK) {
+            ret = i2c_master_read_byte(cmd_handle, data + data_len - 1, I2C_MASTER_NACK);
         }
     }
+
+    return ret;
+}
+
+static inline bool i2c_cmd_is_single_byte(const i2c_cmd_t *cmd) {
+    return cmd->total_bytes == 1;
 }
 
 static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
 {
     i2c_obj_t *p_i2c = p_i2c_obj[i2c_num];
     portBASE_TYPE HPTaskAwoken = pdFALSE;
-    i2c_cmd_evt_t evt;
+    i2c_cmd_evt_t evt = { 0 };
     if (p_i2c->cmd_link.head != NULL && p_i2c->status == I2C_STATUS_READ) {
         i2c_cmd_t *cmd = &p_i2c->cmd_link.head->cmd;
-        i2c_hal_read_rxfifo(&(i2c_context[i2c_num].hal), cmd->data, p_i2c->rx_cnt);
-        cmd->data += p_i2c->rx_cnt;
-        if (cmd->hw_cmd.byte_num > 0) {
+        i2c_hal_read_rxfifo(&(i2c_context[i2c_num].hal), cmd->data + cmd->bytes_used, p_i2c->rx_cnt);
+        /* rx_cnt bytes have just been read, increment the number of bytes used from the buffer */
+        cmd->bytes_used += p_i2c->rx_cnt;
+
+        /* Test if there are still some remaining bytes to send. */
+        if (cmd->bytes_used != cmd->total_bytes) {
             p_i2c->cmd_idx = 0;
         } else {
             p_i2c->cmd_link.head = p_i2c->cmd_link.head->next;
+            p_i2c->cmd_link.head->cmd.bytes_used = 0;
         }
     } else if ((p_i2c->status == I2C_STATUS_ACK_ERROR)
                || (p_i2c->status == I2C_STATUS_TIMEOUT)) {
@@ -1085,37 +1283,47 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
     };
     while (p_i2c->cmd_link.head) {
         i2c_cmd_t *cmd = &p_i2c->cmd_link.head->cmd;
+        const size_t remaining_bytes = cmd->total_bytes - cmd->bytes_used;
+
         i2c_hw_cmd_t hw_cmd = cmd->hw_cmd;
+        uint8_t fifo_fill = 0;
+
         if (cmd->hw_cmd.op_code == I2C_LL_CMD_WRITE) {
-            uint8_t wr_filled = 0;
             uint8_t *write_pr = NULL;
+
             //TODO: to reduce interrupt number
-            if (cmd->data) {
-                wr_filled = (cmd->hw_cmd.byte_num > SOC_I2C_FIFO_LEN) ? SOC_I2C_FIFO_LEN : cmd->hw_cmd.byte_num;
-                write_pr = cmd->data;
-                cmd->data += wr_filled;
-                cmd->hw_cmd.byte_num -= wr_filled;
+            if (!i2c_cmd_is_single_byte(cmd)) {
+                fifo_fill = MIN(remaining_bytes, SOC_I2C_FIFO_LEN);
+                /* cmd->data shall not be altered!
+                 * Else it would not be possible to reuse the commands list. */
+                write_pr = cmd->data + cmd->bytes_used;
+                cmd->bytes_used += fifo_fill;
             } else {
-                wr_filled = 1;
-                write_pr = &(cmd->byte_cmd);
-                cmd->hw_cmd.byte_num--;
+                fifo_fill = 1;
+                /* `data_byte` field contains the data itself.
+                 * NOTE: It is possible to get the correct data (and not 0s)
+                 * because both Xtensa and RISC-V architectures used on ESP
+                 * boards are little-endian.
+                 */
+                write_pr = (uint8_t*) &cmd->data_byte;
             }
-            hw_cmd.byte_num = wr_filled;
-            i2c_hal_write_txfifo(&(i2c_context[i2c_num].hal), write_pr, wr_filled);
+            hw_cmd.byte_num = fifo_fill;
+            i2c_hal_write_txfifo(&(i2c_context[i2c_num].hal), write_pr, fifo_fill);
             i2c_hal_write_cmd_reg(&(i2c_context[i2c_num].hal), hw_cmd, p_i2c->cmd_idx);
             i2c_hal_write_cmd_reg(&(i2c_context[i2c_num].hal), hw_end_cmd, p_i2c->cmd_idx + 1);
             i2c_hal_enable_master_tx_it(&(i2c_context[i2c_num].hal));
             p_i2c->cmd_idx = 0;
-            if (cmd->hw_cmd.byte_num == 0) {
+            if (i2c_cmd_is_single_byte(cmd) || cmd->total_bytes == cmd->bytes_used) {
                 p_i2c->cmd_link.head = p_i2c->cmd_link.head->next;
+                p_i2c->cmd_link.head->cmd.bytes_used = 0;
             }
             p_i2c->status = I2C_STATUS_WRITE;
             break;
         } else if (cmd->hw_cmd.op_code == I2C_LL_CMD_READ) {
             //TODO: to reduce interrupt number
-            p_i2c->rx_cnt = cmd->hw_cmd.byte_num > SOC_I2C_FIFO_LEN ? SOC_I2C_FIFO_LEN : cmd->hw_cmd.byte_num;
-            cmd->hw_cmd.byte_num -= p_i2c->rx_cnt;
-            hw_cmd.byte_num = p_i2c->rx_cnt;
+            fifo_fill = MIN(remaining_bytes, SOC_I2C_FIFO_LEN);
+            p_i2c->rx_cnt = fifo_fill;
+            hw_cmd.byte_num = fifo_fill;
             i2c_hal_write_cmd_reg(&(i2c_context[i2c_num].hal), hw_cmd, p_i2c->cmd_idx);
             i2c_hal_write_cmd_reg(&(i2c_context[i2c_num].hal), hw_end_cmd, p_i2c->cmd_idx + 1);
             i2c_hal_enable_master_rx_it(&(i2c_context[i2c_num].hal));
@@ -1191,6 +1399,10 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
     i2c_reset_tx_fifo(i2c_num);
     i2c_reset_rx_fifo(i2c_num);
     const i2c_cmd_desc_t *cmd = (const i2c_cmd_desc_t *) cmd_handle;
+    /* Before starting the transfer, resetset the number of bytes sent to 0.
+     * `i2c_master_cmd_begin_static` will also reset this field for each node
+     * while browsing the command list. */
+    cmd->head->cmd.bytes_used = 0;
     p_i2c->cmd_link.free = cmd->free;
     p_i2c->cmd_link.cur = cmd->cur;
     p_i2c->cmd_link.head = cmd->head;

components/driver/include/driver/i2c.h

@@ -36,6 +36,32 @@ extern "C" {
 #define I2C_SCLK_SRC_FLAG_AWARE_DFS       (1 << 0)    /*!< For REF tick clock, it won't change with APB.*/
 #define I2C_SCLK_SRC_FLAG_LIGHT_SLEEP     (1 << 1)    /*!< For light sleep mode.*/
 
+/**
+ * @brief Minimum size, in bytes, of the internal private structure used to describe
+ * I2C commands link.
+ */
+#define I2C_INTERNAL_STRUCT_SIZE (24)
+
+/**
+ * @brief The following macro is used to determine the recommended size of the
+ * buffer to pass to `i2c_cmd_link_create_static()` function.
+ * It requires one parameter, `TRANSACTIONS`, describing the number of transactions
+ * intended to be performed on the I2C port.
+ * For example, if one wants to perform a read on an I2C device register, `TRANSACTIONS`
+ * must be at least 2, because the commands required are the following:
+ *  - write device register
+ *  - read register content
+ *
+ * Signals such as "(repeated) start", "stop", "nack", "ack" shall not be counted.
+ */
+#define I2C_LINK_RECOMMENDED_SIZE(TRANSACTIONS)     (2 * I2C_INTERNAL_STRUCT_SIZE + I2C_INTERNAL_STRUCT_SIZE * \
+                                                        (5 * TRANSACTIONS)) /* Make the assumption that each transaction
+                                                                             * of the user is surrounded by a "start", device address
+                                                                             * and a "nack/ack" signal. Allocate one more room for
+                                                                             * "stop" signal at the end.
+                                                                             * Allocate 2 more internal struct size for headers.
+                                                                             */
+
 /**
  * @brief I2C initialization parameters
  */
@@ -62,18 +88,14 @@ typedef struct{
 typedef void *i2c_cmd_handle_t;    /*!< I2C command handle  */
 
 /**
- * @brief I2C driver install
+ * @brief Install an I2C driver
  *
  * @param i2c_num I2C port number
- * @param mode I2C mode( master or slave )
- * @param slv_rx_buf_len receiving buffer size for slave mode
- *        @note
- *        Only slave mode will use this value, driver will ignore this value in master mode.
- * @param slv_tx_buf_len sending buffer size for slave mode
- *        @note
- *        Only slave mode will use this value, driver will ignore this value in master mode.
- * @param intr_alloc_flags Flags used to allocate the interrupt. One or multiple (ORred)
- *            ESP_INTR_FLAG_* values. See esp_intr_alloc.h for more info.
+ * @param mode I2C mode (either master or slave)
+ * @param slv_rx_buf_len Receiving buffer size. Only slave mode will use this value, it is ignored in master mode.
+ * @param slv_tx_buf_len Sending buffer size. Only slave mode will use this value, it is ignored in master mode.
+ * @param intr_alloc_flags Flags used to allocate the interrupt. One or multiple (ORred) ESP_INTR_FLAG_* values.
+ *                         See esp_intr_alloc.h for more info.
  *        @note
  *        In master mode, if the cache is likely to be disabled(such as write flash) and the slave is time-sensitive,
  *        `ESP_INTR_FLAG_IRAM` is suggested to be used. In this case, please use the memory allocated from internal RAM in i2c read and write function,
@@ -82,17 +104,17 @@ typedef void *i2c_cmd_handle_t;    /*!< I2C command handle  */
  * @return
  *     - ESP_OK   Success
  *     - ESP_ERR_INVALID_ARG Parameter error
- *     - ESP_FAIL Driver install error
+ *     - ESP_FAIL Driver installation error
  */
 esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_buf_len, size_t slv_tx_buf_len, int intr_alloc_flags);
 
 /**
- * @brief I2C driver delete
+ * @brief Delete I2C driver
  *
  * @note This function does not guarantee thread safety.
  *       Please make sure that no thread will continuously hold semaphores before calling the delete function.
  *
- * @param i2c_num I2C port number
+ * @param i2c_num I2C port to delete
  *
  * @return
  *     - ESP_OK Success
@@ -101,10 +123,10 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_
 esp_err_t i2c_driver_delete(i2c_port_t i2c_num);
 
 /**
- * @brief I2C parameter initialization
+ * @brief Configure an I2C bus with the given configuration.
  *
- * @param i2c_num I2C port number
- * @param i2c_conf pointer to I2C parameter settings
+ * @param i2c_num I2C port to configure
+ * @param i2c_conf Pointer to the I2C configuration
  *
  * @return
  *     - ESP_OK Success
@@ -135,14 +157,14 @@ esp_err_t i2c_reset_tx_fifo(i2c_port_t i2c_num);
 esp_err_t i2c_reset_rx_fifo(i2c_port_t i2c_num);
 
 /**
- * @brief I2C isr handler register
+ * @brief Register an I2C ISR handler.
  *
- * @param i2c_num I2C port number
- * @param fn isr handler function
- * @param arg parameter for isr handler function
+ * @param i2c_num I2C port number to attach handler to
+ * @param fn ISR handler function
+ * @param arg Parameter for the ISR handler
  * @param intr_alloc_flags Flags used to allocate the interrupt. One or multiple (ORred)
- *            ESP_INTR_FLAG_* values. See esp_intr_alloc.h for more info.
- * @param handle handle return from esp_intr_alloc.
+ *                         ESP_INTR_FLAG_* values. See esp_intr_alloc.h for more info.
+ * @param handle Handle return from esp_intr_alloc.
  *
  * @return
  *     - ESP_OK Success
@@ -151,9 +173,9 @@ esp_err_t i2c_reset_rx_fifo(i2c_port_t i2c_num);
 esp_err_t i2c_isr_register(i2c_port_t i2c_num, void (*fn)(void *), void *arg, int intr_alloc_flags, intr_handle_t *handle);
 
 /**
- * @brief to delete and free I2C isr.
+ * @brief Delete and free I2C ISR handle.
  *
- * @param handle handle of isr.
+ * @param handle Handle of isr to delete.
  *
  * @return
  *     - ESP_OK Success
@@ -162,13 +184,13 @@ esp_err_t i2c_isr_register(i2c_port_t i2c_num, void (*fn)(void *), void *arg, in
 esp_err_t i2c_isr_free(intr_handle_t handle);
 
 /**
- * @brief Configure GPIO signal for I2C sck and sda
+ * @brief Configure GPIO pins for I2C SCK and SDA signals.
  *
  * @param i2c_num I2C port number
- * @param sda_io_num GPIO number for I2C sda signal
- * @param scl_io_num GPIO number for I2C scl signal
- * @param sda_pullup_en Whether to enable the internal pullup for sda pin
- * @param scl_pullup_en Whether to enable the internal pullup for scl pin
+ * @param sda_io_num GPIO number for I2C SDA signal
+ * @param scl_io_num GPIO number for I2C SCL signal
+ * @param sda_pullup_en Enable the internal pullup for SDA pin
+ * @param scl_pullup_en Enable the internal pullup for SCL pin
  * @param mode I2C mode
  *
  * @return
@@ -179,191 +201,289 @@ esp_err_t i2c_set_pin(i2c_port_t i2c_num, int sda_io_num, int scl_io_num,
                       bool sda_pullup_en, bool scl_pullup_en, i2c_mode_t mode);
 
 /**
- * @brief Create and init I2C command link
- *        @note
- *        Before we build I2C command link, we need to call i2c_cmd_link_create() to create
- *        a command link.
- *        After we finish sending the commands, we need to call i2c_cmd_link_delete() to
- *        release and return the resources.
+ * @brief Perform a write to a device connected to a particular I2C port.
+ *        This function is a wrapper to `i2c_master_start()`, `i2c_master_write()`, `i2c_master_read()`, etc...
+ *        It shall only be called in I2C master mode.
+ *
+ * @param i2c_num I2C port number to perform the transfer on
+ * @param device_address I2C device's 7-bit address
+ * @param write_buffer Bytes to send on the bus
+ * @param write_size Size, in bytes, of the write buffer
+ * @param ticks_to_wait Maximum ticks to wait before issuing a timeout.
+ *
+ * @return
+ *     - ESP_OK Success
+ *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_FAIL Sending command error, slave hasn't ACK the transfer.
+ *     - ESP_ERR_INVALID_STATE I2C driver not installed or not in master mode.
+ *     - ESP_ERR_TIMEOUT Operation timeout because the bus is busy.
+ */
+esp_err_t i2c_master_write_to_device(i2c_port_t i2c_num, uint8_t device_address,
+                                     const uint8_t* write_buffer, size_t write_size,
+                                     TickType_t ticks_to_wait);
+
+/**
+ * @brief Perform a read to a device connected to a particular I2C port.
+ *        This function is a wrapper to `i2c_master_start()`, `i2c_master_write()`, `i2c_master_read()`, etc...
+ *        It shall only be called in I2C master mode.
+ *
+ * @param i2c_num I2C port number to perform the transfer on
+ * @param device_address I2C device's 7-bit address
+ * @param read_buffer Buffer to store the bytes received on the bus
+ * @param read_size Size, in bytes, of the read buffer
+ * @param ticks_to_wait Maximum ticks to wait before issuing a timeout.
+ *
+ * @return
+ *     - ESP_OK Success
+ *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_FAIL Sending command error, slave hasn't ACK the transfer.
+ *     - ESP_ERR_INVALID_STATE I2C driver not installed or not in master mode.
+ *     - ESP_ERR_TIMEOUT Operation timeout because the bus is busy.
+ */
+esp_err_t i2c_master_read_from_device(i2c_port_t i2c_num, uint8_t device_address,
+                                      uint8_t* read_buffer, size_t read_size,
+                                      TickType_t ticks_to_wait);
+
+/**
+ * @brief Perform a write followed by a read to a device on the I2C bus.
+ *        A repeated start signal is used between the `write` and `read`, thus, the bus is
+ *        not released until the two transactions are finished.
+ *        This function is a wrapper to `i2c_master_start()`, `i2c_master_write()`, `i2c_master_read()`, etc...
+ *        It shall only be called in I2C master mode.
+ *
+ * @param i2c_num I2C port number to perform the transfer on
+ * @param device_address I2C device's 7-bit address
+ * @param write_buffer Bytes to send on the bus
+ * @param write_size Size, in bytes, of the write buffer
+ * @param read_buffer Buffer to store the bytes received on the bus
+ * @param read_size Size, in bytes, of the read buffer
+ * @param ticks_to_wait Maximum ticks to wait before issuing a timeout.
+ *
+ * @return
+ *     - ESP_OK Success
+ *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_FAIL Sending command error, slave hasn't ACK the transfer.
+ *     - ESP_ERR_INVALID_STATE I2C driver not installed or not in master mode.
+ *     - ESP_ERR_TIMEOUT Operation timeout because the bus is busy.
+ */
+esp_err_t i2c_master_write_read_device(i2c_port_t i2c_num, uint8_t device_address,
+                                       const uint8_t* write_buffer, size_t write_size,
+                                       uint8_t* read_buffer, size_t read_size,
+                                       TickType_t ticks_to_wait);
+
+
+/**
+ * @brief Create and initialize an I2C commands list with a given buffer.
+ *        All the allocations for data or signals (START, STOP, ACK, ...) will be
+ *        performed within this buffer.
+ *        This buffer must be valid during the whole transaction.
+ *        After finishing the I2C transactions, it is required to call `i2c_cmd_link_delete_static()`.
  *
- * @return i2c command link handler
+ * @note It is **highly** advised to not allocate this buffer on the stack. The size of the data
+ *       used underneath may increase in the future, resulting in a possible stack overflow as the macro
+ *       `I2C_LINK_RECOMMENDED_SIZE` would also return a bigger value.
+ *       A better option is to use a buffer allocated statically or dynamically (with `malloc`).
+ *
+ * @param buffer Buffer to use for commands allocations
+ * @param size Size in bytes of the buffer
+ *
+ * @return Handle to the I2C command link or NULL if the buffer provided is too small, please
+ *         use `I2C_LINK_RECOMMENDED_SIZE` macro to get the recommended size for the buffer.
+ */
+i2c_cmd_handle_t i2c_cmd_link_create_static(uint8_t* buffer, uint32_t size);
+
+/**
+ * @brief Create and initialize an I2C commands list with a given buffer.
+ *        After finishing the I2C transactions, it is required to call `i2c_cmd_link_delete()`
+ *        to release and return the resources.
+ *        The required bytes will be dynamically allocated.
+ *
+ * @return Handle to the I2C command link
  */
 i2c_cmd_handle_t i2c_cmd_link_create(void);
 
 /**
- * @brief Free I2C command link
- *        @note
- *        Before we build I2C command link, we need to call i2c_cmd_link_create() to create
- *        a command link.
- *        After we finish sending the commands, we need to call i2c_cmd_link_delete() to
- *        release and return the resources.
+ * @brief Free the I2C commands list allocated statically with `i2c_cmd_link_create_static`.
+ *
+ * @param cmd_handle I2C commands list allocated statically. This handle should be created thanks to
+ *                   `i2c_cmd_link_create_static()` function
+ */
+void i2c_cmd_link_delete_static(i2c_cmd_handle_t cmd_handle);
+
+/**
+ * @brief Free the I2C commands list
  *
- * @param cmd_handle I2C command handle
+ * @param cmd_handle I2C commands list. This handle should be created thanks to
+ *                   `i2c_cmd_link_create()` function
  */
 void i2c_cmd_link_delete(i2c_cmd_handle_t cmd_handle);
 
 /**
- * @brief Queue command for I2C master to generate a start signal
- *        @note
- *        Only call this function in I2C master mode
- *        Call i2c_master_cmd_begin() to send all queued commands
+ * @brief Queue a "START signal" to the given commands list.
+ *        This function shall only be called in I2C master mode.
+ *        Call `i2c_master_cmd_begin()` to send all the queued commands.
  *
- * @param cmd_handle I2C cmd link
+ * @param cmd_handle I2C commands list
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_ERR_NO_MEM The static buffer used to create `cmd_handler` is too small
+ *     - ESP_FAIL No more memory left on the heap
  */
 esp_err_t i2c_master_start(i2c_cmd_handle_t cmd_handle);
 
 /**
- * @brief Queue command for I2C master to write one byte to I2C bus
- *        @note
- *        Only call this function in I2C master mode
- *        Call i2c_master_cmd_begin() to send all queued commands
+ * @brief Queue a "write byte" command to the commands list.
+ *        A single byte will be sent on the I2C port. This function shall only be
+ *        called in I2C master mode.
+ *        Call `i2c_master_cmd_begin()` to send all queued commands
  *
- * @param cmd_handle I2C cmd link
- * @param data I2C one byte command to write to bus
- * @param ack_en enable ack check for master
+ * @param cmd_handle I2C commands list
+ * @param data Byte to send on the port
+ * @param ack_en Enable ACK signal
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_ERR_NO_MEM The static buffer used to create `cmd_handler` is too small
+ *     - ESP_FAIL No more memory left on the heap
  */
 esp_err_t i2c_master_write_byte(i2c_cmd_handle_t cmd_handle, uint8_t data, bool ack_en);
 
 /**
- * @brief Queue command for I2C master to write buffer to I2C bus
- *        @note
- *        Only call this function in I2C master mode
- *        Call i2c_master_cmd_begin() to send all queued commands
+ * @brief Queue a "write (multiple) bytes" command to the commands list.
+ *        This function shall only be called in I2C master mode.
+ *        Call `i2c_master_cmd_begin()` to send all queued commands
  *
- * @param cmd_handle I2C cmd link
- * @param data data to send
- *        @note
- *        If the psram is enabled and intr_flag is `ESP_INTR_FLAG_IRAM`, please use the memory allocated from internal RAM.
- * @param data_len data length
- * @param ack_en enable ack check for master
+ * @param cmd_handle I2C commands list
+ * @param data Bytes to send. This buffer shall remain **valid** until the transaction is finished.
+ *             If the PSRAM is enabled and `intr_flag` is set to `ESP_INTR_FLAG_IRAM`,
+ *             `data` should be allocated from internal RAM.
+ * @param data_len Length, in bytes, of the data buffer
+ * @param ack_en Enable ACK signal
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_ERR_NO_MEM The static buffer used to create `cmd_handler` is too small
+ *     - ESP_FAIL No more memory left on the heap
  */
 esp_err_t i2c_master_write(i2c_cmd_handle_t cmd_handle, const uint8_t *data, size_t data_len, bool ack_en);
 
 /**
- * @brief Queue command for I2C master to read one byte from I2C bus
- *        @note
- *        Only call this function in I2C master mode
- *        Call i2c_master_cmd_begin() to send all queued commands
+ * @brief Queue a "read byte" command to the commands list.
+ *        A single byte will be read on the I2C bus. This function shall only be
+ *        called in I2C master mode.
+ *        Call `i2c_master_cmd_begin()` to send all queued commands
  *
- * @param cmd_handle I2C cmd link
- * @param data pointer accept the data byte
- *        @note
- *        If the psram is enabled and intr_flag is `ESP_INTR_FLAG_IRAM`, please use the memory allocated from internal RAM.
- * @param ack ack value for read command
+ * @param cmd_handle I2C commands list
+ * @param data Pointer where the received byte will the stored. This buffer shall remain **valid**
+ *             until the transaction is finished.
+ * @param ack ACK signal
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_ERR_NO_MEM The static buffer used to create `cmd_handler` is too small
+ *     - ESP_FAIL No more memory left on the heap
  */
 esp_err_t i2c_master_read_byte(i2c_cmd_handle_t cmd_handle, uint8_t *data, i2c_ack_type_t ack);
 
 /**
- * @brief Queue command for I2C master to read data from I2C bus
- *        @note
- *        Only call this function in I2C master mode
- *        Call i2c_master_cmd_begin() to send all queued commands
+ * @brief Queue a "read (multiple) bytes" command to the commands list.
+ *        Multiple bytes will be read on the I2C bus. This function shall only be
+ *        called in I2C master mode.
+ *        Call `i2c_master_cmd_begin()` to send all queued commands
  *
- * @param cmd_handle I2C cmd link
- * @param data data buffer to accept the data from bus
- *        @note
- *        If the psram is enabled and intr_flag is `ESP_INTR_FLAG_IRAM`, please use the memory allocated from internal RAM.
- * @param data_len read data length
- * @param ack ack value for read command
+ * @param cmd_handle I2C commands list
+ * @param data Pointer where the received bytes will the stored. This buffer shall remain **valid**
+ *             until the transaction is finished.
+ * @param data_len Size, in bytes, of the `data` buffer
+ * @param ack ACK signal
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_ERR_NO_MEM The static buffer used to create `cmd_handler` is too small
+ *     - ESP_FAIL No more memory left on the heap
  */
 esp_err_t i2c_master_read(i2c_cmd_handle_t cmd_handle, uint8_t *data, size_t data_len, i2c_ack_type_t ack);
 
 /**
- * @brief Queue command for I2C master to generate a stop signal
- *        @note
- *        Only call this function in I2C master mode
- *        Call i2c_master_cmd_begin() to send all queued commands
+ * @brief Queue a "STOP signal" to the given commands list.
+ *        This function shall only be called in I2C master mode.
+ *        Call `i2c_master_cmd_begin()` to send all the queued commands.
  *
- * @param cmd_handle I2C cmd link
+ * @param cmd_handle I2C commands list
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
+ *     - ESP_ERR_NO_MEM The static buffer used to create `cmd_handler` is too small
+ *     - ESP_FAIL No more memory left on the heap
  */
 esp_err_t i2c_master_stop(i2c_cmd_handle_t cmd_handle);
 
 /**
- * @brief I2C master send queued commands.
- *        This function will trigger sending all queued commands.
+ * @brief Send all the queued commands on the I2C bus, in master mode.
  *        The task will be blocked until all the commands have been sent out.
  *        The I2C APIs are not thread-safe, if you want to use one I2C port in different tasks,
  *        you need to take care of the multi-thread issue.
- *        @note
- *        Only call this function in I2C master mode
+ *        This function shall only be called in I2C master mode.
  *
  * @param i2c_num I2C port number
- * @param cmd_handle I2C command handler
- * @param ticks_to_wait maximum wait ticks.
+ * @param cmd_handle I2C commands list
+ * @param ticks_to_wait Maximum ticks to wait before issuing a timeout.
  *
  * @return
  *     - ESP_OK Success
  *     - ESP_ERR_INVALID_ARG Parameter error
- *     - ESP_FAIL Sending command error, slave doesn't ACK the transfer.
+ *     - ESP_FAIL Sending command error, slave hasn't ACK the transfer.
  *     - ESP_ERR_INVALID_STATE I2C driver not installed or not in master mode.
  *     - ESP_ERR_TIMEOUT Operation timeout because the bus is busy.
  */
 esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle, TickType_t ticks_to_wait);
 
 /**
- * @brief I2C slave write data to internal ringbuffer, when tx fifo empty, isr will fill the hardware
- *        fifo from the internal ringbuffer
- *        @note
- *        Only call this function in I2C slave mode
+ * @brief Write bytes to internal ringbuffer of the I2C slave data. When the TX fifo empty, the ISR will
+ *        fill the hardware FIFO with the internal ringbuffer's data.
+ *        @note This function shall only be called in I2C slave mode.
  *
  * @param i2c_num I2C port number
- * @param data data pointer to write into internal buffer
- * @param size data size
- * @param ticks_to_wait Maximum waiting ticks
+ * @param data Bytes to write into internal buffer
+ * @param size Size, in bytes, of `data` buffer
+ * @param ticks_to_wait Maximum ticks to wait.
  *
  * @return
- *     - ESP_FAIL(-1) Parameter error
- *     - Others(>=0) The number of data bytes that pushed to the I2C slave buffer.
+ *     - ESP_FAIL (-1) Parameter error
+ *     - Other (>=0) The number of data bytes pushed to the I2C slave buffer.
  */
 int i2c_slave_write_buffer(i2c_port_t i2c_num, const uint8_t *data, int size, TickType_t ticks_to_wait);
 
 /**
- * @brief I2C slave read data from internal buffer. When I2C slave receive data, isr will copy received data
- *        from hardware rx fifo to internal ringbuffer. Then users can read from internal ringbuffer.
- *        @note
- *        Only call this function in I2C slave mode
+ * @brief Read bytes from I2C internal buffer. When the I2C bus receives data, the ISR will copy them
+ *        from the hardware RX FIFO to the internal ringbuffer.
+ *        Calling this function will then copy bytes from the internal ringbuffer to the `data` user buffer.
+ *        @note This function shall only be called in I2C slave mode.
  *
  * @param i2c_num I2C port number
- * @param data data pointer to accept data from internal buffer
- * @param max_size Maximum data size to read
+ * @param data Buffer to fill with ringbuffer's bytes
+ * @param max_size Maximum bytes to read
  * @param ticks_to_wait Maximum waiting ticks
  *
  * @return
  *     - ESP_FAIL(-1) Parameter error
- *     - Others(>=0) The number of data bytes that read from I2C slave buffer.
+ *     - Others(>=0) The number of data bytes read from I2C slave buffer.
  */
 int i2c_slave_read_buffer(i2c_port_t i2c_num, uint8_t *data, size_t max_size, TickType_t ticks_to_wait);
 
 /**
- * @brief set I2C master clock period
+ * @brief Set I2C master clock period
  *
  * @param i2c_num I2C port number
- * @param high_period clock cycle number during SCL is high level, high_period is a 14 bit value
- * @param low_period clock cycle number during SCL is low level, low_period is a 14 bit value
+ * @param high_period Clock cycle number during SCL is high level, high_period is a 14 bit value
+ * @param low_period Clock cycle number during SCL is low level, low_period is a 14 bit value
  *
  * @return
  *     - ESP_OK Success
@@ -372,7 +492,7 @@ int i2c_slave_read_buffer(i2c_port_t i2c_num, uint8_t *data, size_t max_size, Ti
 esp_err_t i2c_set_period(i2c_port_t i2c_num, int high_period, int low_period);
 
 /**
- * @brief get I2C master clock period
+ * @brief Get I2C master clock period
  *
  * @param i2c_num I2C port number
  * @param high_period pointer to get clock cycle number during SCL is high level, will get a 14 bit value
@@ -385,15 +505,14 @@ esp_err_t i2c_set_period(i2c_port_t i2c_num, int high_period, int low_period);
 esp_err_t i2c_get_period(i2c_port_t i2c_num, int *high_period, int *low_period);
 
 /**
- * @brief enable hardware filter on I2C bus
+ * @brief Enable hardware filter on I2C bus
  *        Sometimes the I2C bus is disturbed by high frequency noise(about 20ns), or the rising edge of
- *        the SCL clock is very slow, these may cause the master state machine broken. enable hardware
- *        filter can filter out high frequency interference and make the master more stable.
- *        @note
- *        Enable filter will slow the SCL clock.
+ *        the SCL clock is very slow, these may cause the master state machine to break.
+ *        Enable hardware filter can filter out high frequency interference and make the master more stable.
+ *        @note Enable filter will slow down the SCL clock.
  *
- * @param i2c_num I2C port number
- * @param cyc_num the APB cycles need to be filtered(0<= cyc_num <=7).
+ * @param i2c_num I2C port number to filter
+ * @param cyc_num the APB cycles need to be filtered (0<= cyc_num <=7).
  *        When the period of a pulse is less than cyc_num * APB_cycle, the I2C controller will ignore this pulse.
  *
  * @return
@@ -403,7 +522,7 @@ esp_err_t i2c_get_period(i2c_port_t i2c_num, int *high_period, int *low_period);
 esp_err_t i2c_filter_enable(i2c_port_t i2c_num, uint8_t cyc_num);
 
 /**
- * @brief disable filter on I2C bus
+ * @brief Disable filter on I2C bus
  *
  * @param i2c_num I2C port number
  *

components/hal/esp32/include/hal/i2c_ll.h

@@ -25,7 +25,7 @@ extern "C" {
 #define I2C_LL_INTR_MASK          (0x3fff) /*!< I2C all interrupt bitmap */
 
 /**
- * @brief I2C hardware cmd register filed.
+ * @brief I2C hardware cmd register fields.
  */
 typedef union {
     struct {

components/hal/esp32c3/include/hal/i2c_ll.h

@@ -28,7 +28,7 @@ extern "C" {
 #define I2C_LL_INTR_MASK          (0x3fff) /*!< I2C all interrupt bitmap */
 
 /**
- * @brief I2C hardware cmd register filed.
+ * @brief I2C hardware cmd register fields.
  */
 typedef union {
     struct {

components/hal/esp32s2/include/hal/i2c_ll.h

@@ -25,7 +25,7 @@ extern "C" {
 #define I2C_LL_INTR_MASK          (0x1ffff) /*!< I2C all interrupt bitmap */
 
 /**
- * @brief I2C hardware cmd register filed.
+ * @brief I2C hardware cmd register fields.
  */
 typedef union {
     struct {

components/hal/esp32s3/include/hal/i2c_ll.h

@@ -25,7 +25,7 @@ extern "C" {
 
 #define I2C_LL_INTR_MASK          (0x3fff) /*!< I2C all interrupt bitmap */
 /**
- * @brief I2C hardware cmd register filed.
+ * @brief I2C hardware cmd register fields.
  */
 typedef union {
     struct {

components/hal/include/hal/i2c_hal.h

@@ -66,7 +66,7 @@ typedef struct {
 #define i2c_hal_write_cmd_reg(hal,cmd, cmd_idx)    i2c_ll_write_cmd_reg((hal)->dev,cmd,cmd_idx)
 
 /**
- * @brief  Configure the I2C to triger a trasaction
+ * @brief  Configure the I2C to triger a transaction
  *
  * @param  hal Context of the HAL layer
  *