Hello everybody, I have noticed problematic pattern in the imxRT code which is probably copied from STM32 code
./arch/arm/src/stm32f7/stm32_tickless.c: while (tm <= stm32_get_counter()) ./arch/arm/src/imxrt/imxrt_tickless.c: while (tm <= imxrt_get_counter()) Criticality is minor, because overflow or 64-bit counter occurs for microsecond resolution only once per ~ 5e5 years and has to coincide with miss of he event. But problem reported anyway, because it similar problem appear in other places where cyclic arithmetics is intended The code https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/imxrt/imxrt_tickless.c#L539 size_t offset = 1; uint64_t tm .... .... while (tm <= imxrt_get_counter()) { tm = imxrt_get_counter() + offset++; putreg32(tm, g_tickless.base + IMXRT_GPT_OCR1_OFFSET + \ (4 * (g_tickless.out_compare - 1))); } When the cyclic nature of the tm requires cyclic (modulo arithmetics) compare. But as is shown in the example below, the comparison can lead incorrect decision that alarm has been set successfully even when the time is and counter overflows. The correct condition for cyclic compare is while ((int64_t)(tm - imxrt_get_counter()) <= 0) Modulo, cyclic arithmetics and overflows are guaranteed only for unsigned types by C language standard. That was correct. But for substraction to find which way is the shorter path on the cycle between two points, it is necessary to cast result to the signed type no larger than each of the inputs. In this case uint64_t - uint64_t, so same or smaller type with sign is int64_t . The code has another problem, that alarm time can be easily far after hardware counter overflow. But that should not be a problem in such case, because compare to value truncated to 32-bits is triggered earlier and if all other logic goes well, the next event would be shedulled after complete cycle of the 32-bit part of the counter. The analysis would get much more complex when calibration of the clock and fine scaling is considered in the future. In such case, maximam revet realizable time has to be considered and value has to be limited to what is supported in the hardware. I can help to help read Linux kernel part of this code... I have contributed to it years ago when I have extended i.MX1 support to switch from tick based solution to high resolution timing introduced by Thomas Gleixner at the time. Ability to use clock/timer which period is not multiple of 1 usec could be considered for NuttX in the long term perspective. Best wishes, Pavel PS: I personally prefer to document cyclic nature of given comparison by macros ul_cyclic_gt(x,y) and #define ul_cyclic_ge(x,y) https://gitlab.com/pikron/sw-base/ulut/-/blob/master/ulut/ul_utdefs.h#L93 but solution is a little fragile for case where some integer type can be defined on some target as using only 16 bits from 20 or 24-bit char on some DSP with memory organized by words. But even in DSP cases I have not found such problem yet. !6-bit type forced to align to 4 bytes could be possible even on less obscure platform. --------------------------------------------------------------------------------------------- #include <stdio.h> #include <stdint.h> int main(int argc, char *argv[]) { uint64_t counter; uint64_t tm; int cmp11, cmp12, cmp13, cmp21, cmp22, cmp23; counter = 1234; tm = counter + 10; counter = counter + 20; cmp11 = (tm <= counter); cmp12 = ((tm - counter) <= 0); cmp13 = ((int64_t)(tm - counter) <= 0); printf("compare 1 default %d, substract %d, cyclic %d\n", cmp11, cmp12, cmp13); counter = (uint64_t)-15; tm = counter + 10; counter = counter + 20; cmp21 = (tm <= counter); cmp22 = ((tm - counter) <= 0); cmp23 = ((int64_t)(tm - counter) <= 0); printf("compare 2 default %d, substract %d, cyclic %d\n", cmp21, cmp22, cmp23); return 0; } ---------------------------------------------------------------------------------------------
