xiaoxiang781216 commented on code in PR #15929:
URL: https://github.com/apache/nuttx/pull/15929#discussion_r1981854908
##########
arch/risc-v/src/common/riscv_mtimer.c:
##########
@@ -277,9 +278,7 @@ static int riscv_mtimer_cancel(struct oneshot_lowerhalf_s
*lower,
riscv_mtimer_set_mtimecmp(priv, mtime + UINT64_MAX);
- nsec = (alarm - mtime) * NSEC_PER_SEC / priv->freq;
- ts->tv_sec = nsec / NSEC_PER_SEC;
- ts->tv_nsec = nsec % NSEC_PER_SEC;
+ *ticks = (alarm - mtime) / priv->cycle_per_tick;
Review Comment:
```suggestion
*ticks = (alarm - mtime) * TICK_PER_SEC / priv->freq;
```
##########
arch/risc-v/src/common/riscv_mtimer.c:
##########
@@ -310,18 +309,15 @@ static int riscv_mtimer_cancel(struct oneshot_lowerhalf_s
*lower,
****************************************************************************/
static int riscv_mtimer_current(struct oneshot_lowerhalf_s *lower,
- struct timespec *ts)
+ clock_t *ticks)
{
struct riscv_mtimer_lowerhalf_s *priv =
(struct riscv_mtimer_lowerhalf_s *)lower;
uint64_t mtime = riscv_mtimer_get_mtime(priv);
- uint64_t left;
- ts->tv_sec = mtime / priv->freq;
- left = mtime - ts->tv_sec * priv->freq;
- ts->tv_nsec = NSEC_PER_SEC * left / priv->freq;
+ *ticks = mtime / priv->cycle_per_tick;
Review Comment:
```suggestion
*ticks = mtime * TICK_PER_SEC / priv->freq;
```
##########
arch/risc-v/src/common/riscv_mtimer.c:
##########
@@ -349,11 +345,11 @@ riscv_mtimer_initialize(uintreg_t mtime, uintreg_t
mtimecmp,
priv = kmm_zalloc(sizeof(*priv));
if (priv != NULL)
{
- priv->lower.ops = &g_riscv_mtimer_ops;
- priv->mtime = mtime;
- priv->mtimecmp = mtimecmp;
- priv->freq = freq;
- priv->alarm = UINT64_MAX;
+ priv->lower.ops = &g_riscv_mtimer_ops;
+ priv->mtime = mtime;
+ priv->mtimecmp = mtimecmp;
+ priv->cycle_per_tick = freq / TICK_PER_SEC;
Review Comment:
Please see my inline comment:
the right approach:
```
*ticks = mtime * TICK_PER_SEC / freq;
```
and the bad one:
```
*ticks = mtime / priv->cycle_per_tick = mtime / (freq / TICK_PER_SEC);
```
the delta will increase when mtime is growing if freq can't be divided by
TICK_PER_SEC evenly.
##########
arch/risc-v/src/common/riscv_mtimer.c:
##########
@@ -222,8 +221,11 @@ static int riscv_mtimer_start(struct oneshot_lowerhalf_s
*lower,
flags = up_irq_save();
mtime = riscv_mtimer_get_mtime(priv);
- alarm = mtime + ts->tv_sec * priv->freq +
- ts->tv_nsec * priv->freq / NSEC_PER_SEC;
+
+ /* Align each tick start to cycle_per_tick boundary to avoid clock drift */
+
+ alarm = mtime / priv->cycle_per_tick * priv->cycle_per_tick +
Review Comment:
```suggestion
alarm = mtime + tick * priv->freq / TICK_PER_SEC;
```
--
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
