adc_i2s: solve the i2s_adc issue when using wifi

components/driver/adc.c

@@ -78,6 +78,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)
@@ -89,36 +97,60 @@ static _lock_t adc1_i2s_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();
@@ -283,6 +315,7 @@ esp_err_t adc1_i2s_mode_acquire(void)
     _lock_acquire( &adc1_i2s_lock );
     ESP_LOGD( ADC_TAG, "i2s mode takes adc1 lock." );
     ADC_ENTER_CRITICAL();
+    adc_power_acquire();
     adc_hal_set_power_manage(ADC_POWER_SW_ON);
     /* switch SARADC into DIG channel */
     adc_hal_set_controller(ADC_NUM_1, ADC_CTRL_DIG);
@@ -296,6 +329,7 @@ esp_err_t adc1_adc_mode_acquire(void)
        for adc1, block until acquire the lock. */
     _lock_acquire( &adc1_i2s_lock );
     ADC_ENTER_CRITICAL();
+    adc_power_acquire();
     /* switch SARADC into RTC channel. */
     adc_hal_set_controller(ADC_NUM_1, ADC_CTRL_RTC);
     ADC_EXIT_CRITICAL();
@@ -317,7 +351,7 @@ int adc1_get_raw(adc1_channel_t channel)
     ADC_CHANNEL_CHECK(ADC_NUM_1, channel);
     adc1_adc_mode_acquire();
 
-    adc_power_on();
+    adc_power_acquire();
     ADC_ENTER_CRITICAL();
     /* disable other peripherals. */
     adc_hal_hall_disable();
@@ -329,6 +363,7 @@ int adc1_get_raw(adc1_channel_t channel)
     adc_value = adc_convert(ADC_NUM_1, channel);
     ADC_EXIT_CRITICAL();
 
+    adc_power_release();
     adc1_lock_release();
     return adc_value;
 }
@@ -340,7 +375,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);
@@ -372,6 +407,7 @@ esp_err_t adc2_wifi_acquire(void)
 {
     /* Wi-Fi module will use adc2. Use locks to avoid conflicts. */
     _lock_acquire( &adc2_wifi_lock );
+    adc_power_acquire();
     ESP_LOGD( ADC_TAG, "Wi-Fi takes adc2 lock." );
     return ESP_OK;
 }
@@ -444,7 +480,7 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int *
     ADC_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);
@@ -452,6 +488,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;
     }
 
@@ -468,15 +505,16 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int *
     adc_value = adc_convert(ADC_NUM_2, channel);
     _lock_release( &adc2_wifi_lock );
     portEXIT_CRITICAL(&adc2_spinlock);
-
+    adc_power_release();
     *raw_out = (int)adc_value;
     return ESP_OK;
 }
 
 esp_err_t adc2_vref_to_gpio(gpio_num_t gpio)
 {
-    adc_power_always_on();               //Select power source of ADC
+    adc_power_acquire();               //Select power source of ADC
     if (adc_hal_vref_output(gpio) != true) {
+        adc_power_release();
         return ESP_ERR_INVALID_ARG;
     } else {
         //Configure RTC gpio
@@ -496,7 +534,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. */

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(void);
-
 /**
  * @brief For I2S dma to claim the usage of ADC1.
  *

components/driver/i2s.c

@@ -828,7 +828,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.
     i2s_hal_config_param(&(p_i2s_obj[i2s_num]->hal), i2s_config);
@@ -1041,6 +1041,8 @@ esp_err_t i2s_adc_enable(i2s_port_t i2s_num)
 
     adc1_i2s_mode_acquire();
     _i2s_adc_mode_recover();
+    i2s_hal_start_rx(&(p_i2s_obj[i2s_num]->hal));
+    i2s_hal_reset(&(p_i2s_obj[i2s_num]->hal));
     return i2s_set_clk(i2s_num, p_i2s_obj[i2s_num]->sample_rate, p_i2s_obj[i2s_num]->bits_per_sample, p_i2s_obj[i2s_num]->channel_num);
 }
 

components/driver/include/driver/adc.h

@@ -187,14 +187,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);
+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

components/driver/include/driver/gpio.h

@@ -81,9 +81,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/test/test_adc2.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"
 
 #if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2BETA)
 
@@ -18,6 +20,9 @@ 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 void wifi_event_handler(void* arg, esp_event_base_t event_base,
                                 int32_t event_id, void* event_data)
@@ -151,4 +156,110 @@ 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.");
 }
 
+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();
+    i2s_adc_test();
+    //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());
+
+    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

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)

components/esp_wifi/src/wifi_init.c

@@ -21,6 +21,7 @@
 #include "esp_wpa.h"
 #include "esp_netif.h"
 #include "tcpip_adapter_compatible/tcpip_adapter_compat.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!"
@@ -54,6 +55,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(void)
 {
     /* WiFi libraries aren't compiled to know CONFIG_LOG_DEFAULT_LEVEL,
@@ -220,3 +223,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();
+    }
+}

components/soc/esp32/include/hal/adc_ll.h

@@ -447,6 +447,20 @@ static inline void adc_ll_output_invert(adc_ll_num_t adc_n, bool inv_en)
     }
 }
 
+/**
+ * ADC module Digital output data invert or not.
+ *
+ * @prarm adc_n ADC unit.
+ */
+static inline void adc_ll_dig_output_invert(adc_ll_num_t adc_n, bool inv_en)
+{
+    if (adc_n == ADC_NUM_1) {
+        SYSCON.saradc_ctrl2.sar1_inv = inv_en;   // Enable / Disable ADC data invert
+    } else { // adc_n == ADC_NUM_2
+        SYSCON.saradc_ctrl2.sar2_inv = inv_en;  // Enable / Disable ADC data invert
+    }
+}
+
 /**
  * Set ADC module controller.
  * There are five SAR ADC controllers:

components/soc/esp32s2beta/include/hal/adc_ll.h

@@ -2,6 +2,7 @@
 
 #include "soc/adc_periph.h"
 #include "hal/adc_types.h"
+#include "soc/apb_ctrl_struct.h"
 #include <stdbool.h>
 
 typedef enum {
@@ -444,6 +445,20 @@ static inline void adc_ll_output_invert(adc_ll_num_t adc_n, bool inv_en)
     }
 }
 
+/**
+ * ADC module Digital output data invert or not.
+ *
+ * @prarm adc_n ADC unit.
+ */
+static inline void adc_ll_dig_output_invert(adc_ll_num_t adc_n, bool inv_en)
+{
+    if (adc_n == ADC_NUM_1) {
+        APB_CTRL.saradc_ctrl2.sar1_inv = inv_en;   // Enable / Disable ADC data invert
+    } else { // adc_n == ADC_NUM_2
+        APB_CTRL.saradc_ctrl2.sar2_inv = inv_en;  // Enable / Disable ADC data invert
+    }
+}
+
 /**
  * Set ADC module controller.
  * There are five SAR ADC controllers:

components/soc/src/hal/adc_hal.c

@@ -16,13 +16,14 @@
 
 void adc_hal_init(void)
 {
-    adc_ll_set_power_manage(ADC_POWER_BY_FSM);
     // Set internal FSM wait time, fixed value.
     adc_ll_dig_set_fsm_time(SOC_ADC_FSM_RSTB_WAIT_DEFAULT, SOC_ADC_FSM_START_WAIT_DEFAULT,
                             SOC_ADC_FSM_STANDBY_WAIT_DEFAULT);
     adc_ll_dig_set_sample_cycle(ADC_FSM_SAMPLE_CYCLE_DEFAULT);
     adc_ll_output_invert(ADC_NUM_1, SOC_ADC1_DATA_INVERT_DEFAULT);
+    adc_ll_dig_output_invert(ADC_NUM_1, SOC_ADC1_DATA_INVERT_DEFAULT);
     adc_ll_output_invert(ADC_NUM_2, SOC_ADC2_DATA_INVERT_DEFAULT);
+    adc_ll_dig_output_invert(ADC_NUM_2, SOC_ADC2_DATA_INVERT_DEFAULT);
 }
 
 void adc_hal_dig_controller_config(const adc_hal_dig_config_t *cfg)