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 2023/01/02 16:11:36 UTC

[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #7984: register sysview to note drivers list

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


##########
include/nuttx/segger/sysview.h:
##########
@@ -25,7 +25,7 @@
  * Included Files
  ****************************************************************************/
 
-#include <nuttx/config.h>
+#include <nuttx/note/note_driver.h>

Review Comment:
   revert the change



##########
drivers/note/note_driver.c:
##########
@@ -40,6 +40,7 @@
 #include <nuttx/note/notelog_driver.h>
 #include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
+#include <nuttx/segger/sysview.h>

Review Comment:
   remove



##########
drivers/segger/sysview.c:
##########
@@ -365,63 +262,63 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
             }
         }
 
-      g_sysview.irq[up_cpu_index()] = 0;
+      g_note_sysview_driver.irq[up_cpu_index()] = 0;
     }
 }
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-void sched_note_syscall_enter(int nr, int argc, ...)
+static void note_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)
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr,
+      &g_note_sysview_driver.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);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_note_sysview_driver.syscall_marker);

Review Comment:
   ditto



##########
drivers/segger/sysview.c:
##########
@@ -333,16 +234,12 @@ void sched_note_resume(FAR struct tcb_s *tcb)
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
+static void note_sysview_irqhandler(FAR struct note_driver_s *drv, int irq,
+                                    FAR void *handler, bool enter)
 {
-  if (!sysview_isenabled_irq(irq, enter))

Review Comment:
   let's cast drv to avoid using the global variable(g_note_sysview_driver) as much as possible:
   FAR struct note_sysview_driver_s *sysview = ...



##########
drivers/segger/sysview.c:
##########
@@ -365,63 +262,63 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
             }
         }
 
-      g_sysview.irq[up_cpu_index()] = 0;
+      g_note_sysview_driver.irq[up_cpu_index()] = 0;
     }
 }
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-void sched_note_syscall_enter(int nr, int argc, ...)
+static void note_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)
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr,
+      &g_note_sysview_driver.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);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_note_sysview_driver.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)
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0,
+            &g_note_sysview_driver.syscall_marker) == 0)
         {
-          NOTE_FILTER_SYSCALLMASK_SET(0, &g_sysview.syscall_marker);
+          NOTE_FILTER_SYSCALLMASK_SET(0,
+            &g_note_sysview_driver.syscall_marker);
         }
     }
 
   SEGGER_SYSVIEW_MarkStart(nr);
 }
 
-void sched_note_syscall_leave(int nr, uintptr_t result)
+static void note_sysview_syscall_leave(FAR struct note_driver_s *drv,
+                                       int nr, uintptr_t result)
 {
   nr -= CONFIG_SYS_RESERVED;
 
-  if (sysview_isenabled_syscall(nr) == 0)
-    {
-      return;
-    }
-
-  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_marker) != 0)
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr,

Review Comment:
   ditto



##########
drivers/segger/sysview.c:
##########
@@ -365,63 +262,63 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
             }
         }
 
-      g_sysview.irq[up_cpu_index()] = 0;
+      g_note_sysview_driver.irq[up_cpu_index()] = 0;
     }
 }
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-void sched_note_syscall_enter(int nr, int argc, ...)
+static void note_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)
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr,
+      &g_note_sysview_driver.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);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_note_sysview_driver.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)
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0,
+            &g_note_sysview_driver.syscall_marker) == 0)

Review Comment:
   ditto



##########
drivers/segger/sysview.c:
##########
@@ -39,47 +39,93 @@
  * Private Types
  ****************************************************************************/
 
-struct sysview_s
+struct note_sysview_driver_s
 {
-  unsigned int                 irq[CONFIG_SMP_NCPUS];
-#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s    mode;
+  struct note_driver_s driver;
+  unsigned int irq[CONFIG_SMP_NCPUS];
+  struct note_filter_syscall_s syscall_marker;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static void note_sysview_start(FAR struct note_driver_s *drv,
+                               FAR struct tcb_s *tcb);
+static void note_sysview_stop(FAR struct note_driver_s *drv,
+                              FAR struct tcb_s *tcb);
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SWITCH
+static void note_sysview_suspend(FAR struct note_driver_s *drv,
+                                 FAR struct tcb_s *tcb);
+static void note_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 note_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 note_sysview_syscall_enter(FAR struct note_driver_s *drv,
+                                       int nr, int argc, va_list *ap);
+static void note_sysview_syscall_leave(FAR struct note_driver_s *drv,
+                                       int nr, uintptr_t result);
 #endif
-};
 
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
-static struct sysview_s g_sysview =
+static const struct note_driver_ops_s g_note_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 */
+  note_sysview_start,         /* start */
+  note_sysview_stop,          /* stop */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SWITCH
+  note_sysview_suspend,       /* suspend */
+  note_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
+  note_sysview_syscall_enter, /* syscall_enter */
+  note_sysview_syscall_leave, /* syscall_leave */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+  note_sysview_irqhandler,    /* irqhandler */
 #endif
 };
 
+struct note_sysview_driver_s g_note_sysview_driver =

Review Comment:
   add static



##########
drivers/segger/sysview.c:
##########
@@ -365,63 +262,63 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
             }
         }
 
-      g_sysview.irq[up_cpu_index()] = 0;
+      g_note_sysview_driver.irq[up_cpu_index()] = 0;
     }
 }
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-void sched_note_syscall_enter(int nr, int argc, ...)
+static void note_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)
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr,
+      &g_note_sysview_driver.syscall_marker) == 0)

Review Comment:
   ditto



##########
drivers/segger/sysview.c:
##########
@@ -365,63 +262,63 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
             }
         }
 
-      g_sysview.irq[up_cpu_index()] = 0;
+      g_note_sysview_driver.irq[up_cpu_index()] = 0;
     }
 }
 #endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
-void sched_note_syscall_enter(int nr, int argc, ...)
+static void note_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)
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr,
+      &g_note_sysview_driver.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);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_note_sysview_driver.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)
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0,
+            &g_note_sysview_driver.syscall_marker) == 0)
         {
-          NOTE_FILTER_SYSCALLMASK_SET(0, &g_sysview.syscall_marker);
+          NOTE_FILTER_SYSCALLMASK_SET(0,
+            &g_note_sysview_driver.syscall_marker);

Review Comment:
   ditto



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