From 4cd74f51dbdc4f4c2148bb15ced51a8263d1f87d Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Mon, 1 Jul 2024 19:43:40 +0800 Subject: [PATCH 1/2] fix(ledc): clear ledc_mem_force_pd when LEDC peripheral is in use And enable target test for C5 and P4 --- components/esp_driver_ledc/src/ledc.c | 8 ++++---- .../test_apps/.build-test-rules.yml | 4 ---- .../test_apps/ledc/pytest_ledc.py | 5 ++--- components/hal/esp32/include/hal/ledc_ll.h | 19 +++++++++++++++---- components/hal/esp32c2/include/hal/ledc_ll.h | 19 +++++++++++++++---- components/hal/esp32c3/include/hal/ledc_ll.h | 19 +++++++++++++++---- components/hal/esp32c5/include/hal/ledc_ll.h | 19 ++++++++++++++++--- components/hal/esp32c6/include/hal/ledc_ll.h | 18 ++++++++++++++---- components/hal/esp32h2/include/hal/ledc_ll.h | 18 ++++++++++++++---- components/hal/esp32p4/include/hal/ledc_ll.h | 19 +++++++++++++++---- components/hal/esp32s2/include/hal/ledc_ll.h | 19 +++++++++++++++---- components/hal/esp32s3/include/hal/ledc_ll.h | 19 +++++++++++++++---- components/hal/ledc_hal.c | 7 ++++--- 13 files changed, 144 insertions(+), 49 deletions(-) diff --git a/components/esp_driver_ledc/src/ledc.c b/components/esp_driver_ledc/src/ledc.c index f2b36f133e..bf06d97000 100644 --- a/components/esp_driver_ledc/src/ledc.c +++ b/components/esp_driver_ledc/src/ledc.c @@ -304,16 +304,16 @@ static bool ledc_speed_mode_ctx_create(ledc_mode_t speed_mode) ledc_obj_t *ledc_new_mode_obj = (ledc_obj_t *) heap_caps_calloc(1, sizeof(ledc_obj_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); if (ledc_new_mode_obj) { new_ctx = true; + LEDC_BUS_CLOCK_ATOMIC() { + ledc_ll_enable_bus_clock(true); + ledc_ll_enable_reset_reg(false); + } ledc_hal_init(&(ledc_new_mode_obj->ledc_hal), speed_mode); ledc_new_mode_obj->glb_clk = LEDC_SLOW_CLK_UNINIT; #if SOC_LEDC_HAS_TIMER_SPECIFIC_MUX memset(ledc_new_mode_obj->timer_specific_clk, LEDC_TIMER_SPECIFIC_CLK_UNINIT, sizeof(ledc_clk_src_t) * LEDC_TIMER_MAX); #endif p_ledc_obj[speed_mode] = ledc_new_mode_obj; - LEDC_BUS_CLOCK_ATOMIC() { - ledc_ll_enable_bus_clock(true); - ledc_ll_enable_reset_reg(false); - } } } _lock_release(&s_ledc_mutex[speed_mode]); diff --git a/components/esp_driver_ledc/test_apps/.build-test-rules.yml b/components/esp_driver_ledc/test_apps/.build-test-rules.yml index 9fe6d7d7f3..50e40b72c0 100644 --- a/components/esp_driver_ledc/test_apps/.build-test-rules.yml +++ b/components/esp_driver_ledc/test_apps/.build-test-rules.yml @@ -3,9 +3,5 @@ components/esp_driver_ledc/test_apps/ledc: disable: - if: SOC_LEDC_SUPPORTED != 1 - disable_test: - - if: IDF_TARGET in ["esp32p4", "esp32c5"] - temporary: true - reason: test not pass, should be re-enable # TODO: [ESP32P4] IDF-8969, [ESP32C5] IDF-10333 depends_components: - esp_driver_ledc diff --git a/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py b/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py index 995b5793db..672aadb28f 100644 --- a/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py +++ b/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py @@ -5,9 +5,8 @@ from pytest_embedded_idf import IdfDut @pytest.mark.supported_targets -# TODO: [ESP32P4] IDF-8969, [ESP32C5] IDF-10333 -@pytest.mark.temp_skip_ci(targets=['esp32s3', 'esp32p4', 'esp32c5'], - reason='skip due to duplication with test_ledc_psram, p4 TBD, c5 test failed') +@pytest.mark.temp_skip_ci(targets=['esp32s3'], + reason='skip due to duplication with test_ledc_psram') @pytest.mark.generic @pytest.mark.parametrize( 'config', diff --git a/components/hal/esp32/include/hal/ledc_ll.h b/components/hal/esp32/include/hal/ledc_ll.h index e460ebe98b..0e5b1f7fa8 100644 --- a/components/hal/esp32/include/hal/ledc_ll.h +++ b/components/hal/esp32/include/hal/ledc_ll.h @@ -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 */ @@ -46,7 +46,8 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ if (enable) { DPORT_SET_PERI_REG_MASK(DPORT_PERIP_CLK_EN_REG, DPORT_LEDC_CLK_EN); } else { @@ -61,7 +62,8 @@ static inline void ledc_ll_enable_bus_clock(bool enable) { /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ if (enable) { DPORT_SET_PERI_REG_MASK(DPORT_PERIP_RST_EN_REG, DPORT_LEDC_RST); } else { @@ -73,6 +75,14 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance #define ledc_ll_enable_reset_reg(...) (void)__DECLARE_RCC_ATOMIC_ENV; ledc_ll_enable_reset_reg(__VA_ARGS__) +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No LEDC mem block on ESP32 +} + /** * @brief Enable LEDC function clock * @@ -81,7 +91,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ //resolve for compatibility } diff --git a/components/hal/esp32c2/include/hal/ledc_ll.h b/components/hal/esp32c2/include/hal/ledc_ll.h index e70282c9b7..3849494743 100644 --- a/components/hal/esp32c2/include/hal/ledc_ll.h +++ b/components/hal/esp32c2/include/hal/ledc_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -40,7 +40,8 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ SYSTEM.perip_clk_en0.ledc_clk_en = enable; } @@ -51,7 +52,8 @@ static inline void ledc_ll_enable_bus_clock(bool enable) { /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ SYSTEM.perip_rst_en0.ledc_rst = enable; } @@ -59,6 +61,14 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance #define ledc_ll_enable_reset_reg(...) (void)__DECLARE_RCC_ATOMIC_ENV; ledc_ll_enable_reset_reg(__VA_ARGS__) +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No LEDC mem block on C2 +} + /** * @brief Enable LEDC function clock * @@ -67,7 +77,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ //resolve for compatibility } diff --git a/components/hal/esp32c3/include/hal/ledc_ll.h b/components/hal/esp32c3/include/hal/ledc_ll.h index 373a3c679e..7d6fd942c5 100644 --- a/components/hal/esp32c3/include/hal/ledc_ll.h +++ b/components/hal/esp32c3/include/hal/ledc_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -41,7 +41,8 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ SYSTEM.perip_clk_en0.reg_ledc_clk_en = enable; } @@ -52,7 +53,8 @@ static inline void ledc_ll_enable_bus_clock(bool enable) { /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ SYSTEM.perip_rst_en0.reg_ledc_rst = enable; } @@ -60,6 +62,14 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance #define ledc_ll_enable_reset_reg(...) (void)__DECLARE_RCC_ATOMIC_ENV; ledc_ll_enable_reset_reg(__VA_ARGS__) +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No LEDC mem block on C3 +} + /** * @brief Enable LEDC function clock * @@ -68,7 +78,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ //resolve for compatibility } diff --git a/components/hal/esp32c5/include/hal/ledc_ll.h b/components/hal/esp32c5/include/hal/ledc_ll.h index 1c86aa700a..e6fd636379 100644 --- a/components/hal/esp32c5/include/hal/ledc_ll.h +++ b/components/hal/esp32c5/include/hal/ledc_ll.h @@ -35,17 +35,29 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ PCR.ledc_conf.ledc_clk_en = enable; } /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ PCR.ledc_conf.ledc_rst_en = enable; } +/** + * @brief Enable the power for LEDC memory block + * + * Note. This function cannot overwrite the power control of the mem block in sleep mode + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + PCR.ledc_pd_ctrl.ledc_mem_force_pd = !enable; +} + /** * @brief Enable LEDC function clock * @@ -54,7 +66,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ (void)hw; PCR.ledc_sclk_conf.ledc_sclk_en = en; } diff --git a/components/hal/esp32c6/include/hal/ledc_ll.h b/components/hal/esp32c6/include/hal/ledc_ll.h index cc858d4dee..b73e0214af 100644 --- a/components/hal/esp32c6/include/hal/ledc_ll.h +++ b/components/hal/esp32c6/include/hal/ledc_ll.h @@ -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 */ @@ -38,17 +38,27 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ PCR.ledc_conf.ledc_clk_en = enable; } /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ PCR.ledc_conf.ledc_rst_en = enable; } +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No register to control the power for LEDC memory block on C6 +} + /** * @brief Enable LEDC function clock * @@ -539,7 +549,7 @@ static inline void ledc_ll_get_fade_param(ledc_dev_t *hw, ledc_mode_t speed_mode static inline void ledc_ll_get_fade_param_range(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, uint8_t range, uint32_t *dir, uint32_t *cycle, uint32_t *scale, uint32_t *step) { // On ESP32C6/H2, gamma ram read/write has the APB and LEDC clock domain sync issue - // To make sure the parameter read is from the correct gamma ram addr, add a delay in between to ensure syncronization + // To make sure the parameter read is from the correct gamma ram addr, add a delay in between to ensure synchronization ledc_ll_set_duty_range_rd_addr(hw, speed_mode, channel_num, range); esp_rom_delay_us(5); ledc_ll_get_fade_param(hw, speed_mode, channel_num, dir, cycle, scale, step); diff --git a/components/hal/esp32h2/include/hal/ledc_ll.h b/components/hal/esp32h2/include/hal/ledc_ll.h index 81e8177b7b..731ee3f3ea 100644 --- a/components/hal/esp32h2/include/hal/ledc_ll.h +++ b/components/hal/esp32h2/include/hal/ledc_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -36,17 +36,27 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ PCR.ledc_conf.ledc_clk_en = enable; } /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ PCR.ledc_conf.ledc_rst_en = enable; } +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No register to control the power for LEDC memory block on H2 +} + /** * @brief Enable LEDC function clock * @@ -537,7 +547,7 @@ static inline void ledc_ll_get_fade_param(ledc_dev_t *hw, ledc_mode_t speed_mode static inline void ledc_ll_get_fade_param_range(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, uint8_t range, uint32_t *dir, uint32_t *cycle, uint32_t *scale, uint32_t *step) { // On ESP32C6/H2, gamma ram read/write has the APB and LEDC clock domain sync issue - // To make sure the parameter read is from the correct gamma ram addr, add a delay in between to ensure syncronization + // To make sure the parameter read is from the correct gamma ram addr, add a delay in between to ensure synchronization ledc_ll_set_duty_range_rd_addr(hw, speed_mode, channel_num, range); esp_rom_delay_us(5); ledc_ll_get_fade_param(hw, speed_mode, channel_num, dir, cycle, scale, step); diff --git a/components/hal/esp32p4/include/hal/ledc_ll.h b/components/hal/esp32p4/include/hal/ledc_ll.h index ca655dabb5..dea593b889 100644 --- a/components/hal/esp32p4/include/hal/ledc_ll.h +++ b/components/hal/esp32p4/include/hal/ledc_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -35,7 +35,8 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ HP_SYS_CLKRST.soc_clk_ctrl3.reg_ledc_apb_clk_en = enable; } @@ -46,7 +47,8 @@ static inline void ledc_ll_enable_bus_clock(bool enable) { /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ HP_SYS_CLKRST.hp_rst_en1.reg_rst_en_ledc = enable; } @@ -54,6 +56,14 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance #define ledc_ll_enable_reset_reg(...) (void)__DECLARE_RCC_ATOMIC_ENV; ledc_ll_enable_reset_reg(__VA_ARGS__) +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No register to control the power for LEDC memory block on P4 +} + /** * @brief Enable LEDC function clock * @@ -62,7 +72,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ (void)hw; HP_SYS_CLKRST.peri_clk_ctrl22.reg_ledc_clk_en = en; } diff --git a/components/hal/esp32s2/include/hal/ledc_ll.h b/components/hal/esp32s2/include/hal/ledc_ll.h index a9ed6babcd..8a5670a428 100644 --- a/components/hal/esp32s2/include/hal/ledc_ll.h +++ b/components/hal/esp32s2/include/hal/ledc_ll.h @@ -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 */ @@ -46,7 +46,8 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ if (enable) { DPORT_SET_PERI_REG_MASK(DPORT_PERIP_CLK_EN_REG, DPORT_LEDC_CLK_EN); } else { @@ -61,7 +62,8 @@ static inline void ledc_ll_enable_bus_clock(bool enable) { /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ if (enable) { DPORT_SET_PERI_REG_MASK(DPORT_PERIP_RST_EN_REG, DPORT_LEDC_RST); } else { @@ -73,6 +75,14 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance #define ledc_ll_enable_reset_reg(...) (void)__DECLARE_RCC_ATOMIC_ENV; ledc_ll_enable_reset_reg(__VA_ARGS__) +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No LEDC mem block on S2 +} + /** * @brief Enable LEDC function clock * @@ -81,7 +91,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ //resolve for compatibility } diff --git a/components/hal/esp32s3/include/hal/ledc_ll.h b/components/hal/esp32s3/include/hal/ledc_ll.h index 35c374c10b..56fca02e4a 100644 --- a/components/hal/esp32s3/include/hal/ledc_ll.h +++ b/components/hal/esp32s3/include/hal/ledc_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -41,7 +41,8 @@ extern "C" { * * @param enable Enable/Disable */ -static inline void ledc_ll_enable_bus_clock(bool enable) { +static inline void ledc_ll_enable_bus_clock(bool enable) +{ SYSTEM.perip_clk_en0.ledc_clk_en = enable; } @@ -52,7 +53,8 @@ static inline void ledc_ll_enable_bus_clock(bool enable) { /** * @brief Reset whole peripheral register to init value defined by HW design */ -static inline void ledc_ll_enable_reset_reg(bool enable) { +static inline void ledc_ll_enable_reset_reg(bool enable) +{ SYSTEM.perip_rst_en0.ledc_rst = enable; } @@ -60,6 +62,14 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance #define ledc_ll_enable_reset_reg(...) (void)__DECLARE_RCC_ATOMIC_ENV; ledc_ll_enable_reset_reg(__VA_ARGS__) +/** + * @brief Enable the power for LEDC memory block + */ +static inline void ledc_ll_enable_mem_power(bool enable) +{ + // No LEDC mem block on S3 +} + /** * @brief Enable LEDC function clock * @@ -68,7 +78,8 @@ static inline void ledc_ll_enable_reset_reg(bool enable) { * * @return None */ -static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) { +static inline void ledc_ll_enable_clock(ledc_dev_t *hw, bool en) +{ //resolve for compatibility } diff --git a/components/hal/ledc_hal.c b/components/hal/ledc_hal.c index 6971957124..a5a32394cf 100644 --- a/components/hal/ledc_hal.c +++ b/components/hal/ledc_hal.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 */ @@ -14,6 +14,7 @@ void ledc_hal_init(ledc_hal_context_t *hal, ledc_mode_t speed_mode) //Get hardware instance. hal->dev = LEDC_LL_GET_HW(); hal->speed_mode = speed_mode; + ledc_ll_enable_mem_power(true); } void ledc_hal_get_clk_cfg(ledc_hal_context_t *hal, ledc_timer_t timer_sel, ledc_clk_cfg_t *clk_cfg) @@ -34,9 +35,9 @@ void ledc_hal_get_clk_cfg(ledc_hal_context_t *hal, ledc_timer_t timer_sel, ledc_ #endif { /* If the timer-specific mux is not set to REF_TICK, it either means that: - * - The controler is in fast mode, and thus using APB clock (driver_clk + * - The controller is in fast mode, and thus using APB clock (driver_clk * variable's default value) - * - The controler is in slow mode and so, using a global clock, + * - The controller is in slow mode and so, using a global clock, * so we have to retrieve that clock here. */ if (hal->speed_mode == LEDC_LOW_SPEED_MODE) { From 51a7f7895c24948480d705670e82f146e7f9e681 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Tue, 2 Jul 2024 12:27:30 +0800 Subject: [PATCH 2/2] feat(ledc): add esp_gpio_reserve to ledc driver --- .../esp_driver_ledc/include/driver/ledc.h | 4 +- components/esp_driver_ledc/src/ledc.c | 35 ++++++++---- .../test_apps/ledc/CMakeLists.txt | 5 ++ .../test_apps/ledc/main/test_app_main.c | 2 +- .../test_apps/ledc/main/test_ledc.c | 57 ++++++++++--------- .../test_apps/ledc/pytest_ledc.py | 4 +- 6 files changed, 64 insertions(+), 43 deletions(-) diff --git a/components/esp_driver_ledc/include/driver/ledc.h b/components/esp_driver_ledc/include/driver/ledc.h index 593503c6f6..bd644e1d68 100644 --- a/components/esp_driver_ledc/include/driver/ledc.h +++ b/components/esp_driver_ledc/include/driver/ledc.h @@ -175,13 +175,13 @@ esp_err_t ledc_update_duty(ledc_mode_t speed_mode, ledc_channel_t channel); * * @param gpio_num The LEDC output gpio * @param speed_mode Select the LEDC channel group with specified speed mode. Note that not all targets support high speed mode. - * @param ledc_channel LEDC channel (0 - LEDC_CHANNEL_MAX-1), select from ledc_channel_t + * @param channel LEDC channel (0 - LEDC_CHANNEL_MAX-1), select from ledc_channel_t * * @return * - ESP_OK Success * - ESP_ERR_INVALID_ARG Parameter error */ -esp_err_t ledc_set_pin(int gpio_num, ledc_mode_t speed_mode, ledc_channel_t ledc_channel); +esp_err_t ledc_set_pin(int gpio_num, ledc_mode_t speed_mode, ledc_channel_t channel); /** * @brief LEDC stop. diff --git a/components/esp_driver_ledc/src/ledc.c b/components/esp_driver_ledc/src/ledc.c index bf06d97000..7a64a6e4ad 100644 --- a/components/esp_driver_ledc/src/ledc.c +++ b/components/esp_driver_ledc/src/ledc.c @@ -11,7 +11,6 @@ #include "freertos/idf_additions.h" #include "esp_log.h" #include "esp_check.h" -#include "soc/gpio_periph.h" #include "soc/ledc_periph.h" #include "esp_clk_tree.h" #include "soc/soc_caps.h" @@ -19,10 +18,10 @@ #include "hal/gpio_hal.h" #include "driver/ledc.h" #include "esp_rom_gpio.h" -#include "esp_rom_sys.h" #include "clk_ctrl_os.h" #include "esp_private/periph_ctrl.h" #include "esp_private/gpio.h" +#include "esp_private/esp_gpio_reserve.h" #include "esp_memory_utils.h" static __attribute__((unused)) const char *LEDC_TAG = "ledc"; @@ -88,7 +87,9 @@ typedef struct { #endif } ledc_obj_t; -static ledc_obj_t *p_ledc_obj[LEDC_SPEED_MODE_MAX] = {0}; +static ledc_obj_t *p_ledc_obj[LEDC_SPEED_MODE_MAX] = { + [0 ... LEDC_SPEED_MODE_MAX - 1] = NULL, +}; static ledc_fade_t *s_ledc_fade_rec[LEDC_SPEED_MODE_MAX][LEDC_CHANNEL_MAX]; static ledc_isr_handle_t s_ledc_fade_isr_handle = NULL; static portMUX_TYPE ledc_spinlock = portMUX_INITIALIZER_UNLOCKED; @@ -641,17 +642,30 @@ esp_err_t ledc_timer_config(const ledc_timer_config_t *timer_conf) return ret; } -esp_err_t ledc_set_pin(int gpio_num, ledc_mode_t speed_mode, ledc_channel_t ledc_channel) +esp_err_t _ledc_set_pin(int gpio_num, bool out_inv, ledc_mode_t speed_mode, ledc_channel_t channel) { - LEDC_ARG_CHECK(ledc_channel < LEDC_CHANNEL_MAX, "ledc_channel"); - LEDC_ARG_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(gpio_num), "gpio_num"); - LEDC_ARG_CHECK(speed_mode < LEDC_SPEED_MODE_MAX, "speed_mode"); gpio_func_sel(gpio_num, PIN_FUNC_GPIO); + gpio_set_level(gpio_num, out_inv); gpio_set_direction(gpio_num, GPIO_MODE_OUTPUT); - esp_rom_gpio_connect_out_signal(gpio_num, ledc_periph_signal[speed_mode].sig_out0_idx + ledc_channel, 0, 0); + // reserve the GPIO output path, because we don't expect another peripheral to signal to the same GPIO + uint64_t old_gpio_rsv_mask = esp_gpio_reserve(BIT64(gpio_num)); + // check if the GPIO is already used by others, LEDC signal only uses the output path of the GPIO + if (old_gpio_rsv_mask & BIT64(gpio_num)) { + ESP_LOGW(LEDC_TAG, "GPIO %d is not usable, maybe conflict with others", gpio_num); + } + esp_rom_gpio_connect_out_signal(gpio_num, ledc_periph_signal[speed_mode].sig_out0_idx + channel, out_inv, 0); return ESP_OK; } +// One LEDC channel signal can be directed to multiple IOs as outputs +esp_err_t ledc_set_pin(int gpio_num, ledc_mode_t speed_mode, ledc_channel_t channel) +{ + LEDC_ARG_CHECK(channel < LEDC_CHANNEL_MAX, "channel"); + LEDC_ARG_CHECK(speed_mode < LEDC_SPEED_MODE_MAX, "speed_mode"); + LEDC_ARG_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(gpio_num), "gpio_num"); + return _ledc_set_pin(gpio_num, false, speed_mode, channel); +} + esp_err_t ledc_channel_config(const ledc_channel_config_t *ledc_conf) { LEDC_ARG_CHECK(ledc_conf, "ledc_conf"); @@ -710,10 +724,7 @@ esp_err_t ledc_channel_config(const ledc_channel_config_t *ledc_conf) ESP_LOGD(LEDC_TAG, "LEDC_PWM CHANNEL %"PRIu32"|GPIO %02u|Duty %04"PRIu32"|Time %"PRIu32, ledc_channel, gpio_num, duty, timer_select); /*set LEDC signal in gpio matrix*/ - gpio_func_sel(gpio_num, PIN_FUNC_GPIO); - gpio_set_level(gpio_num, output_invert); - gpio_set_direction(gpio_num, GPIO_MODE_OUTPUT); - esp_rom_gpio_connect_out_signal(gpio_num, ledc_periph_signal[speed_mode].sig_out0_idx + ledc_channel, output_invert, 0); + _ledc_set_pin(gpio_num, output_invert, speed_mode, ledc_channel); return ret; } diff --git a/components/esp_driver_ledc/test_apps/ledc/CMakeLists.txt b/components/esp_driver_ledc/test_apps/ledc/CMakeLists.txt index 1a8d35567e..34bad95b94 100644 --- a/components/esp_driver_ledc/test_apps/ledc/CMakeLists.txt +++ b/components/esp_driver_ledc/test_apps/ledc/CMakeLists.txt @@ -20,3 +20,8 @@ if(CONFIG_COMPILER_DUMP_RTL_FILES) DEPENDS ${elf} ) endif() + +message(STATUS "Checking ledc registers are not read-write by half-word") +include($ENV{IDF_PATH}/tools/ci/check_register_rw_half_word.cmake) +check_register_rw_half_word(SOC_MODULES "ledc" "pcr" "hp_sys_clkrst" + HAL_MODULES "ledc") diff --git a/components/esp_driver_ledc/test_apps/ledc/main/test_app_main.c b/components/esp_driver_ledc/test_apps/ledc/main/test_app_main.c index 3818d8c7de..5beebdc515 100644 --- a/components/esp_driver_ledc/test_apps/ledc/main/test_app_main.c +++ b/components/esp_driver_ledc/test_apps/ledc/main/test_app_main.c @@ -9,7 +9,7 @@ #include "esp_heap_caps.h" // Some resources are lazy allocated in LEDC driver, the threshold is left for that case -#define TEST_MEMORY_LEAK_THRESHOLD (230) +#define TEST_MEMORY_LEAK_THRESHOLD (400) void setUp(void) { diff --git a/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c b/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c index 45d775434b..72c387ddbc 100644 --- a/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c +++ b/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -110,12 +110,12 @@ static void timer_duty_test(ledc_channel_t channel, ledc_timer_bit_t timer_bit, TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); vTaskDelay(5 / portTICK_PERIOD_MS); - // duty ratio: (2^duty)/(2^timer_bit) + // duty ratio: (duty)/(2^timer_bit) timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, 0); timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, 1); - timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, 1 << 12); // 50% duty - timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, (1 << 13) - 1); - timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, (1 << 13) - 2); + timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, 1 << (timer_bit - 1)); // 50% duty + timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, (1 << timer_bit) - 1); + timer_duty_set_get(ledc_ch_config.speed_mode, ledc_ch_config.channel, (1 << timer_bit) - 2); } TEST_CASE("LEDC channel config wrong gpio", "[ledc]") @@ -533,15 +533,6 @@ static void frequency_set_get(ledc_mode_t speed_mode, ledc_timer_t timer, uint32 static void timer_frequency_test(ledc_channel_t channel, ledc_timer_bit_t timer_bit, ledc_timer_t timer, ledc_mode_t speed_mode) { - ledc_channel_config_t ledc_ch_config = { - .gpio_num = PULSE_IO, - .speed_mode = speed_mode, - .channel = channel, - .intr_type = LEDC_INTR_DISABLE, - .timer_sel = timer, - .duty = 4000, - .hpoint = 0, - }; ledc_timer_config_t ledc_time_config = { .speed_mode = speed_mode, .duty_resolution = timer_bit, @@ -549,8 +540,12 @@ static void timer_frequency_test(ledc_channel_t channel, ledc_timer_bit_t timer_ .freq_hz = TEST_PWM_FREQ, .clk_cfg = TEST_DEFAULT_CLK_CFG, }; - TEST_ESP_OK(ledc_channel_config(&ledc_ch_config)); TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); + TEST_ESP_OK(ledc_bind_channel_timer(speed_mode, channel, timer)); + + TEST_ESP_OK(ledc_set_duty(speed_mode, channel, (1 << (timer_bit - 1)))); // 50% duty cycle + TEST_ESP_OK(ledc_update_duty(speed_mode, channel)); + frequency_set_get(speed_mode, timer, 100, 100, 20); #if SOC_CLK_TREE_SUPPORTED frequency_set_get(speed_mode, timer, 5000, 5000, 50); @@ -575,16 +570,23 @@ static void timer_frequency_test(ledc_channel_t channel, ledc_timer_bit_t timer_ TEST_CASE("LEDC set and get frequency", "[ledc][timeout=60]") { setup_testbench(); + + ledc_channel_config_t ledc_ch_config = initialize_channel_config(); #if SOC_LEDC_SUPPORT_HS_MODE + ledc_ch_config.speed_mode = LEDC_HIGH_SPEED_MODE; + TEST_ESP_OK(ledc_channel_config(&ledc_ch_config)); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_0, LEDC_HIGH_SPEED_MODE); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_1, LEDC_HIGH_SPEED_MODE); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_2, LEDC_HIGH_SPEED_MODE); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_3, LEDC_HIGH_SPEED_MODE); #endif // SOC_LEDC_SUPPORT_HS_MODE + ledc_ch_config.speed_mode = LEDC_LOW_SPEED_MODE; + TEST_ESP_OK(ledc_channel_config(&ledc_ch_config)); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_0, LEDC_LOW_SPEED_MODE); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_1, LEDC_LOW_SPEED_MODE); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_2, LEDC_LOW_SPEED_MODE); timer_frequency_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, LEDC_TIMER_3, LEDC_LOW_SPEED_MODE); + tear_testbench(); } @@ -600,6 +602,7 @@ static void timer_set_clk_src_and_freq_test(ledc_mode_t speed_mode, ledc_clk_cfg .clk_cfg = clk_src, }; TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); + TEST_ESP_OK(ledc_update_duty(speed_mode, LEDC_CHANNEL_0)); // Start vTaskDelay(100 / portTICK_PERIOD_MS); if (clk_src == LEDC_USE_RC_FAST_CLK) { // RC_FAST_CLK freq is get from calibration, it is reasonable that divider calculation does a rounding @@ -663,6 +666,16 @@ TEST_CASE("LEDC timer pause and resume", "[ledc]") setup_testbench(); const ledc_mode_t test_speed_mode = TEST_SPEED_MODE; int count; + + ledc_timer_config_t ledc_time_config = { + .speed_mode = test_speed_mode, + .duty_resolution = LEDC_TIMER_13_BIT, + .timer_num = LEDC_TIMER_0, + .freq_hz = TEST_PWM_FREQ, + .clk_cfg = TEST_DEFAULT_CLK_CFG, + }; + TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); + ledc_channel_config_t ledc_ch_config = { .gpio_num = PULSE_IO, .speed_mode = test_speed_mode, @@ -674,15 +687,6 @@ TEST_CASE("LEDC timer pause and resume", "[ledc]") }; TEST_ESP_OK(ledc_channel_config(&ledc_ch_config)); - ledc_timer_config_t ledc_time_config = { - .speed_mode = test_speed_mode, - .duty_resolution = LEDC_TIMER_13_BIT, - .timer_num = LEDC_TIMER_0, - .freq_hz = TEST_PWM_FREQ, - .clk_cfg = TEST_DEFAULT_CLK_CFG, - }; - TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); - vTaskDelay(10 / portTICK_PERIOD_MS); count = wave_count(1000); TEST_ASSERT_INT16_WITHIN(5, TEST_PWM_FREQ, count); @@ -712,11 +716,12 @@ TEST_CASE("LEDC timer pause and resume", "[ledc]") static void ledc_cpu_reset_test_first_stage(void) { + ledc_timer_config_t ledc_time_config = create_default_timer_config(); + TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); + ledc_channel_config_t ledc_ch_config = initialize_channel_config(); TEST_ESP_OK(ledc_channel_config(&ledc_ch_config)); - ledc_timer_config_t ledc_time_config = create_default_timer_config(); - TEST_ESP_OK(ledc_timer_config(&ledc_time_config)); vTaskDelay(50 / portTICK_PERIOD_MS); esp_restart(); } diff --git a/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py b/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py index 672aadb28f..0b7ae925e9 100644 --- a/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py +++ b/components/esp_driver_ledc/test_apps/ledc/pytest_ledc.py @@ -17,7 +17,7 @@ from pytest_embedded_idf import IdfDut indirect=True, ) def test_ledc(dut: IdfDut) -> None: - dut.run_all_single_board_cases() + dut.run_all_single_board_cases(reset=True) @pytest.mark.esp32s3 @@ -31,4 +31,4 @@ def test_ledc(dut: IdfDut) -> None: indirect=True, ) def test_ledc_psram(dut: IdfDut) -> None: - dut.run_all_single_board_cases() + dut.run_all_single_board_cases(reset=True)