From 58dd50120f0a2eec6b4cf03c77e0e6c2c66abf3f Mon Sep 17 00:00:00 2001 From: paul Date: Tue, 12 Dec 2023 22:12:02 +0300 Subject: [PATCH 1/2] fix(ulp_adc): Provide ulp_adc_deinit() API to fix ADC1 handle leakage --- components/ulp/ulp_common/include/ulp_adc.h | 7 +++++++ components/ulp/ulp_common/ulp_adc.c | 21 +++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/components/ulp/ulp_common/include/ulp_adc.h b/components/ulp/ulp_common/include/ulp_adc.h index 5f1b93967b..b8958efb8a 100644 --- a/components/ulp/ulp_common/include/ulp_adc.h +++ b/components/ulp/ulp_common/include/ulp_adc.h @@ -29,6 +29,13 @@ typedef struct { */ esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg); +/** + * @brief Deinitialize ADC after use with ULP, allowing it to be reclaimed + * + * @return esp_err_t ESP_OK for successful. +*/ +esp_err_t ulp_adc_deinit(); + #ifdef __cplusplus } #endif diff --git a/components/ulp/ulp_common/ulp_adc.c b/components/ulp/ulp_common/ulp_adc.c index 9f3cd6cc5f..fa7568d250 100644 --- a/components/ulp/ulp_common/ulp_adc.c +++ b/components/ulp/ulp_common/ulp_adc.c @@ -15,17 +15,16 @@ #include "esp_private/adc_share_hw_ctrl.h" static const char *TAG = "ulp_adc"; +static adc_oneshot_unit_handle_t s_adc1_handle = NULL; esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg) { esp_err_t ret = ESP_OK; - ESP_GOTO_ON_FALSE(cfg, ESP_ERR_INVALID_ARG, err, TAG, "cfg == NULL"); - ESP_GOTO_ON_FALSE(cfg->adc_n == ADC_UNIT_1, ESP_ERR_INVALID_ARG, err, TAG, "Only ADC_UNIT_1 is supported for now"); + ESP_RETURN_ON_FALSE(cfg, ESP_ERR_INVALID_ARG, TAG, "cfg == NULL"); + ESP_RETURN_ON_FALSE(cfg->adc_n == ADC_UNIT_1, ESP_ERR_INVALID_ARG, TAG, "Only ADC_UNIT_1 is supported for now"); //-------------ADC1 Init---------------// - adc_oneshot_unit_handle_t adc1_handle; - adc_oneshot_unit_init_cfg_t init_config1 = { .unit_id = cfg->adc_n, .ulp_mode = cfg->ulp_mode, @@ -37,21 +36,27 @@ esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg) init_config1.ulp_mode = ADC_ULP_MODE_RISCV; } - - ESP_ERROR_CHECK(adc_oneshot_new_unit(&init_config1, &adc1_handle)); + ret = adc_oneshot_new_unit(&init_config1, &s_adc1_handle); + if (ret != ESP_OK) return ret; //-------------ADC1 Config---------------// adc_oneshot_chan_cfg_t config = { .bitwidth = cfg->width, .atten = cfg->atten, }; - ESP_ERROR_CHECK(adc_oneshot_config_channel(adc1_handle, cfg->channel, &config)); + ret = adc_oneshot_config_channel(s_adc1_handle, cfg->channel, &config); + if (ret != ESP_OK) return ret; //Calibrate the ADC #if SOC_ADC_CALIBRATION_V1_SUPPORTED adc_set_hw_calibration_code(cfg->adc_n, cfg->atten); #endif -err: return ret; } + +esp_err_t ulp_adc_deinit() +{ + // No need to check for null-pointer and stuff, oneshot driver already does that + return adc_oneshot_del_unit(s_adc1_handle); +} \ No newline at end of file From 0a2199d81b655e527d49f06b646a9a91a719e6fb Mon Sep 17 00:00:00 2001 From: Marius Vikhammer Date: Thu, 14 Dec 2023 19:02:32 +0800 Subject: [PATCH 2/2] change(ulp): added test for ulp adc init/deinit --- .../test_apps/ulp_riscv/main/test_ulp_riscv.c | 43 ++++++++++++++++++- components/ulp/ulp_common/include/ulp_adc.h | 2 +- components/ulp/ulp_common/ulp_adc.c | 14 +++--- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/components/ulp/test_apps/ulp_riscv/main/test_ulp_riscv.c b/components/ulp/test_apps/ulp_riscv/main/test_ulp_riscv.c index 857211f2cd..e4f7ec12d3 100644 --- a/components/ulp/test_apps/ulp_riscv/main/test_ulp_riscv.c +++ b/components/ulp/test_apps/ulp_riscv/main/test_ulp_riscv.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2010-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2010-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -13,6 +13,7 @@ #include "soc/rtc_periph.h" #include "ulp_riscv.h" #include "ulp_riscv_lock.h" +#include "ulp_adc.h" #include "ulp_test_app.h" #include "ulp_test_app2.h" #include "ulp_test_shared.h" @@ -21,6 +22,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/semphr.h" +#include "esp_adc/adc_oneshot.h" #define ULP_WAKEUP_PERIOD 1000000 // 1 second @@ -434,3 +436,42 @@ TEST_CASE("ULP-RISC-V interrupt signals can be handled via ISRs on the main core firmware_loaded = false; load_and_start_ulp_firmware(ulp_main_bin_start, ulp_main_bin_length); } + +#define ATTEN 3 +#define WIDTH 0 +#define CHANNEL 6 +#define ADC_UNIT ADC_UNIT_1 + +TEST_CASE("ULP ADC can init-deinit-init", "[ulp]") +{ + + + ulp_adc_cfg_t riscv_adc_cfg = { + .adc_n = ADC_UNIT, + .channel = CHANNEL, + .width = WIDTH, + .atten = ATTEN, + .ulp_mode = ADC_ULP_MODE_FSM, + }; + + TEST_ASSERT_EQUAL(ESP_OK, ulp_adc_init(&riscv_adc_cfg)); + TEST_ASSERT_EQUAL(ESP_OK, ulp_adc_deinit()); + + /* Check that we can init one-shot ADC */ + adc_oneshot_unit_handle_t adc1_handle; + adc_oneshot_unit_init_cfg_t init_config1 = { + .unit_id = ADC_UNIT, + }; + TEST_ASSERT_EQUAL(ESP_OK, adc_oneshot_new_unit(&init_config1, &adc1_handle)); + + adc_oneshot_chan_cfg_t config = { + .bitwidth = WIDTH, + .atten = ATTEN, + }; + TEST_ASSERT_EQUAL(ESP_OK, adc_oneshot_config_channel(adc1_handle, CHANNEL, &config)); + TEST_ASSERT_EQUAL(ESP_OK, adc_oneshot_del_unit(adc1_handle)); + + /* Re-init ADC for ULP */ + TEST_ASSERT_EQUAL(ESP_OK, ulp_adc_init(&riscv_adc_cfg)); + TEST_ASSERT_EQUAL(ESP_OK, ulp_adc_deinit()); +} diff --git a/components/ulp/ulp_common/include/ulp_adc.h b/components/ulp/ulp_common/include/ulp_adc.h index b8958efb8a..e838d4cdbb 100644 --- a/components/ulp/ulp_common/include/ulp_adc.h +++ b/components/ulp/ulp_common/include/ulp_adc.h @@ -34,7 +34,7 @@ esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg); * * @return esp_err_t ESP_OK for successful. */ -esp_err_t ulp_adc_deinit(); +esp_err_t ulp_adc_deinit(void); #ifdef __cplusplus } diff --git a/components/ulp/ulp_common/ulp_adc.c b/components/ulp/ulp_common/ulp_adc.c index fa7568d250..cf9ffa1c6f 100644 --- a/components/ulp/ulp_common/ulp_adc.c +++ b/components/ulp/ulp_common/ulp_adc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -37,7 +37,9 @@ esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg) } ret = adc_oneshot_new_unit(&init_config1, &s_adc1_handle); - if (ret != ESP_OK) return ret; + if (ret != ESP_OK) { + return ret; + } //-------------ADC1 Config---------------// adc_oneshot_chan_cfg_t config = { @@ -45,7 +47,9 @@ esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg) .atten = cfg->atten, }; ret = adc_oneshot_config_channel(s_adc1_handle, cfg->channel, &config); - if (ret != ESP_OK) return ret; + if (ret != ESP_OK) { + return ret; + } //Calibrate the ADC #if SOC_ADC_CALIBRATION_V1_SUPPORTED @@ -55,8 +59,8 @@ esp_err_t ulp_adc_init(const ulp_adc_cfg_t *cfg) return ret; } -esp_err_t ulp_adc_deinit() +esp_err_t ulp_adc_deinit(void) { // No need to check for null-pointer and stuff, oneshot driver already does that return adc_oneshot_del_unit(s_adc1_handle); -} \ No newline at end of file +}