Merge branch 'bugfix/phy_set_xpd_sar' into 'master'

wifi: add set_xpd_sar override

Closes IDFGH-235, IDFGH-1442, IDFGH-1917, IDFGH-2473, and IDFGH-2168

See merge request espressif/esp-idf!7185

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

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;
     }
 

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;
 }
 

components/driver/i2s.c

@@ -872,7 +872,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.

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()``

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);
  *

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

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}")

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();
+    }
+}