jlaitine commented on PR #17221: URL: https://github.com/apache/nuttx/pull/17221#issuecomment-3433979933
> > Thank you @linguini1 for the PR and @xiaoxiang781216 for the hints in #17011 :-) > > One remark here, if we revert this commit there may be a short window of time where timers will be again inaccurate until fixes come from #17011? Or is it safe / necessary to merge in order to proceed with #17011 ? > > This change should make the timers more accurate, by adding back the accurate `ndelay`. It was less accurate after this removal because the implementation exclusively used busy-waiting, which is worse. The only difference is I imagine busy-waiting has better granularity than the accurate `ndelay` on some systems where the tick is large, assuming the busy-wait is not interrupted by other tasks, etc. I think if we want to improve the accuracy of `ndelay` then its implementation needs to change to something that's not busy waiting. We could maybe introduce some `ifdef` so if the architecture doesn't have good granularity on `ndelay`, it can default back to the busy-wait implementation? Might need a mailing list discussion after this is merged. > > Ultimately I think both ways have trade-offs, but the PR #14450 only says "Fixes many issues where up_udelay is used", not really citing any drivers/functionality that broke. However, concretely, removing this accurate `ndelay` function breaks delays entirely on `sim`, so no delays are respected. Did you even read the summary of PR #14450 ? It doesn't "only say" what you claim, but explain the problem that the PR fixes. The ndelay_accurate was not very accurate when it quatized the delay to *systick* length. This caused e.g. udelay(1) to delay for close to 10000 microseconds instead of busylooping for 1 microsecond, on a system with 10ms tick. For any hardware driver initialization, where one needed to wait for a few hundred nanoseconds, the udelay(1) was commontly used. With the "accurate" ndelay everything just blew up. So when you revert that, please make sure that it doesn't happen again. I made a fix for mpfs to use architecture specific delays, which really check the time from a timer when busylooping, and that is of course the way to go. But *not* using the current time from oneshot_current, which calculated it from the tick! -- 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]
