From 02600309c8e12cf1786ce3326d81709639462264 Mon Sep 17 00:00:00 2001 From: Armando Date: Mon, 18 Jan 2021 21:59:18 +0800 Subject: [PATCH] adc: fix some regression issues --- components/driver/CMakeLists.txt | 2 +- components/driver/esp32c3/adc2_init_cal.c | 2 +- .../driver/esp32c3/include/driver/adc.h | 33 ------------------ components/driver/esp32s2/adc2_init_cal.c | 2 +- components/driver/include/driver/adc_common.h | 8 ++--- .../{ => include}/esp_private/adc_cali.h | 0 components/driver/test/test_adc_dma.c | 34 +++++++------------ components/hal/adc_hal.c | 3 +- components/hal/esp32c3/adc_hal.c | 2 +- .../hal/esp32c3/include/hal/clk_gate_ll.h | 2 -- components/hal/esp32c3/include/hal/gdma_ll.h | 7 ---- components/hal/esp32s2/adc_hal.c | 14 ++++---- components/hal/include/hal/adc_types.h | 6 ++-- .../adc/adc_dma/main/adc_dma_example_main.c | 2 +- 14 files changed, 33 insertions(+), 84 deletions(-) rename components/driver/{ => include}/esp_private/adc_cali.h (100%) diff --git a/components/driver/CMakeLists.txt b/components/driver/CMakeLists.txt index 777d0afae0..4e71325b50 100644 --- a/components/driver/CMakeLists.txt +++ b/components/driver/CMakeLists.txt @@ -22,7 +22,7 @@ set(srcs "twai.c" "uart.c") -set(includes "include" "${target}/include" "esp_private") +set(includes "include" "${target}/include") if(${target} STREQUAL "esp32") # SDMMC and MCPWM are in ESP32 only. diff --git a/components/driver/esp32c3/adc2_init_cal.c b/components/driver/esp32c3/adc2_init_cal.c index 19e7d98c19..944d8fa46f 100644 --- a/components/driver/esp32c3/adc2_init_cal.c +++ b/components/driver/esp32c3/adc2_init_cal.c @@ -18,7 +18,7 @@ Don't put any other code into this file. */ #include "adc2_wifi_private.h" #include "hal/adc_hal.h" -#include "adc_cali.h" +#include "esp_private/adc_cali.h" /** * @brief Set initial code to ADC2 after calibration. ADC2 RTC and ADC2 PWDET controller share the initial code. diff --git a/components/driver/esp32c3/include/driver/adc.h b/components/driver/esp32c3/include/driver/adc.h index fc8b59412d..463778a3a9 100644 --- a/components/driver/esp32c3/include/driver/adc.h +++ b/components/driver/esp32c3/include/driver/adc.h @@ -186,39 +186,6 @@ esp_err_t adc_digi_isr_register(void (*fn)(void *), void *arg, int intr_alloc_fl */ esp_err_t adc_digi_isr_deregister(void); -/*--------------------------------------------------------------- - RTC controller setting ----------------------------------------------------------------*/ - -/*--------------------------------------------------------------- - Deprecated API ----------------------------------------------------------------*/ -/** - * @brief Set I2S data source - * - * @param src I2S DMA data source, I2S DMA can get data from digital signals or from ADC. - * - * @deprecated The ESP32C3 doesn't use I2S DMA. Call ``adc_digi_controller_config`` instead. - * - * @return - * - ESP_OK success - */ -esp_err_t adc_set_i2s_data_source(adc_i2s_source_t src) __attribute__((deprecated)); - -/** - * @brief Initialize I2S ADC mode - * - * @param adc_unit ADC unit index - * @param channel ADC channel index - * - * @deprecated The ESP32C3 doesn't use I2S DMA. Call ``adc_digi_controller_config`` instead. - * - * @return - * - ESP_OK success - * - ESP_ERR_INVALID_ARG Parameter error - */ -esp_err_t adc_i2s_mode_init(adc_unit_t adc_unit, adc_channel_t channel) __attribute__((deprecated)); - #ifdef __cplusplus } #endif diff --git a/components/driver/esp32s2/adc2_init_cal.c b/components/driver/esp32s2/adc2_init_cal.c index 19e7d98c19..944d8fa46f 100644 --- a/components/driver/esp32s2/adc2_init_cal.c +++ b/components/driver/esp32s2/adc2_init_cal.c @@ -18,7 +18,7 @@ Don't put any other code into this file. */ #include "adc2_wifi_private.h" #include "hal/adc_hal.h" -#include "adc_cali.h" +#include "esp_private/adc_cali.h" /** * @brief Set initial code to ADC2 after calibration. ADC2 RTC and ADC2 PWDET controller share the initial code. diff --git a/components/driver/include/driver/adc_common.h b/components/driver/include/driver/adc_common.h index abb1b17e57..2ac5110c43 100644 --- a/components/driver/include/driver/adc_common.h +++ b/components/driver/include/driver/adc_common.h @@ -38,8 +38,7 @@ typedef enum { ADC1_CHANNEL_7, /*!< ADC1 channel 7 is GPIO35 */ ADC1_CHANNEL_MAX, } adc1_channel_t; -#elif CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 -//S3 is not sure. Need to be checked when bringing up S3 +#elif CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 // TODO ESP32-S3 channels are wrong IDF-1776 /**** `adc1_channel_t` will be deprecated functions, combine into `adc_channel_t` ********/ typedef enum { ADC1_CHANNEL_0 = 0, /*!< ADC1 channel 0 is GPIO1 */ @@ -64,10 +63,9 @@ typedef enum { ADC1_CHANNEL_4, /*!< ADC1 channel 4 is GPIO34 */ ADC1_CHANNEL_MAX, } adc1_channel_t; -#endif +#endif // CONFIG_IDF_TARGET_* -#if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 -//S3 is not sure. Need to be checked when bringing up S3 +#if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 // TODO ESP32-S3 channels are wrong IDF-1776 /**** `adc2_channel_t` will be deprecated functions, combine into `adc_channel_t` ********/ typedef enum { ADC2_CHANNEL_0 = 0, /*!< ADC2 channel 0 is GPIO4 (ESP32), GPIO11 (ESP32-S2) */ diff --git a/components/driver/esp_private/adc_cali.h b/components/driver/include/esp_private/adc_cali.h similarity index 100% rename from components/driver/esp_private/adc_cali.h rename to components/driver/include/esp_private/adc_cali.h diff --git a/components/driver/test/test_adc_dma.c b/components/driver/test/test_adc_dma.c index db32120d8e..04d2c4c57a 100644 --- a/components/driver/test/test_adc_dma.c +++ b/components/driver/test/test_adc_dma.c @@ -36,7 +36,7 @@ static int insert_point(uint32_t value) if (s_adc_offset < 0) { if (fixed_size) { - assert(MAX_ARRAY_SIZE >= 4096); + TEST_ASSERT_GREATER_OR_EQUAL(4096, MAX_ARRAY_SIZE); s_adc_offset = 0; //Fixed to 0 because the array can hold all the data in 12 bits } else { s_adc_offset = MAX((int)value - MAX_ARRAY_SIZE/2, 0); @@ -44,8 +44,8 @@ static int insert_point(uint32_t value) } if (!fixed_size && (value < s_adc_offset || value >= s_adc_offset + MAX_ARRAY_SIZE)) { - assert(value >= s_adc_offset); - assert(value < s_adc_offset + MAX_ARRAY_SIZE); + TEST_ASSERT_GREATER_OR_EQUAL(s_adc_offset, value); + TEST_ASSERT_LESS_THAN(s_adc_offset + MAX_ARRAY_SIZE, value); } s_adc_count[value - s_adc_offset] ++; @@ -119,9 +119,6 @@ static void print_summary(bool figure) static void continuous_adc_init(uint16_t adc1_chan_mask, uint16_t adc2_chan_mask, adc_channel_t *channel, uint8_t channel_num, adc_atten_t atten) { - esp_err_t ret = ESP_OK; - assert(ret == ESP_OK); - adc_digi_init_config_t adc_dma_config = { .max_store_buf_size = TEST_COUNT*2, .conv_num_each_intr = 128, @@ -129,10 +126,9 @@ static void continuous_adc_init(uint16_t adc1_chan_mask, uint16_t adc2_chan_mask .adc1_chan_mask = adc1_chan_mask, .adc2_chan_mask = adc2_chan_mask, }; - ret = adc_digi_initialize(&adc_dma_config); - assert(ret == ESP_OK); + TEST_ESP_OK(adc_digi_initialize(&adc_dma_config)); - adc_digi_pattern_table_t adc_pattern[10]; + adc_digi_pattern_table_t adc_pattern[10] = {0}; adc_digi_config_t dig_cfg = { .conv_limit_en = 0, .conv_limit_num = 250, @@ -152,13 +148,11 @@ static void continuous_adc_init(uint16_t adc1_chan_mask, uint16_t adc2_chan_mask adc_pattern[i].unit = unit; } dig_cfg.adc_pattern = adc_pattern; - ret = adc_digi_controller_config(&dig_cfg); - assert(ret == ESP_OK); + TEST_ESP_OK(adc_digi_controller_config(&dig_cfg)); } TEST_CASE("test_adc_dma", "[adc][ignore][manual]") { - esp_err_t ret; uint16_t adc1_chan_mask = BIT(2); uint16_t adc2_chan_mask = 0; adc_channel_t channel[1] = {ADC1_CHANNEL_2}; @@ -168,7 +162,7 @@ TEST_CASE("test_adc_dma", "[adc][ignore][manual]") int buffer_size = TEST_COUNT*output_data_size; uint8_t* read_buf = malloc(buffer_size); - assert(read_buf); + TEST_ASSERT_NOT_NULL(read_buf); adc_atten_t atten; bool print_figure; @@ -187,7 +181,7 @@ TEST_CASE("test_adc_dma", "[adc][ignore][manual]") esp_adc_cal_characteristics_t chan1_char = {}; esp_adc_cal_value_t cal_ret = esp_adc_cal_characterize(ADC_UNIT_1, atten, ADC_WIDTH_12Bit, 0, &chan1_char); - assert(cal_ret == ESP_ADC_CAL_VAL_EFUSE_TP); + TEST_ASSERT(cal_ret == ESP_ADC_CAL_VAL_EFUSE_TP); continuous_adc_init(adc1_chan_mask, adc2_chan_mask, channel, sizeof(channel) / sizeof(adc_channel_t), atten); adc_digi_start(); @@ -196,11 +190,10 @@ TEST_CASE("test_adc_dma", "[adc][ignore][manual]") while (remain_count) { int already_got = TEST_COUNT - remain_count; uint32_t ret_num; - ret = adc_digi_read_bytes(read_buf + already_got*output_data_size, - remain_count*output_data_size, &ret_num, ADC_MAX_DELAY); + TEST_ESP_OK(adc_digi_read_bytes(read_buf + already_got*output_data_size, + remain_count*output_data_size, &ret_num, ADC_MAX_DELAY)); - ESP_ERROR_CHECK(ret); - assert((ret_num % output_data_size) == 0); + TEST_ASSERT((ret_num % output_data_size) == 0); remain_count -= ret_num / output_data_size; } @@ -217,8 +210,7 @@ TEST_CASE("test_adc_dma", "[adc][ignore][manual]") printf("Voltage = %d mV\n", voltage_mv); adc_digi_stop(); - ret = adc_digi_deinitialize(); - assert(ret == ESP_OK); + TEST_ESP_OK(adc_digi_deinitialize()); if (atten == target_atten) { break; @@ -254,7 +246,7 @@ TEST_CASE("test_adc_single", "[adc][ignore][manual]") esp_adc_cal_characteristics_t chan1_char = {}; esp_adc_cal_value_t cal_ret = esp_adc_cal_characterize(ADC_UNIT_1, atten, ADC_WIDTH_12Bit, 0, &chan1_char); - assert(cal_ret == ESP_ADC_CAL_VAL_EFUSE_TP); + TEST_ASSERT(cal_ret == ESP_ADC_CAL_VAL_EFUSE_TP); const int test_count = TEST_COUNT; diff --git a/components/hal/adc_hal.c b/components/hal/adc_hal.c index 4389a1df2e..73d6a7887d 100644 --- a/components/hal/adc_hal.c +++ b/components/hal/adc_hal.c @@ -17,6 +17,7 @@ #if CONFIG_IDF_TARGET_ESP32C3 +#include "soc/gdma_channel.h" #include "soc/soc.h" #include "esp_rom_sys.h" #endif @@ -126,7 +127,7 @@ void adc_hal_digi_init(adc_dma_hal_context_t *adc_dma_ctx, adc_dma_hal_config_t adc_dma_ctx->dev = &GDMA; gdma_ll_enable_clock(adc_dma_ctx->dev, true); gdma_ll_clear_interrupt_status(adc_dma_ctx->dev, dma_config->dma_chan, UINT32_MAX); - gdma_ll_rx_connect_to_periph(adc_dma_ctx->dev, dma_config->dma_chan, GDMA_LL_TRIG_SRC_ADC_DAC); + gdma_ll_rx_connect_to_periph(adc_dma_ctx->dev, dma_config->dma_chan, SOC_GDMA_TRIG_PERIPH_ADC0); } /*--------------------------------------------------------------- diff --git a/components/hal/esp32c3/adc_hal.c b/components/hal/esp32c3/adc_hal.c index 7f1e49d4f6..a2ab50c02b 100644 --- a/components/hal/esp32c3/adc_hal.c +++ b/components/hal/esp32c3/adc_hal.c @@ -43,7 +43,7 @@ void adc_hal_digi_controller_config(const adc_digi_config_t *cfg) if (cfg->adc_pattern_len) { adc_ll_digi_clear_pattern_table(pattern_both); adc_ll_digi_set_pattern_table_len(pattern_both, cfg->adc_pattern_len); - for (int i = 0; i < cfg->adc_pattern_len; i++) { + for (uint32_t i = 0; i < cfg->adc_pattern_len; i++) { adc_ll_digi_set_pattern_table(pattern_both, i, cfg->adc_pattern[i]); } } diff --git a/components/hal/esp32c3/include/hal/clk_gate_ll.h b/components/hal/esp32c3/include/hal/clk_gate_ll.h index 7af8ed61ba..f7382650d5 100644 --- a/components/hal/esp32c3/include/hal/clk_gate_ll.h +++ b/components/hal/esp32c3/include/hal/clk_gate_ll.h @@ -172,7 +172,6 @@ static uint32_t periph_ll_get_clk_en_reg(periph_module_t periph) case PERIPH_SHA_MODULE: case PERIPH_GDMA_MODULE: return SYSTEM_PERIP_CLK_EN1_REG; - case PERIPH_SARADC_MODULE: default: return SYSTEM_PERIP_CLK_EN0_REG; } @@ -196,7 +195,6 @@ static uint32_t periph_ll_get_rst_en_reg(periph_module_t periph) case PERIPH_SHA_MODULE: case PERIPH_GDMA_MODULE: return SYSTEM_PERIP_RST_EN1_REG; - case PERIPH_SARADC_MODULE: default: return SYSTEM_PERIP_RST_EN0_REG; } diff --git a/components/hal/esp32c3/include/hal/gdma_ll.h b/components/hal/esp32c3/include/hal/gdma_ll.h index c3ad226bce..22410d3bdd 100644 --- a/components/hal/esp32c3/include/hal/gdma_ll.h +++ b/components/hal/esp32c3/include/hal/gdma_ll.h @@ -39,13 +39,6 @@ extern "C" { #define GDMA_LL_EVENT_RX_SUC_EOF (1<<1) #define GDMA_LL_EVENT_RX_DONE (1<<0) -#define GDMA_LL_TRIG_SRC_SPI2 (0) -#define GDMA_LL_TRIG_SRC_UART (2) -#define GDMA_LL_TRIG_SRC_I2S0 (3) -#define GDMA_LL_TRIG_SRC_AES (6) -#define GDMA_LL_TRIG_SRC_SHA (7) -#define GDMA_LL_TRIG_SRC_ADC_DAC (8) - ///////////////////////////////////// Common ///////////////////////////////////////// /** * @brief Enable DMA channel M2M mode (TX channel n forward data to RX channel n), disabled by default diff --git a/components/hal/esp32s2/adc_hal.c b/components/hal/esp32s2/adc_hal.c index c244432d76..68e8b98071 100644 --- a/components/hal/esp32s2/adc_hal.c +++ b/components/hal/esp32s2/adc_hal.c @@ -46,7 +46,7 @@ void adc_hal_digi_controller_config(const adc_digi_config_t *cfg) if (cfg->adc1_pattern_len) { adc_ll_digi_clear_pattern_table(ADC_NUM_1); adc_ll_digi_set_pattern_table_len(ADC_NUM_1, cfg->adc1_pattern_len); - for (int i = 0; i < cfg->adc1_pattern_len; i++) { + for (uint32_t i = 0; i < cfg->adc1_pattern_len; i++) { adc_ll_digi_set_pattern_table(ADC_NUM_1, i, cfg->adc1_pattern[i]); } } @@ -55,7 +55,7 @@ void adc_hal_digi_controller_config(const adc_digi_config_t *cfg) if (cfg->adc2_pattern_len) { adc_ll_digi_clear_pattern_table(ADC_NUM_2); adc_ll_digi_set_pattern_table_len(ADC_NUM_2, cfg->adc2_pattern_len); - for (int i = 0; i < cfg->adc2_pattern_len; i++) { + for (uint32_t i = 0; i < cfg->adc2_pattern_len; i++) { adc_ll_digi_set_pattern_table(ADC_NUM_2, i, cfg->adc2_pattern[i]); } } @@ -181,11 +181,11 @@ uint32_t adc_hal_self_calibration(adc_ll_num_t adc_n, adc_channel_t channel, adc adc_ll_set_atten(adc_n, channel, atten); } - uint32_t code_list[ADC_HAL_CAL_TIMES] = {0}; - uint32_t code_sum = 0; - uint32_t code_h = 0; - uint32_t code_l = 0; - uint32_t chk_code = 0; + uint32_t code_list[ADC_HAL_CAL_TIMES] = {0}; + uint32_t code_sum = 0; + uint32_t code_h = 0; + uint32_t code_l = 0; + uint32_t chk_code = 0; for (uint8_t rpt = 0 ; rpt < ADC_HAL_CAL_TIMES ; rpt ++) { code_h = ADC_HAL_CAL_OFFSET_RANGE; diff --git a/components/hal/include/hal/adc_types.h b/components/hal/include/hal/adc_types.h index eddf6a96c5..3718899407 100644 --- a/components/hal/include/hal/adc_types.h +++ b/components/hal/include/hal/adc_types.h @@ -88,10 +88,10 @@ typedef enum { ADC_WIDTH_BIT_10 = 1, /*!< ADC capture width is 10Bit. */ ADC_WIDTH_BIT_11 = 2, /*!< ADC capture width is 11Bit. */ ADC_WIDTH_BIT_12 = 3, /*!< ADC capture width is 12Bit. */ -#elif CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 - ADC_WIDTH_BIT_13 = 4, /*!< ADC capture width is 13Bit. */ -#elif CONFIG_IDF_TARGET_ESP32C3 +#elif SOC_ADC_MAX_BITWIDTH == 12 ADC_WIDTH_BIT_12 = 3, /*!< ADC capture width is 12Bit. */ +#elif SOC_ADC_MAX_BITWIDTH == 13 + ADC_WIDTH_BIT_13 = 4, /*!< ADC capture width is 13Bit. */ #endif ADC_WIDTH_MAX, } adc_bits_width_t; diff --git a/examples/peripherals/adc/adc_dma/main/adc_dma_example_main.c b/examples/peripherals/adc/adc_dma/main/adc_dma_example_main.c index 6604e1d6e4..99ef45de50 100644 --- a/examples/peripherals/adc/adc_dma/main/adc_dma_example_main.c +++ b/examples/peripherals/adc/adc_dma/main/adc_dma_example_main.c @@ -24,7 +24,7 @@ static void continuous_adc_init(uint16_t adc1_chan_mask, uint16_t adc2_chan_mask ret = adc_digi_initialize(&adc_dma_config); assert(ret == ESP_OK); - adc_digi_pattern_table_t adc_pattern[10]; + adc_digi_pattern_table_t adc_pattern[10] = {0}; adc_digi_config_t dig_cfg = { .conv_limit_en = 0, .conv_limit_num = 250,