Merge branch 'feature/mcpwm_update_compare_iram_safe' into 'master'

mcpwm: make set_compare_value iram safe

Closes IDFGH-8314

See merge request espressif/esp-idf!20254

components/driver/Kconfig

@@ -352,6 +352,14 @@ menu "Driver Configurations"
                 cache misses, and also be able to run whilst the cache is disabled.
                 (e.g. SPI Flash write)
 
+        config MCPWM_CTRL_FUNC_IN_IRAM
+            bool "Place MCPWM control functions into IRAM"
+            default n
+            help
+                Place MCPWM control functions (like set_compare_value) into IRAM,
+                so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context.
+                Enabling this option can improve driver performance as well.
+
         config MCPWM_SUPPRESS_DEPRECATE_WARN
             bool "Suppress leagcy driver deprecated warning"
             default n

components/driver/deprecated/driver/mcpwm_types_legacy.h

@@ -68,7 +68,9 @@ typedef struct {
  */
 typedef enum {
     MCPWM_UNIT_0,   /*!<MCPWM unit0 selected*/
+#if SOC_MCPWM_GROUPS > 1
     MCPWM_UNIT_1,   /*!<MCPWM unit1 selected*/
+#endif
     MCPWM_UNIT_MAX, /*!<Max number of MCPWM units*/
 } mcpwm_unit_t;
 

components/driver/deprecated/mcpwm_legacy.c

@@ -8,7 +8,6 @@
 #include "sdkconfig.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/semphr.h"
-#include "freertos/xtensa_api.h"
 #include "freertos/task.h"
 #include "esp_log.h"
 #include "esp_err.h"
@@ -104,6 +103,7 @@ static mcpwm_context_t context[SOC_MCPWM_GROUPS] = {
         .mcpwm_intr_handle = NULL,
         .cap_isr_func = {[0 ... SOC_MCPWM_CAPTURE_CHANNELS_PER_TIMER - 1] = {NULL, NULL}},
     },
+#if SOC_MCPWM_GROUPS > 1
     [1] = {
         .hal = {MCPWM_LL_GET_HW(1)},
         .spinlock = portMUX_INITIALIZER_UNLOCKED,
@@ -116,6 +116,7 @@ static mcpwm_context_t context[SOC_MCPWM_GROUPS] = {
         .mcpwm_intr_handle = NULL,
         .cap_isr_func = {[0 ... SOC_MCPWM_CAPTURE_CHANNELS_PER_TIMER - 1] = {NULL, NULL}},
     }
+#endif
 };
 
 typedef void (*mcpwm_ll_gen_set_event_action_t)(mcpwm_dev_t *mcpwm, int op, int gen, int action);

components/driver/linker.lf

@@ -17,3 +17,5 @@ entries:
         gpio: gpio_intr_disable (noflash)
     if SDM_CTRL_FUNC_IN_IRAM = y:
         sdm: sdm_channel_set_duty (noflash)
+    if MCPWM_CTRL_FUNC_IN_IRAM = y:
+        mcpwm_cmpr: mcpwm_comparator_set_compare_value (noflash)

components/driver/mcpwm/mcpwm_cap.c

@@ -112,7 +112,7 @@ esp_err_t mcpwm_new_capture_timer(const mcpwm_capture_timer_config_t *config, mc
     cap_timer->spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
     cap_timer->fsm = MCPWM_CAP_TIMER_FSM_INIT;
     *ret_cap_timer = cap_timer;
-    ESP_LOGD(TAG, "new capture timer at %p, in group (%d)", cap_timer, group_id);
+    ESP_LOGD(TAG, "new capture timer at %p, in group (%d), resolution %"PRIu32, cap_timer, group_id, cap_timer->resolution_hz);
     return ESP_OK;
 
 err:

components/driver/mcpwm/mcpwm_cmpr.c

@@ -124,12 +124,12 @@ esp_err_t mcpwm_del_comparator(mcpwm_cmpr_handle_t cmpr)
 
 esp_err_t mcpwm_comparator_set_compare_value(mcpwm_cmpr_handle_t cmpr, uint32_t cmp_ticks)
 {
-    ESP_RETURN_ON_FALSE(cmpr, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
+    ESP_RETURN_ON_FALSE_ISR(cmpr, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
     mcpwm_oper_t *oper = cmpr->oper;
     mcpwm_group_t *group = oper->group;
     mcpwm_timer_t *timer = oper->timer;
-    ESP_RETURN_ON_FALSE(timer, ESP_ERR_INVALID_STATE, TAG, "timer and operator are not connected");
-    ESP_RETURN_ON_FALSE(cmp_ticks < timer->peak_ticks, ESP_ERR_INVALID_ARG, TAG, "compare value out of range");
+    ESP_RETURN_ON_FALSE_ISR(timer, ESP_ERR_INVALID_STATE, TAG, "timer and operator are not connected");
+    ESP_RETURN_ON_FALSE_ISR(cmp_ticks < timer->peak_ticks, ESP_ERR_INVALID_ARG, TAG, "compare value out of range");
 
     portENTER_CRITICAL_SAFE(&cmpr->spinlock);
     mcpwm_ll_operator_set_compare_value(group->hal.dev, oper->oper_id, cmpr->cmpr_id, cmp_ticks);
@@ -181,6 +181,7 @@ static void IRAM_ATTR mcpwm_comparator_default_isr(void *args)
 {
     mcpwm_cmpr_t *cmpr = (mcpwm_cmpr_t *)args;
     mcpwm_oper_t *oper = cmpr->oper;
+    mcpwm_timer_t *timer = oper->timer;
     mcpwm_group_t *group = oper->group;
     mcpwm_hal_context_t *hal = &group->hal;
     int oper_id = oper->oper_id;
@@ -192,7 +193,7 @@ static void IRAM_ATTR mcpwm_comparator_default_isr(void *args)
 
     mcpwm_compare_event_data_t edata = {
         .compare_ticks = cmpr->compare_ticks,
-        // .direction = TODO
+        .direction = mcpwm_ll_timer_get_count_direction(hal->dev, timer->timer_id),
     };
 
     if (status & MCPWM_LL_EVENT_CMP_EQUAL(oper_id, cmpr_id)) {

components/driver/mcpwm/mcpwm_fault.c

@@ -274,7 +274,7 @@ static void IRAM_ATTR mcpwm_gpio_fault_default_isr(void *args)
     mcpwm_ll_intr_clear_status(hal->dev, status & MCPWM_LL_EVENT_FAULT_MASK(fault_id));
 
     mcpwm_fault_event_data_t edata = {
-        // TODO
+        // TBD
     };
 
     if (status & MCPWM_LL_EVENT_FAULT_ENTER(fault_id)) {

components/driver/mcpwm/mcpwm_private.h

@@ -22,7 +22,7 @@
 extern "C" {
 #endif
 
-#if CONFIG_MCPWM_ISR_IRAM_SAFE
+#if CONFIG_MCPWM_ISR_IRAM_SAFE || CONFIG_MCPWM_CTRL_FUNC_IN_IRAM
 #define MCPWM_MEM_ALLOC_CAPS      (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT)
 #else
 #define MCPWM_MEM_ALLOC_CAPS      MALLOC_CAP_DEFAULT

components/driver/test_apps/mcpwm/main/test_mcpwm_iram.c

@@ -12,8 +12,7 @@
 #include "unity_test_utils.h"
 #include "soc/soc_caps.h"
 #include "esp_private/esp_clk.h"
-#include "driver/mcpwm_cap.h"
-#include "driver/mcpwm_sync.h"
+#include "driver/mcpwm_prelude.h"
 #include "driver/gpio.h"
 #include "test_mcpwm_utils.h"
 
@@ -90,3 +89,88 @@ TEST_CASE("mcpwm_capture_iram_safe", "[mcpwm]")
     TEST_ESP_OK(mcpwm_capture_timer_disable(cap_timer));
     TEST_ESP_OK(mcpwm_del_capture_timer(cap_timer));
 }
+
+static bool IRAM_ATTR test_compare_on_reach(mcpwm_cmpr_handle_t cmpr, const mcpwm_compare_event_data_t *ev_data, void *user_data)
+{
+    uint32_t cmp_val = ev_data->compare_ticks;
+    cmp_val += 10;
+    // compare ticks can't exceed the timer's period ticks
+    if (cmp_val >= 50) {
+        cmp_val = 0;
+    }
+    mcpwm_comparator_set_compare_value(cmpr, cmp_val);
+    return false;
+}
+
+static void IRAM_ATTR test_delay_post_cache_disable(void *args)
+{
+    esp_rom_delay_us(1000);
+}
+
+TEST_CASE("mcpwm_comparator_iram_safe", "[mcpwm]")
+{
+    mcpwm_timer_handle_t timer;
+    mcpwm_oper_handle_t oper;
+    mcpwm_cmpr_handle_t comparator;
+    mcpwm_gen_handle_t gen;
+
+    mcpwm_timer_config_t timer_config = {
+        .group_id = 0,
+        .clk_src = MCPWM_TIMER_CLK_SRC_DEFAULT,
+        .resolution_hz = 1 * 1000 * 1000,
+        .period_ticks = 50, // 50us <-> 20KHz
+        .count_mode = MCPWM_TIMER_COUNT_MODE_UP,
+    };
+    mcpwm_operator_config_t operator_config = {
+        .group_id = 0,
+    };
+    mcpwm_comparator_config_t comparator_config = {
+        .flags.update_cmp_on_tep = true,
+        .flags.update_cmp_on_tez = true,
+    };
+    printf("install timer, operator and comparator\r\n");
+    TEST_ESP_OK(mcpwm_new_timer(&timer_config, &timer));
+    TEST_ESP_OK(mcpwm_new_operator(&operator_config, &oper));
+    TEST_ESP_OK(mcpwm_new_comparator(oper, &comparator_config, &comparator));
+
+    printf("connect MCPWM timer and operators\r\n");
+    TEST_ESP_OK(mcpwm_operator_connect_timer(oper, timer));
+    TEST_ESP_OK(mcpwm_comparator_set_compare_value(comparator, 10));
+
+    printf("install MCPWM generator\r\n");
+    mcpwm_generator_config_t gen_config = {
+        .gen_gpio_num = 0,
+    };
+    TEST_ESP_OK(mcpwm_new_generator(oper, &gen_config, &gen));
+
+    printf("set generator actions on timer and compare events\r\n");
+    TEST_ESP_OK(mcpwm_generator_set_actions_on_timer_event(gen,
+                MCPWM_GEN_TIMER_EVENT_ACTION(MCPWM_TIMER_DIRECTION_UP, MCPWM_TIMER_EVENT_EMPTY, MCPWM_GEN_ACTION_HIGH),
+                MCPWM_GEN_TIMER_EVENT_ACTION_END()));
+    TEST_ESP_OK(mcpwm_generator_set_actions_on_compare_event(gen,
+                MCPWM_GEN_COMPARE_EVENT_ACTION(MCPWM_TIMER_DIRECTION_UP, comparator, MCPWM_GEN_ACTION_LOW),
+                MCPWM_GEN_COMPARE_EVENT_ACTION_END()));
+
+    printf("register compare event callback\r\n");
+    mcpwm_comparator_event_callbacks_t cbs = {
+        .on_reach = test_compare_on_reach,
+    };
+    TEST_ESP_OK(mcpwm_comparator_register_event_callbacks(comparator, &cbs, NULL));
+
+    printf("start timer\r\n");
+    TEST_ESP_OK(mcpwm_timer_enable(timer));
+    TEST_ESP_OK(mcpwm_timer_start_stop(timer, MCPWM_TIMER_START_NO_STOP));
+
+    printf("disable flash cache and check the compare events are still in working\r\n");
+    for (int i = 0; i < 50; i++) {
+        unity_utils_run_cache_disable_stub(test_delay_post_cache_disable, NULL);
+    }
+
+    printf("uninstall timer, operator and comparator\r\n");
+    TEST_ESP_OK(mcpwm_timer_start_stop(timer, MCPWM_TIMER_STOP_EMPTY));
+    TEST_ESP_OK(mcpwm_timer_disable(timer));
+    TEST_ESP_OK(mcpwm_del_generator(gen));
+    TEST_ESP_OK(mcpwm_del_comparator(comparator));
+    TEST_ESP_OK(mcpwm_del_operator(oper));
+    TEST_ESP_OK(mcpwm_del_timer(timer));
+}

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

@@ -1,5 +1,6 @@
 CONFIG_COMPILER_DUMP_RTL_FILES=y
 CONFIG_MCPWM_ISR_IRAM_SAFE=y
+CONFIG_MCPWM_CTRL_FUNC_IN_IRAM=y
 CONFIG_GPIO_CTRL_FUNC_IN_IRAM=y
 CONFIG_COMPILER_OPTIMIZATION_NONE=y
 # silent the error check, as the error string are stored in rodata, causing RTL check failure

docs/en/api-reference/peripherals/mcpwm.rst

@@ -861,19 +861,26 @@ There's a Kconfig option :ref:`CONFIG_MCPWM_ISR_IRAM_SAFE` that will:
 
 This will allow the interrupt to run while the cache is disabled but will come at the cost of increased IRAM consumption.
 
+There is another Kconfig option :ref:`CONFIG_MCPWM_CTRL_FUNC_IN_IRAM` that can put commonly used IO control functions into IRAM as well. So, these functions can also be executable when the cache is disabled. These IO control functions are as follows:
+
+- :cpp:func:`mcpwm_comparator_set_compare_value`
+
 Thread Safety
 ^^^^^^^^^^^^^
 
 The factory functions like :cpp:func:`mcpwm_new_timer` are guaranteed to be thread safe by the driver, which means, you can call it from different RTOS tasks without protection by extra locks.
 
-No functions are allowed to run within ISR environment.
+The following functions are allowed to run under ISR context, as the driver uses a critical section to prevent them being called concurrently in the task and ISR.
+
+- :cpp:func:`mcpwm_comparator_set_compare_value`
 
-Functions that are not related to `Resource Allocation <#resource-allocation-and-initialization>`__, are not thread safe. Thus, you should avoid calling them in different tasks without mutex protection.
+Other functions that are not related to `Resource Allocation <#resource-allocation-and-initialization>`__, are not thread safe. Thus, you should avoid calling them in different tasks without mutex protection.
 
 Kconfig Options
 ^^^^^^^^^^^^^^^
 
 - :ref:`CONFIG_MCPWM_ISR_IRAM_SAFE` controls whether the default ISR handler can work when cache is disabled, see `IRAM Safe <#iram-safe>`__ for more information.
+- :ref:`CONFIG_MCPWM_CTRL_FUNC_IN_IRAM` controls where to place the MCPWM control functions (IRAM or flash), see `IRAM Safe <#iram-safe>`__ for more information.
 - :ref:`CONFIG_MCPWM_ENABLE_DEBUG_LOG` is used to enabled the debug log output. Enable this option will increase the firmware binary size.
 
 Application Examples