Hello,

thanks for the replies. I attached a gzipped patch and also posted it in clear below. It is based on 12.9 branch but should apply correctly to master branch as well. Hopefully someone with GitHub account finds the time. (Provided such a change indeed does not negatively affect other architectures.)

Have a nice day!


From c0f2067bb6461314340b7a0d37ebe37fb4e5592b Mon Sep 17 00:00:00 2001
From: Kerogit <kr....@kerogit.eu>
Date: Mon, 24 Mar 2025 10:41:59 +0100
Subject: [PATCH] nuttx/clock: make NSEC_PER_USEC and others long

On AVR architecture, the compiler apparently sometimes truncates NSEC_PER_TICK to 16bit value, leading to clock_time2ticks returning incorrect results. This
was encountered while attempting to add tickless OS support for AVR
architecture but seemed to affect non-tickless mode of operation as well.

This patch marks NSEC_PER_USEC (and to be safe, USEC_PER_MSEC and MSEC_PER_SEC
too) as long.
---
 include/nuttx/clock.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/nuttx/clock.h b/include/nuttx/clock.h
index b750e5347d..e64f3fee6c 100644
--- a/include/nuttx/clock.h
+++ b/include/nuttx/clock.h
@@ -103,7 +103,7 @@

 #define NSEC_PER_SEC          1000000000L /* Seconds */
 #define USEC_PER_SEC             1000000L
-#define MSEC_PER_SEC                1000
+#define MSEC_PER_SEC                1000L
 #define DSEC_PER_SEC                  10
 #define HSEC_PER_SEC                   2

@@ -117,9 +117,9 @@
 #define MSEC_PER_DSEC                100

 #define NSEC_PER_MSEC            1000000L /* Milliseconds */
-#define USEC_PER_MSEC               1000
+#define USEC_PER_MSEC               1000L

-#define NSEC_PER_USEC               1000  /* Microseconds */
+#define NSEC_PER_USEC               1000L /* Microseconds */

 #define SEC_PER_MIN                   60
 #define NSEC_PER_MIN           (NSEC_PER_SEC * SEC_PER_MIN)
--
2.39.5




Dne 2025-03-24 09:58, Sebastien Lorquet napsal:
Hello,

I also dont have a github account (anymore), also for reasons.

If you contribution is small enough (like a few lines) you can just show it on the mailing list and people with a github account may pick it up when they have time.

Or send a git patch that would be easy to apply for people with github accounts.

Thats what I plan to do for my next potential contribution, at least.

Sebastien


On 23/03/2025 11:58, kr....@kerogit.eu wrote:
Hello,

I decided to try NuttX out - just as a hobbyist for learning experience. I am currently attempting to create port for DA/DB family of AVR architecture with support for tickless OS. My initial implementation worked sort-of well: I created a simple application with a LED blinking in 1/2 second interval using usleep() and tried it with USEC_PER_TICK set to 62500. There was some imprecision in the frequency but that is to be expected. However, when I tried to tweak USEC_PER_TICK, the results became weird. NuttX requested (via up_alarm_start) to set the alarm in nonsensical intervals - 29 seconds, a day... or a few microseconds.

After some debugging I found out that up_alarm_tick_cancel in sched/sched/sched_timerexpiration.c reads an incorrect ticks value from clock_time2ticks macro. It received 0 seconds, 0x77358ns as a parameter, which - rounded up - should yield 1 tick with USEC_PER_TICK set to 1000. Instead, it was returning 29 ticks. I tried to make a simple standalone reimplementation of clock_time2ticks and NSEC2TICK from include/nuttx/clock.h to see what exactly is going on and it worked well, returned correct result of 1. I then tried to compare the generated assembly and found out the difference: while the reimplementation was calculating (nsec + 999999) / 1000000, NuttX was counting (nsec + 16959) / 16960. The former is 0xf423f and 0xf4240 in hex, the latter is 0x423f and 0x4240 which led me to the idea that the compiler truncates something to 16bit - that something turned to be NSEC_PER_TICK.

I then changed NSEC_PER_USEC to 1000L, the calculation in clock_time2ticks corrected itself and the timer seems to work like a charm. (I can't explain why the reimplementation worked corectly though, it was setting NSEC_PER_USEC to 1000 without the "L" as well. Yet in this case, the compiler did the right thing.)

In case I ever decided that my code is actually worth contributing(*), is this change (and to be safe, same thing for USEC_PER_MSEC and MSEC_PER_SEC) acceptable? I have to admit that I have little clue what it can do (ie. break) for other architectures.

Thanks for any input

(*) which may be problematic in itself - as far as I found on the NuttX website, PR on GitHub is the only listed way contributions are accepted. I can provide publicly accessible Git repo but I don't have a GitHub account and am not willing to make one for a few reasons.

Attachment: 0001-nuttx-clock-make-NSEC_PER_USEC-and-others-long.patch.gz
Description: application/gzip

Reply via email to