Merge branch 'bugfix/phy_regi2c_critical_section_v4.1' into 'release/v4.1'

esp_phy: use spinlock to avoid regi2c access conflicts (v4.1)

See merge request espressif/esp-idf!17694

components/esp_wifi/CMakeLists.txt

@@ -16,23 +16,27 @@ else()
 endif()
 
 idf_component_register(SRCS "src/coexist.c"
-                            "src/lib_printf.c"
+                        "src/lib_printf.c"
-                            "src/mesh_event.c"
+                        "src/mesh_event.c"
-                            "src/phy_init.c"
+                        "src/phy_init.c"
-                            "src/smartconfig.c"
+                        "src/smartconfig.c"
-                            "src/smartconfig_ack.c"
+                        "src/smartconfig_ack.c"
-                            "src/wifi_init.c"
+                        "src/wifi_init.c"
-                            "src/wifi_default.c"
+                        "src/wifi_default.c"
-                            "src/wifi_netif.c"
+                        "src/wifi_netif.c"
-                            "${idf_target}/esp_adapter.c"
+                        "${idf_target}/esp_adapter.c"
-                    INCLUDE_DIRS "include" "${idf_target}/include"
+                        "src/phy_override.c"
-                    PRIV_REQUIRES wpa_supplicant nvs_flash esp_netif driver ${extra_priv_requires}
+            INCLUDE_DIRS "include" "${idf_target}/include"
-                    REQUIRES esp_event
+            PRIV_REQUIRES wpa_supplicant nvs_flash esp_netif driver ${extra_priv_requires}
-                    LDFRAGMENTS "${ldfragments}")
+            REQUIRES esp_event
+            LDFRAGMENTS "${ldfragments}")
 
 idf_build_get_property(build_dir BUILD_DIR)
 target_link_libraries(${COMPONENT_LIB} PUBLIC "-L ${CMAKE_CURRENT_SOURCE_DIR}/lib/${idf_target}")
 
+# Override functions in PHY lib with the functions in 'phy_override.c'
+target_link_libraries(${COMPONENT_LIB} INTERFACE "-u include_esp_phy_override")
+
 if(link_binary_libs)
     if(CONFIG_IDF_TARGET_ESP32)
         set(phy phy)

components/esp_wifi/src/phy_override.c

@@ -0,0 +1,53 @@
+/*
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdbool.h>
+#include "esp_attr.h"
+#include "driver/adc.h"
+
+/*
+ * This file is used to override the hooks provided by the PHY lib for some system features.
+ * Call phy_override() so that this file will be linked.
+ */
+
+static bool s_wifi_adc_xpd_flag;
+
+void include_esp_phy_override(void)
+{
+    /* When this empty function is called, all functions below will be linked. */
+}
+
+/* Coordinate ADC power with other modules. */
+// It seems that it is only required on ESP32, but we still compile it for all chips, in case it is
+// called by PHY unexpectedly.
+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();
+    }
+}
+
+//add spinlock protection
+extern void regi2c_enter_critical(void);
+extern void regi2c_exit_critical(void);
+
+IRAM_ATTR void phy_i2c_enter_critical(void)
+{
+    regi2c_enter_critical();
+}
+
+IRAM_ATTR void phy_i2c_exit_critical(void)
+{
+    regi2c_exit_critical();
+}

components/esp_wifi/src/wifi_init.c

@@ -21,7 +21,6 @@
 #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!"
@@ -52,8 +51,6 @@ 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,
@@ -221,18 +218,3 @@ void wifi_apb80m_release(void)
 }
 #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/i2c_rtc_clk.h

@@ -56,6 +56,10 @@ uint8_t regi2c_ctrl_read_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_ad
 void regi2c_ctrl_write_reg(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t data);
 void regi2c_ctrl_write_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t msb, uint8_t lsb, uint8_t data);
 
+/* enter the critical section that protects internal registers. Don't use it in SDK. Use the functions above. */
+void regi2c_enter_critical(void);
+void regi2c_exit_critical(void);
+
 #endif // BOOTLOADER_BUILD
 
 /* Convenience macros for the above functions, these use register definitions

components/soc/esp32s2beta/i2c_rtc_clk.h

@@ -56,6 +56,10 @@ uint8_t regi2c_ctrl_read_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_ad
 void regi2c_ctrl_write_reg(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t data);
 void regi2c_ctrl_write_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t msb, uint8_t lsb, uint8_t data);
 
+/* enter the critical section that protects internal registers. Don't use it in SDK. Use the functions above. */
+void regi2c_enter_critical(void);
+void regi2c_exit_critical(void);
+
 #endif // BOOTLOADER_BUILD
 
 /* Convenience macros for the above functions, these use register definitions

components/soc/src/regi2c_ctrl.c

@@ -20,30 +20,40 @@ static portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED;
 
 uint8_t IRAM_ATTR regi2c_ctrl_read_reg(uint8_t block, uint8_t host_id, uint8_t reg_add)
 {
-    portENTER_CRITICAL_ISR(&mux);
+    portENTER_CRITICAL_SAFE(&mux);
     uint8_t value = i2c_read_reg_raw(block, host_id, reg_add);
-    portEXIT_CRITICAL_ISR(&mux);
+    portEXIT_CRITICAL_SAFE(&mux);
     return value;
 }
 
 uint8_t IRAM_ATTR regi2c_ctrl_read_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t msb, uint8_t lsb)
 {
-    portENTER_CRITICAL_ISR(&mux);
+    portENTER_CRITICAL_SAFE(&mux);
     uint8_t value = i2c_read_reg_mask_raw(block, host_id, reg_add, msb, lsb);
-    portEXIT_CRITICAL_ISR(&mux);
+    portEXIT_CRITICAL_SAFE(&mux);
     return value;
 }
 
 void IRAM_ATTR regi2c_ctrl_write_reg(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t data)
 {
-    portENTER_CRITICAL_ISR(&mux);
+    portENTER_CRITICAL_SAFE(&mux);
     i2c_write_reg_raw(block, host_id, reg_add, data);
-    portEXIT_CRITICAL_ISR(&mux);
+    portEXIT_CRITICAL_SAFE(&mux);
 }
 
 void IRAM_ATTR regi2c_ctrl_write_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t msb, uint8_t lsb, uint8_t data)
 {
-    portENTER_CRITICAL_ISR(&mux);
+    portENTER_CRITICAL_SAFE(&mux);
     i2c_write_reg_mask_raw(block, host_id, reg_add, msb, lsb, data);
-    portEXIT_CRITICAL_ISR(&mux);
+    portEXIT_CRITICAL_SAFE(&mux);
+}
+
+void IRAM_ATTR regi2c_enter_critical(void)
+{
+    portENTER_CRITICAL_SAFE(&mux);
+}
+
+void IRAM_ATTR regi2c_exit_critical(void)
+{
+    portEXIT_CRITICAL_SAFE(&mux);
 }