[En-Nut-Discussion] NutGetTickClock rounding (again)

[Henrik Maier]

> Henrik, if ms is directly multiplied by 'Ticks in Hertz', then the
> maximum value of argument 'ms' has to be ((2^32)-1)/'Ticks in Hertz'.
> If there are 1000 ticks per seconds, that's about 1 hour and 11 minutes,
> very far from 49 days so already in that function the overflow problem
> is to be handled.

Bernhard you are correct here. The 49 days was an oversight from my side. 

This even means that the current Nut/OS implementation is flawed with the
overflow problem since it was changed on 2006/12/20. 

Even more reasons to overhaul the current implementation!

> I think that the 'arm arch' function is good, however I wonder why the

The 'arm arch' function  is certainly better for overflow handling. However
Duane pointed out in his previous post how the Linux kernel is dealing with
these issues. The Linux kernel code applies a similar rounding technique to
my suggestion and also an overflow code:

274 static inline unsigned long msecs_to_jiffies(const unsigned int m)
275 {
276         if (m > jiffies_to_msecs(MAX_JIFFY_OFFSET))
277                 return MAX_JIFFY_OFFSET;
278 #if HZ <= MSEC_PER_SEC && !(MSEC_PER_SEC % HZ)
279         return (m + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ);
280 #elif HZ > MSEC_PER_SEC && !(HZ % MSEC_PER_SEC)
281         return m * (HZ / MSEC_PER_SEC);
282 #else
283         return (m * HZ + MSEC_PER_SEC - 1) / MSEC_PER_SEC;
284 #endif
285 }

Unfortunately NutGetTickClock is a function not a pre-processor macro so we
can't do conditional compilation based on it like the Linux kernel code does
to optimise for the first two special cases. For that reason we can only
apply the last case of above #if clauses.

Based on above I would refine my suggestion as follow:

-----------------------------------------------------
u_long NutTimerMillisToTicks(u_long ms)
{
   if (ms >= 0x3E8000UL) 
      return (ms / 1000UL) * NutGetTickClock();
   else
      return (ms * NutGetTickClock() + 500UL - 1UL) / 1000UL; 
}
-----------------------------------------------------

> person who wrote the code split the choice between  the two calculus
> with 0x3E8000. If overflow is to be avoided because of a multiplication
> by 1000, then it should be ((2^32)-1)/1000 (4294967 in decimal, or hex
> 0x418937. Using 1024 ticks per second would give 0x400000).

No idea either why the magic value of 0x3E8000 was used.

> Now the rounding problem comes from having a number of ticks per second
> slightly lower than the divisor (1000). I don't think that adding 500
> fixes the problem, since you then have rounding errors  if you have a
> number of ticks slightly lower than 500 :)

Adding half of the divisor to an integer division is a common rounding
technique and yields in more accurate results than the current truncation
method. Refer to Linux kernel code above.
It is equivalent to adding 0.5 to a floating point value before converting
it to an integer and is also what the standard C round() function does.

Also the case of having a tick of 500 Hz is improved with my suggested
rounding method as the case of ms=2 yields in one tick which would be
correct. The case of ms=1 is not a rounding problem as such, we simply don't
have the resolution to express one ms as a tick in this scenario. We should
not mix rounding and resolution issues.

> What about (for the second calculus, the first being kept 'as is' even
> if 0x3E8000 is still 'magical'):
> 
> ((ms * NutGetTickClock()) + (NutGetTickClock ()/2)) / 1000UL; 

This would not be mathematically correct rounding, works only for special
cases and is even worse in case of NutGetTickClock being multiples of 1000.

Henrik