jlaitine commented on PR #17221: URL: https://github.com/apache/nuttx/pull/17221#issuecomment-3436324868
So the intent is to now put back the systick drift, which will randomly break the hardware drivers by waking up the tick based sleeps too early? The change https://github.com/apache/nuttx/pull/15929 was done because in I2C and SPI drivers on MPFS platforms the nxsem_tickwait_uninterruptible randombly woke up too early (asked to sleep at least for 1 tick but randomly woke up on the next tick start). And the same issue occured also on arm64, and on *every* platform I tried to use. If currently the only broken architecture is "sim", why not add specific up_ndelay and up_udelay for "sim", and not break every other arch? It is easy to add a architecture specific variant of those functions (like I did in https://github.com/apache/nuttx/pull/16485 for mpfs). Accurate versions of up_ndelay etc. should just be done by the arch (hence the name up_*), because that is the place where all the architecture specific details of the timers are known. I am seeing many very strange things around tick timer, for example this doesn't make sense to me: https://github.com/apache/nuttx/pull/15938 . If someone is measuring / sleeping long times by using systick, he is doing something very wrong. The systick accuracy should not matter, but systick time should be *constant*. It should not vary! Instead of making systick time vary, one should have a constant defining the *exact* systick time after all the HW timer dependent roundings, and use that for calculations (how many ticks are needed for a certain time). Anyhow, I am glad to hear that there are improvements coming! Unfortunately I was unable to participate the workshop this year, I will look into the presentation for sure! Before that, below is a quick draft of how I imagined the timers should be architected. Again, I am not pushing any changes or anything, below is just an opinion. I really need to look into the @Fix-Point 's work, I am sure it will be good :) ``` Arch/TimerSource - provide accurate up_ndelay etc. - registers to TimerDriver (at boot) - provides timer compare interrupt for TimerDriver ^ | | Common/TimerDriver - list of timer sources (or just one per architecture) - sorted queue of bucketed callbacks - selects TimerSource and sets it to interrupt at next callback bucket expiration - at interrupt, calls all callbacks of the bucket - provides coarse up_ndelay etc. if accurate ones are not available for the architecture - provides interface for registering oneshot and periodic callbacks ^ | | Common/SystickDriver - registers a periodic callback to TimerDriver ``` Here any driver could register for accurate periodic callbacks to the common TimerDriver, and the systick could be just one of these. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
