Re: [PR] driver/note: To support clock_gettime for CPU sleep scenario [nuttx]

2025-05-13 Thread via GitHub 


xiaoxiang781216 commented on code in PR #16369:
URL: https://github.com/apache/nuttx/pull/16369#discussion_r2086770652


##
drivers/note/Kconfig:
##
@@ -36,6 +36,19 @@ config DRIVERS_NOTECTL
If this option is selected, the instrumentation filter control 
device
/dev/notectl is provided.
 
+config NOTE_GET_PERF_TIME

Review Comment:
   let's change to choice



##
drivers/note/note_driver.c:
##
@@ -217,7 +217,17 @@ static void note_common(FAR struct tcb_s *tcb,
   note->nc_pid = tcb->pid;
 }
 
+#if defined (CONFIG_NOTE_GET_PERF_TIME)

Review Comment:
   ```suggestion
   #if defined(CONFIG_NOTE_GET_PERF_TIME)
   ```



##
drivers/note/note_driver.c:
##
@@ -217,7 +217,17 @@ static void note_common(FAR struct tcb_s *tcb,
   note->nc_pid = tcb->pid;
 }
 
+#if defined (CONFIG_NOTE_GET_PERF_TIME)
   note->nc_systime = perf_gettime();
+
+#elif defined (CONFIG_NOTE_GET_CLOCK_TIME)
+  struct timespec ts;
+  clock_gettime(CLOCK_REALTIME, &ts);
+  note->nc_systime = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
+
+#else

Review Comment:
   remove



##
drivers/note/Kconfig:
##
@@ -36,6 +36,19 @@ config DRIVERS_NOTECTL
If this option is selected, the instrumentation filter control 
device
/dev/notectl is provided.
 
+config NOTE_GET_PERF_TIME
+   bool "Note get time config"
+   default y
+   ---help---
+   If this option is selected, then note get time from perf time
+
+config NOTE_GET_CLOCK_TIME

Review Comment:
   DRIVERS_NOTE_CLOCK_TIME



##
drivers/note/Kconfig:
##
@@ -36,6 +36,19 @@ config DRIVERS_NOTECTL
If this option is selected, the instrumentation filter control 
device
/dev/notectl is provided.
 
+config NOTE_GET_PERF_TIME

Review Comment:
   DRIVERS_NOTE_PERF_TIME



##
drivers/note/note_driver.c:
##
@@ -217,7 +217,17 @@ static void note_common(FAR struct tcb_s *tcb,
   note->nc_pid = tcb->pid;
 }
 
+#if defined (CONFIG_NOTE_GET_PERF_TIME)
   note->nc_systime = perf_gettime();
+
+#elif defined (CONFIG_NOTE_GET_CLOCK_TIME)
+  struct timespec ts;

Review Comment:
   doesn't allow define the variable in the middle of function



##
drivers/note/note_driver.c:
##
@@ -217,7 +217,17 @@ static void note_common(FAR struct tcb_s *tcb,
   note->nc_pid = tcb->pid;
 }
 
+#if defined (CONFIG_NOTE_GET_PERF_TIME)
   note->nc_systime = perf_gettime();
+
+#elif defined (CONFIG_NOTE_GET_CLOCK_TIME)

Review Comment:
   ```suggestion
   #elif defined(CONFIG_NOTE_GET_CLOCK_TIME)
   ```



-- 
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-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] driver/note: To support clock_gettime for CPU sleep scenario [nuttx]

2025-05-13 Thread via GitHub 


acassis commented on code in PR #16369:
URL: https://github.com/apache/nuttx/pull/16369#discussion_r2086713775


##
drivers/note/Kconfig:
##
@@ -36,6 +36,19 @@ config DRIVERS_NOTECTL
If this option is selected, the instrumentation filter control 
device
/dev/notectl is provided.
 
+config NOTE_GET_PERF_TIME
+   bool "Note get time config"
+   default y
+   ---help---
+   If this option is selected, then note get time from perf time
+
+config NOTE_GET_CLOCK_TIME
+   bool "Note get time config"

Review Comment:
   Please don't use the same message ("Note get time config") in boot configs, 
it will confuse users. The description in the config should be clear to the 
users!



##
drivers/note/Kconfig:
##
@@ -36,6 +36,19 @@ config DRIVERS_NOTECTL
If this option is selected, the instrumentation filter control 
device
/dev/notectl is provided.
 
+config NOTE_GET_PERF_TIME
+   bool "Note get time config"
+   default y
+   ---help---
+   If this option is selected, then note get time from perf time

Review Comment:
   Please improve the "---help---", what is "perf time"? What difference to 
"clock time", etc. These are question that user need to know before enabling 
this option



-- 
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-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] driver/note: To support clock_gettime for CPU sleep scenario [nuttx]

2025-05-13 Thread via GitHub 


yangwei-x commented on code in PR #16369:
URL: https://github.com/apache/nuttx/pull/16369#discussion_r2086665253


##
drivers/note/noteram_driver.c:
##
@@ -794,7 +794,17 @@ static int noteram_dump_header(FAR struct lib_outstream_s 
*s,
   pid_t pid;
   int ret;
 
+

Review Comment:
   Remove this line



-- 
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-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org