From 12a0f93f872280895b1fd15893c8b6704058c08c Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Wed, 9 Dec 2020 20:13:09 +0800 Subject: [PATCH] adc_i2s: solve the i2s_adc issue when using wifi --- components/driver/adc1_private.h | 10 -- components/driver/adc_common.c | 79 +++++++++--- components/driver/esp32/adc.c | 3 +- components/driver/i2s.c | 2 +- components/driver/include/driver/adc_common.h | 26 +++- components/driver/include/driver/gpio.h | 30 ++--- components/driver/test/test_adc2_with_wifi.c | 114 ++++++++++++++++++ components/esp_wifi/CMakeLists.txt | 2 +- components/esp_wifi/src/wifi_init.c | 19 +++ 9 files changed, 238 insertions(+), 47 deletions(-) diff --git a/components/driver/adc1_private.h b/components/driver/adc1_private.h index 40b5ee6852..210988c19d 100644 --- a/components/driver/adc1_private.h +++ b/components/driver/adc1_private.h @@ -20,16 +20,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 e11c5d5aff..0cf33ff7ef 100644 --- a/components/driver/adc_common.c +++ b/components/driver/adc_common.c @@ -73,6 +73,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; @@ -111,36 +119,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(); @@ -338,7 +370,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_ESP32S2 @@ -351,6 +383,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; } @@ -362,7 +395,7 @@ int adc1_get_voltage(adc1_channel_t channel) //Deprecated. Use adc1_get_raw() void adc1_ulp_enable(void) { - adc_power_on(); + adc_power_acquire(); ADC_ENTER_CRITICAL(); adc_hal_set_controller(ADC_NUM_1, ADC_CTRL_ULP); @@ -422,6 +455,7 @@ esp_err_t adc2_config_channel_atten(adc2_channel_t channel, adc_atten_t atten) 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; } adc_rtc_chan_init(ADC_UNIT_2); @@ -482,7 +516,7 @@ 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 @@ -529,8 +563,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; } @@ -542,8 +580,12 @@ 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) { + adc_power_acquire(); #ifdef CONFIG_IDF_TARGET_ESP32 - if (adc_unit & ADC_UNIT_1) return ESP_ERR_INVALID_ARG; + if (adc_unit & ADC_UNIT_1) { + adc_power_release(); + return ESP_ERR_INVALID_ARG; + } #endif adc2_channel_t ch = ADC2_CHANNEL_MAX; /* Check if the GPIO supported. */ @@ -553,7 +595,10 @@ esp_err_t adc_vref_to_gpio(adc_unit_t adc_unit, gpio_num_t gpio) break; } } - if (ch == ADC2_CHANNEL_MAX) return ESP_ERR_INVALID_ARG; + if (ch == ADC2_CHANNEL_MAX) { + adc_power_release(); + return ESP_ERR_INVALID_ARG; + } ADC_ENTER_CRITICAL(); adc_hal_set_power_manage(ADC_POWER_SW_ON); diff --git a/components/driver/esp32/adc.c b/components/driver/esp32/adc.c index f35d8e6ad3..1e00c28b99 100644 --- a/components/driver/esp32/adc.c +++ b/components/driver/esp32/adc.c @@ -163,7 +163,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. */ @@ -175,6 +175,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 264a9bcdd5..84d02ba0a8 100644 --- a/components/driver/i2s.c +++ b/components/driver/i2s.c @@ -862,7 +862,7 @@ static esp_err_t i2s_param_config(i2s_port_t i2s_num, const i2s_config_t *i2s_co //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(); + adc_power_acquire(); } #endif // configure I2S data port interface. diff --git a/components/driver/include/driver/adc_common.h b/components/driver/include/driver/adc_common.h index 719f60a4d2..38e4d9dd8a 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 @@ -312,6 +330,8 @@ 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/include/driver/gpio.h b/components/driver/include/driver/gpio.h index 270d738ada..8e9fdfca56 100644 --- a/components/driver/include/driver/gpio.h +++ b/components/driver/include/driver/gpio.h @@ -78,20 +78,22 @@ esp_err_t gpio_reset_pin(gpio_num_t gpio_num); */ 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. - * 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. - * - * @param gpio_num GPIO number. If you want to enable an interrupt on e.g. GPIO16, gpio_num should be GPIO_NUM_16 (16); - * - * @return - * - ESP_OK Success - * - ESP_ERR_INVALID_ARG Parameter error - * - */ + /** + * @brief Enable GPIO module interrupt signal + * + * @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); + * + * @return + * - ESP_OK Success + * - ESP_ERR_INVALID_ARG Parameter error + * + */ esp_err_t gpio_intr_enable(gpio_num_t gpio_num); /** diff --git a/components/driver/test/test_adc2_with_wifi.c b/components/driver/test/test_adc2_with_wifi.c index 080d05eef4..478ba9e897 100644 --- a/components/driver/test/test_adc2_with_wifi.c +++ b/components/driver/test/test_adc2_with_wifi.c @@ -11,6 +11,8 @@ #include "esp_log.h" #include "nvs_flash.h" #include "test_utils.h" +#include "driver/i2s.h" +#include "driver/gpio.h" static const char* TAG = "test_adc2"; @@ -21,6 +23,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) @@ -165,3 +170,112 @@ TEST_CASE("adc2 work with wifi","[adc]") TEST_IGNORE_MESSAGE("this test case is ignored due to the critical memory leak of esp_netif and event_loop."); } + +#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 diff --git a/components/esp_wifi/CMakeLists.txt b/components/esp_wifi/CMakeLists.txt index f2416a9c51..b7ccf99bfb 100644 --- a/components/esp_wifi/CMakeLists.txt +++ b/components/esp_wifi/CMakeLists.txt @@ -26,8 +26,8 @@ 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 wpa_supplicant nvs_flash esp_netif ${extra_priv_requires} LDFRAGMENTS "${ldfragments}") idf_build_get_property(build_dir BUILD_DIR) diff --git a/components/esp_wifi/src/wifi_init.c b/components/esp_wifi/src/wifi_init.c index 0159e48fa4..5c298e40d4 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" #if (CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_DYNAMIC_RX_BUFFER_NUM) @@ -55,6 +56,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) @@ -241,3 +244,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(); + } +}