From b84434b39d88d6ad93201ee6db486e990690e3a4 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Fri, 5 Jan 2024 12:01:28 +0800 Subject: [PATCH] fix(tsens,adc): Fix issue that disable adc will make temperature sensor crash, Closes https://github.com/espressif/esp-idf/issues/12921 --- components/driver/deprecated/adc_dma_legacy.c | 9 +- components/driver/deprecated/adc_legacy.c | 10 +-- components/esp_adc/adc_common.c | 38 +-------- components/esp_adc/adc_continuous.c | 9 +- .../esp_adc/include/esp_private/adc_private.h | 19 +---- .../esp_adc/test_apps/.build-test-rules.yml | 1 + .../esp_adc/test_apps/adc/main/CMakeLists.txt | 2 + .../test_apps/adc/main/test_adc_tsens.c | 83 +++++++++++++++++++ components/esp_driver_dac/esp32s2/dac_dma.c | 7 +- components/esp_hw_support/adc_share_hw_ctrl.c | 40 ++++++++- .../include/esp_private/adc_share_hw_ctrl.h | 20 ++++- .../esp_hw_support/sar_periph_ctrl_common.c | 9 +- .../esp32p4/include/soc/Kconfig.soc_caps.in | 4 + components/soc/esp32p4/include/soc/soc_caps.h | 1 + 14 files changed, 174 insertions(+), 78 deletions(-) create mode 100644 components/esp_adc/test_apps/adc/main/test_adc_tsens.c diff --git a/components/driver/deprecated/adc_dma_legacy.c b/components/driver/deprecated/adc_dma_legacy.c index 846276c17f..03c7ba5e58 100644 --- a/components/driver/deprecated/adc_dma_legacy.c +++ b/components/driver/deprecated/adc_dma_legacy.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2016-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2016-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -177,7 +177,7 @@ esp_err_t adc_digi_deinitialize(void) free(s_adc_digi_ctx); s_adc_digi_ctx = NULL; - periph_module_disable(PERIPH_SARADC_MODULE); + adc_apb_periph_free(); return ESP_OK; } @@ -321,10 +321,7 @@ esp_err_t adc_digi_initialize(const adc_digi_init_config_t *init_config) }; adc_hal_dma_ctx_config(&s_adc_digi_ctx->hal, &config); - //enable ADC digital part - periph_module_enable(PERIPH_SARADC_MODULE); - //reset ADC digital part - periph_module_reset(PERIPH_SARADC_MODULE); + adc_apb_periph_claim(); #if SOC_ADC_CALIBRATION_V1_SUPPORTED adc_hal_calibration_init(ADC_UNIT_1); diff --git a/components/driver/deprecated/adc_legacy.c b/components/driver/deprecated/adc_legacy.c index a88c42465d..9afbf4fec5 100644 --- a/components/driver/deprecated/adc_legacy.c +++ b/components/driver/deprecated/adc_legacy.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -777,7 +777,7 @@ int adc1_get_raw(adc1_channel_t channel) return ESP_ERR_TIMEOUT; } - periph_module_enable(PERIPH_SARADC_MODULE); + adc_apb_periph_claim(); sar_periph_ctrl_adc_oneshot_power_acquire(); adc_ll_digi_clk_sel(ADC_DIGI_CLK_SRC_DEFAULT); @@ -792,7 +792,7 @@ int adc1_get_raw(adc1_channel_t channel) ADC_REG_LOCK_EXIT(); sar_periph_ctrl_adc_oneshot_power_release(); - periph_module_disable(PERIPH_SARADC_MODULE); + adc_apb_periph_free(); adc_lock_release(ADC_UNIT_1); return raw_out; @@ -834,7 +834,7 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int * return ESP_ERR_TIMEOUT; } - periph_module_enable(PERIPH_SARADC_MODULE); + adc_apb_periph_claim(); sar_periph_ctrl_adc_oneshot_power_acquire(); adc_ll_digi_clk_sel(ADC_DIGI_CLK_SRC_DEFAULT); @@ -852,7 +852,7 @@ esp_err_t adc2_get_raw(adc2_channel_t channel, adc_bits_width_t width_bit, int * ADC_REG_LOCK_EXIT(); sar_periph_ctrl_adc_oneshot_power_release(); - periph_module_disable(PERIPH_SARADC_MODULE); + adc_apb_periph_free(); adc_lock_release(ADC_UNIT_2); return ret; diff --git a/components/esp_adc/adc_common.c b/components/esp_adc/adc_common.c index 4a2da9625b..01301c310f 100644 --- a/components/esp_adc/adc_common.c +++ b/components/esp_adc/adc_common.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -18,42 +18,6 @@ #include "soc/adc_periph.h" static const char *TAG = "adc_common"; -static portMUX_TYPE s_spinlock = portMUX_INITIALIZER_UNLOCKED; -extern portMUX_TYPE rtc_spinlock; - -/*------------------------------------------------------------------------------ -* For those who use APB_SARADC periph -*----------------------------------------------------------------------------*/ -static int s_adc_digi_ctrlr_cnt; - -void adc_apb_periph_claim(void) -{ - portENTER_CRITICAL(&s_spinlock); - s_adc_digi_ctrlr_cnt++; - if (s_adc_digi_ctrlr_cnt == 1) { - //enable ADC digital part - periph_module_enable(PERIPH_SARADC_MODULE); - //reset ADC digital part - periph_module_reset(PERIPH_SARADC_MODULE); - } - - portEXIT_CRITICAL(&s_spinlock); -} - -void adc_apb_periph_free(void) -{ - portENTER_CRITICAL(&s_spinlock); - s_adc_digi_ctrlr_cnt--; - if (s_adc_digi_ctrlr_cnt == 0) { - periph_module_disable(PERIPH_SARADC_MODULE); - } else if (s_adc_digi_ctrlr_cnt < 0) { - portEXIT_CRITICAL(&s_spinlock); - ESP_LOGE(TAG, "%s called, but `s_adc_digi_ctrlr_cnt == 0`", __func__); - abort(); - } - - portEXIT_CRITICAL(&s_spinlock); -} /*--------------------------------------------------------------- ADC IOs diff --git a/components/esp_adc/adc_continuous.c b/components/esp_adc/adc_continuous.c index 71d405ccb8..36e4a29fd4 100644 --- a/components/esp_adc/adc_continuous.c +++ b/components/esp_adc/adc_continuous.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2016-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2016-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -247,10 +247,7 @@ esp_err_t adc_continuous_new_handle(const adc_continuous_handle_cfg_t *hdl_confi adc_ctx->fsm = ADC_FSM_INIT; *ret_handle = adc_ctx; - //enable ADC digital part - periph_module_enable(PERIPH_SARADC_MODULE); - //reset ADC digital part - periph_module_reset(PERIPH_SARADC_MODULE); + adc_apb_periph_claim(); #if SOC_ADC_CALIBRATION_V1_SUPPORTED adc_hal_calibration_init(ADC_UNIT_1); @@ -504,7 +501,7 @@ esp_err_t adc_continuous_deinit(adc_continuous_handle_t handle) free(handle); handle = NULL; - periph_module_disable(PERIPH_SARADC_MODULE); + adc_apb_periph_free(); return ESP_OK; } diff --git a/components/esp_adc/include/esp_private/adc_private.h b/components/esp_adc/include/esp_private/adc_private.h index 805614a976..a714181e2e 100644 --- a/components/esp_adc/include/esp_private/adc_private.h +++ b/components/esp_adc/include/esp_private/adc_private.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,23 +15,6 @@ extern "C" { #endif -/*------------------------------------------------------------------------------ -* For those who use APB_SARADC periph -*----------------------------------------------------------------------------*/ -/** - * @brief Claim the usage of the APB_SARADC periph - * - * Reference count inside - */ -void adc_apb_periph_claim(void); - -/** - * @brief Free the usage of the APB_SARADC periph - * - * Reference count inside - */ -void adc_apb_periph_free(void); - /*--------------------------------------------------------------- ADC IOs ---------------------------------------------------------------*/ diff --git a/components/esp_adc/test_apps/.build-test-rules.yml b/components/esp_adc/test_apps/.build-test-rules.yml index 9c6297a2ff..aa97546c7d 100644 --- a/components/esp_adc/test_apps/.build-test-rules.yml +++ b/components/esp_adc/test_apps/.build-test-rules.yml @@ -10,3 +10,4 @@ components/esp_adc/test_apps/adc: - esp_driver_i2s # ADC continuous driver relies on I2S on ESP32 - efuse - esp_driver_spi # ADC continuous driver relies on SPI on ESP32S2 + - esp_driver_tsens diff --git a/components/esp_adc/test_apps/adc/main/CMakeLists.txt b/components/esp_adc/test_apps/adc/main/CMakeLists.txt index 8b8cc4b085..e7eed57470 100644 --- a/components/esp_adc/test_apps/adc/main/CMakeLists.txt +++ b/components/esp_adc/test_apps/adc/main/CMakeLists.txt @@ -4,10 +4,12 @@ set(srcs "test_app_main.c" "test_adc_driver.c" "test_adc_driver_iram.c" "test_adc_wifi.c" + "test_adc_tsens.c" "test_common_adc.c") # In order for the cases defined by `TEST_CASE` to be linked into the final elf, # the component can be registered as WHOLE_ARCHIVE idf_component_register(SRCS ${srcs} PRIV_REQUIRES esp_driver_gptimer esp_driver_gpio esp_wifi nvs_flash esp_adc test_utils efuse + esp_driver_tsens WHOLE_ARCHIVE) diff --git a/components/esp_adc/test_apps/adc/main/test_adc_tsens.c b/components/esp_adc/test_apps/adc/main/test_adc_tsens.c new file mode 100644 index 0000000000..159fa1b7ef --- /dev/null +++ b/components/esp_adc/test_apps/adc/main/test_adc_tsens.c @@ -0,0 +1,83 @@ +/* + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include "esp_log.h" +#include "soc/adc_periph.h" +#include "esp_adc/adc_oneshot.h" +#include "driver/gpio.h" +#include "driver/rtc_io.h" +#include "driver/temperature_sensor.h" +#include "nvs_flash.h" +#include "esp_event.h" +#include "esp_wifi.h" +#include "test_common_adc.h" +#include "test_utils.h" + +#if SOC_TEMP_SENSOR_SUPPORTED && SOC_ADC_SUPPORTED + +static const char *TAG = "adc_tsens"; + +#define EXAMPLE_ADC1_CHAN0 ADC_CHANNEL_2 +#define EXAMPLE_ADC1_CHAN1 ADC_CHANNEL_4 +#define EXAMPLE_ADC_ATTEN ADC_ATTEN_DB_12 + +static int adc_raw[2][10]; + +TEST_CASE("Test temperature sensor cannot be influenced by ADC", "[adc]") +{ + ESP_LOGI(TAG, "Install temperature sensor, expected temp ranger range: 10~50 ℃"); + temperature_sensor_handle_t temp_sensor = NULL; + temperature_sensor_config_t temp_sensor_config = TEMPERATURE_SENSOR_CONFIG_DEFAULT(-10, 80); + TEST_ESP_OK(temperature_sensor_install(&temp_sensor_config, &temp_sensor)); + int cnt = 2; + float tsens_value; + while (cnt--) { + temperature_sensor_enable(temp_sensor); + TEST_ESP_OK(temperature_sensor_get_celsius(temp_sensor, &tsens_value)); + ESP_LOGI(TAG, "Temperature value %.02f ℃", tsens_value); + vTaskDelay(pdMS_TO_TICKS(100)); + TEST_ESP_OK(temperature_sensor_disable(temp_sensor)); + } + + adc_oneshot_unit_handle_t adc1_handle; + adc_oneshot_unit_init_cfg_t init_config1 = { + .unit_id = ADC_UNIT_1, + }; + TEST_ESP_OK(adc_oneshot_new_unit(&init_config1, &adc1_handle)); + + //-------------ADC1 Config---------------// + adc_oneshot_chan_cfg_t config = { + .bitwidth = ADC_BITWIDTH_DEFAULT, + .atten = EXAMPLE_ADC_ATTEN, + }; + TEST_ESP_OK(adc_oneshot_config_channel(adc1_handle, EXAMPLE_ADC1_CHAN0, &config)); + TEST_ESP_OK(adc_oneshot_config_channel(adc1_handle, EXAMPLE_ADC1_CHAN1, &config)); + + cnt = 2; + while (cnt--) { + TEST_ESP_OK(adc_oneshot_read(adc1_handle, EXAMPLE_ADC1_CHAN0, &adc_raw[0][0])); + ESP_LOGI(TAG, "ADC%d Channel[%d] Raw Data: %d", ADC_UNIT_1 + 1, EXAMPLE_ADC1_CHAN0, adc_raw[0][0]); + vTaskDelay(pdMS_TO_TICKS(100)); + } + + TEST_ESP_OK(adc_oneshot_del_unit(adc1_handle)); + + cnt = 2; + while (cnt--) { + temperature_sensor_enable(temp_sensor); + TEST_ESP_OK(temperature_sensor_get_celsius(temp_sensor, &tsens_value)); + ESP_LOGI(TAG, "Temperature value %.02f ℃", tsens_value); + vTaskDelay(pdMS_TO_TICKS(100)); + TEST_ESP_OK(temperature_sensor_disable(temp_sensor)); + } + + TEST_ESP_OK(temperature_sensor_uninstall(temp_sensor)); +} + +#endif diff --git a/components/esp_driver_dac/esp32s2/dac_dma.c b/components/esp_driver_dac/esp32s2/dac_dma.c index 6e715bc4dc..52b250fbce 100644 --- a/components/esp_driver_dac/esp32s2/dac_dma.c +++ b/components/esp_driver_dac/esp32s2/dac_dma.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,6 +15,7 @@ #include "sdkconfig.h" #include "esp_private/spi_common_internal.h" #include "esp_private/periph_ctrl.h" +#include "esp_private/adc_share_hw_ctrl.h" #include "hal/spi_ll.h" #include "hal/dac_ll.h" #include "hal/adc_ll.h" @@ -130,7 +131,7 @@ esp_err_t dac_dma_periph_init(uint32_t freq_hz, bool is_alternate, bool is_apll) esp_err_t ret = ESP_OK; /* Acquire DMA peripheral */ ESP_RETURN_ON_FALSE(spicommon_periph_claim(DAC_DMA_PERIPH_SPI_HOST, "dac_dma"), ESP_ERR_NOT_FOUND, TAG, "Failed to acquire DAC DMA peripheral"); - periph_module_enable(PERIPH_SARADC_MODULE); + adc_apb_periph_claim(); /* Allocate DAC DMA peripheral object */ s_ddp = (dac_dma_periph_spi_t *)heap_caps_calloc(1, sizeof(dac_dma_periph_spi_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); ESP_GOTO_ON_FALSE(s_ddp, ESP_ERR_NO_MEM, err, TAG, "No memory for DAC DMA object"); @@ -163,7 +164,7 @@ esp_err_t dac_dma_periph_deinit(void) } ESP_RETURN_ON_FALSE(spicommon_periph_free(DAC_DMA_PERIPH_SPI_HOST), ESP_FAIL, TAG, "Failed to release DAC DMA peripheral"); spi_ll_disable_intr(s_ddp->periph_dev, SPI_LL_INTR_OUT_EOF | SPI_LL_INTR_OUT_TOTAL_EOF); - periph_module_disable(PERIPH_SARADC_MODULE); + adc_apb_periph_free(); if (s_ddp) { if (s_ddp->use_apll) { periph_rtc_apll_release(); diff --git a/components/esp_hw_support/adc_share_hw_ctrl.c b/components/esp_hw_support/adc_share_hw_ctrl.c index f9b8167258..c993797e1d 100644 --- a/components/esp_hw_support/adc_share_hw_ctrl.c +++ b/components/esp_hw_support/adc_share_hw_ctrl.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -28,6 +28,8 @@ #include "hal/adc_ll.h" #include "esp_private/adc_share_hw_ctrl.h" #include "esp_private/sar_periph_ctrl.h" +#include "esp_private/periph_ctrl.h" +#include "soc/periph_defs.h" //For calibration #if CONFIG_IDF_TARGET_ESP32S2 #include "esp_efuse_rtc_table.h" @@ -190,3 +192,39 @@ esp_err_t adc2_wifi_release(void) return ESP_OK; } + +static portMUX_TYPE s_spinlock = portMUX_INITIALIZER_UNLOCKED; + +/*------------------------------------------------------------------------------ +* For those who use APB_SARADC periph +*----------------------------------------------------------------------------*/ +static int s_adc_digi_ctrlr_cnt; + +void adc_apb_periph_claim(void) +{ + portENTER_CRITICAL(&s_spinlock); + s_adc_digi_ctrlr_cnt++; + if (s_adc_digi_ctrlr_cnt == 1) { + //enable ADC digital part + periph_module_enable(PERIPH_SARADC_MODULE); + //reset ADC digital part + periph_module_reset(PERIPH_SARADC_MODULE); + } + + portEXIT_CRITICAL(&s_spinlock); +} + +void adc_apb_periph_free(void) +{ + portENTER_CRITICAL(&s_spinlock); + s_adc_digi_ctrlr_cnt--; + if (s_adc_digi_ctrlr_cnt == 0) { + periph_module_disable(PERIPH_SARADC_MODULE); + } else if (s_adc_digi_ctrlr_cnt < 0) { + portEXIT_CRITICAL(&s_spinlock); + ESP_LOGE(TAG, "%s called, but `s_adc_digi_ctrlr_cnt == 0`", __func__); + abort(); + } + + portEXIT_CRITICAL(&s_spinlock); +} diff --git a/components/esp_hw_support/include/esp_private/adc_share_hw_ctrl.h b/components/esp_hw_support/include/esp_private/adc_share_hw_ctrl.h index 4d9de5270c..5bf1084730 100644 --- a/components/esp_hw_support/include/esp_private/adc_share_hw_ctrl.h +++ b/components/esp_hw_support/include/esp_private/adc_share_hw_ctrl.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -156,6 +156,24 @@ void adc2_cal_include(void); #define adc2_cal_include() #endif //CONFIG_IDF_TARGET_* +/*------------------------------------------------------------------------------ +* For those who use APB_SARADC periph +*----------------------------------------------------------------------------*/ +/** + * @brief Claim the usage of the APB_SARADC periph + * + * Reference count inside + */ +void adc_apb_periph_claim(void); + +/** + * @brief Free the usage of the APB_SARADC periph + * + * Reference count inside + */ +void adc_apb_periph_free(void); + + #ifdef __cplusplus } #endif diff --git a/components/esp_hw_support/sar_periph_ctrl_common.c b/components/esp_hw_support/sar_periph_ctrl_common.c index 90b55d6608..f13ca42869 100644 --- a/components/esp_hw_support/sar_periph_ctrl_common.c +++ b/components/esp_hw_support/sar_periph_ctrl_common.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,6 +15,7 @@ #include "soc/temperature_sensor_periph.h" #include "soc/periph_defs.h" #include "esp_private/periph_ctrl.h" +#include "esp_private/adc_share_hw_ctrl.h" extern __attribute__((unused)) portMUX_TYPE rtc_spinlock; @@ -44,6 +45,9 @@ void temperature_sensor_power_acquire(void) s_temperature_sensor_power_cnt++; if (s_temperature_sensor_power_cnt == 1) { regi2c_saradc_enable(); +#if !SOC_TEMPERATURE_SENSOR_IS_INDEPENDENT_FROM_ADC + adc_apb_periph_claim(); +#endif TSENS_RCC_ATOMIC() { temperature_sensor_ll_bus_clk_enable(true); temperature_sensor_ll_reset_module(); @@ -71,6 +75,9 @@ void temperature_sensor_power_release(void) TSENS_RCC_ATOMIC() { temperature_sensor_ll_bus_clk_enable(false); } +#if !SOC_TEMPERATURE_SENSOR_IS_INDEPENDENT_FROM_ADC + adc_apb_periph_free(); +#endif regi2c_saradc_disable(); } portEXIT_CRITICAL(&rtc_spinlock); diff --git a/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in b/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in index 361d7bd73e..8644c0c61a 100644 --- a/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in @@ -1339,6 +1339,10 @@ config SOC_TEMPERATURE_SENSOR_INTR_SUPPORT bool default y +config SOC_TEMPERATURE_SENSOR_IS_INDEPENDENT_FROM_ADC + bool + default y + config SOC_MEM_TCM_SUPPORTED bool default y diff --git a/components/soc/esp32p4/include/soc/soc_caps.h b/components/soc/esp32p4/include/soc/soc_caps.h index 6db49e7d84..f4391a6324 100644 --- a/components/soc/esp32p4/include/soc/soc_caps.h +++ b/components/soc/esp32p4/include/soc/soc_caps.h @@ -577,6 +577,7 @@ /*-------------------------- Temperature Sensor CAPS -------------------------------------*/ #define SOC_TEMPERATURE_SENSOR_LP_PLL_SUPPORT (1) #define SOC_TEMPERATURE_SENSOR_INTR_SUPPORT (1) +#define SOC_TEMPERATURE_SENSOR_IS_INDEPENDENT_FROM_ADC (1) /*!< Temperature sensor is a separate module, not share regs with ADC */ /*-------------------------- Memory CAPS --------------------------*/ #define SOC_MEM_TCM_SUPPORTED (1)