fix(pcnt): fix the accum_value missing when overflow

components/esp_driver_pcnt/README.md

@@ -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.

components/esp_driver_pcnt/src/pulse_cnt.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -451,10 +451,32 @@ 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;
+            }
+        }
+    }
+
+    *value = temp_value + unit->accum_value;
     portEXIT_CRITICAL_SAFE(&unit->spinlock);
 
     return ESP_OK;
@@ -824,25 +846,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)) {
+        // 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));
 
-        // points watcher event
-        uint32_t event_status = pcnt_ll_get_event_status(group->hal.dev, unit_id);
-        // iter on each event_id
-        while (event_status) {
-            int event_id = __builtin_ffs(event_status) - 1;
-            event_status &= (event_status - 1); // clear the right most bit
-
-            portENTER_CRITICAL_ISR(&unit->spinlock);
         if (unit->flags.accum_count) {
-                if (event_id == PCNT_LL_WATCH_EVENT_LOW_LIMIT) {
+            if (event_status & BIT(PCNT_LL_WATCH_EVENT_LOW_LIMIT)) {
                 unit->accum_value += unit->low_limit;
-                } else if (event_id == PCNT_LL_WATCH_EVENT_HIGH_LIMIT) {
+            } else if (event_status & BIT(PCNT_LL_WATCH_EVENT_HIGH_LIMIT)) {
                 unit->accum_value += unit->high_limit;
             }
         }
         portEXIT_CRITICAL_ISR(&unit->spinlock);
 
+        // using while loop so that we don't miss any event
+        while (event_status) {
+            int event_id = __builtin_ffs(event_status) - 1;
+            event_status &= (event_status - 1); // clear the right most bit
+
             // invoked user registered callback
             if (on_reach) {
                 pcnt_watch_event_data_t edata = {

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

@@ -286,6 +286,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

docs/zh_CN/api-reference/peripherals/pcnt.rst

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