From 6adee5052f7fd8a97ed31ba3b8e31fea619a165e Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Thu, 22 Aug 2024 16:20:29 +0800 Subject: [PATCH] fix(i2c_master): Fix an I2C issue that slave streth happen but master timeout set seems doesn't work Closes https://github.com/espressif/esp-idf/issues/14129 Closes https://github.com/espressif/esp-idf/issues/14401 --- components/driver/i2c/i2c_master.c | 13 ++++++++++--- components/hal/esp32c2/include/hal/i2c_ll.h | 13 +++++++------ components/hal/esp32c3/include/hal/i2c_ll.h | 15 ++++++++------- components/hal/esp32c6/include/hal/i2c_ll.h | 5 +++-- components/hal/esp32h2/include/hal/i2c_ll.h | 15 ++++++++------- components/hal/esp32p4/include/hal/i2c_ll.h | 15 ++++++++------- components/hal/esp32s3/include/hal/i2c_ll.h | 13 +++++++------ 7 files changed, 51 insertions(+), 38 deletions(-) diff --git a/components/driver/i2c/i2c_master.c b/components/driver/i2c/i2c_master.c index fbd5d4c5bc..afbc993359 100644 --- a/components/driver/i2c/i2c_master.c +++ b/components/driver/i2c/i2c_master.c @@ -40,6 +40,8 @@ static const char *TAG = "i2c.master"; #define I2C_ADDRESS_TRANS_WRITE(device_address) (((device_address) << 1) | 0) #define I2C_ADDRESS_TRANS_READ(device_address) (((device_address) << 1) | 1) +#define I2C_CLR_BUS_TIMEOUT_MS (50) // 50ms is sufficient for clearing the bus + static esp_err_t s_i2c_master_clear_bus(i2c_bus_handle_t handle) { #if !SOC_I2C_SUPPORT_HW_CLR_BUS @@ -548,12 +550,13 @@ static esp_err_t s_i2c_transaction_start(i2c_master_dev_handle_t i2c_dev, int xf i2c_master->rx_cnt = 0; i2c_master->read_len_static = 0; - i2c_hal_master_set_scl_timeout_val(hal, i2c_dev->scl_wait_us, i2c_master->base->clk_src_freq_hz); - I2C_CLOCK_SRC_ATOMIC() { - i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); i2c_hal_set_bus_timing(hal, i2c_dev->scl_speed_hz, i2c_master->base->clk_src, i2c_master->base->clk_src_freq_hz); } + + // Set the timeout value + i2c_hal_master_set_scl_timeout_val(hal, i2c_dev->scl_wait_us, i2c_master->base->clk_src_freq_hz); + i2c_ll_master_set_fractional_divider(hal->dev, 0, 0); i2c_ll_update(hal->dev); @@ -891,6 +894,10 @@ esp_err_t i2c_new_master_bus(const i2c_master_bus_config_t *bus_config, i2c_mast } ESP_GOTO_ON_ERROR(i2c_param_master_config(i2c_master->base, bus_config), err, TAG, "i2c configure parameter failed"); + I2C_CLOCK_SRC_ATOMIC() { + i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); + } + i2c_master->bus_lock_mux = xSemaphoreCreateBinaryWithCaps(I2C_MEM_ALLOC_CAPS); ESP_GOTO_ON_FALSE(i2c_master->bus_lock_mux, ESP_ERR_NO_MEM, err, TAG, "No memory for binary semaphore"); xSemaphoreGive(i2c_master->bus_lock_mux); diff --git a/components/hal/esp32c2/include/hal/i2c_ll.h b/components/hal/esp32c2/include/hal/i2c_ll.h index 8fa30c2256..1fe3d4b46a 100644 --- a/components/hal/esp32c2/include/hal/i2c_ll.h +++ b/components/hal/esp32c2/include/hal/i2c_ll.h @@ -583,7 +583,7 @@ static inline void i2c_ll_get_stop_timing(i2c_dev_t *hw, int *setup_time, int *h * * @param hw Beginning address of the peripheral registers * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written * * @return None. */ @@ -647,7 +647,7 @@ static inline void i2c_ll_master_get_filter(i2c_dev_t *hw, uint8_t *filter_conf) } /** - * @brief Reste I2C master FSM. When the master FSM is stuck, call this function to reset the FSM + * @brief reset I2C master FSM. When the master FSM is stuck, call this function to reset the FSM * * @param hw Beginning address of the peripheral registers * @@ -675,7 +675,7 @@ static inline void i2c_ll_master_clr_bus(i2c_dev_t *hw, uint32_t slave_pulses) hw->scl_sp_conf.scl_rst_slv_num = slave_pulses; hw->scl_sp_conf.scl_rst_slv_en = 1; hw->ctr.conf_upgate = 1; - // hardward will clear scl_rst_slv_en after sending SCL pulses, + // hardware will clear scl_rst_slv_en after sending SCL pulses, // and we should set conf_upgate bit to synchronize register value. while (hw->scl_sp_conf.scl_rst_slv_en); hw->ctr.conf_upgate = 1; @@ -742,7 +742,8 @@ static inline volatile void *i2c_ll_get_interrupt_status_reg(i2c_dev_t *dev) static inline uint32_t i2c_ll_calculate_timeout_us_to_reg_val(uint32_t src_clk_hz, uint32_t timeout_us) { uint32_t clk_cycle_num_per_us = src_clk_hz / (1 * 1000 * 1000); - return 31 - __builtin_clz(clk_cycle_num_per_us * timeout_us); + // round up to an integer + return 32 - __builtin_clz(clk_cycle_num_per_us * timeout_us); } //////////////////////////////////////////Deprecated Functions////////////////////////////////////////////////////////// @@ -777,7 +778,7 @@ typedef enum { * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param high_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param high_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * @param wait_high_period The I2C SCL wait rising edge period. * @@ -892,7 +893,7 @@ static inline void i2c_ll_master_disable_rx_it(i2c_dev_t *hw) * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param hight_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param hight_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * * @return None. diff --git a/components/hal/esp32c3/include/hal/i2c_ll.h b/components/hal/esp32c3/include/hal/i2c_ll.h index f22d15947f..5d06f2afb5 100644 --- a/components/hal/esp32c3/include/hal/i2c_ll.h +++ b/components/hal/esp32c3/include/hal/i2c_ll.h @@ -663,7 +663,7 @@ static inline void i2c_ll_get_stop_timing(i2c_dev_t *hw, int *setup_time, int *h * * @param hw Beginning address of the peripheral registers * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written * * @return None. */ @@ -698,7 +698,7 @@ static inline void i2c_ll_read_rxfifo(i2c_dev_t *hw, uint8_t *ptr, uint8_t len) * @param hw Beginning address of the peripheral registers * @param ram_offset Offset value of I2C RAM. * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written */ static inline void i2c_ll_write_by_nonfifo(i2c_dev_t *hw, uint8_t ram_offset, const uint8_t *ptr, uint8_t len) { @@ -770,7 +770,7 @@ static inline void i2c_ll_master_get_filter(i2c_dev_t *hw, uint8_t *filter_conf) } /** - * @brief Reste I2C master FSM. When the master FSM is stuck, call this function to reset the FSM + * @brief reset I2C master FSM. When the master FSM is stuck, call this function to reset the FSM * * @param hw Beginning address of the peripheral registers * @@ -798,7 +798,7 @@ static inline void i2c_ll_master_clr_bus(i2c_dev_t *hw, uint32_t slave_pulses) hw->scl_sp_conf.scl_rst_slv_num = slave_pulses; hw->scl_sp_conf.scl_rst_slv_en = 1; hw->ctr.conf_upgate = 1; - // hardward will clear scl_rst_slv_en after sending SCL pulses, + // hardware will clear scl_rst_slv_en after sending SCL pulses, // and we should set conf_upgate bit to synchronize register value. while (hw->scl_sp_conf.scl_rst_slv_en); hw->ctr.conf_upgate = 1; @@ -915,7 +915,8 @@ static inline void i2c_ll_slave_clear_stretch(i2c_dev_t *dev) static inline uint32_t i2c_ll_calculate_timeout_us_to_reg_val(uint32_t src_clk_hz, uint32_t timeout_us) { uint32_t clk_cycle_num_per_us = src_clk_hz / (1 * 1000 * 1000); - return 31 - __builtin_clz(clk_cycle_num_per_us * timeout_us); + // round up to an integer + return 32 - __builtin_clz(clk_cycle_num_per_us * timeout_us); } //////////////////////////////////////////Deprecated Functions////////////////////////////////////////////////////////// @@ -954,7 +955,7 @@ typedef enum { * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param high_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param high_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * @param wait_high_period The I2C SCL wait rising edge period. * @@ -1141,7 +1142,7 @@ static inline void i2c_ll_slave_disable_rx_it(i2c_dev_t *hw) * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param hight_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param hight_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * * @return None. diff --git a/components/hal/esp32c6/include/hal/i2c_ll.h b/components/hal/esp32c6/include/hal/i2c_ll.h index 2d465c20a4..55de5ccdd4 100644 --- a/components/hal/esp32c6/include/hal/i2c_ll.h +++ b/components/hal/esp32c6/include/hal/i2c_ll.h @@ -723,7 +723,7 @@ static inline void i2c_ll_master_get_filter(i2c_dev_t *hw, uint8_t *filter_conf) } /** - * @brief Reste I2C master FSM. When the master FSM is stuck, call this function to reset the FSM + * @brief reset I2C master FSM. When the master FSM is stuck, call this function to reset the FSM * * @param hw Beginning address of the peripheral registers * @@ -947,7 +947,8 @@ static inline void i2c_ll_slave_clear_stretch(i2c_dev_t *dev) static inline uint32_t i2c_ll_calculate_timeout_us_to_reg_val(uint32_t src_clk_hz, uint32_t timeout_us) { uint32_t clk_cycle_num_per_us = src_clk_hz / (1 * 1000 * 1000); - return 31 - __builtin_clz(clk_cycle_num_per_us * timeout_us); + // round up to an integer + return 32 - __builtin_clz(clk_cycle_num_per_us * timeout_us); } //////////////////////////////////////////Deprecated Functions////////////////////////////////////////////////////////// diff --git a/components/hal/esp32h2/include/hal/i2c_ll.h b/components/hal/esp32h2/include/hal/i2c_ll.h index 1f21cae77c..7e990d6c0c 100644 --- a/components/hal/esp32h2/include/hal/i2c_ll.h +++ b/components/hal/esp32h2/include/hal/i2c_ll.h @@ -610,7 +610,7 @@ static inline void i2c_ll_get_stop_timing(i2c_dev_t *hw, int *setup_time, int *h * * @param hw Beginning address of the peripheral registers * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written * * @return None. */ @@ -645,7 +645,7 @@ static inline void i2c_ll_read_rxfifo(i2c_dev_t *hw, uint8_t *ptr, uint8_t len) * @param hw Beginning address of the peripheral registers * @param ram_offset Offset value of I2C RAM. * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written */ static inline void i2c_ll_write_by_nonfifo(i2c_dev_t *hw, uint8_t ram_offset, const uint8_t *ptr, uint8_t len) { @@ -717,7 +717,7 @@ static inline void i2c_ll_master_get_filter(i2c_dev_t *hw, uint8_t *filter_conf) } /** - * @brief Reste I2C master FSM. When the master FSM is stuck, call this function to reset the FSM + * @brief reset I2C master FSM. When the master FSM is stuck, call this function to reset the FSM * * @param hw Beginning address of the peripheral registers * @@ -745,7 +745,7 @@ static inline void i2c_ll_master_clr_bus(i2c_dev_t *hw, uint32_t slave_pulses) hw->scl_sp_conf.scl_rst_slv_num = slave_pulses; hw->scl_sp_conf.scl_rst_slv_en = 1; hw->ctr.conf_upgate = 1; - // hardward will clear scl_rst_slv_en after sending SCL pulses, + // hardware will clear scl_rst_slv_en after sending SCL pulses, // and we should set conf_upgate bit to synchronize register value. while (hw->scl_sp_conf.scl_rst_slv_en); hw->ctr.conf_upgate = 1; @@ -862,7 +862,8 @@ static inline void i2c_ll_slave_clear_stretch(i2c_dev_t *dev) static inline uint32_t i2c_ll_calculate_timeout_us_to_reg_val(uint32_t src_clk_hz, uint32_t timeout_us) { uint32_t clk_cycle_num_per_us = src_clk_hz / (1 * 1000 * 1000); - return 31 - __builtin_clz(clk_cycle_num_per_us * timeout_us); + // round up to an integer + return 32 - __builtin_clz(clk_cycle_num_per_us * timeout_us); } //////////////////////////////////////////Deprecated Functions////////////////////////////////////////////////////////// @@ -901,7 +902,7 @@ typedef enum { * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param high_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param high_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * @param wait_high_period The I2C SCL wait rising edge period. * @@ -1089,7 +1090,7 @@ static inline void i2c_ll_slave_disable_rx_it(i2c_dev_t *hw) * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param hight_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param hight_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * * @return None. diff --git a/components/hal/esp32p4/include/hal/i2c_ll.h b/components/hal/esp32p4/include/hal/i2c_ll.h index cb2a0af0cd..a87188b472 100644 --- a/components/hal/esp32p4/include/hal/i2c_ll.h +++ b/components/hal/esp32p4/include/hal/i2c_ll.h @@ -614,7 +614,7 @@ static inline void i2c_ll_get_stop_timing(i2c_dev_t *hw, int *setup_time, int *h * * @param hw Beginning address of the peripheral registers * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written * * @return None. */ @@ -649,7 +649,7 @@ static inline void i2c_ll_read_rxfifo(i2c_dev_t *hw, uint8_t *ptr, uint8_t len) * @param hw Beginning address of the peripheral registers * @param ram_offset Offset value of I2C RAM. * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written */ static inline void i2c_ll_write_by_nonfifo(i2c_dev_t *hw, uint8_t ram_offset, const uint8_t *ptr, uint8_t len) { @@ -721,7 +721,7 @@ static inline void i2c_ll_master_get_filter(i2c_dev_t *hw, uint8_t *filter_conf) } /** - * @brief Reste I2C master FSM. When the master FSM is stuck, call this function to reset the FSM + * @brief reset I2C master FSM. When the master FSM is stuck, call this function to reset the FSM * * @param hw Beginning address of the peripheral registers * @@ -749,7 +749,7 @@ static inline void i2c_ll_master_clr_bus(i2c_dev_t *hw, uint32_t slave_pulses) hw->scl_sp_conf.scl_rst_slv_num = slave_pulses; hw->scl_sp_conf.scl_rst_slv_en = 1; hw->ctr.conf_upgate = 1; - // hardward will clear scl_rst_slv_en after sending SCL pulses, + // hardware will clear scl_rst_slv_en after sending SCL pulses, // and we should set conf_upgate bit to synchronize register value. while (hw->scl_sp_conf.scl_rst_slv_en); hw->ctr.conf_upgate = 1; @@ -902,7 +902,8 @@ static inline void i2c_ll_slave_clear_stretch(i2c_dev_t *dev) static inline uint32_t i2c_ll_calculate_timeout_us_to_reg_val(uint32_t src_clk_hz, uint32_t timeout_us) { uint32_t clk_cycle_num_per_us = src_clk_hz / (1 * 1000 * 1000); - return 31 - __builtin_clz(clk_cycle_num_per_us * timeout_us); + // round up to an integer + return 32 - __builtin_clz(clk_cycle_num_per_us * timeout_us); } //////////////////////////////////////////Deprecated Functions////////////////////////////////////////////////////////// @@ -941,7 +942,7 @@ typedef enum { * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param high_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param high_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * @param wait_high_period The I2C SCL wait rising edge period. * @@ -1129,7 +1130,7 @@ static inline void i2c_ll_slave_disable_rx_it(i2c_dev_t *hw) * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param hight_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param hight_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * * @return None. diff --git a/components/hal/esp32s3/include/hal/i2c_ll.h b/components/hal/esp32s3/include/hal/i2c_ll.h index 97131a888b..5e3006b497 100644 --- a/components/hal/esp32s3/include/hal/i2c_ll.h +++ b/components/hal/esp32s3/include/hal/i2c_ll.h @@ -671,7 +671,7 @@ static inline void i2c_ll_get_stop_timing(i2c_dev_t *hw, int *setup_time, int *h * * @param hw Beginning address of the peripheral registers * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written * * @return None. */ @@ -706,7 +706,7 @@ static inline void i2c_ll_read_rxfifo(i2c_dev_t *hw, uint8_t *ptr, uint8_t len) * @param hw Beginning address of the peripheral registers * @param ram_offset Offset value of I2C RAM. * @param ptr Pointer to data buffer - * @param len Amount of data needs to be writen + * @param len Amount of data needs to be written */ static inline void i2c_ll_write_by_nonfifo(i2c_dev_t *hw, uint8_t ram_offset, const uint8_t *ptr, uint8_t len) { @@ -778,7 +778,7 @@ static inline void i2c_ll_master_get_filter(i2c_dev_t *hw, uint8_t *filter_conf) } /** - * @brief Reste I2C master FSM. When the master FSM is stuck, call this function to reset the FSM + * @brief reset I2C master FSM. When the master FSM is stuck, call this function to reset the FSM * * @param hw Beginning address of the peripheral registers * @@ -917,7 +917,8 @@ static inline void i2c_ll_slave_clear_stretch(i2c_dev_t *dev) static inline uint32_t i2c_ll_calculate_timeout_us_to_reg_val(uint32_t src_clk_hz, uint32_t timeout_us) { uint32_t clk_cycle_num_per_us = src_clk_hz / (1 * 1000 * 1000); - return 31 - __builtin_clz(clk_cycle_num_per_us * timeout_us); + // round up to an integer + return 32 - __builtin_clz(clk_cycle_num_per_us * timeout_us); } //////////////////////////////////////////Deprecated Functions////////////////////////////////////////////////////////// @@ -972,7 +973,7 @@ static inline void i2c_ll_get_scl_clk_timing(i2c_dev_t *hw, int *high_period, in * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param high_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param high_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * @param wait_high_period The I2C SCL wait rising edge period. * @@ -1143,7 +1144,7 @@ static inline void i2c_ll_slave_disable_rx_it(i2c_dev_t *hw) * @brief Configure I2C SCL timing * * @param hw Beginning address of the peripheral registers - * @param high_period The I2C SCL hight period (in core clock cycle, hight_period > 2) + * @param high_period The I2C SCL height period (in core clock cycle, hight_period > 2) * @param low_period The I2C SCL low period (in core clock cycle, low_period > 1) * * @return None.