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


##########
drivers/segger/sysview.c:
##########
@@ -22,55 +22,94 @@
  * Included Files
  ****************************************************************************/
 
-#include <nuttx/config.h>
-#include <syslog.h>
-
 #include <nuttx/clock.h>
 #include <nuttx/sched.h>
 #include <nuttx/sched_note.h>
+#include <nuttx/note/note_driver.h>
 #include <nuttx/segger/sysview.h>
 
+#include <stddef.h>
+#include <syslog.h>
 #include <SEGGER_RTT.h>
 #include <SEGGER_SYSVIEW.h>
 
 #include "sched/sched.h"
 
 /****************************************************************************
- * Private Types
+ * Private Functions
  ****************************************************************************/
 
-struct sysview_s
-{
-  unsigned int                 irq[CONFIG_SMP_NCPUS];
-#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s    mode;
+static void sysview_start(FAR struct note_driver_s *drv,
+                          FAR struct tcb_s *tcb);
+static void sysview_stop(FAR struct note_driver_s *drv,
+                         FAR struct tcb_s *tcb);
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SWITCH
+static void sysview_suspend(FAR struct note_driver_s *drv,
+                            FAR struct tcb_s *tcb);
+static void sysview_resume(FAR struct note_driver_s *drv,
+                           FAR struct tcb_s *tcb);
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s     irq_mask;
+static void sysview_irqhandler(FAR struct note_driver_s *drv, int irq,
+                               FAR void *handler, bool enter);
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-  struct note_filter_syscall_s syscall_mask;
-  struct note_filter_syscall_s syscall_marker;
+static void sysview_syscall_enter(FAR struct note_driver_s *drv,
+                                  int nr, int argc, va_list *ap);
+static void sysview_syscall_leave(FAR struct note_driver_s *drv,
+                                  int nr, uintptr_t result);
 #endif
-};
 
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
-static struct sysview_s g_sysview =
+static struct note_driver_ops_s g_sysview_ops =

Review Comment:
   add const



##########
include/nuttx/segger/sysview.h:
##########
@@ -25,7 +25,23 @@
  * Included Files
  ****************************************************************************/
 
-#include <nuttx/config.h>
+#include <nuttx/note/note_driver.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+struct note_sysview_driver_s
+{
+  struct note_driver_s driver;
+  unsigned int         irq[CONFIG_SMP_NCPUS];
+};
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+extern struct note_sysview_driver_s g_sysview_driver;

Review Comment:
   add ifdef/endif guard



##########
drivers/segger/sysview.c:
##########
@@ -22,55 +22,94 @@
  * Included Files
  ****************************************************************************/
 
-#include <nuttx/config.h>
-#include <syslog.h>
-
 #include <nuttx/clock.h>
 #include <nuttx/sched.h>
 #include <nuttx/sched_note.h>
+#include <nuttx/note/note_driver.h>
 #include <nuttx/segger/sysview.h>
 
+#include <stddef.h>
+#include <syslog.h>
 #include <SEGGER_RTT.h>
 #include <SEGGER_SYSVIEW.h>
 
 #include "sched/sched.h"
 
 /****************************************************************************
- * Private Types
+ * Private Functions

Review Comment:
   Private Function Prototypes



##########
drivers/segger/sysview.c:
##########
@@ -365,63 +255,33 @@ void sched_note_irqhandler(int irq, FAR void *handler, 
bool enter)
             }
         }
 
-      g_sysview.irq[up_cpu_index()] = 0;
+      g_sysview_driver.irq[up_cpu_index()] = 0;
     }
 }
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-void sched_note_syscall_enter(int nr, int argc, ...)
+static void sysview_syscall_enter(FAR struct note_driver_s *drv, int nr,
+                                  int argc, va_list *ap)
 {
   nr -= CONFIG_SYS_RESERVED;
-
-  if (sysview_isenabled_syscall(nr) == 0)
-    {
-      return;
-    }
-
-  /* Set the name marker if the current syscall nr is not active */
-
-  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_marker) == 0)
-    {
-      /* Set the name marker */
-
-      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
-
-      /* Mark the syscall active */
-
-      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_marker);
-
-      /* Use the Syscall "0" to identify whether the syscall is enabled,
-       * if the host tool is closed abnormally, use this bit to clear
-       * the active set.
-       */
-
-      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_marker) == 0)
-        {
-          NOTE_FILTER_SYSCALLMASK_SET(0, &g_sysview.syscall_marker);
-        }
-    }
-
+  SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);

Review Comment:
   it's inefficient to send the name marker every time, why remove the original 
logic which avoid send the name more than once.



##########
drivers/segger/sysview.c:
##########
@@ -22,55 +22,94 @@
  * Included Files
  ****************************************************************************/
 
-#include <nuttx/config.h>
-#include <syslog.h>
-
 #include <nuttx/clock.h>
 #include <nuttx/sched.h>
 #include <nuttx/sched_note.h>
+#include <nuttx/note/note_driver.h>
 #include <nuttx/segger/sysview.h>
 
+#include <stddef.h>
+#include <syslog.h>
 #include <SEGGER_RTT.h>
 #include <SEGGER_SYSVIEW.h>
 
 #include "sched/sched.h"
 
 /****************************************************************************
- * Private Types
+ * Private Functions
  ****************************************************************************/
 
-struct sysview_s
-{
-  unsigned int                 irq[CONFIG_SMP_NCPUS];
-#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s    mode;
+static void sysview_start(FAR struct note_driver_s *drv,
+                          FAR struct tcb_s *tcb);
+static void sysview_stop(FAR struct note_driver_s *drv,
+                         FAR struct tcb_s *tcb);
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SWITCH
+static void sysview_suspend(FAR struct note_driver_s *drv,
+                            FAR struct tcb_s *tcb);
+static void sysview_resume(FAR struct note_driver_s *drv,
+                           FAR struct tcb_s *tcb);
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s     irq_mask;
+static void sysview_irqhandler(FAR struct note_driver_s *drv, int irq,
+                               FAR void *handler, bool enter);
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-  struct note_filter_syscall_s syscall_mask;
-  struct note_filter_syscall_s syscall_marker;
+static void sysview_syscall_enter(FAR struct note_driver_s *drv,
+                                  int nr, int argc, va_list *ap);
+static void sysview_syscall_leave(FAR struct note_driver_s *drv,
+                                  int nr, uintptr_t result);
 #endif
-};
 
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
-static struct sysview_s g_sysview =
+static struct note_driver_ops_s g_sysview_ops =
 {
-#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  .mode =
-    {
-      .flag = CONFIG_SCHED_INSTRUMENTATION_FILTER_DEFAULT_MODE,
-#ifdef CONFIG_SMP
-      .cpuset = CONFIG_SCHED_INSTRUMENTATION_CPUSET,
+  NULL,                  /* add */
+  sysview_start,         /* start */
+  sysview_stop,          /* stop */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SWITCH
+  sysview_suspend,       /* suspend */
+  sysview_resume,        /* resume */
+#  ifdef CONFIG_SMP
+  NULL,                  /* cpu_start */
+  NULL,                  /* cpu_started */
+  NULL,                  /* cpu_pause */
+  NULL,                  /* cpu_paused */
+  NULL,                  /* cpu_resume */
+  NULL,                  /* cpu_resumed */
+#  endif
 #endif
-    }
+#ifdef CONFIG_SCHED_INSTRUMENTATION_PREEMPTION
+  NULL,                  /* premption */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
+  NULL,                  /* csection */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  NULL,                  /* spinlock */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  sysview_syscall_enter, /* syscall_enter */
+  sysview_syscall_leave, /* syscall_leave */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+  sysview_irqhandler,    /* irqhandler */
 #endif
 };
 
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct note_sysview_driver_s g_sysview_driver =
+{
+    {

Review Comment:
   remove two extra spaces



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

Reply via email to