From 049ebf985327b98952d0b0026b84ecf7113ee64f Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Thu, 3 Mar 2022 16:55:08 +0800 Subject: [PATCH 1/2] regi2c: use safe version of spinlock, instead of ISR ver --- components/esp_hw_support/regi2c_ctrl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/esp_hw_support/regi2c_ctrl.c b/components/esp_hw_support/regi2c_ctrl.c index 8401b7ecee..fe2ce74146 100644 --- a/components/esp_hw_support/regi2c_ctrl.c +++ b/components/esp_hw_support/regi2c_ctrl.c @@ -15,32 +15,32 @@ 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 = regi2c_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 = regi2c_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); regi2c_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); regi2c_write_reg_mask_raw(block, host_id, reg_add, msb, lsb, data); - portEXIT_CRITICAL_ISR(&mux); + portEXIT_CRITICAL_SAFE(&mux); } /** From f3adbf995314fd95fb39472c282da502546cc9bc Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Tue, 1 Mar 2022 16:12:45 +0800 Subject: [PATCH 2/2] esp_phy: use spinlock to avoid regi2c access conflicts --- .../include/esp_private/regi2c_ctrl.h | 5 ++ components/esp_hw_support/regi2c_ctrl.c | 10 ++++ components/esp_phy/CMakeLists.txt | 31 ++++++------ components/esp_phy/src/phy_override.c | 50 +++++++++++++++++++ components/esp_wifi/src/wifi_init.c | 20 -------- 5 files changed, 81 insertions(+), 35 deletions(-) create mode 100644 components/esp_phy/src/phy_override.c diff --git a/components/esp_hw_support/include/esp_private/regi2c_ctrl.h b/components/esp_hw_support/include/esp_private/regi2c_ctrl.h index f6308920fd..17918dfb85 100644 --- a/components/esp_hw_support/include/esp_private/regi2c_ctrl.h +++ b/components/esp_hw_support/include/esp_private/regi2c_ctrl.h @@ -34,11 +34,16 @@ extern "C" { #else +/* Access internal registers, don't use in application */ uint8_t regi2c_ctrl_read_reg(uint8_t block, uint8_t host_id, uint8_t reg_add); uint8_t regi2c_ctrl_read_reg_mask(uint8_t block, uint8_t host_id, uint8_t reg_add, uint8_t msb, uint8_t lsb); 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/esp_hw_support/regi2c_ctrl.c b/components/esp_hw_support/regi2c_ctrl.c index fe2ce74146..e6c23fe353 100644 --- a/components/esp_hw_support/regi2c_ctrl.c +++ b/components/esp_hw_support/regi2c_ctrl.c @@ -43,6 +43,16 @@ void IRAM_ATTR regi2c_ctrl_write_reg_mask(uint8_t block, uint8_t host_id, uint8_ 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); +} + /** * Restore regi2c analog calibration related configuration registers. * This is a workaround, and is fixed on later chips diff --git a/components/esp_phy/CMakeLists.txt b/components/esp_phy/CMakeLists.txt index a38fd9743d..7ce8f6ea14 100644 --- a/components/esp_phy/CMakeLists.txt +++ b/components/esp_phy/CMakeLists.txt @@ -5,6 +5,8 @@ if(IDF_TARGET STREQUAL "esp32c2") return() endif() +set(srcs "src/phy_override.c" "src/lib_printf.c") + if(CONFIG_APP_NO_BLOBS) set(link_binary_libs 0) set(ldfragments) @@ -14,11 +16,10 @@ else() endif() if(IDF_TARGET STREQUAL "esp32h2") - set(srcs "src/phy_init_esp32hxx.c") + list(APPEND srcs "src/phy_init_esp32hxx.c") else() - set(srcs "src/phy_init.c") + list(APPEND srcs "src/phy_init.c") endif() - list(APPEND srcs "src/lib_printf.c") idf_build_get_property(build_dir BUILD_DIR) @@ -29,23 +30,23 @@ if(CONFIG_ESP_PHY_MULTIPLE_INIT_DATA_BIN) endif() if(CONFIG_ESP_PHY_MULTIPLE_INIT_DATA_BIN_EMBED) - idf_component_register(SRCS "${srcs}" - INCLUDE_DIRS "include" "${idf_target}/include" - PRIV_REQUIRES nvs_flash efuse - LDFRAGMENTS "${ldfragments}" - EMBED_FILES "${build_dir}/phy_multiple_init_data.bin" - ) -else() - idf_component_register(SRCS "${srcs}" - INCLUDE_DIRS "include" "${idf_target}/include" - PRIV_REQUIRES nvs_flash efuse - LDFRAGMENTS "${ldfragments}" - ) + set(embed_files "${build_dir}/phy_multiple_init_data.bin") endif() +# [refactor-todo]: requires "driver" component for periph_ctrl header file +idf_component_register(SRCS "${srcs}" + INCLUDE_DIRS "include" "${idf_target}/include" + PRIV_REQUIRES nvs_flash driver efuse + LDFRAGMENTS "${ldfragments}" + EMBED_FILES ${embed_files} + ) + 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) target_link_libraries(${COMPONENT_LIB} PUBLIC phy) diff --git a/components/esp_phy/src/phy_override.c b/components/esp_phy/src/phy_override.c new file mode 100644 index 0000000000..4b2c47fce4 --- /dev/null +++ b/components/esp_phy/src/phy_override.c @@ -0,0 +1,50 @@ +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "esp_private/regi2c_ctrl.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 +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 25ddb7fba7..f4787db820 100644 --- a/components/esp_wifi/src/wifi_init.c +++ b/components/esp_wifi/src/wifi_init.c @@ -15,7 +15,6 @@ #include "soc/rtc.h" #include "esp_wpa.h" #include "esp_netif.h" -#include "driver/adc.h" #include "esp_coexist_internal.h" #include "esp_phy_init.h" #include "phy.h" @@ -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"; @@ -278,24 +276,6 @@ void wifi_apb80m_release(void) } #endif //CONFIG_PM_ENABLE -/* Coordinate ADC power with other modules. This overrides the function from PHY lib. */ -// 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(); - } -} - #ifndef CONFIG_ESP_WIFI_FTM_ENABLE void ieee80211_ftm_attach(void) {