fix(ledc): duty_start bit should wait for its self-clear before next set on esp32

This commit is contained in:
Song Ruo Jing
2025-06-17 20:11:17 +08:00
parent 92b84443f3
commit d87de032df
14 changed files with 38 additions and 41 deletions

View File

@@ -975,7 +975,7 @@ esp_err_t ledc_channel_config(const ledc_channel_config_t *ledc_conf)
static void _ledc_update_duty(ledc_mode_t speed_mode, ledc_channel_t channel)
{
ledc_hal_set_sig_out_en(&(p_ledc_obj[speed_mode]->ledc_hal), channel, true);
ledc_hal_set_duty_start(&(p_ledc_obj[speed_mode]->ledc_hal), channel, true);
ledc_hal_set_duty_start(&(p_ledc_obj[speed_mode]->ledc_hal), channel);
ledc_ls_channel_update(speed_mode, channel);
}
@@ -998,7 +998,6 @@ esp_err_t ledc_stop(ledc_mode_t speed_mode, ledc_channel_t channel, uint32_t idl
portENTER_CRITICAL_SAFE(&ledc_spinlock);
ledc_hal_set_idle_level(&(p_ledc_obj[speed_mode]->ledc_hal), channel, idle_level);
ledc_hal_set_sig_out_en(&(p_ledc_obj[speed_mode]->ledc_hal), channel, false);
ledc_hal_set_duty_start(&(p_ledc_obj[speed_mode]->ledc_hal), channel, false);
ledc_ls_channel_update(speed_mode, channel);
portEXIT_CRITICAL_SAFE(&ledc_spinlock);
return ESP_OK;
@@ -1261,7 +1260,7 @@ static void IRAM_ATTR ledc_fade_isr(void *arg)
cycle,
scale);
s_ledc_fade_rec[speed_mode][channel]->fsm = LEDC_FSM_HW_FADE;
ledc_hal_set_duty_start(&(p_ledc_obj[speed_mode]->ledc_hal), channel, true);
ledc_hal_set_duty_start(&(p_ledc_obj[speed_mode]->ledc_hal), channel);
ledc_ls_channel_update(speed_mode, channel);
}
portEXIT_CRITICAL_ISR(&ledc_spinlock);

View File

@@ -150,6 +150,12 @@ TEST_CASE("LEDC output idle level test", "[ledc]")
TEST_ESP_OK(ledc_stop(test_speed_mode, LEDC_CHANNEL_0, !current_level));
vTaskDelay(1000 / portTICK_PERIOD_MS);
TEST_ASSERT_EQUAL_INT32(!current_level, LEDC.channel_group[test_speed_mode].channel[LEDC_CHANNEL_0].conf0.idle_lv);
// check real output level over some period
gpio_input_enable(PULSE_IO);
for (int i = 0; i < 40; i++) {
TEST_ASSERT_EQUAL_INT32(!current_level, gpio_get_level(PULSE_IO));
esp_rom_delay_us(50);
}
}
TEST_CASE("LEDC iterate over all channel and timer configs", "[ledc]")

View File

@@ -481,13 +481,15 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, high-speed mode or low-speed mode
* @param channel_num LEDC channel index (0-7), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
// wait until the last duty change took effect (duty_start bit will be self-cleared when duty update or fade is done)
// this is necessary on ESP32 only, otherwise, internal logic might mess up (later targets with SOC_LEDC_SUPPORT_FADE_STOP allow to re-configure parameters while last update is still in progress)
while (hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start);
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -458,13 +458,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, high-speed mode or low-speed mode
* @param channel_num LEDC channel index (0-7), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -459,13 +459,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, high-speed mode or low-speed mode
* @param channel_num LEDC channel index (0-7), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -458,13 +458,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, low-speed mode only
* @param channel_num LEDC channel index (0-5), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -579,13 +579,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, low-speed mode only
* @param channel_num LEDC channel index (0-5), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -458,13 +458,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, low-speed mode only
* @param channel_num LEDC channel index (0-5), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -577,13 +577,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, low-speed mode only
* @param channel_num LEDC channel index (0-5), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -484,13 +484,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, low-speed mode only
* @param channel_num LEDC channel index (0-5), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -498,13 +498,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, high-speed mode or low-speed mode
* @param channel_num LEDC channel index (0-7), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -459,13 +459,12 @@ static inline void ledc_ll_set_sig_out_en(ledc_dev_t *hw, ledc_mode_t speed_mode
* @param hw Beginning address of the peripheral registers
* @param speed_mode LEDC speed_mode, high-speed mode or low-speed mode
* @param channel_num LEDC channel index (0-7), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num, bool duty_start)
static inline void ledc_ll_set_duty_start(ledc_dev_t *hw, ledc_mode_t speed_mode, ledc_channel_t channel_num)
{
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = duty_start;
hw->channel_group[speed_mode].channel[channel_num].conf1.duty_start = 1;
}
/**

View File

@@ -263,11 +263,10 @@ void ledc_hal_ls_channel_update(ledc_hal_context_t *hal, ledc_channel_t channel_
*
* @param hal Context of the HAL layer
* @param channel_num LEDC channel index (0-7), select from ledc_channel_t
* @param duty_start The duty start
*
* @return None
*/
void ledc_hal_set_duty_start(ledc_hal_context_t *hal, ledc_channel_t channel_num, bool duty_start);
void ledc_hal_set_duty_start(ledc_hal_context_t *hal, ledc_channel_t channel_num);
/**
* @brief Set LEDC the integer part of duty value

View File

@@ -16,9 +16,9 @@ void ledc_hal_ls_channel_update(ledc_hal_context_t *hal, ledc_channel_t channel_
ledc_ll_ls_channel_update(hal->dev, hal->speed_mode, channel_num);
}
void ledc_hal_set_duty_start(ledc_hal_context_t *hal, ledc_channel_t channel_num, bool duty_start)
void ledc_hal_set_duty_start(ledc_hal_context_t *hal, ledc_channel_t channel_num)
{
ledc_ll_set_duty_start(hal->dev, hal->speed_mode, channel_num, duty_start);
ledc_ll_set_duty_start(hal->dev, hal->speed_mode, channel_num);
}
void ledc_hal_set_duty_int_part(ledc_hal_context_t *hal, ledc_channel_t channel_num, uint32_t duty_val)