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


##########
include/nuttx/hrtimer.h:
##########
@@ -30,7 +30,7 @@
 #include <nuttx/config.h>
 #include <nuttx/clock.h>
 #include <nuttx/compiler.h>
-#include <nuttx/seqlock.h>
+#include <nuttx/spinlock.h>

Review Comment:
   let's merge to the first patch



##########
sched/hrtimer/hrtimer.h:
##########
@@ -156,14 +153,13 @@ int hrtimer_starttimer(uint64_t ns)
  *   b - Pointer to the second hrtimer node.
  *
  * Returned Value:
- *   >0 if b expires before a
- *    0 if a and b expire at the same time
- *   <0 if b expires after a
+ *   <0 if a expires before b
+ *   >=0 if a expires after b
  ****************************************************************************/
 
 static inline_function
 int hrtimer_compare(FAR const hrtimer_node_t *a,
-                    FAR const hrtimer_node_t *b)
+                FAR const hrtimer_node_t *b)

Review Comment:
   why change



##########
sched/hrtimer/hrtimer_cancel.c:
##########
@@ -40,6 +40,40 @@
 
 #define HRTIMER_CANCEL_SYNC_DELAY_MS  5
 
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************

Review Comment:
   merge to the first patch



##########
sched/hrtimer/hrtimer.h:
##########
@@ -58,6 +58,8 @@ extern seqcount_t g_hrtimer_spinlock;
 
 extern struct hrtimer_tree_s g_hrtimer_tree;
 
+extern FAR hrtimer_t *g_running_hrtimers[CONFIG_SMP_NCPUS];

Review Comment:
   g_hrtimer_running



##########
sched/hrtimer/hrtimer_initialize.c:
##########
@@ -28,10 +28,22 @@
 
 #include "hrtimer/hrtimer.h"
 
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define RUNNING_INITIALIZER  { NULL }
+
 /****************************************************************************
  * Public Data
  ****************************************************************************/
 
+/* Array of pointers to currently running high-resolution timers
+ * for each CPU in SMP configurations. Index corresponds to CPU ID.
+ */
+
+FAR hrtimer_t *g_running_hrtimers[CONFIG_SMP_NCPUS] = RUNNING_INITIALIZER;

Review Comment:
   remove `= RUNNING_INITIALIZER` and line 31-36



##########
sched/hrtimer/hrtimer.h:
##########
@@ -182,6 +178,90 @@ int hrtimer_compare(FAR const hrtimer_node_t *a,
 
 RB_PROTOTYPE(hrtimer_tree_s, hrtimer_node_s, entry, hrtimer_compare);
 
-#endif /* CONFIG_HRTIMER */
+/****************************************************************************
+ * Name: hrtimer_is_armed
+ *
+ * Description:
+ *   Test whether a timer is currently armed (inserted into the RB-tree).
+ *
+ * Returned Value:
+ *   true if armed, false otherwise.
+ ****************************************************************************/
+
+static inline_function bool hrtimer_is_armed(FAR hrtimer_t *hrtimer)
+{
+  /* RB-tree root has NULL parent, so root must be checked explicitly */
+
+  return RB_PARENT(&hrtimer->node, entry) != NULL ||
+         RB_ROOT(&g_hrtimer_tree) == &hrtimer->node;
+}
+
+/****************************************************************************
+ * Name: hrtimer_remove
+ *
+ * Description:
+ *   Remove a timer from the RB-tree and mark it as unarmed.
+ ****************************************************************************/
+
+static inline_function void hrtimer_remove(FAR hrtimer_t *hrtimer)
+{
+  RB_REMOVE(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node);
+
+  /* Explicitly clear parent to mark the timer as unarmed */
+
+  RB_PARENT(&hrtimer->node, entry) = NULL;
+}
 
+/****************************************************************************
+ * Name: hrtimer_insert
+ *
+ * Description:
+ *   Insert a timer into the RB-tree according to its expiration time.
+ ****************************************************************************/
+
+static inline_function void hrtimer_insert(FAR hrtimer_t *hrtimer)
+{
+  RB_INSERT(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node);
+}
+
+/****************************************************************************
+ * Name: hrtimer_get_first
+ *
+ * Description:
+ *   Return the earliest expiring armed timer.
+ *
+ * Returned Value:
+ *   Pointer to the earliest timer, or NULL if none are armed.
+ ****************************************************************************/
+
+static inline_function FAR hrtimer_t *hrtimer_get_first(void)
+{
+  return (FAR hrtimer_t *)RB_MIN(hrtimer_tree_s, &g_hrtimer_tree);
+}
+
+/****************************************************************************
+ * Name: hrtimer_is_first
+ *
+ * Description:
+ *   Test whether the given high-resolution timer is the earliest
+ *   expiring timer in the RB-tree.
+ *
+ *   In a red-black tree ordered by expiration time, the earliest timer
+ *   is represented by the left-most node. Therefore, a timer is the
+ *   earliest one if it has no left child.
+ *
+ * Input Parameters:
+ *   hrtimer - Pointer to the high-resolution timer to be tested.
+ *
+ * Returned Value:
+ *   true  - The timer is the earliest expiring armed timer.
+ *   false - The timer is not the earliest timer.
+ ****************************************************************************/
+
+static inline_function bool hrtimer_is_first(FAR hrtimer_t *hrtimer)
+{
+  return (RB_LEFT(&hrtimer->node, entry) == NULL);

Review Comment:
   remove ()



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