From 8aee667873143a4f2c029d21a4bfff358d287db4 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/esp_driver_i2c/i2c_master.c | 31 +++++++++++--------- components/hal/esp32c2/include/hal/i2c_ll.h | 13 ++++---- components/hal/esp32c3/include/hal/i2c_ll.h | 15 +++++----- components/hal/esp32c5/include/hal/i2c_ll.h | 3 +- components/hal/esp32c6/include/hal/i2c_ll.h | 5 ++-- components/hal/esp32c61/include/hal/i2c_ll.h | 3 +- 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 ++++---- 9 files changed, 62 insertions(+), 51 deletions(-) diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c index 771c74fce7..f66628661d 100644 --- a/components/esp_driver_i2c/i2c_master.c +++ b/components/esp_driver_i2c/i2c_master.c @@ -586,23 +586,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); - - if (!i2c_master->base->is_lp_i2c) { - I2C_CLOCK_SRC_ATOMIC() { - i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); - } - } -#if SOC_LP_I2C_SUPPORTED - else { - LP_I2C_SRC_CLK_ATOMIC() { - lp_i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); - } - } -#endif I2C_CLOCK_SRC_ATOMIC() { 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); @@ -941,6 +931,19 @@ 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"); + if (!i2c_master->base->is_lp_i2c) { + I2C_CLOCK_SRC_ATOMIC() { + i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); + } + } +#if SOC_LP_I2C_SUPPORTED + else { + LP_I2C_SRC_CLK_ATOMIC() { + lp_i2c_ll_set_source_clk(hal->dev, i2c_master->base->clk_src); + } + } +#endif + 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 bc6ae54802..1d43bce85a 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; @@ -756,7 +756,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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////////////////////////////////////////////////////////// @@ -791,7 +792,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. * @@ -906,7 +907,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 9fd18c694f..4c5faed2c4 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; @@ -929,7 +929,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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////////////////////////////////////////////////////////// @@ -968,7 +969,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. * @@ -1155,7 +1156,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/esp32c5/include/hal/i2c_ll.h b/components/hal/esp32c5/include/hal/i2c_ll.h index c81f0103f3..e6440281d9 100644 --- a/components/hal/esp32c5/include/hal/i2c_ll.h +++ b/components/hal/esp32c5/include/hal/i2c_ll.h @@ -967,7 +967,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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/esp32c6/include/hal/i2c_ll.h b/components/hal/esp32c6/include/hal/i2c_ll.h index 5d0ce895d3..3bd42d6038 100644 --- a/components/hal/esp32c6/include/hal/i2c_ll.h +++ b/components/hal/esp32c6/include/hal/i2c_ll.h @@ -734,7 +734,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 * @@ -972,7 +972,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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/esp32c61/include/hal/i2c_ll.h b/components/hal/esp32c61/include/hal/i2c_ll.h index 9a671075ce..b6caabdaaf 100644 --- a/components/hal/esp32c61/include/hal/i2c_ll.h +++ b/components/hal/esp32c61/include/hal/i2c_ll.h @@ -893,7 +893,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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 9935228463..8e9870c944 100644 --- a/components/hal/esp32h2/include/hal/i2c_ll.h +++ b/components/hal/esp32h2/include/hal/i2c_ll.h @@ -613,7 +613,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. */ @@ -648,7 +648,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) { @@ -720,7 +720,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 * @@ -748,7 +748,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; @@ -879,7 +879,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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////////////////////////////////////////////////////////// @@ -918,7 +919,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. * @@ -1106,7 +1107,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 e4fdb1622b..9693707256 100644 --- a/components/hal/esp32p4/include/hal/i2c_ll.h +++ b/components/hal/esp32p4/include/hal/i2c_ll.h @@ -615,7 +615,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. */ @@ -650,7 +650,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) { @@ -722,7 +722,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 * @@ -750,7 +750,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; @@ -976,7 +976,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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////////////////////////////////////////////////////////// @@ -1015,7 +1016,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. * @@ -1203,7 +1204,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 d881c05416..caf58b9022 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 * @@ -931,7 +931,8 @@ static inline bool i2c_ll_master_is_cmd_done(i2c_dev_t *hw, int cmd_idx) 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////////////////////////////////////////////////////////// @@ -986,7 +987,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. * @@ -1157,7 +1158,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.