From 5490dbcaf7daba44aeb053fab1e7d76d2cbd985b Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Thu, 3 Dec 2020 13:58:24 +0800 Subject: [PATCH] adc_i2s: solve the i2s_adc issue when using wifi --- components/driver/adc1_i2s_private.h | 10 --- components/driver/i2s.c | 2 +- components/driver/include/driver/adc.h | 24 +++++- components/driver/include/driver/gpio.h | 4 +- components/driver/rtc_module.c | 72 +++++++++++----- components/driver/test/test_adc2.c | 108 ++++++++++++++++++++++++ components/esp32/wifi_init.c | 19 +++++ 7 files changed, 203 insertions(+), 36 deletions(-) diff --git a/components/driver/adc1_i2s_private.h b/components/driver/adc1_i2s_private.h index cc592ffe1d0..b86d91c3918 100644 --- a/components/driver/adc1_i2s_private.h +++ b/components/driver/adc1_i2s_private.h @@ -21,16 +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(); - /** * @brief For I2S dma to claim the usage of ADC1. * diff --git a/components/driver/i2s.c b/components/driver/i2s.c index 58deb4d2fe6..e64358482cc 100644 --- a/components/driver/i2s.c +++ b/components/driver/i2s.c @@ -903,7 +903,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(); } // configure I2S data port interface. //reset i2s diff --git a/components/driver/include/driver/adc.h b/components/driver/include/driver/adc.h index b5a9a9848e3..023dfbd5014 100644 --- a/components/driver/include/driver/adc.h +++ b/components/driver/include/driver/adc.h @@ -228,14 +228,32 @@ int adc1_get_voltage(adc1_channel_t channel) __attribute__((deprecated)); /** * @brief Enable ADC power + * @deprecated Use adc_power_acquire and adc_power_release instead. */ -void adc_power_on(); +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 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 diff --git a/components/driver/include/driver/gpio.h b/components/driver/include/driver/gpio.h index c2231a625b1..4539a54f81f 100644 --- a/components/driver/include/driver/gpio.h +++ b/components/driver/include/driver/gpio.h @@ -276,9 +276,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); * diff --git a/components/driver/rtc_module.c b/components/driver/rtc_module.c index c5b515fc52b..9eaa258edc5 100644 --- a/components/driver/rtc_module.c +++ b/components/driver/rtc_module.c @@ -100,6 +100,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); + //prevent ADC2 being used by wifi and other tasks at the same time. static _lock_t adc2_wifi_lock; //prevent ADC2 being used by tasks (regardless of WIFI) @@ -1143,32 +1151,49 @@ static esp_err_t adc_set_atten(adc_unit_t adc_unit, adc_channel_t channel, adc_a return ESP_OK; } -void adc_power_always_on() +void adc_power_acquire() { + bool powered_on = false; portENTER_CRITICAL(&rtc_spinlock); - SENS.sar_meas_wait2.force_xpd_sar = SENS_FORCE_XPD_SAR_PU; + s_adc_power_on_cnt++; + if (s_adc_power_on_cnt == 1) { + adc_power_on_internal(); + powered_on = true; + } + portEXIT_CRITICAL(&rtc_spinlock); + if (powered_on) { + ESP_LOGV(TAG, "%s: ADC powered on", __func__); + } +} + +void adc_power_release(void) +{ + bool powered_off = false; + portENTER_CRITICAL(&rtc_spinlock); + s_adc_power_on_cnt--; + if (s_adc_power_on_cnt < 0) { + portEXIT_CRITICAL(&rtc_spinlock); + } else if (s_adc_power_on_cnt == 0) { + adc_power_off_internal(); + powered_off = true; + } portEXIT_CRITICAL(&rtc_spinlock); + if (powered_off) { + ESP_LOGV(TAG, "%s: ADC powered off", __func__); + } } -void adc_power_on() +static void adc_power_on_internal(void) { portENTER_CRITICAL(&rtc_spinlock); - //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. SENS.sar_meas_wait2.force_xpd_sar = SENS_FORCE_XPD_SAR_PU; -#else - //Use the FSM to turn off the power while not used to save power. - if (SENS.sar_meas_wait2.force_xpd_sar & SENS_FORCE_XPD_SAR_SW_M) { - SENS.sar_meas_wait2.force_xpd_sar = SENS_FORCE_XPD_SAR_PU; - } else { - SENS.sar_meas_wait2.force_xpd_sar = SENS_FORCE_XPD_SAR_FSM; - } -#endif portEXIT_CRITICAL(&rtc_spinlock); } -void adc_power_off() +void adc_power_on(void) __attribute__((alias("adc_power_on_internal"))); + +static void adc_power_off_internal(void) { portENTER_CRITICAL(&rtc_spinlock); //Bit1 0:Fsm 1: SW mode @@ -1177,6 +1202,8 @@ void adc_power_off() portEXIT_CRITICAL(&rtc_spinlock); } +void adc_power_off(void) __attribute__((alias("adc_power_off_internal"))); + esp_err_t adc_set_clk_div(uint8_t clk_div) { portENTER_CRITICAL(&rtc_spinlock); @@ -1395,7 +1422,7 @@ esp_err_t adc_i2s_mode_init(adc_unit_t adc_unit, adc_channel_t channel) uint8_t table_len = 1; //POWER ON SAR - adc_power_always_on(); + adc_power_acquire(); adc_gpio_init(adc_unit, channel); adc_set_i2s_data_len(adc_unit, table_len); adc_set_i2s_data_pattern(adc_unit, 0, channel, ADC_WIDTH_BIT_12, ADC_ATTEN_DB_11); @@ -1540,7 +1567,7 @@ int adc1_get_raw(adc1_channel_t channel) uint16_t adc_value; RTC_MODULE_CHECK(channel < ADC1_CHANNEL_MAX, "ADC Channel Err", ESP_ERR_INVALID_ARG); adc1_adc_mode_acquire(); - adc_power_on(); + adc_power_acquire(); portENTER_CRITICAL(&rtc_spinlock); //disable other peripherals @@ -1551,6 +1578,7 @@ int adc1_get_raw(adc1_channel_t channel) //start conversion adc_value = adc_convert( ADC_UNIT_1, channel ); portEXIT_CRITICAL(&rtc_spinlock); + adc_power_release(); adc1_lock_release(); return adc_value; } @@ -1562,7 +1590,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(); portENTER_CRITICAL(&rtc_spinlock); adc_set_controller( ADC_UNIT_1, ADC_CTRL_ULP ); @@ -1701,7 +1729,7 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int* RTC_MODULE_CHECK(channel < ADC2_CHANNEL_MAX, "ADC Channel Err", ESP_ERR_INVALID_ARG); //in critical section with whole rtc module - adc_power_on(); + adc_power_acquire(); //avoid collision with other tasks portENTER_CRITICAL(&adc2_spinlock); @@ -1709,6 +1737,7 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int* //try the lock, return if failed (wifi using). if ( _lock_try_acquire( &adc2_wifi_lock ) == -1 ) { portEXIT_CRITICAL( &adc2_spinlock ); + adc_power_release(); return ESP_ERR_TIMEOUT; } @@ -1725,7 +1754,7 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int* adc_value = adc_convert( ADC_UNIT_2, channel ); _lock_release( &adc2_wifi_lock ); portEXIT_CRITICAL(&adc2_spinlock); - + adc_power_release(); *raw_out = (int)adc_value; return ESP_OK; } @@ -1750,7 +1779,7 @@ esp_err_t adc2_vref_to_gpio(gpio_num_t gpio) rtc_gpio_pullup_dis(gpio); rtc_gpio_pulldown_dis(gpio); //force fsm - adc_power_always_on(); //Select power source of ADC + adc_power_acquire(); //Select power source of ADC RTCCNTL.bias_conf.dbg_atten = 0; //Check DBG effect outside sleep mode //set dtest (MUX_SEL : 0 -> RTC; 1-> vdd_sar2) @@ -1931,7 +1960,7 @@ static int hall_sensor_get_value() //hall sensor without LNA int Sens_Vn1; int hall_value; - adc_power_on(); + adc_power_acquire(); portENTER_CRITICAL(&rtc_spinlock); //disable other peripherals @@ -1948,6 +1977,7 @@ static int hall_sensor_get_value() //hall sensor without LNA Sens_Vn1 = adc_convert( ADC_UNIT_1, ADC1_CHANNEL_3 ); portEXIT_CRITICAL(&rtc_spinlock); hall_value = (Sens_Vp1 - Sens_Vp0) - (Sens_Vn1 - Sens_Vn0); + adc_power_release(); return hall_value; } diff --git a/components/driver/test/test_adc2.c b/components/driver/test/test_adc2.c index 40549428efc..773c040bb33 100644 --- a/components/driver/test/test_adc2.c +++ b/components/driver/test/test_adc2.c @@ -11,11 +11,16 @@ #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"; #define DEFAULT_SSID "TEST_SSID" #define DEFAULT_PWD "TEST_PASS" +#define ADC1_CHANNEL_4_IO (32) +#define SAMPLE_RATE (36000) +#define SAMPLE_BITS (16) static esp_err_t event_handler(void *ctx, system_event_t *event) { @@ -116,3 +121,106 @@ TEST_CASE("adc2 work with wifi","[adc]") TEST_IGNORE_MESSAGE("this test case is ignored due to the critical memory leak of tcpip_adapter and event_loop."); } + +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]") +{ + + 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); + tcpip_adapter_init(); + 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()); + + 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."); +} diff --git a/components/esp32/wifi_init.c b/components/esp32/wifi_init.c index 7b700dd4ec6..be9b4241d5a 100644 --- a/components/esp32/wifi_init.c +++ b/components/esp32/wifi_init.c @@ -19,6 +19,7 @@ #include "esp_pm.h" #include "soc/rtc.h" #include "esp_mesh.h" +#include "driver/adc.h" #if (CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_DYNAMIC_RX_BUFFER_NUM) #error "WiFi configuration check: WARNING, WIFI_RX_BA_WIN should not be larger than WIFI_DYNAMIC_RX_BUFFER_NUM!" @@ -38,6 +39,8 @@ static esp_pm_lock_handle_t s_wifi_modem_sleep_lock; /* Callback function to update WiFi MAC time */ wifi_mac_time_update_cb_t s_wifi_mac_time_update_cb = NULL; +static bool s_wifi_adc_xpd_flag; + static void __attribute__((constructor)) s_set_default_wifi_log_level() { /* WiFi libraries aren't compiled to know CONFIG_LOG_DEFAULT_LEVEL, @@ -132,3 +135,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(); + } +}