You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/07 12:43:49 UTC

[GitHub] [incubator-nuttx] zyfeier opened a new pull request, #7033: nuttx/timer: add arch_alarm tick interface

zyfeier opened a new pull request, #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033

   ## Summary
   
   Change sched and up timer interface to tick count.
   
   ## Impact
   
   ## Testing
   
   


-- 
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] [incubator-nuttx] zyfeier commented on pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
zyfeier commented on PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#issuecomment-1244932157

   > Hi @zyfeier please include more information about this PR, what was the main motivation and what are the benefits of using this new interface? Did it make the system more accurate for timer measuring?
   
   Hi,  acassis. This PR is not ready for review, it's a temporary change. I'll add more information if it was ready.


-- 
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] [incubator-nuttx] zyfeier commented on a diff in pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
zyfeier commented on code in PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#discussion_r992224306


##########
arch/arm/src/stm32h7/stm32_tickless.c:
##########
@@ -729,9 +729,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   Done



-- 
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] [incubator-nuttx] zyfeier commented on a diff in pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
zyfeier commented on code in PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#discussion_r992224033


##########
arch/arm/src/stm32f7/stm32_tickless.c:
##########
@@ -776,7 +776,7 @@ int up_timer_getcounter(uint64_t *cycles)
  *
  ****************************************************************************/
 
-void up_timer_getmask(uint64_t *mask)
+void up_timer_getmask(FAR clock_t *mask)

Review Comment:
   Done



-- 
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] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#discussion_r992649770


##########
include/nuttx/clock.h:
##########
@@ -251,6 +251,19 @@ EXTERN volatile clock_t g_system_ticks;
  * Public Function Prototypes
  ****************************************************************************/
 
+#define timespec_from_tick(ts, tick) \
+  do \
+    { \
+      clock_t _tick = (tick); \
+      (ts)->tv_sec = _tick / TICK_PER_SEC; \
+      _tick -= (clock_t)(ts)->tv_sec * TICK_PER_SEC; \
+      (ts)->tv_nsec = (clock_t)_tick * NSEC_PER_TICK; \

Review Comment:
   ```suggestion
         (ts)->tv_nsec = _tick * NSEC_PER_TICK; \
   ```
   since `_tick` is already `clock_t` type



-- 
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] [incubator-nuttx] zyfeier commented on a diff in pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
zyfeier commented on code in PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#discussion_r992223618


##########
arch/arm/src/stm32/stm32_tickless.c:
##########
@@ -698,9 +698,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   Done



##########
arch/arm/src/stm32/stm32_tickless.c:
##########
@@ -718,7 +718,7 @@ int up_timer_getcounter(uint64_t *cycles)
  *
  ****************************************************************************/
 
-void up_timer_getmask(uint64_t *mask)
+void up_timer_getmask(FAR clock_t *mask)

Review Comment:
   Done



##########
arch/arm/src/stm32f7/stm32_tickless.c:
##########
@@ -756,9 +756,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   Done



-- 
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] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#discussion_r991918819


##########
arch/arm/src/stm32/stm32_tickless.c:
##########
@@ -698,9 +698,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   ```suggestion
   int up_timer_gettick(clock_t *ticks)
   ```



##########
arch/arm/src/stm32h7/stm32_tickless.c:
##########
@@ -749,7 +749,7 @@ int up_timer_getcounter(uint64_t *cycles)
  *
  ****************************************************************************/
 
-void up_timer_getmask(uint64_t *mask)
+void up_timer_getmask(FAR clock_t *mask)

Review Comment:
   ```suggestion
   void up_timer_getmask(clock_t *mask)
   ```



##########
arch/arm/src/stm32h7/stm32_tickless.c:
##########
@@ -729,9 +729,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   ```suggestion
   int up_timer_gettick(clock_t *ticks)
   ```



##########
arch/arm/src/stm32f7/stm32_tickless.c:
##########
@@ -756,9 +756,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   ```suggestion
   int up_timer_gettick(clock_t *ticks)
   ```



##########
arch/arm/src/stm32f7/stm32_tickless.c:
##########
@@ -776,7 +776,7 @@ int up_timer_getcounter(uint64_t *cycles)
  *
  ****************************************************************************/
 
-void up_timer_getmask(uint64_t *mask)
+void up_timer_getmask(FAR clock_t *mask)

Review Comment:
   ```suggestion
   void up_timer_getmask(clock_t *mask)
   ```



##########
arch/arm/src/stm32wb/stm32wb_tickless.c:
##########
@@ -592,7 +592,7 @@ int up_timer_getcounter(uint64_t *cycles)
  *
  ****************************************************************************/
 
-void up_timer_getmask(uint64_t *mask)
+void up_timer_getmask(FAR clock_t *mask)

Review Comment:
   ```suggestion
   void up_timer_getmask(clock_t *mask)
   ```



##########
include/nuttx/timers/oneshot.h:
##########
@@ -192,6 +204,15 @@ struct oneshot_operations_s
                      FAR struct timespec *ts);
   CODE int (*current)(struct oneshot_lowerhalf_s *lower,
                       FAR struct timespec *ts);
+  CODE int (*tick_max_delay)(FAR struct oneshot_lowerhalf_s *lower,
+                             FAR clock_t *ticks);
+  CODE int (*tick_start)(FAR struct oneshot_lowerhalf_s *lower,
+                         oneshot_callback_t callback, FAR void *arg,
+                         clock_t ticks);
+  CODE int (*tick_cancel)(struct oneshot_lowerhalf_s *lower,
+                          FAR clock_t *ticks);
+  CODE int (*tick_current)(struct oneshot_lowerhalf_s *lower,

Review Comment:
   ```suggestion
     CODE int (*tick_current)(FAR struct oneshot_lowerhalf_s *lower,
   ```



##########
include/nuttx/clock.h:
##########
@@ -251,6 +251,19 @@ EXTERN volatile clock_t g_system_ticks;
  * Public Function Prototypes
  ****************************************************************************/
 
+#define timespec_from_tick(ts, tick) \
+  do \
+    { \
+      clock_t _tick = tick; \

Review Comment:
   ```suggestion
         clock_t _tick = (tick); \
   ```



##########
include/nuttx/timers/oneshot.h:
##########
@@ -237,6 +258,120 @@ extern "C"
  * Public Function Prototypes
  ****************************************************************************/
 
+static inline
+int oneshot_max_delay(FAR struct oneshot_lowerhalf_s *lower,
+                      FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_max_delay);
+
+  ret = lower->ops->tick_max_delay(lower, &tick);
+  timespec_from_tick(ts, tick);
+  return ret;
+}
+
+static inline
+int oneshot_start(FAR struct oneshot_lowerhalf_s *lower,
+                  oneshot_callback_t callback, FAR void *arg,
+                  FAR const struct timespec *ts)
+{
+  clock_t tick;
+
+  DEBUGASSERT(lower->ops->tick_start);
+
+  tick = timespec_to_tick(ts);
+  return lower->ops->tick_start(lower, callback, arg, tick);
+}
+
+static inline
+int oneshot_cancel(struct oneshot_lowerhalf_s *lower,
+                   FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_cancel);
+
+  ret = lower->ops->tick_cancel(lower, &tick);
+  timespec_from_tick(ts, tick);
+
+  return ret;
+}
+
+static inline
+int oneshot_current(struct oneshot_lowerhalf_s *lower,

Review Comment:
   ```suggestion
   int oneshot_current(FAR struct oneshot_lowerhalf_s *lower,
   ```



##########
include/nuttx/timers/oneshot.h:
##########
@@ -237,6 +258,120 @@ extern "C"
  * Public Function Prototypes
  ****************************************************************************/
 
+static inline
+int oneshot_max_delay(FAR struct oneshot_lowerhalf_s *lower,
+                      FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_max_delay);
+
+  ret = lower->ops->tick_max_delay(lower, &tick);
+  timespec_from_tick(ts, tick);
+  return ret;
+}
+
+static inline
+int oneshot_start(FAR struct oneshot_lowerhalf_s *lower,
+                  oneshot_callback_t callback, FAR void *arg,
+                  FAR const struct timespec *ts)
+{
+  clock_t tick;
+
+  DEBUGASSERT(lower->ops->tick_start);
+
+  tick = timespec_to_tick(ts);
+  return lower->ops->tick_start(lower, callback, arg, tick);
+}
+
+static inline
+int oneshot_cancel(struct oneshot_lowerhalf_s *lower,

Review Comment:
   ```suggestion
   int oneshot_cancel(FAR struct oneshot_lowerhalf_s *lower,
   ```



##########
include/nuttx/timers/oneshot.h:
##########
@@ -192,6 +204,15 @@ struct oneshot_operations_s
                      FAR struct timespec *ts);
   CODE int (*current)(struct oneshot_lowerhalf_s *lower,
                       FAR struct timespec *ts);
+  CODE int (*tick_max_delay)(FAR struct oneshot_lowerhalf_s *lower,
+                             FAR clock_t *ticks);
+  CODE int (*tick_start)(FAR struct oneshot_lowerhalf_s *lower,
+                         oneshot_callback_t callback, FAR void *arg,
+                         clock_t ticks);
+  CODE int (*tick_cancel)(struct oneshot_lowerhalf_s *lower,

Review Comment:
   ```suggestion
     CODE int (*tick_cancel)(FAR struct oneshot_lowerhalf_s *lower,
   ```



##########
arch/arm/src/stm32wb/stm32wb_tickless.c:
##########
@@ -572,9 +572,9 @@ int up_timer_gettime(struct timespec *ts)
  *
  ****************************************************************************/
 
-int up_timer_getcounter(uint64_t *cycles)
+int up_timer_gettick(FAR clock_t *ticks)

Review Comment:
   ```suggestion
   int up_timer_gettick(clock_t *ticks)
   ```



##########
arch/arm/src/stm32/stm32_tickless.c:
##########
@@ -718,7 +718,7 @@ int up_timer_getcounter(uint64_t *cycles)
  *
  ****************************************************************************/
 
-void up_timer_getmask(uint64_t *mask)
+void up_timer_getmask(FAR clock_t *mask)

Review Comment:
   ```suggestion
   void up_timer_getmask(clock_t *mask)
   ```



##########
include/nuttx/timers/oneshot.h:
##########
@@ -237,6 +258,120 @@ extern "C"
  * Public Function Prototypes
  ****************************************************************************/
 
+static inline
+int oneshot_max_delay(FAR struct oneshot_lowerhalf_s *lower,
+                      FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_max_delay);
+
+  ret = lower->ops->tick_max_delay(lower, &tick);
+  timespec_from_tick(ts, tick);
+  return ret;
+}
+
+static inline
+int oneshot_start(FAR struct oneshot_lowerhalf_s *lower,
+                  oneshot_callback_t callback, FAR void *arg,
+                  FAR const struct timespec *ts)
+{
+  clock_t tick;
+
+  DEBUGASSERT(lower->ops->tick_start);
+
+  tick = timespec_to_tick(ts);
+  return lower->ops->tick_start(lower, callback, arg, tick);
+}
+
+static inline
+int oneshot_cancel(struct oneshot_lowerhalf_s *lower,
+                   FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_cancel);
+
+  ret = lower->ops->tick_cancel(lower, &tick);
+  timespec_from_tick(ts, tick);
+
+  return ret;
+}
+
+static inline
+int oneshot_current(struct oneshot_lowerhalf_s *lower,
+                    FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_current);
+
+  ret = lower->ops->tick_current(lower, &tick);
+  timespec_from_tick(ts, tick);
+
+  return ret;
+}
+
+static inline
+int oneshot_tick_max_delay(FAR struct oneshot_lowerhalf_s *lower,
+                           FAR clock_t *ticks)
+{
+  struct timespec ts;
+  int ret;
+
+  DEBUGASSERT(lower->ops->max_delay);
+
+  ret = lower->ops->max_delay(lower, &ts);
+  *ticks = timespec_to_tick(&ts);
+  return ret;
+}
+
+static inline
+int oneshot_tick_start(FAR struct oneshot_lowerhalf_s *lower,
+                       oneshot_callback_t callback, FAR void *arg,
+                       clock_t ticks)
+{
+  struct timespec ts;
+
+  DEBUGASSERT(lower->ops->start);
+
+  timespec_from_tick(&ts, ticks);
+  return lower->ops->start(lower, callback, arg, &ts);
+}
+
+static inline
+int oneshot_tick_cancel(struct oneshot_lowerhalf_s *lower,

Review Comment:
   ```suggestion
   int oneshot_tick_cancel(FAR struct oneshot_lowerhalf_s *lower,
   ```



##########
drivers/timers/arch_timer.c:
##########
@@ -315,6 +302,21 @@ int weak_function up_timer_gettime(FAR struct timespec *ts)
 }
 #endif
 
+#if defined(CONFIG_SCHED_TICKLESS_TICK_ARGUMENT) || defined(CONFIG_CLOCK_TIMEKEEPING)
+int weak_function up_timer_gettick(FAR clock_t *ticks)
+{
+  int ret = -EAGAIN;
+
+  if (g_timer.lower != NULL)
+    {
+      *ticks = current_usec() / USEC_PER_TICK;
+      ret = 0;

Review Comment:
   ```suggestion
         ret = OK;
   ```



##########
include/nuttx/timers/oneshot.h:
##########
@@ -237,6 +258,120 @@ extern "C"
  * Public Function Prototypes
  ****************************************************************************/
 
+static inline
+int oneshot_max_delay(FAR struct oneshot_lowerhalf_s *lower,
+                      FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_max_delay);
+
+  ret = lower->ops->tick_max_delay(lower, &tick);
+  timespec_from_tick(ts, tick);
+  return ret;
+}
+
+static inline
+int oneshot_start(FAR struct oneshot_lowerhalf_s *lower,
+                  oneshot_callback_t callback, FAR void *arg,
+                  FAR const struct timespec *ts)
+{
+  clock_t tick;
+
+  DEBUGASSERT(lower->ops->tick_start);
+
+  tick = timespec_to_tick(ts);
+  return lower->ops->tick_start(lower, callback, arg, tick);
+}
+
+static inline
+int oneshot_cancel(struct oneshot_lowerhalf_s *lower,
+                   FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_cancel);
+
+  ret = lower->ops->tick_cancel(lower, &tick);
+  timespec_from_tick(ts, tick);
+
+  return ret;
+}
+
+static inline
+int oneshot_current(struct oneshot_lowerhalf_s *lower,
+                    FAR struct timespec *ts)
+{
+  clock_t tick;
+  int ret;
+
+  DEBUGASSERT(lower->ops->tick_current);
+
+  ret = lower->ops->tick_current(lower, &tick);
+  timespec_from_tick(ts, tick);
+
+  return ret;
+}
+
+static inline
+int oneshot_tick_max_delay(FAR struct oneshot_lowerhalf_s *lower,
+                           FAR clock_t *ticks)
+{
+  struct timespec ts;
+  int ret;
+
+  DEBUGASSERT(lower->ops->max_delay);
+
+  ret = lower->ops->max_delay(lower, &ts);
+  *ticks = timespec_to_tick(&ts);
+  return ret;
+}
+
+static inline
+int oneshot_tick_start(FAR struct oneshot_lowerhalf_s *lower,
+                       oneshot_callback_t callback, FAR void *arg,
+                       clock_t ticks)
+{
+  struct timespec ts;
+
+  DEBUGASSERT(lower->ops->start);
+
+  timespec_from_tick(&ts, ticks);
+  return lower->ops->start(lower, callback, arg, &ts);
+}
+
+static inline
+int oneshot_tick_cancel(struct oneshot_lowerhalf_s *lower,
+                        FAR clock_t *ticks)
+{
+  struct timespec ts;
+  int ret;
+
+  DEBUGASSERT(lower->ops->cancel);
+
+  ret = lower->ops->cancel(lower, &ts);
+  *ticks = timespec_to_tick(&ts);
+
+  return ret;
+}
+
+static inline
+int oneshot_tick_current(struct oneshot_lowerhalf_s *lower,

Review Comment:
   ```suggestion
   int oneshot_tick_current(FAR struct oneshot_lowerhalf_s *lower,
   ```



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#issuecomment-1273619475

   @acassis this patch is ready for review. The motivation of this patch is to remove tick<->timespec conversion in tickless mode to improve the performance. Of course, the old interface still keep, the user need enable SCHED_TICKLESS_TICK_ARGUMENT to get the benefit.


-- 
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] [incubator-nuttx] masayuki2009 commented on pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#issuecomment-1274679995

   @zyfeier 
   Please rebase this PR to the latest upstream master.
   


-- 
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] [incubator-nuttx] xiaoxiang781216 merged pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033


-- 
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] [incubator-nuttx] acassis commented on pull request #7033: nuttx/timer: add arch_alarm tick interface

Posted by GitBox <gi...@apache.org>.
acassis commented on PR #7033:
URL: https://github.com/apache/incubator-nuttx/pull/7033#issuecomment-1242991767

   Hi @zyfeier please include more information about this PR, what was the main motivation and what are the benefits of using this new interface? Did it make the system more accurate for timer measuring?


-- 
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