You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "jlaitine (via GitHub)" <gi...@apache.org> on 2023/01/31 06:49:12 UTC
[GitHub] [nuttx] jlaitine opened a new pull request, #8371: Change sec2tick macros to round up
jlaitine opened a new pull request, #8371:
URL: https://github.com/apache/nuttx/pull/8371
## Summary
This change is discussed in https://github.com/apache/nuttx/issues/8346 ; in short, this proposes changing USEC2TICK, MSEC2TICK and SEC2TICK to round up instead of round to nearest. This is the same practice that is used in many other OSs (like linux msecs_to_jiffies ; when converting a value to ticks the integer result is always greater or equivalent of the exact result.
The rationale behind this change is that typical use is a timeout or schedule wq event. That is, to go to sleep for *at least* N msec.
## Impact
This fixes regression in i2c drivers, networking and iob caused by https://github.com/apache/nuttx/commit/1fb8c13e5e80ee4cd5758e76821378162451c2fa
## Testing
It has been tested that the change fixes the found problems on stm32f7 and mpfs targets with 10ms systick. However, this affects many drivers in many platforms, I am not able to test them all.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 merged pull request #8371: Change sec2tick macros to round up
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8371:
URL: https://github.com/apache/nuttx/pull/8371
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8371: Change sec2tick macros to round up
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8371:
URL: https://github.com/apache/nuttx/pull/8371#discussion_r1091580504
##########
include/nuttx/clock.h:
##########
@@ -145,41 +145,49 @@
* preferred for that reason (at the risk of overflow)
*/
-#define TICK_PER_HOUR (USEC_PER_HOUR / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_MIN (USEC_PER_MIN / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_SEC (USEC_PER_SEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_MSEC (USEC_PER_MSEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_DSEC (USEC_PER_DSEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_HSEC (USEC_PER_HSEC / USEC_PER_TICK) /* Truncates! */
+/* TICK_PER_* truncates! */
-#define MSEC_PER_TICK (USEC_PER_TICK / USEC_PER_MSEC) /* Truncates! */
-#define NSEC_PER_TICK (USEC_PER_TICK * NSEC_PER_USEC) /* Exact */
+#define TICK_PER_HOUR (USEC_PER_HOUR / USEC_PER_TICK)
Review Comment:
should we use the roundup for these constant too?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jlaitine commented on a diff in pull request #8371: Change sec2tick macros to round up
Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #8371:
URL: https://github.com/apache/nuttx/pull/8371#discussion_r1091664006
##########
include/nuttx/clock.h:
##########
@@ -145,41 +145,49 @@
* preferred for that reason (at the risk of overflow)
*/
-#define TICK_PER_HOUR (USEC_PER_HOUR / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_MIN (USEC_PER_MIN / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_SEC (USEC_PER_SEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_MSEC (USEC_PER_MSEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_DSEC (USEC_PER_DSEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_HSEC (USEC_PER_HSEC / USEC_PER_TICK) /* Truncates! */
+/* TICK_PER_* truncates! */
-#define MSEC_PER_TICK (USEC_PER_TICK / USEC_PER_MSEC) /* Truncates! */
-#define NSEC_PER_TICK (USEC_PER_TICK * NSEC_PER_USEC) /* Exact */
+#define TICK_PER_HOUR (USEC_PER_HOUR / USEC_PER_TICK)
Review Comment:
Intuitively I'd say no:
- The intended use for those macros is different than of those that I was now changing
- There is a comment warning about these macros a bit above, describing the truncation/inaccuracy
- They haven't been used in many places (basically only TICK_PER_SEC is used) and in most (normal) cases it wouldn't make any difference either
- The TICK_PER_SEC is used in clock_ticks2time.c in a similar way as some other OSs do tick->timespec conversion, so it is currenly expecting it to truncate.
- They are also not directly related to the SEC2TICK macros, as due to rounding it is not anyway possible to do 1<->1 conversion between those, regardless of what rounding is used
So I wouldn't touch them now unless there is a good reason to do so (?)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] btashton commented on pull request #8371: Change sec2tick macros to round up
Posted by "btashton (via GitHub)" <gi...@apache.org>.
btashton commented on PR #8371:
URL: https://github.com/apache/nuttx/pull/8371#issuecomment-1409880880
This looks reasonable to me, but it would be really good to validate that we have not broken anything on a few different core platforms. Maybe not prior to merging but at least prior to the next release.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8371: Change sec2tick macros to round up
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8371:
URL: https://github.com/apache/nuttx/pull/8371#discussion_r1091685095
##########
include/nuttx/clock.h:
##########
@@ -145,41 +145,49 @@
* preferred for that reason (at the risk of overflow)
*/
-#define TICK_PER_HOUR (USEC_PER_HOUR / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_MIN (USEC_PER_MIN / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_SEC (USEC_PER_SEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_MSEC (USEC_PER_MSEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_DSEC (USEC_PER_DSEC / USEC_PER_TICK) /* Truncates! */
-#define TICK_PER_HSEC (USEC_PER_HSEC / USEC_PER_TICK) /* Truncates! */
+/* TICK_PER_* truncates! */
-#define MSEC_PER_TICK (USEC_PER_TICK / USEC_PER_MSEC) /* Truncates! */
-#define NSEC_PER_TICK (USEC_PER_TICK * NSEC_PER_USEC) /* Exact */
+#define TICK_PER_HOUR (USEC_PER_HOUR / USEC_PER_TICK)
Review Comment:
Ok.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org