ppisa commented on code in PR #16491:
URL: https://github.com/apache/nuttx/pull/16491#discussion_r2132843957
##########
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)
Review Comment:
If the rule that interrupt is not blocked for longer time than minimal time
at which one half of range from zero to limit is counted (obligatory condition
to allow the code to work at all), then the while cycle is not required. When
the limit reach and count zeroing event occurs then the counter value start
from zero and the opposite event cannot appear before opposite limit is
reached. So only single even needs to be corrected during the single read. If
one limit is near zero then the logic cannot work,
So I would simply check for max limit reached and if true and below one half
of max limit or even below zero do the correction, if this even is not ending
check for min limit and if true and read value is above one half on min (which
is negative) or count is positive (both checked by single compare), then do the
correction for low limit overflow.
##########
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)
Review Comment:
I would nest vice-versa the conditions. When accumulation is not enabled
then pcnt_ll_get_intr_status() is necessary overhead.
--
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
