diff --git a/components/esp_adc/test_apps/adc/main/test_adc.c b/components/esp_adc/test_apps/adc/main/test_adc.c index f91debf34c..4faecc01e8 100644 --- a/components/esp_adc/test_apps/adc/main/test_adc.c +++ b/components/esp_adc/test_apps/adc/main/test_adc.c @@ -15,6 +15,7 @@ #include "driver/gpio.h" #include "driver/rtc_io.h" #include "test_common_adc.h" +#include "esp_rom_sys.h" const __attribute__((unused)) static char *TAG = "TEST_ADC"; @@ -117,6 +118,44 @@ TEST_CASE("ADC oneshot high/low test", "[adc_oneshot]") #endif //#if ADC_TEST_ONESHOT_HIGH_LOW_TEST_ADC2 } +TEST_CASE("ADC oneshot stress test that get zero even if convent done", "[adc_oneshot]") +{ + //There is a hardware limitation. After ADC get DONE signal, it still need a delay to synchronize ADC raw data or it may get zero even if getting DONE signal. + + int test_num = 100; + adc_channel_t channel = ADC1_TEST_CHAN1; + adc_atten_t atten = ADC_ATTEN_DB_11; + adc_unit_t unit_id = ADC_UNIT_1; + + adc_oneshot_unit_handle_t adc1_handle; + adc_oneshot_unit_init_cfg_t init_config1 = { + .unit_id = unit_id, + .ulp_mode = ADC_ULP_MODE_DISABLE, + }; + + adc_oneshot_chan_cfg_t config = { + .bitwidth = SOC_ADC_RTC_MAX_BITWIDTH, + .atten = atten, + }; + + int raw_data = 0; + srand(199); + + for (int i = 0; i < test_num; i++) { + test_adc_set_io_level(unit_id, ADC1_TEST_CHAN1, 1); + + TEST_ESP_OK(adc_oneshot_new_unit(&init_config1, &adc1_handle)); + TEST_ESP_OK(adc_oneshot_config_channel(adc1_handle, channel, &config)); + TEST_ESP_OK(adc_oneshot_read(adc1_handle, channel, &raw_data)); + + TEST_ASSERT_NOT_EQUAL(0, raw_data); + + TEST_ESP_OK(adc_oneshot_del_unit(adc1_handle)); + + esp_rom_delay_us(rand() % 512); + } +} + #if SOC_ADC_CALIBRATION_V1_SUPPORTED /*--------------------------------------------------------------- ADC Oneshot with Light Sleep diff --git a/components/hal/adc_oneshot_hal.c b/components/hal/adc_oneshot_hal.c index ce6cb193c6..feddc3178c 100644 --- a/components/hal/adc_oneshot_hal.c +++ b/components/hal/adc_oneshot_hal.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -60,6 +60,8 @@ void adc_oneshot_hal_setup(adc_oneshot_hal_ctx_t *hal, adc_channel_t chan) #if SOC_ADC_DIG_CTRL_SUPPORTED && !SOC_ADC_RTC_CTRL_SUPPORTED adc_ll_digi_clk_sel(hal->clk_src); + adc_ll_digi_controller_clk_div(ADC_LL_CLKM_DIV_NUM_DEFAULT, ADC_LL_CLKM_DIV_A_DEFAULT, ADC_LL_CLKM_DIV_B_DEFAULT); + adc_ll_digi_set_clk_div(ADC_LL_DIGI_SAR_CLK_DIV_DEFAULT); #else adc_ll_set_sar_clk_div(unit, ADC_LL_SAR_CLK_DIV_DEFAULT(unit)); if (unit == ADC_UNIT_2) { @@ -83,33 +85,42 @@ static void adc_hal_onetime_start(adc_unit_t unit, uint32_t clk_src_freq_hz) { #if SOC_ADC_DIG_CTRL_SUPPORTED && !SOC_ADC_RTC_CTRL_SUPPORTED (void)unit; - uint32_t delay = 0; /** * There is a hardware limitation. If the APB clock frequency is high, the step of this reg signal: ``onetime_start`` may not be captured by the * ADC digital controller (when its clock frequency is too slow). A rough estimate for this step should be at least 3 ADC digital controller * clock cycle. */ - uint32_t digi_clk = clk_src_freq_hz / (ADC_LL_CLKM_DIV_NUM_DEFAULT + ADC_LL_CLKM_DIV_A_DEFAULT / ADC_LL_CLKM_DIV_B_DEFAULT + 1); + uint32_t adc_ctrl_clk = clk_src_freq_hz / (ADC_LL_CLKM_DIV_NUM_DEFAULT + ADC_LL_CLKM_DIV_A_DEFAULT / ADC_LL_CLKM_DIV_B_DEFAULT + 1); //Convert frequency to time (us). Since decimals are removed by this division operation. Add 1 here in case of the fact that delay is not enough. - delay = (1000 * 1000) / digi_clk + 1; - //3 ADC digital controller clock cycle - delay = delay * 3; - HAL_EARLY_LOGD("adc_hal", "clk_src_freq_hz: %"PRIu32", digi_clk: %"PRIu32", delay: %"PRIu32"", clk_src_freq_hz, digi_clk, delay); + uint32_t sample_delay_us = ((1000 * 1000) / adc_ctrl_clk + 1) * 3; + HAL_EARLY_LOGD("adc_hal", "clk_src_freq_hz: %"PRIu32", adc_ctrl_clk: %"PRIu32", sample_delay_us: %"PRIu32"", clk_src_freq_hz, adc_ctrl_clk, sample_delay_us); //This coefficient (8) is got from test, and verified from DT. When digi_clk is not smaller than ``APB_CLK_FREQ/8``, no delay is needed. - if (digi_clk >= APB_CLK_FREQ/8) { - delay = 0; + if (adc_ctrl_clk >= APB_CLK_FREQ/8) { + sample_delay_us = 0; } - HAL_EARLY_LOGD("adc_hal", "delay: %"PRIu32"", delay); + HAL_EARLY_LOGD("adc_hal", "delay for `onetime_start` signal captured: %"PRIu32"", sample_delay_us); adc_oneshot_ll_start(false); - esp_rom_delay_us(delay); + esp_rom_delay_us(sample_delay_us); adc_oneshot_ll_start(true); - //No need to delay here. Becuase if the start signal is not seen, there won't be a done intr. +#if ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL + /** + * There is a hardware limitation. + * After ADC get DONE signal, it still need a delay to synchronize ADC raw data or it may get zero. + * A rough estimate for this step should be at least ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL ADC sar clock cycle. + */ + uint32_t sar_clk = adc_ctrl_clk / ADC_LL_DIGI_SAR_CLK_DIV_DEFAULT; + uint32_t read_delay_us = ((1000 * 1000) / sar_clk + 1) * ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL; + HAL_EARLY_LOGD("adc_hal", "clk_src_freq_hz: %"PRIu32", sar_clk: %"PRIu32", read_delay_us: %"PRIu32"", clk_src_freq_hz, sar_clk, read_delay_us); + esp_rom_delay_us(read_delay_us); + +#endif //ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL + #else adc_oneshot_ll_start(unit); -#endif +#endif // SOC_ADC_DIG_CTRL_SUPPORTED && !SOC_ADC_RTC_CTRL_SUPPORTED } bool adc_oneshot_hal_convert(adc_oneshot_hal_ctx_t *hal, int *out_raw) diff --git a/components/hal/esp32/include/hal/adc_ll.h b/components/hal/esp32/include/hal/adc_ll.h index 6e5ef9bbcc..08e93a0c6d 100644 --- a/components/hal/esp32/include/hal/adc_ll.h +++ b/components/hal/esp32/include/hal/adc_ll.h @@ -30,6 +30,7 @@ extern "C" { ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (1) #define ADC_LL_SAR_CLK_DIV_DEFAULT(PERIPH_NUM) (1) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (0) /*--------------------------------------------------------------- DMA diff --git a/components/hal/esp32c2/include/hal/adc_ll.h b/components/hal/esp32c2/include/hal/adc_ll.h index 96c3e58bde..9427b0e4c4 100644 --- a/components/hal/esp32c2/include/hal/adc_ll.h +++ b/components/hal/esp32c2/include/hal/adc_ll.h @@ -33,6 +33,7 @@ extern "C" { Oneshot ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (0) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (0) /*--------------------------------------------------------------- DMA diff --git a/components/hal/esp32c3/include/hal/adc_ll.h b/components/hal/esp32c3/include/hal/adc_ll.h index eb3a42651c..f62ffb538c 100644 --- a/components/hal/esp32c3/include/hal/adc_ll.h +++ b/components/hal/esp32c3/include/hal/adc_ll.h @@ -41,6 +41,7 @@ extern "C" { Oneshot ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (0) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (0) /*--------------------------------------------------------------- DMA diff --git a/components/hal/esp32c6/include/hal/adc_ll.h b/components/hal/esp32c6/include/hal/adc_ll.h index c8c52aadfd..d7d689e74a 100644 --- a/components/hal/esp32c6/include/hal/adc_ll.h +++ b/components/hal/esp32c6/include/hal/adc_ll.h @@ -42,6 +42,7 @@ extern "C" { Oneshot ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (0) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (0) /*--------------------------------------------------------------- DMA diff --git a/components/hal/esp32h2/include/hal/adc_ll.h b/components/hal/esp32h2/include/hal/adc_ll.h index 269194b690..31b8976c40 100644 --- a/components/hal/esp32h2/include/hal/adc_ll.h +++ b/components/hal/esp32h2/include/hal/adc_ll.h @@ -42,6 +42,7 @@ extern "C" { Oneshot ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (0) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (2) /*--------------------------------------------------------------- DMA diff --git a/components/hal/esp32s2/include/hal/adc_ll.h b/components/hal/esp32s2/include/hal/adc_ll.h index 8638d7f6e2..09b926399b 100644 --- a/components/hal/esp32s2/include/hal/adc_ll.h +++ b/components/hal/esp32s2/include/hal/adc_ll.h @@ -39,6 +39,7 @@ extern "C" { ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (0) #define ADC_LL_SAR_CLK_DIV_DEFAULT(PERIPH_NUM) (1) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (0) /*--------------------------------------------------------------- DMA diff --git a/components/hal/esp32s3/include/hal/adc_ll.h b/components/hal/esp32s3/include/hal/adc_ll.h index 5e6978f09b..d016316295 100644 --- a/components/hal/esp32s3/include/hal/adc_ll.h +++ b/components/hal/esp32s3/include/hal/adc_ll.h @@ -42,6 +42,7 @@ extern "C" { ---------------------------------------------------------------*/ #define ADC_LL_DATA_INVERT_DEFAULT(PERIPH_NUM) (0) #define ADC_LL_SAR_CLK_DIV_DEFAULT(PERIPH_NUM) (1) +#define ADC_LL_DELAY_CYCLE_AFTER_DONE_SIGNAL (0) /*--------------------------------------------------------------- DMA