Merge branch 'bugfix/fix_adc_read_zero_h2' into 'master'

ADC: fix adc raw data get 0 because of signal delay on ESP32H2

See merge request espressif/esp-idf!26572

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

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)

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

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

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

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

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

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

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