ledc: Move ledc unit test to test_apps pytest framework

Add ledc_hal_set_duty_start and ledc_hal_set_duty_int_part to IRAM
Fix test_ledc.c minor issues

components/driver/.build-test-rules.yml

@@ -34,6 +34,10 @@ components/driver/test_apps/i2s_test_apps/legacy_i2s_driver:
       temporary: true
       reason: target esp32c6 is not supported yet
 
+components/driver/test_apps/ledc:
+  disable:
+    - if: SOC_LEDC_SUPPORTED != 1
+
 components/driver/test_apps/legacy_adc_driver:
   disable:
     - if: IDF_TARGET == "esp32c6"

components/driver/ledc.c

@@ -843,7 +843,7 @@ uint32_t ledc_get_freq(ledc_mode_t speed_mode, ledc_timer_t timer_num)
     return ((uint64_t) src_clk_freq << 8) / precision / clock_divider;
 }
 
-static inline void ledc_calc_fade_end_channel(uint32_t *fade_end_status, uint32_t *channel)
+static inline void IRAM_ATTR ledc_calc_fade_end_channel(uint32_t *fade_end_status, uint32_t *channel)
 {
     uint32_t i = __builtin_ffs((*fade_end_status)) - 1;
     (*fade_end_status) &= ~(1 << i);

components/driver/test_apps/ledc/CMakeLists.txt

@@ -0,0 +1,18 @@
+# This is the project CMakeLists.txt file for the test subproject
+cmake_minimum_required(VERSION 3.16)
+
+include($ENV{IDF_PATH}/tools/cmake/project.cmake)
+project(ledc_test)
+
+if(CONFIG_COMPILER_DUMP_RTL_FILES)
+    add_custom_target(check_test_app_sections ALL
+                      COMMAND ${PYTHON} $ENV{IDF_PATH}/tools/ci/check_callgraph.py
+                      --rtl-dir ${CMAKE_BINARY_DIR}/esp-idf/driver/
+                      --elf-file ${CMAKE_BINARY_DIR}/ledc_test.elf
+                      find-refs
+                      --from-sections=.iram0.text
+                      --to-sections=.flash.text,.flash.rodata
+                      --exit-code
+                      DEPENDS ${elf}
+                      )
+endif()

components/driver/test_apps/ledc/README.md

@@ -0,0 +1,2 @@
+| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C6 | ESP32-S2 | ESP32-S3 |
+| ----------------- | ----- | -------- | -------- | -------- | -------- | -------- |

components/driver/test_apps/ledc/main/CMakeLists.txt

@@ -0,0 +1,7 @@
+set(srcs "test_app_main.c"
+         "test_ledc.c")
+
+# In order for the cases defined by `TEST_CASE` to be linked into the final elf,
+# the component can be registered as WHOLE_ARCHIVE
+idf_component_register(SRCS ${srcs}
+                       WHOLE_ARCHIVE)

components/driver/test_apps/ledc/main/test_app_main.c

@@ -0,0 +1,36 @@
+/*
+ * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include "unity.h"
+#include "unity_test_utils.h"
+#include "unity_test_runner.h"
+#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 (150)
+
+static size_t before_free_8bit;
+static size_t before_free_32bit;
+
+void setUp(void)
+{
+    before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
+    before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);
+}
+
+void tearDown(void)
+{
+    size_t after_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
+    size_t after_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);
+    printf("\n");
+    unity_utils_check_leak(before_free_8bit, after_free_8bit, "8BIT", TEST_MEMORY_LEAK_THRESHOLD);
+    unity_utils_check_leak(before_free_32bit, after_free_32bit, "32BIT", TEST_MEMORY_LEAK_THRESHOLD);
+}
+
+void app_main(void)
+{
+    unity_run_menu();
+}

components/driver/test_apps/ledc/main/test_ledc.c

@@ -10,23 +10,16 @@
  * real duty = duty/2^duty_resolution
  */
 #include <esp_types.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <malloc.h>
 #include <string.h>
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
-#include "freertos/semphr.h"
-#include "freertos/queue.h"
 #include "unity.h"
-#include "soc/ledc_periph.h"
 #include "soc/gpio_periph.h"
 #include "soc/io_mux_reg.h"
 #include "esp_system.h"
 #include "esp_timer.h"
 #include "driver/ledc.h"
-#include "hal/ledc_ll.h"
-#include "driver/gpio.h"
+#include "soc/ledc_struct.h"
 
 #define PULSE_IO      18
 
@@ -197,7 +190,7 @@ TEST_CASE("LEDC output idle level test", "[ledc]")
     uint32_t current_level = LEDC.channel_group[test_speed_mode].channel[LEDC_CHANNEL_0].conf0.idle_lv;
     TEST_ESP_OK(ledc_stop(test_speed_mode, LEDC_CHANNEL_0, !current_level));
     vTaskDelay(1000 / portTICK_PERIOD_MS);
-    TEST_ASSERT_EQUAL_INT32(LEDC.channel_group[test_speed_mode].channel[LEDC_CHANNEL_0].conf0.idle_lv, !current_level);
+    TEST_ASSERT_EQUAL_INT32(!current_level, LEDC.channel_group[test_speed_mode].channel[LEDC_CHANNEL_0].conf0.idle_lv);
 }
 
 TEST_CASE("LEDC iterate over all channel and timer configs", "[ledc]")
@@ -207,17 +200,15 @@ TEST_CASE("LEDC iterate over all channel and timer configs", "[ledc]")
 
     // use all kinds of speed mode, channel, timer combination to test all of possible configuration
     ledc_mode_t speed_mode[LEDC_SPEED_MODE_MAX] = SPEED_MODE_LIST;
-    ledc_channel_t channels[8] = {LEDC_CHANNEL_0, LEDC_CHANNEL_1, LEDC_CHANNEL_2, LEDC_CHANNEL_3, LEDC_CHANNEL_4, LEDC_CHANNEL_5};
-    ledc_timer_t timer_select[4] = {LEDC_TIMER_0, LEDC_TIMER_1, LEDC_TIMER_2, LEDC_TIMER_3};
 
     for (int i = 0; i < LEDC_SPEED_MODE_MAX; i++) {
         ledc_ch_config.speed_mode = speed_mode[i];
         ledc_time_config.speed_mode = speed_mode[i];
-        for (int j = 0; j < 8; j++) {
-            ledc_ch_config.channel = channels[j];
-            for (int k = 0; k < 4; k++) {
-                ledc_ch_config.timer_sel = timer_select[k];
-                ledc_time_config.timer_num = timer_select[k];
+        for (int j = 0; j < LEDC_CHANNEL_MAX; j++) {
+            ledc_ch_config.channel = (ledc_channel_t)j;
+            for (int k = 0; k < LEDC_TIMER_MAX; k++) {
+                ledc_ch_config.timer_sel = (ledc_timer_t)k;
+                ledc_time_config.timer_num = (ledc_timer_t)k;
                 TEST_ESP_OK(ledc_channel_config(&ledc_ch_config));
                 TEST_ESP_OK(ledc_timer_config(&ledc_time_config));
             }
@@ -246,12 +237,12 @@ TEST_CASE("LEDC memory leak test", "[ledc]")
 }
 
 // duty should be manually checked from the waveform using a logic analyzer
-// this test is enabled only for testting the settings
+// this test is enabled only for testing the settings
 TEST_CASE("LEDC set and get duty", "[ledc]")
 {
     ledc_timer_t timer_list[4] = {LEDC_TIMER_0, LEDC_TIMER_1, LEDC_TIMER_2, LEDC_TIMER_3};
     ledc_mode_t speed_mode_list[LEDC_SPEED_MODE_MAX] = SPEED_MODE_LIST;
-    for (int i = 0; i < LEDC_TIMER_MAX - 1; i++) {
+    for (int i = 0; i < LEDC_TIMER_MAX; i++) {
         for (int j = 0; j < LEDC_SPEED_MODE_MAX; j++) {
             timer_duty_test(LEDC_CHANNEL_0, LEDC_TIMER_13_BIT, timer_list[i], speed_mode_list[j]);
         }
@@ -452,7 +443,7 @@ static void frequency_set_get(ledc_mode_t speed_mode, ledc_timer_t timer, uint32
     int count;
     TEST_ESP_OK(ledc_set_freq(speed_mode, timer, freq_hz));
     count = wave_count(1000);
-    TEST_ASSERT_INT16_WITHIN(error, count, real_freq);
+    TEST_ASSERT_INT16_WITHIN(error, real_freq, count);
     TEST_ASSERT_EQUAL_INT32(real_freq, ledc_get_freq(speed_mode, timer));
 }
 
@@ -481,7 +472,7 @@ static void timer_frequency_test(ledc_channel_t channel, ledc_timer_bit_t timer_
     frequency_set_get(ledc_ch_config.speed_mode, ledc_ch_config.timer_sel, 9000, 8992, 50);
 }
 
-TEST_CASE("LEDC set and get frequency", "[ledc][timeout=60][ignore]")
+TEST_CASE("LEDC set and get frequency", "[ledc][timeout=60]")
 {
     setup_testbench();
 #if SOC_LEDC_SUPPORT_HS_MODE
@@ -511,12 +502,12 @@ static void timer_set_clk_src_and_freq_test(ledc_mode_t speed_mode, ledc_clk_cfg
     vTaskDelay(100 / portTICK_PERIOD_MS);
     if (clk_src == LEDC_USE_RTC8M_CLK) {
         // RTC8M_CLK freq is get from calibration, it is reasonable that divider calculation does a rounding
-        TEST_ASSERT_UINT32_WITHIN(5, ledc_get_freq(speed_mode, LEDC_TIMER_0), freq_hz);
+        TEST_ASSERT_UINT32_WITHIN(5, freq_hz, ledc_get_freq(speed_mode, LEDC_TIMER_0));
     } else {
-        TEST_ASSERT_EQUAL_INT32(ledc_get_freq(speed_mode, LEDC_TIMER_0), freq_hz);
+        TEST_ASSERT_EQUAL_INT32(freq_hz, ledc_get_freq(speed_mode, LEDC_TIMER_0));
     }
     int count = wave_count(1000);
-    TEST_ASSERT_UINT32_WITHIN(10, count, freq_hz);
+    TEST_ASSERT_UINT32_WITHIN(10, freq_hz, count);
 }
 
 TEST_CASE("LEDC timer select specific clock source", "[ledc]")
@@ -590,29 +581,28 @@ TEST_CASE("LEDC timer pause and resume", "[ledc]")
 
     vTaskDelay(10 / portTICK_PERIOD_MS);
     count = wave_count(1000);
-    TEST_ASSERT_INT16_WITHIN(5, count, 5000);
+    TEST_ASSERT_INT16_WITHIN(5, 5000, count);
 
     //pause ledc timer, when pause it, will get no waveform count
     printf("Pause ledc timer\n");
     TEST_ESP_OK(ledc_timer_pause(test_speed_mode, LEDC_TIMER_0));
     vTaskDelay(10 / portTICK_PERIOD_MS);
     count = wave_count(1000);
-    TEST_ASSERT_INT16_WITHIN(5, count, 0);
-    TEST_ASSERT_EQUAL_UINT32(count, 0);
+    TEST_ASSERT_INT16_WITHIN(5, 0, count);
 
     //resume ledc timer
     printf("Resume ledc timer\n");
     TEST_ESP_OK(ledc_timer_resume(test_speed_mode, LEDC_TIMER_0));
     vTaskDelay(10 / portTICK_PERIOD_MS);
     count = wave_count(1000);
-    TEST_ASSERT_UINT32_WITHIN(5, count, 5000);
+    TEST_ASSERT_UINT32_WITHIN(5, 5000, count);
 
     //reset ledc timer
     printf("reset ledc timer\n");
     TEST_ESP_OK(ledc_timer_rst(test_speed_mode, LEDC_TIMER_0));
     vTaskDelay(100 / portTICK_PERIOD_MS);
     count = wave_count(1000);
-    TEST_ASSERT_UINT32_WITHIN(5, count, 5000);
+    TEST_ASSERT_UINT32_WITHIN(5, 5000, count);
     tear_testbench();
 }
 

components/driver/test_apps/ledc/pytest_ledc.py

@@ -0,0 +1,19 @@
+# SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: CC0-1.0
+
+import pytest
+from pytest_embedded_idf import IdfDut
+
+
+@pytest.mark.supported_targets
+@pytest.mark.generic
+@pytest.mark.parametrize(
+    'config',
+    [
+        'iram_safe',
+        'release',
+    ],
+    indirect=True,
+)
+def test_ledc(dut: IdfDut) -> None:
+    dut.run_all_single_board_cases()

components/driver/test_apps/ledc/sdkconfig.ci.iram_safe

@@ -0,0 +1,8 @@
+CONFIG_COMPILER_DUMP_RTL_FILES=y
+CONFIG_COMPILER_OPTIMIZATION_NONE=y
+# place non-ISR FreeRTOS functions in Flash
+CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y
+# silent the error check, as the error string are stored in rodata, causing RTL check failure
+CONFIG_COMPILER_OPTIMIZATION_CHECKS_SILENT=y
+# ledc driver uses assert in the ISR code path
+CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y

components/driver/test_apps/ledc/sdkconfig.ci.release

@@ -0,0 +1,5 @@
+CONFIG_PM_ENABLE=y
+CONFIG_FREERTOS_USE_TICKLESS_IDLE=y
+CONFIG_COMPILER_OPTIMIZATION_SIZE=y
+CONFIG_BOOTLOADER_COMPILER_OPTIMIZATION_SIZE=y
+CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y

components/driver/test_apps/ledc/sdkconfig.defaults

@@ -0,0 +1,4 @@
+CONFIG_FREERTOS_HZ=1000
+CONFIG_ESP_TASK_WDT=n
+# Disable memory protection, because "LEDC continue work after software reset" test case requires a cpu reset
+CONFIG_ESP_SYSTEM_MEMPROT_FEATURE=n

components/hal/include/hal/ledc_hal.h

@@ -175,17 +175,6 @@ typedef struct {
  */
 #define ledc_hal_get_hpoint(hal, channel_num, hpoint_val)  ledc_ll_get_hpoint((hal)->dev, (hal)->speed_mode, channel_num, hpoint_val)
 
-/**
- * @brief Set LEDC the integer part of duty value
- *
- * @param hal Context of the HAL layer
- * @param channel_num LEDC channel index (0-7), select from ledc_channel_t
- * @param duty_val LEDC duty value, the range of duty setting is [0, (2**duty_resolution)]
- *
- * @return None
- */
-#define ledc_hal_set_duty_int_part(hal, channel_num, duty_val)  ledc_ll_set_duty_int_part((hal)->dev, (hal)->speed_mode, channel_num, duty_val)
-
 /**
  * @brief Set the output enable
  *
@@ -197,17 +186,6 @@ typedef struct {
  */
 #define ledc_hal_set_sig_out_en(hal, channel_num, sig_out_en)  ledc_ll_set_sig_out_en((hal)->dev, (hal)->speed_mode, channel_num, sig_out_en)
 
-/**
- * @brief Set the duty start
- *
- * @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
- */
-#define ledc_hal_set_duty_start(hal, channel_num, duty_start)  ledc_ll_set_duty_start((hal)->dev, (hal)->speed_mode, channel_num, duty_start)
-
 /**
  * @brief Set output idle level
  *
@@ -272,6 +250,28 @@ void ledc_hal_init(ledc_hal_context_t *hal, ledc_mode_t speed_mode);
  */
 void ledc_hal_ls_channel_update(ledc_hal_context_t *hal, ledc_channel_t channel_num);
 
+/**
+ * @brief Set the duty start
+ *
+ * @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);
+
+/**
+ * @brief Set LEDC the integer part of duty value
+ *
+ * @param hal Context of the HAL layer
+ * @param channel_num LEDC channel index (0-7), select from ledc_channel_t
+ * @param duty_val LEDC duty value, the range of duty setting is [0, (2**duty_resolution)]
+ *
+ * @return None
+ */
+void ledc_hal_set_duty_int_part(ledc_hal_context_t *hal, ledc_channel_t channel_num, uint32_t duty_val);
+
 /**
  * @brief Set LEDC hpoint value
  *

components/hal/ledc_hal_iram.c

@@ -15,6 +15,16 @@ 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)
+{
+    ledc_ll_set_duty_start(hal->dev, hal->speed_mode, channel_num, duty_start);
+}
+
+void ledc_hal_set_duty_int_part(ledc_hal_context_t *hal, ledc_channel_t channel_num, uint32_t duty_val)
+{
+    ledc_ll_set_duty_int_part(hal->dev, hal->speed_mode, channel_num, duty_val);
+}
+
 void ledc_hal_set_hpoint(ledc_hal_context_t *hal, ledc_channel_t channel_num, uint32_t hpoint_val)
 {
     ledc_ll_set_hpoint(hal->dev, hal->speed_mode, channel_num, hpoint_val);