refactor(pcnt): use private shared interrupt groups

Route PCNT unit interrupts through private shared groups bound by module
name so units in the same PCNT instance share one CPU line without
allowing other peripherals to join. Remove redundant PCNT-side priority
conflict checks and rely on the interrupt allocator for conflict
validation.

Switch the PCNT test app to unity leak utilities and clean newlib reent
state during teardown so leak checks are more stable with lazy
allocations.
This commit is contained in:
morris
2026-03-26 12:17:45 +08:00
parent a8c8b831b8
commit 8509ec1958
3 changed files with 41 additions and 55 deletions
+32 -35
View File
@@ -43,9 +43,9 @@
#endif
#if CONFIG_PCNT_ISR_IRAM_SAFE
#define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED)
#define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED_PRIVATE)
#else
#define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED)
#define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED_PRIVATE)
#endif
#define PCNT_ALLOW_INTR_PRIORITY_MASK ESP_INTR_FLAG_LOWMED
@@ -69,7 +69,6 @@ struct pcnt_platform_t {
struct pcnt_group_t {
int group_id; // Group ID, index from 0
int intr_priority; // PCNT interrupt priority
pcnt_clock_source_t clk_src; // PCNT clock source
portMUX_TYPE spinlock; // to protect per-group register level concurrent access
pcnt_hal_context_t hal;
@@ -106,6 +105,7 @@ struct pcnt_unit_t {
int low_limit; // low limit value
int high_limit; // high limit value
int clear_signal_gpio_num; // which gpio clear signal input
int intr_priority; // PCNT interrupt priority
int accum_value; // accumulated count value
pcnt_step_interval_t step_info; // step interval info
pcnt_chan_t *channels[PCNT_LL_GET(CHANS_PER_UNIT)]; // array of PCNT channels
@@ -138,6 +138,32 @@ static void pcnt_release_group_handle(pcnt_group_t *group);
static void pcnt_default_isr(void *args);
static esp_err_t pcnt_select_periph_clock(pcnt_unit_t *unit, pcnt_clock_source_t clk_src);
static esp_err_t pcnt_unit_install_interrupt(pcnt_unit_t *unit)
{
pcnt_group_t *group = unit->group;
int group_id = group->group_id;
int unit_id = unit->unit_id;
int isr_flags = PCNT_INTR_ALLOC_FLAGS;
if (unit->intr_priority) {
isr_flags |= 1 << (unit->intr_priority);
} else {
isr_flags |= PCNT_ALLOW_INTR_PRIORITY_MASK;
}
esp_intr_alloc_info_t intr_info = {
.source = soc_pcnt_signals[group_id].irq_id,
.flags = isr_flags,
.intrstatusreg = (uint32_t)pcnt_ll_get_intr_status_reg(group->hal.dev),
.intrstatusmask = PCNT_LL_UNIT_WATCH_EVENT(unit_id),
.handler = pcnt_default_isr,
.arg = unit,
.bind_by.name = soc_pcnt_signals[group_id].module_name,
};
return esp_intr_alloc_info(&intr_info, &unit->intr);
}
static esp_err_t pcnt_register_to_group(pcnt_unit_t *unit)
{
pcnt_group_t *group = NULL;
@@ -223,30 +249,11 @@ esp_err_t pcnt_new_unit(const pcnt_unit_config_t *config, pcnt_unit_handle_t *re
ESP_GOTO_ON_ERROR(pcnt_select_periph_clock(unit, pcnt_clk_src), err, TAG, "select periph clock failed");
ESP_GOTO_ON_ERROR(esp_clk_tree_enable_src((soc_module_clk_t)group->clk_src, true), err, TAG, "clock source enable failed");
// if interrupt priority specified before, it cannot be changed until the group is released
// check if the new priority specified consistents with the old one
bool intr_priority_conflict = false;
portENTER_CRITICAL(&group->spinlock);
if (group->intr_priority == -1) {
group->intr_priority = config->intr_priority;
} else if (config->intr_priority != 0) {
intr_priority_conflict = (group->intr_priority != config->intr_priority);
}
portEXIT_CRITICAL(&group->spinlock);
ESP_GOTO_ON_FALSE(!intr_priority_conflict, ESP_ERR_INVALID_STATE, err, TAG, "intr_priority conflict, already is %d but attempt to %d", group->intr_priority, config->intr_priority);
// to accumulate count value, we should install the interrupt handler first, and in the ISR we do the accumulation
bool to_install_isr = (config->flags.accum_count == 1);
unit->intr_priority = config->intr_priority;
if (to_install_isr) {
int isr_flags = PCNT_INTR_ALLOC_FLAGS;
if (group->intr_priority) {
isr_flags |= 1 << (group->intr_priority);
} else {
isr_flags |= PCNT_ALLOW_INTR_PRIORITY_MASK;
}
ESP_GOTO_ON_ERROR(esp_intr_alloc_intrstatus(soc_pcnt_signals[group_id].irq_id, isr_flags,
(uint32_t)pcnt_ll_get_intr_status_reg(group->hal.dev), PCNT_LL_UNIT_WATCH_EVENT(unit_id),
pcnt_default_isr, unit, &unit->intr), err,
ESP_GOTO_ON_ERROR(pcnt_unit_install_interrupt(unit), err,
TAG, "install interrupt service failed");
}
@@ -516,7 +523,6 @@ esp_err_t pcnt_unit_register_event_callbacks(pcnt_unit_handle_t unit, const pcnt
ESP_RETURN_ON_FALSE(unit && cbs, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
// unit event callbacks should be registered in init state
pcnt_group_t *group = unit->group;
int group_id = group->group_id;
int unit_id = unit->unit_id;
#if CONFIG_PCNT_ISR_IRAM_SAFE
@@ -531,15 +537,7 @@ esp_err_t pcnt_unit_register_event_callbacks(pcnt_unit_handle_t unit, const pcnt
// lazy install interrupt service
if (!unit->intr) {
ESP_RETURN_ON_FALSE(unit->fsm == PCNT_UNIT_FSM_INIT, ESP_ERR_INVALID_STATE, TAG, "unit not in init state");
int isr_flags = PCNT_INTR_ALLOC_FLAGS;
if (group->intr_priority) {
isr_flags |= 1 << (group->intr_priority);
} else {
isr_flags |= PCNT_ALLOW_INTR_PRIORITY_MASK;
}
ESP_RETURN_ON_ERROR(esp_intr_alloc_intrstatus(soc_pcnt_signals[group_id].irq_id, isr_flags,
(uint32_t)pcnt_ll_get_intr_status_reg(group->hal.dev), PCNT_LL_UNIT_WATCH_EVENT(unit_id),
pcnt_default_isr, unit, &unit->intr),
ESP_RETURN_ON_ERROR(pcnt_unit_install_interrupt(unit),
TAG, "install interrupt service failed");
}
// enable/disable PCNT interrupt events
@@ -907,7 +905,6 @@ static pcnt_group_t *pcnt_acquire_group_handle(int group_id)
s_platform.groups[group_id] = group; // register to platform
// initialize pcnt group members
group->group_id = group_id;
group->intr_priority = -1;
group->spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
// enable APB access pcnt registers
PERIPH_RCC_ATOMIC() {
@@ -1,38 +1,26 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include "unity.h"
#include "unity_test_runner.h"
#include "unity_test_utils.h"
#include "esp_heap_caps.h"
#include "esp_newlib.h"
// Some resources are lazy allocated in pulse_cnt driver, the threshold is left for that case
#define TEST_MEMORY_LEAK_THRESHOLD (-300)
static size_t before_free_8bit;
static size_t before_free_32bit;
static void check_leak(size_t before_free, size_t after_free, const char *type)
{
ssize_t delta = after_free - before_free;
printf("MALLOC_CAP_%s: Before %u bytes free, After %u bytes free (delta %d)\n", type, before_free, after_free, delta);
TEST_ASSERT_MESSAGE(delta >= TEST_MEMORY_LEAK_THRESHOLD, "memory leak");
}
#define TEST_MEMORY_LEAK_THRESHOLD (300)
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);
unity_utils_record_free_mem();
}
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);
check_leak(before_free_8bit, after_free_8bit, "8BIT");
check_leak(before_free_32bit, after_free_32bit, "32BIT");
esp_reent_cleanup(); //clean up some of the newlib's lazy allocations
unity_utils_evaluate_leaks_direct(TEST_MEMORY_LEAK_THRESHOLD);
}
void app_main(void)
@@ -23,6 +23,7 @@ TEST_CASE("pcnt_unit_install_uninstall", "[pcnt]")
.low_limit = -100,
.high_limit = 100,
.intr_priority = 0,
.flags.accum_count = true,
};
pcnt_unit_handle_t units[PCNT_LL_GET(UNITS_PER_INST)];
int count_value = 0;
@@ -36,7 +37,7 @@ TEST_CASE("pcnt_unit_install_uninstall", "[pcnt]")
// unit with a different interrupt priority
unit_config.intr_priority = 3;
TEST_ESP_ERR(ESP_ERR_INVALID_STATE, pcnt_new_unit(&unit_config, &units[PCNT_LL_GET(UNITS_PER_INST) - 1]));
TEST_ESP_ERR(ESP_ERR_INVALID_ARG, pcnt_new_unit(&unit_config, &units[PCNT_LL_GET(UNITS_PER_INST) - 1]));
unit_config.intr_priority = 0;
TEST_ESP_OK(pcnt_new_unit(&unit_config, &units[PCNT_LL_GET(UNITS_PER_INST) - 1]));