From e1f388f114351656a4fc10362a16265f6a96c06d Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Fri, 2 Apr 2021 16:14:57 +0800 Subject: [PATCH 1/3] i2c: add `i2c_cmd_link_create_static()` to create commands from a given buffer Application can now provide a buffer in order to allocate commands link. Fixed few unclear details in the documentation. Added wrappers to simplify I2C transfers. Closes https://github.com/espressif/esp-idf/issues/5108 --- components/driver/i2c.c | 327 +++++++++++++++++----- components/driver/include/driver/i2c.h | 361 ++++++++++++++++--------- 2 files changed, 496 insertions(+), 192 deletions(-) diff --git a/components/driver/i2c.c b/components/driver/i2c.c index 6be9172adb..b3f8e44824 100644 --- a/components/driver/i2c.c +++ b/components/driver/i2c.c @@ -24,6 +24,7 @@ #include "driver/periph_ctrl.h" #include "esp_rom_gpio.h" #include "esp_rom_sys.h" +#include 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,10 @@ 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_CMD_WR_MINIMUM_SIZE (sizeof(i2c_cmd_desc_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),\ @@ -106,6 +112,9 @@ 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; typedef enum { @@ -857,6 +866,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_CMD_WR_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_CMD_WR_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_CMD_WR_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 +1014,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 +1047,89 @@ void i2c_cmd_link_delete(i2c_cmd_handle_t cmd_handle) return; } +static esp_err_t i2c_cmd_allocate(i2c_cmd_desc_t *cmd_desc, size_t n, size_t size, void** outptr) +{ + 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 + *outptr = calloc(n, size); +#else + *outptr = heap_caps_calloc(n, size, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); +#endif + 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) { -#if !CONFIG_SPIRAM_USE_MALLOC - cmd_desc->head = (i2c_cmd_link_t *) calloc(1, sizeof(i2c_cmd_link_t)); -#else - cmd_desc->head = (i2c_cmd_link_t *) heap_caps_calloc(1, sizeof(i2c_cmd_link_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); -#endif - if (cmd_desc->head == NULL) { - ESP_LOGE(I2C_TAG, I2C_CMD_MALLOC_ERR_STR); - goto err; + 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,13 +1138,14 @@ 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; + uint8_t len_tmp = 0; int data_offset = 0; - esp_err_t ret; - while (data_len > 0) { - len_tmp = data_len > 0xff ? 0xff : data_len; + esp_err_t ret = ESP_OK; + i2c_cmd_t cmd; + + while (data_len > 0 && ret == ESP_OK) { + len_tmp = MIN(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; @@ -966,36 +1154,32 @@ esp_err_t i2c_master_write(i2c_cmd_handle_t cmd_handle, const uint8_t *data, siz 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; + + return ret; } 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; + i2c_cmd_t cmd = { 0 }; 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; 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 len_tmp = 0; int data_offset = 0; - esp_err_t ret; - while (data_len > 0) { - len_tmp = data_len > 0xff ? 0xff : data_len; + esp_err_t ret = ESP_OK; + i2c_cmd_t cmd; + + while (data_len > 0 && ret == ESP_OK) { + len_tmp = MIN(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; @@ -1004,11 +1188,9 @@ static esp_err_t i2c_master_read_static(i2c_cmd_handle_t cmd_handle, uint8_t *da 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; + + return ret; } esp_err_t i2c_master_read_byte(i2c_cmd_handle_t cmd_handle, uint8_t *data, i2c_ack_type_t ack) @@ -1017,13 +1199,12 @@ 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; + i2c_cmd_t cmd = { 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; + return i2c_cmd_link_append(cmd_handle, &cmd); } @@ -1034,19 +1215,29 @@ 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 void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num) diff --git a/components/driver/include/driver/i2c.h b/components/driver/include/driver/i2c.h index bbff4aad2f..909cd0bd46 100644 --- a/components/driver/include/driver/i2c.h +++ b/components/driver/include/driver/i2c.h @@ -36,6 +36,28 @@ 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 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, `COMMANDS`, describing the number of commands + * intended to be performed on the I2C port. + * For example, if one wants to perform a read on an I2C device register, `COMMANDS` + * must be at least 2, because the commands required are the following: + * - write device register + * - read register content + * + * Moreover, any command that includes strictly more than 255 bytes shall be divided into multiple commands. + * For example, a write of 256 bytes shall be considered as 2 commands, a write of 1024 bytes shall be considered + * as 5 commands. (1024/255 rounded up) + * Signals such as "(repeated) start", "stop", "nack", "ack" shall not be counted. + */ +#define I2C_LINK_RECOMMENDED_SIZE(COMMANDS) (sizeof(i2c_cmd_desc_t) * \ + (4 * COMMANDS + 1)) /* 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. + */ + /** * @brief I2C initialization parameters */ @@ -62,18 +84,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 +100,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 +119,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 +153,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 +169,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 +180,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 +197,287 @@ 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. * - * @return i2c command link handler + * @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. + * 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()`. + * + * @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 command handle + * @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 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 +486,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 +499,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 +516,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 * From cfcbca1271c128db7f03d44f8e3ab05a4fbe5e8a Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Tue, 11 May 2021 11:14:43 +0800 Subject: [PATCH 2/3] i2c: optimize space allocated for read or write buffers Only a single command will be allocated now when a read or write is prepared in the command list. The size of a command's buffer is not limited to 255 bytes anymore. --- components/driver/i2c.c | 126 ++++++++++---------- components/driver/include/driver/i2c.h | 3 - components/hal/esp32/include/hal/i2c_ll.h | 2 +- components/hal/esp32c3/include/hal/i2c_ll.h | 2 +- components/hal/esp32s2/include/hal/i2c_ll.h | 2 +- components/hal/esp32s3/include/hal/i2c_ll.h | 2 +- components/hal/include/hal/i2c_hal.h | 2 +- 7 files changed, 68 insertions(+), 71 deletions(-) diff --git a/components/driver/i2c.c b/components/driver/i2c.c index b3f8e44824..ac8baa4f69 100644 --- a/components/driver/i2c.c +++ b/components/driver/i2c.c @@ -100,7 +100,7 @@ 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 */ + size_t remaining_bytes; } i2c_cmd_t; typedef struct i2c_cmd_link { @@ -1138,59 +1138,47 @@ 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 = 0; - int data_offset = 0; - esp_err_t ret = ESP_OK; - i2c_cmd_t cmd; + i2c_cmd_t cmd = { + .hw_cmd = { + .ack_en = ack_en, + .op_code = I2C_LL_CMD_WRITE, + }, + .data = (uint8_t*) data, + .remaining_bytes = data_len, + }; - while (data_len > 0 && ret == ESP_OK) { - len_tmp = MIN(0xff, data_len); - data_len -= len_tmp; - 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; - } - - return ret; + 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 = { 0 }; - cmd.hw_cmd.ack_en = ack_en; - cmd.hw_cmd.op_code = I2C_LL_CMD_WRITE; - cmd.hw_cmd.byte_num = 1; - cmd.byte_cmd = data; + + /* The marker to test whether we need to send a single byte is `data` field. + * If `data` is NULL, `remaning_bytes` contains the single byte to send. */ + i2c_cmd_t cmd = { + .hw_cmd = { + .ack_en = ack_en, + .op_code = I2C_LL_CMD_WRITE, + }, + .remaining_bytes = data, + }; + 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 = 0; - int data_offset = 0; - esp_err_t ret = ESP_OK; - i2c_cmd_t cmd; + i2c_cmd_t cmd = { + .hw_cmd = { + .ack_val = ack & 0x1, + .op_code = I2C_LL_CMD_READ, + }, + .data = data, + .remaining_bytes = data_len, + }; - while (data_len > 0 && ret == ESP_OK) { - len_tmp = MIN(0xff, data_len); - data_len -= len_tmp; - 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; - } - - return ret; + 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) @@ -1199,11 +1187,14 @@ 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 = { 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, + .remaining_bytes = 1, + }; return i2c_cmd_link_append(cmd_handle, &cmd); } @@ -1240,16 +1231,20 @@ esp_err_t i2c_master_read(i2c_cmd_handle_t cmd_handle, uint8_t *data, size_t dat return ret; } +static inline bool i2c_cmd_is_single_byte(const i2c_cmd_t *cmd) { + return cmd->data == NULL; +} + 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) { + if (cmd->remaining_bytes > 0) { p_i2c->cmd_idx = 0; } else { p_i2c->cmd_link.head = p_i2c->cmd_link.head->next; @@ -1277,36 +1272,41 @@ 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; 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; + if (!i2c_cmd_is_single_byte(cmd)) { + fifo_fill = MIN(cmd->remaining_bytes, SOC_I2C_FIFO_LEN); write_pr = cmd->data; - cmd->data += wr_filled; - cmd->hw_cmd.byte_num -= wr_filled; + cmd->data += fifo_fill; + cmd->remaining_bytes -= fifo_fill; } else { - wr_filled = 1; - write_pr = &(cmd->byte_cmd); - cmd->hw_cmd.byte_num--; + fifo_fill = 1; + /* In that case, `remaining_bytes` contains the data itself. + * NOTE: It is possible to use `remaining_bytes` as a byte pointer + * because both Xtensa and RISC-V architectures used are little-endian. + */ + write_pr = (uint8_t*) &cmd->remaining_bytes; + cmd->hw_cmd.byte_num = 0; } - 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->remaining_bytes == 0) { p_i2c->cmd_link.head = p_i2c->cmd_link.head->next; } 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(cmd->remaining_bytes, SOC_I2C_FIFO_LEN); + p_i2c->rx_cnt = fifo_fill; + cmd->remaining_bytes -= 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)); diff --git a/components/driver/include/driver/i2c.h b/components/driver/include/driver/i2c.h index 909cd0bd46..e3908994a3 100644 --- a/components/driver/include/driver/i2c.h +++ b/components/driver/include/driver/i2c.h @@ -46,9 +46,6 @@ extern "C" { * - write device register * - read register content * - * Moreover, any command that includes strictly more than 255 bytes shall be divided into multiple commands. - * For example, a write of 256 bytes shall be considered as 2 commands, a write of 1024 bytes shall be considered - * as 5 commands. (1024/255 rounded up) * Signals such as "(repeated) start", "stop", "nack", "ack" shall not be counted. */ #define I2C_LINK_RECOMMENDED_SIZE(COMMANDS) (sizeof(i2c_cmd_desc_t) * \ diff --git a/components/hal/esp32/include/hal/i2c_ll.h b/components/hal/esp32/include/hal/i2c_ll.h index f8a8dfc805..550c1b76d2 100644 --- a/components/hal/esp32/include/hal/i2c_ll.h +++ b/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 { diff --git a/components/hal/esp32c3/include/hal/i2c_ll.h b/components/hal/esp32c3/include/hal/i2c_ll.h index 88d4493b0e..85c946c23b 100644 --- a/components/hal/esp32c3/include/hal/i2c_ll.h +++ b/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 { diff --git a/components/hal/esp32s2/include/hal/i2c_ll.h b/components/hal/esp32s2/include/hal/i2c_ll.h index 6d22bce641..e77fd18e04 100644 --- a/components/hal/esp32s2/include/hal/i2c_ll.h +++ b/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 { diff --git a/components/hal/esp32s3/include/hal/i2c_ll.h b/components/hal/esp32s3/include/hal/i2c_ll.h index 877a916130..1aa9290ba7 100644 --- a/components/hal/esp32s3/include/hal/i2c_ll.h +++ b/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 { diff --git a/components/hal/include/hal/i2c_hal.h b/components/hal/include/hal/i2c_hal.h index cac45a7f49..ae1c95be2c 100644 --- a/components/hal/include/hal/i2c_hal.h +++ b/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 * From 7dd499d1f4235bcaaf28b1b0e8dc756fd7c60c96 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Tue, 11 May 2021 17:55:49 +0800 Subject: [PATCH 3/3] i2c: commands can now be re-used without deleting and creating new cmd links It is now possible to call `i2c_master_cmd_begin()` on the same `i2c_cmd_handle_t` parameter. Thus, no extra allocation will be performed. Moreover, as commands contains pointers to data, these can be modified between two calls to `i2c_master_cmd_begin()`. This becomes very handy for framebuffers. --- components/driver/i2c.c | 77 ++++++++++++++++---------- components/driver/include/driver/i2c.h | 17 ++++-- 2 files changed, 62 insertions(+), 32 deletions(-) diff --git a/components/driver/i2c.c b/components/driver/i2c.c index ac8baa4f69..32bae591b5 100644 --- a/components/driver/i2c.c +++ b/components/driver/i2c.c @@ -80,7 +80,8 @@ 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_CMD_WR_MINIMUM_SIZE (sizeof(i2c_cmd_desc_t) * 8) /* It is required to have allocate one i2c_cmd_desc_t per command: +#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 */ @@ -99,8 +100,12 @@ static const char *I2C_TAG = "i2c"; typedef struct { i2c_hw_cmd_t hw_cmd; - uint8_t *data; /*!< data address */ - size_t remaining_bytes; + 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 { @@ -117,6 +122,10 @@ typedef struct { 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 */ @@ -871,7 +880,7 @@ esp_err_t i2c_master_write_to_device(i2c_port_t i2c_num, uint8_t device_address, TickType_t ticks_to_wait) { esp_err_t err = ESP_OK; - uint8_t buffer[I2C_CMD_WR_MINIMUM_SIZE] = { 0 }; + 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); @@ -905,7 +914,7 @@ esp_err_t i2c_master_read_from_device(i2c_port_t i2c_num, uint8_t device_address TickType_t ticks_to_wait) { esp_err_t err = ESP_OK; - uint8_t buffer[I2C_CMD_WR_MINIMUM_SIZE] = { 0 }; + 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); @@ -940,7 +949,7 @@ esp_err_t i2c_master_write_read_device(i2c_port_t i2c_num, uint8_t device_addres TickType_t ticks_to_wait) { esp_err_t err = ESP_OK; - uint8_t buffer[I2C_CMD_WR_MINIMUM_SIZE] = { 0 }; + 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); @@ -1144,7 +1153,7 @@ esp_err_t i2c_master_write(i2c_cmd_handle_t cmd_handle, const uint8_t *data, siz .op_code = I2C_LL_CMD_WRITE, }, .data = (uint8_t*) data, - .remaining_bytes = data_len, + .total_bytes = data_len, }; return i2c_cmd_link_append(cmd_handle, &cmd); @@ -1154,14 +1163,13 @@ esp_err_t i2c_master_write_byte(i2c_cmd_handle_t cmd_handle, uint8_t data, bool { I2C_CHECK(cmd_handle != NULL, I2C_CMD_LINK_INIT_ERR_STR, ESP_ERR_INVALID_ARG); - /* The marker to test whether we need to send a single byte is `data` field. - * If `data` is NULL, `remaning_bytes` contains the single byte to send. */ i2c_cmd_t cmd = { .hw_cmd = { .ack_en = ack_en, .op_code = I2C_LL_CMD_WRITE, }, - .remaining_bytes = data, + .data_byte = data, + .total_bytes = 1, }; return i2c_cmd_link_append(cmd_handle, &cmd); @@ -1175,7 +1183,7 @@ static esp_err_t i2c_master_read_static(i2c_cmd_handle_t cmd_handle, uint8_t *da .op_code = I2C_LL_CMD_READ, }, .data = data, - .remaining_bytes = data_len, + .total_bytes = data_len, }; return i2c_cmd_link_append(cmd_handle, &cmd); @@ -1193,7 +1201,7 @@ esp_err_t i2c_master_read_byte(i2c_cmd_handle_t cmd_handle, uint8_t *data, i2c_a .op_code = I2C_LL_CMD_READ, }, .data = data, - .remaining_bytes = 1, + .total_bytes = 1, }; return i2c_cmd_link_append(cmd_handle, &cmd); @@ -1232,7 +1240,7 @@ esp_err_t i2c_master_read(i2c_cmd_handle_t cmd_handle, uint8_t *data, size_t dat } static inline bool i2c_cmd_is_single_byte(const i2c_cmd_t *cmd) { - return cmd->data == NULL; + return cmd->total_bytes == 1; } static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num) @@ -1242,12 +1250,16 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num) 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->remaining_bytes > 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)) { @@ -1271,24 +1283,29 @@ 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 *write_pr = NULL; + //TODO: to reduce interrupt number if (!i2c_cmd_is_single_byte(cmd)) { - fifo_fill = MIN(cmd->remaining_bytes, SOC_I2C_FIFO_LEN); - write_pr = cmd->data; - cmd->data += fifo_fill; - cmd->remaining_bytes -= fifo_fill; + 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 { fifo_fill = 1; - /* In that case, `remaining_bytes` contains the data itself. - * NOTE: It is possible to use `remaining_bytes` as a byte pointer - * because both Xtensa and RISC-V architectures used are little-endian. + /* `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->remaining_bytes; - cmd->hw_cmd.byte_num = 0; + write_pr = (uint8_t*) &cmd->data_byte; } hw_cmd.byte_num = fifo_fill; i2c_hal_write_txfifo(&(i2c_context[i2c_num].hal), write_pr, fifo_fill); @@ -1296,16 +1313,16 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num) 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 (i2c_cmd_is_single_byte(cmd) || cmd->remaining_bytes == 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 - fifo_fill = MIN(cmd->remaining_bytes, SOC_I2C_FIFO_LEN); + fifo_fill = MIN(remaining_bytes, SOC_I2C_FIFO_LEN); p_i2c->rx_cnt = fifo_fill; - cmd->remaining_bytes -= 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); @@ -1382,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; diff --git a/components/driver/include/driver/i2c.h b/components/driver/include/driver/i2c.h index e3908994a3..4a5dcc2683 100644 --- a/components/driver/include/driver/i2c.h +++ b/components/driver/include/driver/i2c.h @@ -36,23 +36,30 @@ 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, `COMMANDS`, describing the number of commands + * 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, `COMMANDS` + * 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(COMMANDS) (sizeof(i2c_cmd_desc_t) * \ - (4 * COMMANDS + 1)) /* Make the assumption that each transaction +#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. */ /** @@ -239,6 +246,8 @@ esp_err_t i2c_master_read_from_device(i2c_port_t i2c_num, uint8_t device_address /** * @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. *