From 5070e51dde84e605bbc540cd612850136879c86a Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Tue, 4 Apr 2023 16:28:58 +0800 Subject: [PATCH] ledc: Fix two bugs inside LEDC driver 1. Regression introduced when refactoring on clock sources selection (0d07f859). If channel configuration is called before timer configuration on C6, PWM signal may not be able to output. 2. Missing the improper fade parameter fix inside ledc_set_duty_and_update() function. --- components/driver/ledc/include/driver/ledc.h | 4 ++-- components/driver/ledc/ledc.c | 4 ++-- components/driver/test_apps/ledc/main/test_ledc.c | 6 ++++-- components/hal/esp32c2/include/hal/ledc_ll.h | 2 ++ components/hal/esp32c3/include/hal/ledc_ll.h | 2 ++ components/hal/esp32c6/include/hal/ledc_ll.h | 2 ++ components/hal/esp32h4/include/hal/ledc_ll.h | 2 ++ components/hal/esp32s2/include/hal/ledc_ll.h | 3 +++ components/hal/esp32s3/include/hal/ledc_ll.h | 3 +++ 9 files changed, 22 insertions(+), 6 deletions(-) diff --git a/components/driver/ledc/include/driver/ledc.h b/components/driver/ledc/include/driver/ledc.h index 509b81634d..c0e2f14530 100644 --- a/components/driver/ledc/include/driver/ledc.h +++ b/components/driver/ledc/include/driver/ledc.h @@ -450,10 +450,10 @@ esp_err_t ledc_fade_start(ledc_mode_t speed_mode, ledc_channel_t channel, ledc_f #if SOC_LEDC_SUPPORT_FADE_STOP /** - * @brief Stop LEDC fading. Duty of the channel will stay at its present vlaue. + * @brief Stop LEDC fading. The duty of the channel is garanteed to be fixed at most one PWM cycle after the function returns. * @note This API can be called if a new fixed duty or a new fade want to be set while the last fade operation is still running in progress. * @note Call this API will abort the fading operation only if it was started by calling ledc_fade_start with LEDC_FADE_NO_WAIT mode. - * @note If a fade was started with LEDC_FADE_WAIT_DONE mode, calling this API afterwards is no use in stopping the fade. Fade will continue until it reachs the target duty. + * @note If a fade was started with LEDC_FADE_WAIT_DONE mode, calling this API afterwards HAS no use in stopping the fade. Fade will continue until it reachs the target duty. * @param speed_mode Select the LEDC channel group with specified speed mode. Note that not all targets support high speed mode. * @param channel LEDC channel number * diff --git a/components/driver/ledc/ledc.c b/components/driver/ledc/ledc.c index 3e129913ba..929a35ea6d 100644 --- a/components/driver/ledc/ledc.c +++ b/components/driver/ledc/ledc.c @@ -599,7 +599,7 @@ esp_err_t ledc_channel_config(const ledc_channel_config_t *ledc_conf) // Set channel configurations and update bits before core clock is on could lead to error // Therefore, we should connect the core clock to a real clock source to make it on before any ledc register operation // It can be switched to the other desired clock sources to meet the output pwm freq requirement later at timer configuration - ledc_hal_set_slow_clk_sel(&(p_ledc_obj[speed_mode]->ledc_hal), 1); + ledc_hal_set_slow_clk_sel(&(p_ledc_obj[speed_mode]->ledc_hal), LEDC_LL_GLOBAL_CLK_DEFAULT); #endif } @@ -1214,7 +1214,7 @@ esp_err_t ledc_set_duty_and_update(ledc_mode_t speed_mode, ledc_channel_t channe LEDC_CHECK(ledc_fade_channel_init_check(speed_mode, channel) == ESP_OK, LEDC_FADE_INIT_ERROR_STR, ESP_FAIL); _ledc_fade_hw_acquire(speed_mode, channel); portENTER_CRITICAL(&ledc_spinlock); - ledc_duty_config(speed_mode, channel, hpoint, duty, 1, 0, 0, 0); + ledc_duty_config(speed_mode, channel, hpoint, duty, 1, 1, 1, 0); _ledc_update_duty(speed_mode, channel); portEXIT_CRITICAL(&ledc_spinlock); _ledc_fade_hw_release(speed_mode, channel); diff --git a/components/driver/test_apps/ledc/main/test_ledc.c b/components/driver/test_apps/ledc/main/test_ledc.c index c37a6cd8db..3e3beee44a 100644 --- a/components/driver/test_apps/ledc/main/test_ledc.c +++ b/components/driver/test_apps/ledc/main/test_ledc.c @@ -375,11 +375,13 @@ TEST_CASE("LEDC fade stop test", "[ledc]") // Get duty value right before stopping the fade uint32_t duty_before_stop = ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0); TEST_ESP_OK(ledc_fade_stop(test_speed_mode, LEDC_CHANNEL_0)); + // PWM signal is 2000 Hz. It may take one cycle (500 us) at maximum to stablize the duty. + esp_rom_delay_us(500); + // Get duty value now, which is at least one cycle after the ledc_fade_stop function returns + uint32_t duty_after_stop = ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0); fade_stop = esp_timer_get_time(); time_ms = (fade_stop - fade_start) / 1000; TEST_ASSERT_TRUE(llabs(time_ms - 127) < 20); - // Get duty value after fade_stop returns (give at least one cycle for the duty set in fade_stop to take effective) - uint32_t duty_after_stop = ledc_get_duty(test_speed_mode, LEDC_CHANNEL_0); TEST_ASSERT_INT32_WITHIN(4, duty_before_stop, duty_after_stop); // 4 is the scale for one step in the last fade vTaskDelay(300 / portTICK_PERIOD_MS); // Duty should not change any more after ledc_fade_stop returns diff --git a/components/hal/esp32c2/include/hal/ledc_ll.h b/components/hal/esp32c2/include/hal/ledc_ll.h index 3460ef5a40..3b8d91f6a6 100644 --- a/components/hal/esp32c2/include/hal/ledc_ll.h +++ b/components/hal/esp32c2/include/hal/ledc_ll.h @@ -32,6 +32,8 @@ extern "C" { LEDC_SLOW_CLK_RC_FAST, \ } +#define LEDC_LL_GLOBAL_CLK_DEFAULT LEDC_SLOW_CLK_RC_FAST + /** * @brief Set LEDC low speed timer clock diff --git a/components/hal/esp32c3/include/hal/ledc_ll.h b/components/hal/esp32c3/include/hal/ledc_ll.h index d254373325..0edca33890 100644 --- a/components/hal/esp32c3/include/hal/ledc_ll.h +++ b/components/hal/esp32c3/include/hal/ledc_ll.h @@ -33,6 +33,8 @@ extern "C" { LEDC_SLOW_CLK_RC_FAST, \ } +#define LEDC_LL_GLOBAL_CLK_DEFAULT LEDC_SLOW_CLK_RC_FAST + /** * @brief Set LEDC low speed timer clock * diff --git a/components/hal/esp32c6/include/hal/ledc_ll.h b/components/hal/esp32c6/include/hal/ledc_ll.h index 1dcd353c38..49eaa0a098 100644 --- a/components/hal/esp32c6/include/hal/ledc_ll.h +++ b/components/hal/esp32c6/include/hal/ledc_ll.h @@ -34,6 +34,8 @@ extern "C" { LEDC_SLOW_CLK_RC_FAST, \ } +#define LEDC_LL_GLOBAL_CLK_DEFAULT LEDC_SLOW_CLK_RC_FAST + /** * @brief Enable LEDC function clock diff --git a/components/hal/esp32h4/include/hal/ledc_ll.h b/components/hal/esp32h4/include/hal/ledc_ll.h index d254373325..0edca33890 100644 --- a/components/hal/esp32h4/include/hal/ledc_ll.h +++ b/components/hal/esp32h4/include/hal/ledc_ll.h @@ -33,6 +33,8 @@ extern "C" { LEDC_SLOW_CLK_RC_FAST, \ } +#define LEDC_LL_GLOBAL_CLK_DEFAULT LEDC_SLOW_CLK_RC_FAST + /** * @brief Set LEDC low speed timer clock * diff --git a/components/hal/esp32s2/include/hal/ledc_ll.h b/components/hal/esp32s2/include/hal/ledc_ll.h index 0f7e08e3d8..fbdd6cc2f1 100644 --- a/components/hal/esp32s2/include/hal/ledc_ll.h +++ b/components/hal/esp32s2/include/hal/ledc_ll.h @@ -38,6 +38,9 @@ extern "C" { #define LEDC_LL_IS_TIMER_SPECIFIC_CLOCK(SPEED, CLK) ((CLK) == LEDC_USE_REF_TICK) +#define LEDC_LL_GLOBAL_CLK_DEFAULT LEDC_SLOW_CLK_RC_FAST + + /** * @brief Set LEDC low speed timer clock * diff --git a/components/hal/esp32s3/include/hal/ledc_ll.h b/components/hal/esp32s3/include/hal/ledc_ll.h index d254373325..101443a2f0 100644 --- a/components/hal/esp32s3/include/hal/ledc_ll.h +++ b/components/hal/esp32s3/include/hal/ledc_ll.h @@ -33,6 +33,9 @@ extern "C" { LEDC_SLOW_CLK_RC_FAST, \ } +#define LEDC_LL_GLOBAL_CLK_DEFAULT LEDC_SLOW_CLK_RC_FAST + + /** * @brief Set LEDC low speed timer clock *