espressif/esp-idf
1
0
Fork 1
mirror of https://github.com/espressif/esp-idf.git synced 2025-08-04 13:14:32 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'fix/pcnt_miss_accum_value_when_overflow' into 'master'

Browse Source 
fix(pcnt): fix the accum_value missing when overflow

See merge request espressif/esp-idf!39967
This commit is contained in:
Chen Ji Chang
2025-07-02 19:07:53 +08:00
parent 90b88aff5c 6720cf769e
commit 22cfc7ef57
4 changed files with 93 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
components/esp_driver_pcnt/README.md Normal file
View File

@@ -0,0 +1,39 @@
# PCNT Driver Design
## Concurrency
The count value and the overflow state of the count value are located in *different* registers, resulting in the software being unable to obtain information from both of them in the same read instruction.
The race condition case is as follow:
```mermaid
sequenceDiagram
participant HW as PCNT Hardware
participant CPU0_ISR as CPU0_ISR
participant CPU1_Task as CPU1_Task (pcnt_unit_get_count)
participant REG as Reg and Soft accum counter State
CPU1_Task->>CPU1_Task: Call pcnt_unit_get_count()
Note over REG: intr_status = 0<br/>cnt_reg = cnt_value<br/>accum_value = old_value
CPU1_Task->>CPU1_Task: portENTER_CRITICAL_SAFE()
CPU1_Task->>REG: Read intr_status
Note over CPU1_Task: intr_status=0, no need to do compensation
HW->>REG: Overflow interrupt triggered
Note over REG: intr_status = 1<br/>cnt_reg = 0<br/>accum_value = old_value
REG->>CPU0_ISR: ISR is called
CPU0_ISR->>CPU0_ISR: try portENTER_CRITICAL_SAFE() but spin
CPU1_Task->>REG: Read cnt_reg(0) + accum_value(old)
CPU1_Task->>CPU1_Task: portEXIT_CRITICAL_SAFE()
CPU0_ISR->>CPU0_ISR: portENTER_CRITICAL_SAFE()
CPU0_ISR->>REG: Clear interrupt status and update accum_value
Note over REG: intr_status = 0<br/>accum_value = new_value
CPU0_ISR->>CPU0_ISR: portEXIT_CRITICAL_SAFE()
Note over CPU0_ISR: Process events
Note over CPU1_Task: Return incorrect count ❌
```
In the software, we determine whether to perform compensation by checking whether the count value exceeds half of the limit. This can prevent counting errors when the overflow frequency is not high.

64
components/esp_driver_pcnt/src/pulse_cnt.c
View File

@@ -475,10 +475,37 @@ esp_err_t pcnt_unit_get_count(pcnt_unit_handle_t unit, int *value)
pcnt_group_t *group = NULL;
ESP_RETURN_ON_FALSE_ISR(unit && value, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
group = unit->group;
int temp_value = 0;
// the accum_value is also accessed by the ISR, so adding a critical section
portENTER_CRITICAL_SAFE(&unit->spinlock);
*value = pcnt_ll_get_count(group->hal.dev, unit->unit_id) + unit->accum_value;
temp_value = pcnt_ll_get_count(group->hal.dev, unit->unit_id) ;
// Check for pending overflow interrupts that haven't been processed yet
// Add compensation to get accurate count
if (unit->flags.accum_count) {
uint32_t intr_status = pcnt_ll_get_intr_status(group->hal.dev);
if (intr_status & PCNT_LL_UNIT_WATCH_EVENT(unit->unit_id)) {
uint32_t event_status = pcnt_ll_get_event_status(group->hal.dev, unit->unit_id);
// TODO: DIG-683
// Note, the overflow may be triggered between `pcnt_ll_get_count` and `pcnt_ll_get_event_status`
// In this case, we don't want to do the compensation.
// so we should check the count value is greater(less) than the low(high) limit / 2 to filter this case.
// This workaround is only valid for the case that the counter won't overflow twice between `pcnt_ll_get_count()` and `pcnt_ll_get_intr_status()`
if (event_status & BIT(PCNT_LL_WATCH_EVENT_LOW_LIMIT) && temp_value >= unit->low_limit / 2) {
temp_value += unit->low_limit;
} else if (event_status & BIT(PCNT_LL_WATCH_EVENT_HIGH_LIMIT) && temp_value <= unit->high_limit / 2) {
temp_value += unit->high_limit;
}
#if SOC_PCNT_SUPPORT_STEP_NOTIFY && PCNT_LL_STEP_NOTIFY_DIR_LIMIT
else if (event_status & BIT(PCNT_LL_STEP_EVENT_REACH_LIMIT) && abs(temp_value) <= abs(unit->step_info.step_limit) / 2) {
temp_value += unit->step_info.step_limit;
}
#endif
}
}
*value = temp_value + unit->accum_value;
portEXIT_CRITICAL_SAFE(&unit->spinlock);
return ESP_OK;
@@ -975,11 +1002,27 @@ IRAM_ATTR static void pcnt_default_isr(void *args)
uint32_t intr_status = pcnt_ll_get_intr_status(group->hal.dev);
if (intr_status & PCNT_LL_UNIT_WATCH_EVENT(unit_id)) {
pcnt_ll_clear_intr_status(group->hal.dev, PCNT_LL_UNIT_WATCH_EVENT(unit_id));
// event status word contains information about the real watch event type
uint32_t event_status = pcnt_ll_get_event_status(group->hal.dev, unit_id);
// clear interrupt status and update accum_value atomically
portENTER_CRITICAL_ISR(&unit->spinlock);
pcnt_ll_clear_intr_status(group->hal.dev, PCNT_LL_UNIT_WATCH_EVENT(unit_id));
if (unit->flags.accum_count) {
if (event_status & BIT(PCNT_LL_WATCH_EVENT_LOW_LIMIT)) {
unit->accum_value += unit->low_limit;
} else if (event_status & BIT(PCNT_LL_WATCH_EVENT_HIGH_LIMIT)) {
unit->accum_value += unit->high_limit;
}
#if SOC_PCNT_SUPPORT_STEP_NOTIFY && PCNT_LL_STEP_NOTIFY_DIR_LIMIT
else if (event_status & BIT(PCNT_LL_STEP_EVENT_REACH_LIMIT)) {
unit->accum_value += unit->step_info.step_limit;
}
#endif
}
portEXIT_CRITICAL_ISR(&unit->spinlock);
int count_value = 0;
pcnt_unit_zero_cross_mode_t zc_mode = PCNT_UNIT_ZERO_CROSS_INVALID;
@@ -998,11 +1041,6 @@ IRAM_ATTR static void pcnt_default_isr(void *args)
event_status &= ~BIT(PCNT_LL_STEP_EVENT_REACH_LIMIT);
// adjust current count value to the step limit
count_value = unit->step_info.step_limit;
if (unit->flags.accum_count) {
portENTER_CRITICAL_ISR(&unit->spinlock);
unit->accum_value += unit->step_info.step_limit;
portEXIT_CRITICAL_ISR(&unit->spinlock);
}
}
#else
// step event has higher priority than pointer event
@@ -1027,20 +1065,10 @@ IRAM_ATTR static void pcnt_default_isr(void *args)
event_status &= ~(BIT(PCNT_LL_WATCH_EVENT_LOW_LIMIT));
// adjust the count value to reflect the actual watch point value
count_value = unit->low_limit;
if (unit->flags.accum_count) {
portENTER_CRITICAL_ISR(&unit->spinlock);
unit->accum_value += unit->low_limit;
portEXIT_CRITICAL_ISR(&unit->spinlock);
}
} else if (event_status & BIT(PCNT_LL_WATCH_EVENT_HIGH_LIMIT)) {
event_status &= ~(BIT(PCNT_LL_WATCH_EVENT_HIGH_LIMIT));
// adjust the count value to reflect the actual watch point value
count_value = unit->high_limit;
if (unit->flags.accum_count) {
portENTER_CRITICAL_ISR(&unit->spinlock);
unit->accum_value += unit->high_limit;
portEXIT_CRITICAL_ISR(&unit->spinlock);
}
} else if (event_status & BIT(PCNT_LL_WATCH_EVENT_THRES0)) {
event_status &= ~(BIT(PCNT_LL_WATCH_EVENT_THRES0));
// adjust the count value to reflect the actual watch point value

4
docs/en/api-reference/peripherals/pcnt.rst
View File

@@ -329,6 +329,10 @@ The internal hardware counter will be cleared to zero automatically when it reac
:cpp:func:`pcnt_unit_clear_count` resets the accumulated count value as well.
.. note::
When enabling the count overflow compensation, it is recommended to use as large a high/low count limit as possible, as it can avoid frequent interrupt triggering, improve system performance, and avoid compensation failure due to multiple overflows.
.. _pcnt-power-management:
Power Management

4
docs/zh_CN/api-reference/peripherals/pcnt.rst
View File

@@ -329,6 +329,10 @@ PCNT 内部的硬件计数器会在计数达到高/低门限的时候自动清
:cpp:func:`pcnt_unit_clear_count` 会复位该软件累加器。
.. note::
启用计数溢出补偿时，建议使用尽可能大的高/低计数门限，因为它可以避免中断被频繁触发，提高系统性能，并且避免由于多次溢出而导致的补偿失败。
.. _pcnt-power-management:
电源管理