adc_i2s: solve the i2s_adc issue when using wifi

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.
  *

components/driver/i2s.c

@@ -901,7 +901,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

components/driver/include/driver/adc.h

@@ -214,14 +214,32 @@ int adc1_get_raw(adc1_channel_t channel);
 
 /**
  * @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

components/driver/include/driver/gpio.h

@@ -279,9 +279,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);
  *

components/driver/rtc_module.c

@@ -95,6 +95,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)
@@ -1137,32 +1145,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_on()
+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__);
+    }
+}
+
+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
@@ -1171,6 +1196,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);
@@ -1389,7 +1416,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);
@@ -1532,7 +1559,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
@@ -1543,13 +1570,14 @@ 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;
 }
 
 void adc1_ulp_enable(void)
 {
-    adc_power_on();
+    adc_power_acquire();
 
     portENTER_CRITICAL(&rtc_spinlock);
     adc_set_controller( ADC_UNIT_1, ADC_CTRL_ULP );
@@ -1688,7 +1716,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); 
@@ -1696,6 +1724,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;
     }
 
@@ -1712,6 +1741,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;
@@ -1737,7 +1767,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)
@@ -1889,7 +1919,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
@@ -1906,6 +1936,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;
 }

components/driver/test/test_adc2.c

@@ -11,11 +11,17 @@
 #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 +122,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.");
+}

components/esp_wifi/src/wifi_init.c

@@ -19,6 +19,7 @@
 #include "esp_pm.h"
 #include "soc/rtc.h"
 #include "esp_wpa.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!"
@@ -50,6 +51,8 @@ uint64_t g_wifi_feature_caps =
 
 static const char* TAG = "wifi_init";
 
+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,
@@ -210,3 +213,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();
+    }
+}