From a9d12c0fe96c761d1cf788482000b3abdc2715bb Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 6 Jun 2023 14:14:45 +0800 Subject: [PATCH 1/3] adc: fixed the issue that multiply overflow before type expand --- components/esp_adc/adc_cali_curve_fitting.c | 11 ++--------- .../esp_adc/deprecated/esp32c3/esp_adc_cal_legacy.c | 2 +- .../esp_adc/deprecated/esp32s3/esp_adc_cal_legacy.c | 2 +- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/components/esp_adc/adc_cali_curve_fitting.c b/components/esp_adc/adc_cali_curve_fitting.c index 6360d610bcea..434744817593 100644 --- a/components/esp_adc/adc_cali_curve_fitting.c +++ b/components/esp_adc/adc_cali_curve_fitting.c @@ -22,12 +22,10 @@ const __attribute__((unused)) static char *TAG = "adc_cali"; - // coeff_a is actually a float number // it is scaled to put them into uint32_t so that the headers do not have to be changed static const int coeff_a_scaling = 65536; - /* -------------------- Characterization Helper Data Types ------------------ */ typedef struct { uint32_t voltage; @@ -43,7 +41,6 @@ typedef struct { } ref_data; } adc_calib_info_t; - /* ------------------------ Context Structure--------------------------- */ typedef struct { uint32_t coeff_a; ///< Gradient of ADC-Voltage curve @@ -63,7 +60,6 @@ typedef struct { cali_chars_second_step_t chars_second_step; ///< Calibration second step characteristics } cali_chars_curve_fitting_t; - /* ----------------------- Characterization Functions ----------------------- */ static void get_first_step_reference_point(int version_num, adc_unit_t unit_id, adc_atten_t atten, adc_calib_info_t *calib_info); static void calc_first_step_coefficients(const adc_calib_info_t *parsed_data, cali_chars_curve_fitting_t *chars); @@ -71,7 +67,6 @@ static void calc_second_step_coefficients(const adc_cali_curve_fitting_config_t static int32_t get_reading_error(uint64_t v_cali_1, const cali_chars_second_step_t *param, adc_atten_t atten); static esp_err_t check_valid(const adc_cali_curve_fitting_config_t *config); - /* ------------------------ Interface Functions --------------------------- */ static esp_err_t cali_raw_to_voltage(void *arg, int raw, int *voltage); @@ -131,14 +126,13 @@ esp_err_t adc_cali_delete_scheme_curve_fitting(adc_cali_handle_t handle) return ESP_OK; } - /* ------------------------ Interface Functions --------------------------- */ static esp_err_t cali_raw_to_voltage(void *arg, int raw, int *voltage) { //pointers are checked in the upper layer cali_chars_curve_fitting_t *ctx = arg; - uint64_t v_cali_1 = raw * ctx->chars_first_step.coeff_a / coeff_a_scaling + ctx->chars_first_step.coeff_b; + uint64_t v_cali_1 = (uint64_t)raw * ctx->chars_first_step.coeff_a / coeff_a_scaling + ctx->chars_first_step.coeff_b; int32_t error = get_reading_error(v_cali_1, &(ctx->chars_second_step), ctx->atten); *voltage = (int32_t)v_cali_1 - error; @@ -146,7 +140,6 @@ static esp_err_t cali_raw_to_voltage(void *arg, int raw, int *voltage) return ESP_OK; } - /* ----------------------- Characterization Functions ----------------------- */ //To get the reference point (Dout, Vin) static void get_first_step_reference_point(int version_num, adc_unit_t unit_id, adc_atten_t atten, adc_calib_info_t *calib_info) @@ -217,7 +210,7 @@ static int32_t get_reading_error(uint64_t v_cali_1, const cali_chars_second_step error = (int32_t)term[0] * (*param->sign)[atten][0]; for (int i = 1; i < term_num; i++) { - variable[i] = variable[i-1] * v_cali_1; + variable[i] = variable[i - 1] * v_cali_1; coeff = (*param->coeff)[atten][i][0]; term[i] = variable[i] * coeff; ESP_LOGV(TAG, "big coef is %llu, big term%d is %llu, coef_id is %d", coeff, i, term[i], i); diff --git a/components/esp_adc/deprecated/esp32c3/esp_adc_cal_legacy.c b/components/esp_adc/deprecated/esp32c3/esp_adc_cal_legacy.c index e35e00dfbcf8..1755344799bf 100644 --- a/components/esp_adc/deprecated/esp32c3/esp_adc_cal_legacy.c +++ b/components/esp_adc/deprecated/esp32c3/esp_adc_cal_legacy.c @@ -163,7 +163,7 @@ uint32_t esp_adc_cal_raw_to_voltage(uint32_t adc_reading, const esp_adc_cal_char assert(chars != NULL); int32_t error = 0; - uint64_t v_cali_1 = adc_reading * chars->coeff_a / coeff_a_scaling; + uint64_t v_cali_1 = (uint64_t)adc_reading * chars->coeff_a / coeff_a_scaling; esp_adc_error_calc_param_t param = { .v_cali_input = v_cali_1, .term_num = (chars->atten == 3) ? 5 : 3, diff --git a/components/esp_adc/deprecated/esp32s3/esp_adc_cal_legacy.c b/components/esp_adc/deprecated/esp32s3/esp_adc_cal_legacy.c index 1f98689ec31f..53122d85075c 100644 --- a/components/esp_adc/deprecated/esp32s3/esp_adc_cal_legacy.c +++ b/components/esp_adc/deprecated/esp32s3/esp_adc_cal_legacy.c @@ -167,7 +167,7 @@ uint32_t esp_adc_cal_raw_to_voltage(uint32_t adc_reading, const esp_adc_cal_char uint64_t v_cali_1 = 0; //raw * gradient * 1000000 - v_cali_1 = adc_reading * chars->coeff_a; + v_cali_1 = (uint64_t)adc_reading * chars->coeff_a; //convert to real number v_cali_1 = v_cali_1 / coeff_a_scaling; ESP_LOGV(LOG_TAG, "v_cali_1 is %llu", v_cali_1); From f9cf8db97eed32e90c231399f944d5ba7af68b18 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 2 Jun 2023 10:30:38 +0800 Subject: [PATCH 2/3] drivers: fix issue reported by coverity --- components/esp_hw_support/dma/gdma.c | 2 +- components/hal/esp32s3/include/hal/i2c_ll.h | 5 +- .../soc/esp32s3/include/soc/i2c_struct.h | 184 +----------------- components/touch_element/touch_element.c | 2 +- 4 files changed, 9 insertions(+), 184 deletions(-) diff --git a/components/esp_hw_support/dma/gdma.c b/components/esp_hw_support/dma/gdma.c index 8247b07af798..4c70a4ebbbc9 100644 --- a/components/esp_hw_support/dma/gdma.c +++ b/components/esp_hw_support/dma/gdma.c @@ -716,7 +716,7 @@ static void IRAM_ATTR gdma_default_rx_isr(void *args) gdma_ll_rx_clear_interrupt_status(group->hal.dev, pair->pair_id, intr_status); if (intr_status & GDMA_LL_EVENT_RX_SUC_EOF) { - if (rx_chan && rx_chan->on_recv_eof) { + if (rx_chan->on_recv_eof) { uint32_t eof_addr = gdma_ll_rx_get_success_eof_desc_addr(group->hal.dev, pair->pair_id); gdma_event_data_t edata = { .rx_eof_desc_addr = eof_addr diff --git a/components/hal/esp32s3/include/hal/i2c_ll.h b/components/hal/esp32s3/include/hal/i2c_ll.h index 431b551b30a7..9b051356e2e4 100644 --- a/components/hal/esp32s3/include/hal/i2c_ll.h +++ b/components/hal/esp32s3/include/hal/i2c_ll.h @@ -300,10 +300,7 @@ static inline void i2c_ll_set_slave_addr(i2c_dev_t *hw, uint16_t slave_addr, boo __attribute__((always_inline)) static inline void i2c_ll_write_cmd_reg(i2c_dev_t *hw, i2c_ll_hw_cmd_t cmd, int cmd_idx) { - ESP_STATIC_ASSERT(sizeof(i2c_comd0_reg_t) == sizeof(i2c_ll_hw_cmd_t), - "i2c_comdX_reg_t structure size must be equal to i2c_ll_hw_cmd_t structure size"); - volatile i2c_ll_hw_cmd_t* commands = (volatile i2c_ll_hw_cmd_t*) &hw->comd0; - commands[cmd_idx].val = cmd.val; + hw->comd[cmd_idx].val = cmd.val; } /** diff --git a/components/soc/esp32s3/include/soc/i2c_struct.h b/components/soc/esp32s3/include/soc/i2c_struct.h index 27f6ce9d2b90..513f55770faa 100644 --- a/components/soc/esp32s3/include/soc/i2c_struct.h +++ b/components/soc/esp32s3/include/soc/i2c_struct.h @@ -902,8 +902,8 @@ typedef union { /** Group: Command registers */ -/** Type of comd0 register - * I2C command register 0 +/** Type of command register + * I2C command register */ typedef union { struct { @@ -915,181 +915,16 @@ typedef union { * structure for more * Information. */ - uint32_t command0:14; + uint32_t command:14; uint32_t reserved_14:17; /** command0_done : R/W/SS; bitpos: [31]; default: 0; * When command 0 is done in I2C Master mode, this bit changes to high * level. */ - uint32_t command0_done:1; + uint32_t command_done:1; }; uint32_t val; -} i2c_comd0_reg_t; - -/** Type of comd1 register - * I2C command register 1 - */ -typedef union { - struct { - /** command1 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 1. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command1:14; - uint32_t reserved_14:17; - /** command1_done : R/W/SS; bitpos: [31]; default: 0; - * When command 1 is done in I2C Master mode, this bit changes to high - * level. - */ - uint32_t command1_done:1; - }; - uint32_t val; -} i2c_comd1_reg_t; - -/** Type of comd2 register - * I2C command register 2 - */ -typedef union { - struct { - /** command2 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 2. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command2:14; - uint32_t reserved_14:17; - /** command2_done : R/W/SS; bitpos: [31]; default: 0; - * When command 2 is done in I2C Master mode, this bit changes to high - * Level. - */ - uint32_t command2_done:1; - }; - uint32_t val; -} i2c_comd2_reg_t; - -/** Type of comd3 register - * I2C command register 3 - */ -typedef union { - struct { - /** command3 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 3. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command3:14; - uint32_t reserved_14:17; - /** command3_done : R/W/SS; bitpos: [31]; default: 0; - * When command 3 is done in I2C Master mode, this bit changes to high - * level. - */ - uint32_t command3_done:1; - }; - uint32_t val; -} i2c_comd3_reg_t; - -/** Type of comd4 register - * I2C command register 4 - */ -typedef union { - struct { - /** command4 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 4. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command4:14; - uint32_t reserved_14:17; - /** command4_done : R/W/SS; bitpos: [31]; default: 0; - * When command 4 is done in I2C Master mode, this bit changes to high - * level. - */ - uint32_t command4_done:1; - }; - uint32_t val; -} i2c_comd4_reg_t; - -/** Type of comd5 register - * I2C command register 5 - */ -typedef union { - struct { - /** command5 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 5. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command5:14; - uint32_t reserved_14:17; - /** command5_done : R/W/SS; bitpos: [31]; default: 0; - * When command 5 is done in I2C Master mode, this bit changes to high level. - */ - uint32_t command5_done:1; - }; - uint32_t val; -} i2c_comd5_reg_t; - -/** Type of comd6 register - * I2C command register 6 - */ -typedef union { - struct { - /** command6 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 6. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command6:14; - uint32_t reserved_14:17; - /** command6_done : R/W/SS; bitpos: [31]; default: 0; - * When command 6 is done in I2C Master mode, this bit changes to high level. - */ - uint32_t command6_done:1; - }; - uint32_t val; -} i2c_comd6_reg_t; - -/** Type of comd7 register - * I2C command register 7 - */ -typedef union { - struct { - /** command7 : R/W; bitpos: [13:0]; default: 0; - * This is the content of command 7. It consists of three parts: - * op_code is the command, 0: RSTART; 1: WRITE; 2: READ; 3: STOP; 4: END. - * Byte_num represents the number of bytes that need to be sent or received. - * ack_check_en, ack_exp and ack are used to control the ACK bit. See I2C cmd - * structure for more - * Information. - */ - uint32_t command7:14; - uint32_t reserved_14:17; - /** command7_done : R/W/SS; bitpos: [31]; default: 0; - * When command 7 is done in I2C Master mode, this bit changes to high level. - */ - uint32_t command7_done:1; - }; - uint32_t val; -} i2c_comd7_reg_t; +} i2c_comd_reg_t; /** Group: Version register */ @@ -1158,14 +993,7 @@ typedef struct { volatile i2c_scl_stop_setup_reg_t scl_stop_setup; volatile i2c_filter_cfg_reg_t filter_cfg; volatile i2c_clk_conf_reg_t clk_conf; - volatile i2c_comd0_reg_t comd0; - volatile i2c_comd1_reg_t comd1; - volatile i2c_comd2_reg_t comd2; - volatile i2c_comd3_reg_t comd3; - volatile i2c_comd4_reg_t comd4; - volatile i2c_comd5_reg_t comd5; - volatile i2c_comd6_reg_t comd6; - volatile i2c_comd7_reg_t comd7; + volatile i2c_comd_reg_t comd[8]; volatile i2c_scl_st_time_out_reg_t scl_st_time_out; volatile i2c_scl_main_st_time_out_reg_t scl_main_st_time_out; volatile i2c_scl_sp_conf_reg_t scl_sp_conf; diff --git a/components/touch_element/touch_element.c b/components/touch_element/touch_element.c index 7adf025e7631..43e8ba562a65 100644 --- a/components/touch_element/touch_element.c +++ b/components/touch_element/touch_element.c @@ -361,7 +361,7 @@ static void te_intr_cb(void *arg) static int scan_done_cnt = 0; static uint32_t touch_pre_trig_status = 0; int task_awoken = pdFALSE; - te_intr_msg_t te_intr_msg; + te_intr_msg_t te_intr_msg = {}; /*< Figure out which touch sensor channel is triggered and the trigger type */ uint32_t intr_mask = touch_pad_read_intr_status_mask(); if (intr_mask == 0x0) { //For dummy interrupt From 3d7f7c32e35e1935b252149f319dffcb0dc3f537 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 9 Jun 2023 11:41:16 +0800 Subject: [PATCH 3/3] adc: fix out of bound read when SOC_ADC_PERIPH_NUM==1, the adc_unit should only be assigned with 0 --- components/driver/deprecated/adc_dma_legacy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/driver/deprecated/adc_dma_legacy.c b/components/driver/deprecated/adc_dma_legacy.c index d03bc67e2439..7b1f135d208e 100644 --- a/components/driver/deprecated/adc_dma_legacy.c +++ b/components/driver/deprecated/adc_dma_legacy.c @@ -104,7 +104,7 @@ static void adc_dma_intr_handler(void *arg); static int8_t adc_digi_get_io_num(adc_unit_t adc_unit, uint8_t adc_channel) { - assert(adc_unit <= SOC_ADC_PERIPH_NUM); + assert(adc_unit < SOC_ADC_PERIPH_NUM); uint8_t adc_n = (adc_unit == ADC_UNIT_1) ? 0 : 1; return adc_channel_io_map[adc_n][adc_channel]; }