Re: [PR] espressif[risc-v|xtensa]: Check events when reading PCNT counter value [nuttx]

[via GitHub]

fdcavalcanti commented on code in PR #16491:
URL: https://github.com/apache/nuttx/pull/16491#discussion_r2132658119


##########
arch/risc-v/src/common/espressif/esp_pcnt.c:
##########
@@ -642,8 +645,38 @@ static int esp_pcnt_unit_get_count(struct cap_lowerhalf_s 
*dev, int *ret)
     }
 
   flags = spin_lock_irqsave(&priv->lock);
-  *ret = pcnt_ll_get_count(ctx.dev, priv->unit_id) +
-      priv->accum_value;
+  tmp_count = pcnt_ll_get_count(ctx.dev, priv->unit_id);
+
+  intr_status = pcnt_ll_get_intr_status(ctx.dev);
+  if (intr_status & PCNT_LL_UNIT_WATCH_EVENT(priv->unit_id))
+    {
+      event_status = pcnt_ll_get_event_status(ctx.dev, priv->unit_id);
+      while (event_status)
+        {
+          int event_id = __builtin_ffs(event_status) - 1;
+          event_status &= (event_status - 1);
+
+          if (priv->config.accum_count)
+            {
+              if (event_id == PCNT_LL_WATCH_EVENT_LOW_LIMIT)
+                {
+                  if (tmp_count >= (priv->config.low_limit / 2))
+                    {
+                      tmp_count += priv->config.low_limit;
+                    }

Review Comment:
   Can those go in a single if statement? You"ll probably have to break the 
line but is easier to read.
   ```suggestion
                 if ( (event_id == PCNT_LL_WATCH_EVENT_LOW_LIMIT) &&  \
                       (tmp_count >= (priv->config.low_limit / 2)) )
                   {
                      tmp_count += priv->config.low_limit;
                   }
   ```



##########
arch/risc-v/src/common/espressif/esp_pcnt.c:
##########
@@ -642,8 +645,38 @@ static int esp_pcnt_unit_get_count(struct cap_lowerhalf_s 
*dev, int *ret)
     }
 
   flags = spin_lock_irqsave(&priv->lock);
-  *ret = pcnt_ll_get_count(ctx.dev, priv->unit_id) +
-      priv->accum_value;
+  tmp_count = pcnt_ll_get_count(ctx.dev, priv->unit_id);
+
+  intr_status = pcnt_ll_get_intr_status(ctx.dev);
+  if (intr_status & PCNT_LL_UNIT_WATCH_EVENT(priv->unit_id))
+    {
+      event_status = pcnt_ll_get_event_status(ctx.dev, priv->unit_id);
+      while (event_status)
+        {
+          int event_id = __builtin_ffs(event_status) - 1;
+          event_status &= (event_status - 1);
+
+          if (priv->config.accum_count)
+            {
+              if (event_id == PCNT_LL_WATCH_EVENT_LOW_LIMIT)
+                {
+                  if (tmp_count >= (priv->config.low_limit / 2))
+                    {
+                      tmp_count += priv->config.low_limit;
+                    }

Review Comment:
   Can those go in a single if statement? You"ll probably have to break the 
line but is easier to read.
   Something like:
   ```suggestion
                 if ( (event_id == PCNT_LL_WATCH_EVENT_LOW_LIMIT) &&  \
                       (tmp_count >= (priv->config.low_limit / 2)) )
                   {
                      tmp_count += priv->config.low_limit;
                   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org