From a4f81d0bd3657f61e0747ec74c704a423854f602 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 7 Jan 2020 09:35:43 +0100 Subject: [PATCH 1/4] adc: add adc_power_acquire/release, deprecate adc_power_on/off --- components/driver/i2s.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/components/driver/i2s.c b/components/driver/i2s.c index 3c609336e6..cabfa2001c 100644 --- a/components/driver/i2s.c +++ b/components/driver/i2s.c @@ -867,13 +867,6 @@ static esp_err_t i2s_param_config(i2s_port_t i2s_num, const i2s_config_t *i2s_co periph_module_enable(i2s_periph_signal[i2s_num].module); #if SOC_I2S_SUPPORTS_ADC_DAC - if(i2s_config->mode & I2S_MODE_ADC_BUILT_IN) { - //in ADC built-in mode, we need to call i2s_set_adc_mode to - //initialize the specific ADC channel. - //in the current stage, we only support ADC1 and single channel mode. - //In default data mode, the ADC data is in 12-bit resolution mode. - adc_power_always_on(); - } #endif // configure I2S data port interface. i2s_hal_config_param(&(p_i2s_obj[i2s_num]->hal), i2s_config); From 80e7252c13169832bfad634dce9e5f9e7533aaa5 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 2 Jan 2020 10:44:45 +0100 Subject: [PATCH 2/4] wifi: add set_xpd_sar override Wi-Fi enables and disables ADC when exiting and entering sleep mode. Coordinate ADC power state with other modules, using adc_power_acquire and adc_power_release. --- components/esp_wifi/CMakeLists.txt | 1 + components/esp_wifi/src/wifi_init.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/components/esp_wifi/CMakeLists.txt b/components/esp_wifi/CMakeLists.txt index 329f35d34e..0fe1899f3b 100644 --- a/components/esp_wifi/CMakeLists.txt +++ b/components/esp_wifi/CMakeLists.txt @@ -34,6 +34,7 @@ idf_component_register(SRCS "src/coexist.c" "src/wifi_netif.c" "${idf_target}/esp_adapter.c" INCLUDE_DIRS "include" "${idf_target}/include" + PRIV_REQUIRES wpa_supplicant nvs_flash esp_netif driver ${extra_priv_requires} REQUIRES esp_event PRIV_REQUIRES esp_timer esp_pm wpa_supplicant nvs_flash esp_netif ${extra_priv_requires} LDFRAGMENTS "${ldfragments}") diff --git a/components/esp_wifi/src/wifi_init.c b/components/esp_wifi/src/wifi_init.c index e675105653..b8aca2ce31 100644 --- a/components/esp_wifi/src/wifi_init.c +++ b/components/esp_wifi/src/wifi_init.c @@ -23,6 +23,7 @@ #include "esp_wpa.h" #include "esp_netif.h" #include "tcpip_adapter_compatible/tcpip_adapter_compat.h" +#include "driver/adc.h" #include "driver/adc2_wifi_private.h" #include "esp_coexist_internal.h" @@ -56,6 +57,8 @@ uint64_t g_wifi_feature_caps = #endif 0; +static bool s_wifi_adc_xpd_flag; + static const char* TAG = "wifi_init"; static void __attribute__((constructor)) s_set_default_wifi_log_level(void) @@ -251,3 +254,19 @@ void wifi_apb80m_release(void) esp_pm_lock_release(s_wifi_modem_sleep_lock); } #endif //CONFIG_PM_ENABLE + +/* Coordinate ADC power with other modules. This overrides the function from PHY lib. */ +void set_xpd_sar(bool en) +{ + if (s_wifi_adc_xpd_flag == en) { + /* ignore repeated calls to set_xpd_sar when the state is already correct */ + return; + } + + s_wifi_adc_xpd_flag = en; + if (en) { + adc_power_acquire(); + } else { + adc_power_release(); + } +} From d890a516a1097f0a07788e203fdb1a82bb83520e Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 7 Jan 2020 09:48:07 +0100 Subject: [PATCH 3/4] driver: add notes about GPIO36/39 issue (ECO 3.11) workaround Closes IDFGH-1917 Closes https://github.com/espressif/esp-idf/issues/4117 --- components/driver/include/driver/gpio.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/driver/include/driver/gpio.h b/components/driver/include/driver/gpio.h index 3f382d4931..333f75ddbc 100644 --- a/components/driver/include/driver/gpio.h +++ b/components/driver/include/driver/gpio.h @@ -95,9 +95,11 @@ esp_err_t gpio_set_intr_type(gpio_num_t gpio_num, gpio_int_type_t intr_type); /** * @brief Enable GPIO module interrupt signal * - * @note Please do not use the interrupt of GPIO36 and GPIO39 when using ADC. + * @note Please do not use the interrupt of GPIO36 and GPIO39 when using ADC or Wi-Fi with sleep mode enabled. * Please refer to the comments of `adc1_get_raw`. * Please refer to section 3.11 of 'ECO_and_Workarounds_for_Bugs_in_ESP32' for the description of this issue. + * As a workaround, call adc_power_acquire() in the app. This will result in higher power consumption (by ~1mA), + * but will remove the glitches on GPIO36 and GPIO39. * * @param gpio_num GPIO number. If you want to enable an interrupt on e.g. GPIO16, gpio_num should be GPIO_NUM_16 (16); * From d6f0b4af6f1ece5178eeffa45c2db74fb7d4abf2 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Thu, 3 Dec 2020 13:58:24 +0800 Subject: [PATCH 4/4] adc_i2s: solve the i2s_adc issue when using wifi --- components/driver/adc1_private.h | 9 -- components/driver/adc_common.c | 71 ++++++++--- components/driver/esp32/adc.c | 3 +- components/driver/i2s.c | 7 ++ components/driver/include/driver/adc_common.h | 29 ++++- components/driver/test/test_adc2_with_wifi.c | 114 ++++++++++++++++++ 6 files changed, 205 insertions(+), 28 deletions(-) diff --git a/components/driver/adc1_private.h b/components/driver/adc1_private.h index d2ded4afda..0b0781b67e 100644 --- a/components/driver/adc1_private.h +++ b/components/driver/adc1_private.h @@ -21,15 +21,6 @@ extern "C" { #include "esp_err.h" -/** - * @brief Force power on for SAR ADC. - * This function should be called for the scenario in which ADC are controlled by digital function like DMA. - * When the ADC power is always on, RTC FSM can still be functional. - * This is an internal API for I2S module to call to enable I2S-ADC function. - * Note that adc_power_off() can still power down ADC. - */ -void adc_power_always_on(void); - /** * @brief For I2S dma to claim the usage of ADC1. * diff --git a/components/driver/adc_common.c b/components/driver/adc_common.c index f87c5f6430..d3263a20b2 100644 --- a/components/driver/adc_common.c +++ b/components/driver/adc_common.c @@ -75,6 +75,14 @@ In ADC2, there're two locks used for different cases: adc2_spinlock should be acquired first, then adc2_wifi_lock or rtc_spinlock. */ +// This gets incremented when adc_power_acquire() is called, and decremented when +// adc_power_release() is called. ADC is powered down when the value reaches zero. +// Should be modified within critical section (ADC_ENTER/EXIT_CRITICAL). +static int s_adc_power_on_cnt; + +static void adc_power_on_internal(void); +static void adc_power_off_internal(void); + #ifdef CONFIG_IDF_TARGET_ESP32 //prevent ADC2 being used by wifi and other tasks at the same time. static _lock_t adc2_wifi_lock; @@ -113,36 +121,60 @@ static esp_pm_lock_handle_t s_adc2_arbiter_lock; ADC Common ---------------------------------------------------------------*/ -void adc_power_always_on(void) +void adc_power_acquire(void) { + bool powered_on = false; ADC_ENTER_CRITICAL(); - adc_hal_set_power_manage(ADC_POWER_SW_ON); + s_adc_power_on_cnt++; + if (s_adc_power_on_cnt == 1) { + adc_power_on_internal(); + powered_on = true; + } ADC_EXIT_CRITICAL(); + if (powered_on) { + ESP_LOGV(ADC_TAG, "%s: ADC powered on", __func__); + } } -void adc_power_on(void) +void adc_power_release(void) +{ + bool powered_off = false; + ADC_ENTER_CRITICAL(); + s_adc_power_on_cnt--; + /* Sanity check */ + if (s_adc_power_on_cnt < 0) { + ADC_EXIT_CRITICAL(); + ESP_LOGE(ADC_TAG, "%s called, but s_adc_power_on_cnt == 0", __func__); + abort(); + } else if (s_adc_power_on_cnt == 0) { + adc_power_off_internal(); + powered_off = true; + } + ADC_EXIT_CRITICAL(); + if (powered_off) { + ESP_LOGV(ADC_TAG, "%s: ADC powered off", __func__); + } +} + +static void adc_power_on_internal(void) { ADC_ENTER_CRITICAL(); - /* The power FSM controlled mode saves more power, while the ADC noise may get increased. */ -#ifndef CONFIG_ADC_FORCE_XPD_FSM /* Set the power always on to increase precision. */ adc_hal_set_power_manage(ADC_POWER_SW_ON); -#else - /* Use the FSM to turn off the power while not used to save power. */ - if (adc_hal_get_power_manage() != ADC_POWER_BY_FSM) { - adc_hal_set_power_manage(ADC_POWER_SW_ON); - } -#endif ADC_EXIT_CRITICAL(); } -void adc_power_off(void) +void adc_power_on(void) __attribute__((alias("adc_power_on_internal"))); + +static void adc_power_off_internal(void) { ADC_ENTER_CRITICAL(); adc_hal_set_power_manage(ADC_POWER_SW_OFF); ADC_EXIT_CRITICAL(); } +void adc_power_off(void) __attribute__((alias("adc_power_off_internal"))); + esp_err_t adc_set_clk_div(uint8_t clk_div) { ADC_ENTER_CRITICAL(); @@ -335,7 +367,7 @@ int adc1_get_raw(adc1_channel_t channel) int adc_value; ADC_CHANNEL_CHECK(ADC_NUM_1, channel); adc1_rtc_mode_acquire(); - adc_power_on(); + adc_power_acquire(); ADC_ENTER_CRITICAL(); #ifdef CONFIG_IDF_TARGET_ESP32 @@ -352,6 +384,7 @@ int adc1_get_raw(adc1_channel_t channel) adc_hal_rtc_reset(); //Reset FSM of rtc controller #endif + adc_power_release(); adc1_lock_release(); return adc_value; } @@ -364,7 +397,7 @@ int adc1_get_voltage(adc1_channel_t channel) //Deprecated. Use adc1_get_raw() #if SOC_ULP_SUPPORTED void adc1_ulp_enable(void) { - adc_power_on(); + adc_power_acquire(); ADC_ENTER_CRITICAL(); adc_hal_set_controller(ADC_NUM_1, ADC_CTRL_ULP); @@ -485,12 +518,13 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int * ADC_CHECK(width_bit == ADC_WIDTH_BIT_13, "WIDTH ERR: ESP32S2 support 13 bit width", ESP_ERR_INVALID_ARG); #endif - adc_power_on(); //in critical section with whole rtc module + adc_power_acquire(); //in critical section with whole rtc module ADC2_ENTER_CRITICAL(); //avoid collision with other tasks if ( ADC2_WIFI_LOCK_TRY_ACQUIRE() == -1 ) { //try the lock, return if failed (wifi using). ADC2_EXIT_CRITICAL(); + adc_power_release(); return ESP_ERR_TIMEOUT; } #ifdef CONFIG_ADC_DISABLE_DAC @@ -532,8 +566,12 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int * if (adc_value < 0) { ESP_LOGD( ADC_TAG, "ADC2 ARB: Return data is invalid." ); + adc_power_release(); return ESP_ERR_INVALID_STATE; } + + //in critical section with whole rtc module + adc_power_release(); *raw_out = adc_value; return ESP_OK; } @@ -546,7 +584,9 @@ esp_err_t adc2_vref_to_gpio(gpio_num_t gpio) esp_err_t adc_vref_to_gpio(adc_unit_t adc_unit, gpio_num_t gpio) { #ifdef CONFIG_IDF_TARGET_ESP32 + adc_power_acquire(); if (adc_unit & ADC_UNIT_1) { + adc_power_release(); return ESP_ERR_INVALID_ARG; } #endif @@ -559,6 +599,7 @@ esp_err_t adc_vref_to_gpio(adc_unit_t adc_unit, gpio_num_t gpio) } } if (ch == ADC2_CHANNEL_MAX) { + adc_power_release(); return ESP_ERR_INVALID_ARG; } diff --git a/components/driver/esp32/adc.c b/components/driver/esp32/adc.c index 351601968e..8451c05943 100644 --- a/components/driver/esp32/adc.c +++ b/components/driver/esp32/adc.c @@ -161,7 +161,7 @@ static int hall_sensor_get_value(void) //hall sensor without LNA { int hall_value; - adc_power_on(); + adc_power_acquire(); ADC_ENTER_CRITICAL(); /* disable other peripherals. */ @@ -173,6 +173,7 @@ static int hall_sensor_get_value(void) //hall sensor without LNA adc_hal_hall_disable(); ADC_EXIT_CRITICAL(); + adc_power_release(); return hall_value; } diff --git a/components/driver/i2s.c b/components/driver/i2s.c index cabfa2001c..d7e88e3f1c 100644 --- a/components/driver/i2s.c +++ b/components/driver/i2s.c @@ -867,6 +867,13 @@ static esp_err_t i2s_param_config(i2s_port_t i2s_num, const i2s_config_t *i2s_co periph_module_enable(i2s_periph_signal[i2s_num].module); #if SOC_I2S_SUPPORTS_ADC_DAC + if(i2s_config->mode & I2S_MODE_ADC_BUILT_IN) { + //in ADC built-in mode, we need to call i2s_set_adc_mode to + //initialize the specific ADC channel. + //in the current stage, we only support ADC1 and single channel mode. + //In default data mode, the ADC data is in 12-bit resolution mode. + adc_power_acquire(); + } #endif // configure I2S data port interface. i2s_hal_config_param(&(p_i2s_obj[i2s_num]->hal), i2s_config); diff --git a/components/driver/include/driver/adc_common.h b/components/driver/include/driver/adc_common.h index 28adc883b7..c00992edf0 100644 --- a/components/driver/include/driver/adc_common.h +++ b/components/driver/include/driver/adc_common.h @@ -90,14 +90,32 @@ typedef enum { /** * @brief Enable ADC power + * @deprecated Use adc_power_acquire and adc_power_release instead. */ -void adc_power_on(void); +void adc_power_on(void) __attribute__((deprecated)); /** * @brief Power off SAR ADC - * This function will force power down for ADC + * @deprecated Use adc_power_acquire and adc_power_release instead. + * This function will force power down for ADC. + * This function is deprecated because forcing power ADC power off may + * disrupt operation of other components which may be using the ADC. */ -void adc_power_off(void); +void adc_power_off(void) __attribute__((deprecated)); + +/** + * @brief Increment the usage counter for ADC module. + * ADC will stay powered on while the counter is greater than 0. + * Call adc_power_release when done using the ADC. + */ +void adc_power_acquire(void); + +/** + * @brief Decrement the usage counter for ADC module. + * ADC will stay powered on while the counter is greater than 0. + * Call this function when done using the ADC. + */ +void adc_power_release(void); /** * @brief Initialize ADC pad @@ -199,6 +217,8 @@ esp_err_t adc1_config_width(adc_bits_width_t width_bit); * the input of GPIO36 and GPIO39 will be pulled down for about 80ns. * When enabling power for any of these peripherals, ignore input from GPIO36 and GPIO39. * Please refer to section 3.11 of 'ECO_and_Workarounds_for_Bugs_in_ESP32' for the description of this issue. + * As a workaround, call adc_power_acquire() in the app. This will result in higher power consumption (by ~1mA), + * but will remove the glitches on GPIO36 and GPIO39. * * @note Call ``adc1_config_width()`` before the first time this * function is called. @@ -312,6 +332,9 @@ esp_err_t adc2_config_channel_atten(adc2_channel_t channel, adc_atten_t atten); * the input of GPIO36 and GPIO39 will be pulled down for about 80ns. * When enabling power for any of these peripherals, ignore input from GPIO36 and GPIO39. * Please refer to section 3.11 of 'ECO_and_Workarounds_for_Bugs_in_ESP32' for the description of this issue. + * As a workaround, call adc_power_acquire() in the app. This will result in higher power consumption (by ~1mA), + * but will remove the glitches on GPIO36 and GPIO39. + * * * @note ESP32: * For a given channel, ``adc2_config_channel_atten()`` diff --git a/components/driver/test/test_adc2_with_wifi.c b/components/driver/test/test_adc2_with_wifi.c index 1c5419c40c..0aae193754 100644 --- a/components/driver/test/test_adc2_with_wifi.c +++ b/components/driver/test/test_adc2_with_wifi.c @@ -10,6 +10,8 @@ #include "esp_log.h" #include "nvs_flash.h" #include "test_utils.h" +#include "driver/i2s.h" +#include "driver/gpio.h" #if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S3, ESP32C3) #include "driver/dac.h" @@ -23,6 +25,9 @@ static const char* TAG = "test_adc2"; #define ADC_TEST_CH1 ADC2_CHANNEL_8 #define ADC_TEST_CH2 ADC2_CHANNEL_9 #define ADC_TEST_ERROR (600) + #define ADC1_CHANNEL_4_IO (32) + #define SAMPLE_RATE (36000) + #define SAMPLE_BITS (16) #elif defined CONFIG_IDF_TARGET_ESP32S2 #define ADC_TEST_WIDTH ADC_WIDTH_BIT_13 //ESP32S2 only support 13 bit width #define ADC_TEST_RESOLUTION (8192) @@ -169,3 +174,112 @@ TEST_CASE("adc2 work with wifi","[adc]") } #endif + +#ifdef CONFIG_IDF_TARGET_ESP32 +static void i2s_adc_init(void) +{ + i2s_config_t i2s_config = { + .mode = I2S_MODE_MASTER | I2S_MODE_RX | I2S_MODE_ADC_BUILT_IN, + .sample_rate = SAMPLE_RATE, + .bits_per_sample = SAMPLE_BITS, + .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT, + .intr_alloc_flags = 0, + .dma_buf_count = 2, + .dma_buf_len = 1024, + .use_apll = 0, + }; + // install and start I2S driver + i2s_driver_install(I2S_NUM_0, &i2s_config, 0, NULL); + // init ADC pad + i2s_set_adc_mode(ADC_UNIT_1, ADC1_CHANNEL_4); + // enable adc sampling, ADC_WIDTH_BIT_12, ADC_ATTEN_DB_11 hard-coded in adc_i2s_mode_init + i2s_adc_enable(I2S_NUM_0); +} + +static void i2s_adc_test(void) +{ + uint16_t *i2sReadBuffer = (uint16_t *)calloc(1024, sizeof(uint16_t)); + size_t bytesRead; + for (int loop = 0; loop < 10; loop++) { + for (int level = 0; level <= 1; level++) { + if (level == 0) { + gpio_set_pull_mode(ADC1_CHANNEL_4_IO, GPIO_PULLDOWN_ONLY); + } else { + gpio_set_pull_mode(ADC1_CHANNEL_4_IO, GPIO_PULLUP_ONLY); + } + vTaskDelay(200 / portTICK_RATE_MS); + // read data from adc, will block until buffer is full + i2s_read(I2S_NUM_0, (void *)i2sReadBuffer, 1024 * sizeof(uint16_t), &bytesRead, portMAX_DELAY); + + // calc average + int64_t adcSumValue = 0; + for (size_t i = 0; i < 1024; i++) { + adcSumValue += i2sReadBuffer[i] & 0xfff; + } + int adcAvgValue = adcSumValue / 1024; + printf("adc average val: %d\n", adcAvgValue); + + if (level == 0) { + TEST_ASSERT_LESS_THAN(100, adcAvgValue); + } else { + TEST_ASSERT_GREATER_THAN(4000, adcAvgValue); + } + } + } + free(i2sReadBuffer); +} + +static void i2s_adc_release(void) +{ + i2s_adc_disable(I2S_NUM_0); + i2s_driver_uninstall(I2S_NUM_0); +} + +TEST_CASE("adc1 and i2s work with wifi","[adc][ignore]") +{ + + i2s_adc_init(); + //init wifi + printf("nvs init\n"); + esp_err_t r = nvs_flash_init(); + if (r == ESP_ERR_NVS_NO_FREE_PAGES || r == ESP_ERR_NVS_NEW_VERSION_FOUND) { + printf("no free pages or nvs version mismatch, erase..\n"); + TEST_ESP_OK(nvs_flash_erase()); + r = nvs_flash_init(); + } + TEST_ESP_OK(r); + esp_netif_init(); + event_init(); + esp_netif_create_default_wifi_sta(); + wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT(); + TEST_ESP_OK(esp_wifi_init(&cfg)); + wifi_config_t wifi_config = { + .sta = { + .ssid = DEFAULT_SSID, + .password = DEFAULT_PWD + }, + }; + TEST_ESP_OK(esp_wifi_set_mode(WIFI_MODE_STA)); + TEST_ESP_OK(esp_wifi_set_config(ESP_IF_WIFI_STA, &wifi_config)); + i2s_adc_test(); + //now start wifi + printf("wifi start...\n"); + TEST_ESP_OK(esp_wifi_start()); + //test reading during wifi on + i2s_adc_test(); + //wifi stop again + printf("wifi stop...\n"); + + TEST_ESP_OK( esp_wifi_stop() ); + + TEST_ESP_OK(esp_wifi_deinit()); + + event_deinit(); + + nvs_flash_deinit(); + i2s_adc_test(); + i2s_adc_release(); + printf("test passed...\n"); + TEST_IGNORE_MESSAGE("this test case is ignored due to the critical memory leak of esp_netif and event_loop."); +} +#endif