From 13d0e83cd6abf339863f14ffc270c1d6015a6a6b Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 26 Nov 2024 12:50:04 +0800 Subject: [PATCH] fix(driver): issues reported by coverity --- .../dvp/src/esp_cam_ctlr_dvp_gdma.c | 4 +- .../esp_driver_jpeg/jpeg_parse_marker.c | 4 +- components/esp_driver_parlio/src/parlio_rx.c | 4 +- components/esp_driver_uart/src/uart_vfs.c | 2 +- .../esp_hw_support/debug_probe/debug_probe.c | 43 ++++++++----------- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/components/esp_driver_cam/dvp/src/esp_cam_ctlr_dvp_gdma.c b/components/esp_driver_cam/dvp/src/esp_cam_ctlr_dvp_gdma.c index 4e428b2505..e3c76f20bf 100644 --- a/components/esp_driver_cam/dvp/src/esp_cam_ctlr_dvp_gdma.c +++ b/components/esp_driver_cam/dvp/src/esp_cam_ctlr_dvp_gdma.c @@ -44,7 +44,9 @@ static void IRAM_ATTR esp_cam_ctlr_dvp_config_dma_desc(esp_cam_ctlr_dvp_dma_desc n++; } - desc[n - 1].next = NULL; + if (n > 0) { + desc[n - 1].next = NULL; + } } /** diff --git a/components/esp_driver_jpeg/jpeg_parse_marker.c b/components/esp_driver_jpeg/jpeg_parse_marker.c index 13fcc831d0..0e238a1971 100644 --- a/components/esp_driver_jpeg/jpeg_parse_marker.c +++ b/components/esp_driver_jpeg/jpeg_parse_marker.c @@ -130,10 +130,10 @@ esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) // The vertical and horizontal in process must be divided by mcu block. if (header_info->origin_v % header_info->mcuy != 0) { - header_info->process_v = (uint32_t)(ceil(header_info->origin_v / header_info->mcuy) + 1) * header_info->mcuy; + header_info->process_v = ((header_info->origin_v / header_info->mcuy) + 1) * header_info->mcuy; } if (header_info->origin_h % header_info->mcux != 0) { - header_info->process_h = (uint32_t)(ceil(header_info->origin_h / header_info->mcux) + 1) * header_info->mcux; + header_info->process_h = ((header_info->origin_h / header_info->mcux) + 1) * header_info->mcux; } return ESP_OK; diff --git a/components/esp_driver_parlio/src/parlio_rx.c b/components/esp_driver_parlio/src/parlio_rx.c index 97acef1973..abf0d30159 100644 --- a/components/esp_driver_parlio/src/parlio_rx.c +++ b/components/esp_driver_parlio/src/parlio_rx.c @@ -138,7 +138,9 @@ static IRAM_ATTR size_t s_parlio_mount_transaction_buffer(parlio_rx_unit_handle_ /* Update the current transaction to the next one, and declare the delimiter is under using of the rx unit */ memcpy(&rx_unit->curr_trans, trans, sizeof(parlio_rx_transaction_t)); portENTER_CRITICAL_SAFE(&s_rx_spinlock); - trans->delimiter->under_using = true; + if (trans->delimiter) { + trans->delimiter->under_using = true; + } portEXIT_CRITICAL_SAFE(&s_rx_spinlock); uint32_t desc_num = trans->size / PARLIO_MAX_ALIGNED_DMA_BUF_SIZE; diff --git a/components/esp_driver_uart/src/uart_vfs.c b/components/esp_driver_uart/src/uart_vfs.c index a56630168d..a4ea3ca347 100644 --- a/components/esp_driver_uart/src/uart_vfs.c +++ b/components/esp_driver_uart/src/uart_vfs.c @@ -215,7 +215,7 @@ static int uart_rx_char(int fd) static int uart_rx_char_via_driver(int fd) { uint8_t c; - int timeout = s_ctx[fd]->non_blocking ? 0 : portMAX_DELAY; + TickType_t timeout = s_ctx[fd]->non_blocking ? 0 : portMAX_DELAY; int n = uart_read_bytes(fd, &c, 1, timeout); if (n <= 0) { return NONE; diff --git a/components/esp_hw_support/debug_probe/debug_probe.c b/components/esp_hw_support/debug_probe/debug_probe.c index 94574427b8..19ea956eb2 100644 --- a/components/esp_hw_support/debug_probe/debug_probe.c +++ b/components/esp_hw_support/debug_probe/debug_probe.c @@ -15,10 +15,12 @@ #include "esp_heap_caps.h" #include "soc/soc_caps.h" #include "soc/debug_probe_periph.h" +#include "soc/io_mux_reg.h" #include "hal/debug_probe_ll.h" #include "esp_private/debug_probe.h" +#include "esp_private/gpio.h" +#include "esp_private/esp_gpio_reserve.h" #include "esp_rom_gpio.h" -#include "driver/gpio.h" static const char *TAG = "dbg_probe"; @@ -28,6 +30,7 @@ typedef struct debug_probe_channel_t debug_probe_channel_t; struct debug_probe_unit_t { int unit_id; // unit id debug_probe_channel_t *channels[DEBUG_PROBE_LL_CHANNELS_PER_UNIT]; // channels installed in this unit + uint64_t pin_bit_mask; // bit-mask of the GPIOs used by this unit }; struct debug_probe_channel_t { @@ -53,6 +56,7 @@ static esp_err_t debug_probe_unit_destroy(debug_probe_unit_t *unit) // disable the probe output debug_probe_ll_enable_unit(unit_id, false); + esp_gpio_revoke(unit->pin_bit_mask); // free the memory free(unit); return ESP_OK; @@ -60,7 +64,6 @@ static esp_err_t debug_probe_unit_destroy(debug_probe_unit_t *unit) esp_err_t debug_probe_new_unit(const debug_probe_unit_config_t *config, debug_probe_unit_handle_t *out_handle) { - esp_err_t ret = ESP_OK; debug_probe_unit_t *unit = NULL; int unit_id = -1; ESP_RETURN_ON_FALSE(config && out_handle, ESP_ERR_INVALID_ARG, TAG, "invalid args"); @@ -80,43 +83,35 @@ esp_err_t debug_probe_new_unit(const debug_probe_unit_config_t *config, debug_pr ESP_RETURN_ON_FALSE(unit, ESP_ERR_NO_MEM, TAG, "no mem for unit"); unit->unit_id = unit_id; - // configure the GPIOs - gpio_config_t monitor_io_conf = { - .mode = GPIO_MODE_OUTPUT, - .pin_bit_mask = 0, - }; + uint64_t pin_bit_mask = 0; for (int i = 0; i < SOC_DEBUG_PROBE_MAX_OUTPUT_WIDTH; i++) { - // skip unused IOs - if (config->probe_out_gpio_nums[i] < 0) { - continue; + if (config->probe_out_gpio_nums[i] >= 0) { + pin_bit_mask |= BIT64(config->probe_out_gpio_nums[i]); } - monitor_io_conf.pin_bit_mask |= (1ULL << config->probe_out_gpio_nums[i]); } - if (monitor_io_conf.pin_bit_mask) { - ESP_GOTO_ON_ERROR(gpio_config(&monitor_io_conf), err, TAG, "gpio_config failed"); + // reserve the GPIO output path, because we don't expect another peripheral to signal to the same GPIO + uint64_t old_gpio_rsv_mask = esp_gpio_reserve(pin_bit_mask); + // check if the GPIO is already used by others, RMT TX channel only uses the output path of the GPIO + if (old_gpio_rsv_mask & pin_bit_mask) { + ESP_LOGW(TAG, "GPIO conflict with others"); } // connect the probe output signals to the GPIOs for (int i = 0; i < SOC_DEBUG_PROBE_MAX_OUTPUT_WIDTH; i++) { - if (config->probe_out_gpio_nums[i] < 0) { - continue; + if (config->probe_out_gpio_nums[i] >= 0) { + gpio_func_sel(config->probe_out_gpio_nums[i], PIN_FUNC_GPIO); + esp_rom_gpio_connect_out_signal(config->probe_out_gpio_nums[i], + debug_probe_periph_signals.units[unit_id].out_sig[i], + false, false); } - esp_rom_gpio_connect_out_signal(config->probe_out_gpio_nums[i], - debug_probe_periph_signals.units[unit_id].out_sig[i], - false, false); } + unit->pin_bit_mask = pin_bit_mask; // enable the probe unit debug_probe_ll_enable_unit(unit_id, true); *out_handle = unit; return ESP_OK; - -err: - if (unit) { - debug_probe_unit_destroy(unit); - } - return ret; } esp_err_t debug_probe_del_unit(debug_probe_unit_handle_t unit)