From cb0b143cb37877f06cf22f595884c3144dfa35ac Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Tue, 1 Mar 2022 16:12:45 +0800 Subject: [PATCH] esp_phy: use spinlock to avoid regi2c access conflicts --- components/esp_wifi/CMakeLists.txt | 32 ++++++++------- components/esp_wifi/src/phy_override.c | 52 ++++++++++++++++++++++++ components/esp_wifi/src/wifi_init.c | 17 -------- components/soc/src/esp32/regi2c_ctrl.h | 4 ++ components/soc/src/esp32s2/regi2c_ctrl.h | 4 ++ components/soc/src/regi2c_ctrl.c | 10 +++++ 6 files changed, 88 insertions(+), 31 deletions(-) create mode 100644 components/esp_wifi/src/phy_override.c diff --git a/components/esp_wifi/CMakeLists.txt b/components/esp_wifi/CMakeLists.txt index b7ccf99bfb..a5b073cb1a 100644 --- a/components/esp_wifi/CMakeLists.txt +++ b/components/esp_wifi/CMakeLists.txt @@ -16,19 +16,20 @@ else() endif() idf_component_register(SRCS "src/coexist.c" - "src/lib_printf.c" - "src/mesh_event.c" - "src/phy_init.c" - "src/smartconfig.c" - "src/smartconfig_ack.c" - "src/wifi_init.c" - "src/wifi_default.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 - LDFRAGMENTS "${ldfragments}") + "src/lib_printf.c" + "src/mesh_event.c" + "src/phy_init.c" + "src/smartconfig.c" + "src/smartconfig_ack.c" + "src/wifi_init.c" + "src/wifi_default.c" + "src/wifi_netif.c" + "${idf_target}/esp_adapter.c" + "src/phy_override.c" + INCLUDE_DIRS "include" "${idf_target}/include" + PRIV_REQUIRES wpa_supplicant nvs_flash esp_netif driver ${extra_priv_requires} + REQUIRES esp_event + LDFRAGMENTS "${ldfragments}") idf_build_get_property(build_dir BUILD_DIR) @@ -36,6 +37,9 @@ idf_build_get_property(build_dir BUILD_DIR) set(target_name "${idf_target}") target_link_libraries(${COMPONENT_LIB} PUBLIC "-L ${CMAKE_CURRENT_SOURCE_DIR}/lib/${target_name}") +# 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) set(phy phy) set(blobs coexist core espnow mesh net80211 pp rtc smartconfig ${phy}) @@ -58,7 +62,7 @@ endif() if(CONFIG_ESP32_PHY_INIT_DATA_IN_PARTITION) idf_component_get_property(esp_common_dir esp_common COMPONENT_DIR) partition_table_get_partition_info(phy_partition_offset "--partition-type data --partition-subtype phy" "offset") - + if(CONFIG_ESP32_SUPPORT_MULTIPLE_PHY_INIT_DATA_BIN) set(phy_init_data_bin "${CMAKE_CURRENT_SOURCE_DIR}/phy_multiple_init_data.bin") else() diff --git a/components/esp_wifi/src/phy_override.c b/components/esp_wifi/src/phy_override.c new file mode 100644 index 0000000000..9a2692737f --- /dev/null +++ b/components/esp_wifi/src/phy_override.c @@ -0,0 +1,52 @@ +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#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); + +void phy_i2c_enter_critical(void) +{ + regi2c_enter_critical(); +} + +void phy_i2c_exit_critical(void) +{ + regi2c_exit_critical(); +} diff --git a/components/esp_wifi/src/wifi_init.c b/components/esp_wifi/src/wifi_init.c index 788359403d..bd446b9624 100644 --- a/components/esp_wifi/src/wifi_init.c +++ b/components/esp_wifi/src/wifi_init.c @@ -23,7 +23,6 @@ #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" #if (CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_DYNAMIC_RX_BUFFER_NUM) @@ -56,7 +55,6 @@ uint64_t g_wifi_feature_caps = #endif 0; -static bool s_wifi_adc_xpd_flag; static const char* TAG = "wifi_init"; @@ -245,18 +243,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(); - } -} diff --git a/components/soc/src/esp32/regi2c_ctrl.h b/components/soc/src/esp32/regi2c_ctrl.h index a32d6d744c..4c5726d49d 100644 --- a/components/soc/src/esp32/regi2c_ctrl.h +++ b/components/soc/src/esp32/regi2c_ctrl.h @@ -60,6 +60,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 diff --git a/components/soc/src/esp32s2/regi2c_ctrl.h b/components/soc/src/esp32s2/regi2c_ctrl.h index 51a0a22dcb..34c26c6949 100644 --- a/components/soc/src/esp32s2/regi2c_ctrl.h +++ b/components/soc/src/esp32s2/regi2c_ctrl.h @@ -68,6 +68,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 diff --git a/components/soc/src/regi2c_ctrl.c b/components/soc/src/regi2c_ctrl.c index 842bb2a8e2..d902dcd3ce 100644 --- a/components/soc/src/regi2c_ctrl.c +++ b/components/soc/src/regi2c_ctrl.c @@ -48,3 +48,13 @@ void IRAM_ATTR regi2c_ctrl_write_reg_mask(uint8_t block, uint8_t host_id, uint8_ i2c_write_reg_mask_raw(block, host_id, reg_add, msb, lsb, data); 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); +}