feat(ledc): add esp_gpio_reserve to ledc driver

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.

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);
+    _ledc_set_pin(gpio_num, output_invert, speed_mode, ledc_channel);
-    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);
 
     return ret;
 }

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")

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)
 {

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 << (timer_bit - 1)); // 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 << timer_bit) - 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) - 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();
 }

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)