From 1e3dbeb6a82619fd3ffc0965958f1d2896e79034 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Thu, 5 Dec 2024 11:39:44 +0800 Subject: [PATCH 1/3] fix(ledc): fix ledc driver coverity issues --- components/esp_driver_ledc/src/ledc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/esp_driver_ledc/src/ledc.c b/components/esp_driver_ledc/src/ledc.c index 841d193a46..d7ecef8bcf 100644 --- a/components/esp_driver_ledc/src/ledc.c +++ b/components/esp_driver_ledc/src/ledc.c @@ -1335,6 +1335,7 @@ static esp_err_t _ledc_set_fade_with_step(ledc_mode_t speed_mode, ledc_channel_t ledc_hal_get_duty(&(p_ledc_obj[speed_mode]->ledc_hal), channel, &duty_cur); // When duty == max_duty, meanwhile, if scale == 1 and fade_down == 1, counter would overflow. if (duty_cur == ledc_get_max_duty(speed_mode, channel)) { + assert(duty_cur > 0); duty_cur -= 1; } s_ledc_fade_rec[speed_mode][channel]->speed_mode = speed_mode; @@ -1776,7 +1777,7 @@ esp_err_t ledc_fill_multi_fade_param_list(ledc_mode_t speed_mode, ledc_channel_t } surplus_cycles_last_phase = cycles_per_phase - step * cycle; // If next phase is the last one, then account for all remaining duty and cycles - if (i == linear_phase_num - 2) { + if (linear_phase_num >= 2 && i == linear_phase_num - 2) { phase_tail = end_duty; surplus_cycles_last_phase += total_cycles - avg_cycles_per_phase * linear_phase_num; } From 83c244ecad30c4e777bdf04eb9cccc7dc3bf314f Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Thu, 5 Dec 2024 11:51:50 +0800 Subject: [PATCH 2/3] fix(ledc): fix ledc_get_freq calculation err due to overflow Closes https://github.com/espressif/esp-idf/pull/14882 --- components/esp_driver_ledc/src/ledc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/esp_driver_ledc/src/ledc.c b/components/esp_driver_ledc/src/ledc.c index d7ecef8bcf..be71b40638 100644 --- a/components/esp_driver_ledc/src/ledc.c +++ b/components/esp_driver_ledc/src/ledc.c @@ -1117,7 +1117,7 @@ uint32_t ledc_get_freq(ledc_mode_t speed_mode, ledc_timer_t timer_num) ledc_hal_get_clock_divider(&(p_ledc_obj[speed_mode]->ledc_hal), timer_num, &clock_divider); ledc_hal_get_duty_resolution(&(p_ledc_obj[speed_mode]->ledc_hal), timer_num, &duty_resolution); ledc_hal_get_clk_cfg(&(p_ledc_obj[speed_mode]->ledc_hal), timer_num, &clk_cfg); - uint32_t precision = (0x1 << duty_resolution); + uint64_t precision = (0x1 << duty_resolution); uint32_t src_clk_freq = 0; esp_clk_tree_src_get_freq_hz((soc_module_clk_t)clk_cfg, LEDC_CLK_SRC_FREQ_PRECISION, &src_clk_freq); portEXIT_CRITICAL(&ledc_spinlock); From 1cd5736e759c688e6cc2aa8aaa9267a63a138606 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Thu, 5 Dec 2024 15:10:54 +0800 Subject: [PATCH 3/3] refactor(ledc): deprecate ledc_timer_set API Closes https://github.com/espressif/esp-idf/issues/14884 --- components/esp_driver_ledc/include/driver/ledc.h | 10 ++++++++-- components/esp_driver_ledc/src/ledc.c | 12 +++++++++--- docs/en/api-reference/peripherals/ledc.rst | 7 +++---- docs/zh_CN/api-reference/peripherals/ledc.rst | 7 +++---- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/components/esp_driver_ledc/include/driver/ledc.h b/components/esp_driver_ledc/include/driver/ledc.h index 4245ae83da..1e209d5b2b 100644 --- a/components/esp_driver_ledc/include/driver/ledc.h +++ b/components/esp_driver_ledc/include/driver/ledc.h @@ -350,7 +350,13 @@ esp_err_t ledc_set_fade(ledc_mode_t speed_mode, ledc_channel_t channel, uint32_t esp_err_t ledc_isr_register(void (*fn)(void *), void *arg, int intr_alloc_flags, ledc_isr_handle_t *handle); /** - * @brief Configure LEDC settings + * @brief Configure LEDC timer settings + * + * This function does not take care of whether the chosen clock source is enabled or not, also does not handle the clock source + * to meet channel sleep mode choice. + * + * If the chosen clock source is a new clock source to the LEDC timer, please use `ledc_timer_config`; + * If the clock source is kept to be the same, but frequency needs to be updated, please use `ledc_set_freq`. * * @param speed_mode Select the LEDC channel group with specified speed mode. Note that not all targets support high speed mode. * @param timer_sel Timer index (0-3), there are 4 timers in LEDC module @@ -362,7 +368,7 @@ esp_err_t ledc_isr_register(void (*fn)(void *), void *arg, int intr_alloc_flags, * - (-1) Parameter error * - Other Current LEDC duty */ -esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_t clock_divider, uint32_t duty_resolution, ledc_clk_src_t clk_src); +esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_t clock_divider, uint32_t duty_resolution, ledc_clk_src_t clk_src) __attribute__((deprecated("Please use ledc_timer_config() or ledc_set_freq()"))); /** * @brief Reset LEDC timer diff --git a/components/esp_driver_ledc/src/ledc.c b/components/esp_driver_ledc/src/ledc.c index be71b40638..fdc9a042ef 100644 --- a/components/esp_driver_ledc/src/ledc.c +++ b/components/esp_driver_ledc/src/ledc.c @@ -248,8 +248,7 @@ static uint32_t ledc_get_max_duty(ledc_mode_t speed_mode, ledc_channel_t channel return max_duty; } -esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_t clock_divider, uint32_t duty_resolution, - ledc_clk_src_t clk_src) +static esp_err_t ledc_set_timer_params(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_t clock_divider, uint32_t duty_resolution, ledc_clk_src_t clk_src) { LEDC_ARG_CHECK(speed_mode < LEDC_SPEED_MODE_MAX, "speed_mode"); LEDC_ARG_CHECK(timer_sel < LEDC_TIMER_MAX, "timer_select"); @@ -268,6 +267,13 @@ esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_ return ESP_OK; } +// Deprecated public API +esp_err_t ledc_timer_set(ledc_mode_t speed_mode, ledc_timer_t timer_sel, uint32_t clock_divider, uint32_t duty_resolution, + ledc_clk_src_t clk_src) +{ + return ledc_set_timer_params(speed_mode, timer_sel, clock_divider, duty_resolution, clk_src); +} + static IRAM_ATTR esp_err_t ledc_duty_config(ledc_mode_t speed_mode, ledc_channel_t channel, int hpoint_val, int duty_val, ledc_duty_direction_t duty_direction, uint32_t duty_num, uint32_t duty_cycle, uint32_t duty_scale) { @@ -711,7 +717,7 @@ static esp_err_t ledc_set_timer_div(ledc_mode_t speed_mode, ledc_timer_t timer_n } /* The divisor is correct, we can write in the hardware. */ - ledc_timer_set(speed_mode, timer_num, div_param, duty_resolution, timer_clk_src); + ledc_set_timer_params(speed_mode, timer_num, div_param, duty_resolution, timer_clk_src); portENTER_CRITICAL(&ledc_spinlock); if (p_ledc_obj[speed_mode]->timer_xpd_ref_cnt[timer_num] > 0 && !p_ledc_obj[speed_mode]->glb_clk_xpd) { diff --git a/docs/en/api-reference/peripherals/ledc.rst b/docs/en/api-reference/peripherals/ledc.rst index 8005b6b558..317ce17b70 100644 --- a/docs/en/api-reference/peripherals/ledc.rst +++ b/docs/en/api-reference/peripherals/ledc.rst @@ -319,14 +319,13 @@ The LEDC API provides several ways to change the PWM frequency "on the fly": More Control Over PWM """"""""""""""""""""" -There are several lower level timer-specific functions that can be used to change PWM settings: +There are several individual timer-specific functions that can be used to change PWM output: -* :cpp:func:`ledc_timer_set` * :cpp:func:`ledc_timer_rst` * :cpp:func:`ledc_timer_pause` * :cpp:func:`ledc_timer_resume` -The first two functions are called "behind the scenes" by :cpp:func:`ledc_channel_config` to provide a startup of a timer after it is configured. +The first function is called "behind the scenes" by :cpp:func:`ledc_timer_config` to provide a startup of a timer after it is configured. Use Interrupts @@ -356,7 +355,7 @@ If signal output needs to be maintained in Light-sleep, then select :cpp:enumera LEDC High and Low Speed Mode ---------------------------- - High speed mode enables a glitch-free changeover of timer settings. This means that if the timer settings are modified, the changes will be applied automatically on the next overflow interrupt of the timer. In contrast, when updating the low-speed timer, the change of settings should be explicitly triggered by software. The LEDC driver handles it in the background, e.g., when :cpp:func:`ledc_timer_config` or :cpp:func:`ledc_timer_set` is called. + High speed mode enables a glitch-free changeover of timer settings. This means that if the timer settings are modified, the changes will be applied automatically on the next overflow interrupt of the timer. In contrast, when updating the low-speed timer, the change of settings should be explicitly triggered by software. The LEDC driver handles it in the background, e.g., when :cpp:func:`ledc_timer_config` is called. For additional details regarding speed modes, see **{IDF_TARGET_NAME} Technical Reference Manual** > **LED PWM Controller (LEDC)** [`PDF <{IDF_TARGET_TRM_EN_URL}#ledpwm>`__]. diff --git a/docs/zh_CN/api-reference/peripherals/ledc.rst b/docs/zh_CN/api-reference/peripherals/ledc.rst index 51ce5785ec..3137f3c4f1 100644 --- a/docs/zh_CN/api-reference/peripherals/ledc.rst +++ b/docs/zh_CN/api-reference/peripherals/ledc.rst @@ -319,14 +319,13 @@ LED PWM 控制器 API 有多种方式即时改变 PWM 频率： 控制 PWM 的更多方式 """"""""""""""""""""" -有一些较底层的定时器特定函数可用于更改 PWM 设置： +有一些较独立的定时器特定函数可用于更改 PWM 输出： -* :cpp:func:`ledc_timer_set` * :cpp:func:`ledc_timer_rst` * :cpp:func:`ledc_timer_pause` * :cpp:func:`ledc_timer_resume` -前两个功能可通过函数 :cpp:func:`ledc_channel_config` 在后台运行，在定时器配置后启动。 +第一个定时器复位函数在函数 :cpp:func:`ledc_timer_config` 内部完成所有定时器配置后会被调用一次。 使用中断 @@ -356,7 +355,7 @@ LEDC 驱动不使用电源管理锁来防止系统进入 Light-sleep 。相反 LED PWM 控制器高速和低速模式 ---------------------------------- - 高速模式的优点是可平稳地改变定时器设置。也就是说，高速模式下如定时器设置改变，此变更会自动应用于定时器的下一次溢出中断。而更新低速定时器时，设置变更应由软件显式触发。LED PWM 驱动的设置将在硬件层面被修改，比如在调用函数 :cpp:func:`ledc_timer_config` 或 :cpp:func:`ledc_timer_set` 时。 + 高速模式的优点是可平稳地改变定时器设置。也就是说，高速模式下如定时器设置改变，此变更会自动应用于定时器的下一次溢出中断。而更新低速定时器时，设置变更应由软件显式触发。LED PWM 驱动的设置将在硬件层面被修改，比如在调用函数 :cpp:func:`ledc_timer_config` 时。 更多关于速度模式的详细信息请参阅 **{IDF_TARGET_NAME} 技术参考手册** > **LED PWM 控制器 (LEDC)** [`PDF <{IDF_TARGET_TRM_EN_URL}#ledpwm>`__]。