pkarashchenko commented on code in PR #6673:
URL: https://github.com/apache/incubator-nuttx/pull/6673#discussion_r927129228


##########
drivers/power/pm_activity.c:
##########
@@ -35,6 +36,55 @@
 
 #ifdef CONFIG_PM
 
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct pm_wakelock_s g_wakelock[CONFIG_PM_NDOMAINS][PM_COUNT];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static void pm_waklock_cb(wdparm_t arg)
+{
+  pm_wakelock_relax((FAR struct pm_wakelock_s *)arg);
+}
+
+#ifdef CONFIG_PM_PROCFS
+static void pm_wakelock_stats_rm(FAR struct pm_wakelock_s *wakelock)
+{
+  FAR struct pm_domain_s *pdom = &g_pmglobals.domain[wakelock->domain];
+
+  dq_rem(&wakelock->fsnode, &pdom->wakelockall);
+}
+
+static void pm_wakelock_stats(FAR struct pm_wakelock_s *wakelock, bool stay)
+{
+  FAR struct pm_domain_s *pdom = &g_pmglobals.domain[wakelock->domain];
+  struct timespec ts;
+
+  if (stay)
+    {
+      if (!wakelock->fsnode.blink && !wakelock->fsnode.flink)
+        {
+          dq_addlast(&wakelock->fsnode, &pdom->wakelockall);
+        }
+
+      clock_systime_timespec(&wakelock->start);
+    }
+  else
+    {
+      clock_systime_timespec(&ts);
+      clock_timespec_subtract(&ts, &wakelock->start, &ts);
+      clock_timespec_add(&ts, &wakelock->elapse, &wakelock->elapse);
+    }
+}
+#else
+#define pm_wakelock_stats_rm(w)
+#define pm_wakelock_stats(w, s)

Review Comment:
   ```suggestion
   #  define pm_wakelock_stats_rm(w)
   #  define pm_wakelock_stats(w, s)
   ```



##########
drivers/power/greedy_governor.c:
##########
@@ -46,52 +46,44 @@
  * Private Types
  ****************************************************************************/
 
+struct pm_domain_state_s
+{
+  struct wdog_s wdog;
+};
+
+struct pm_greedy_governor_s
+{
+  struct pm_domain_state_s domain_states[CONFIG_PM_NDOMAINS];
+};
+
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
 
 /* PM governor methods */
 
-static void greedy_governor_initialize(void);
-static void greedy_governor_statechanged(int domain,
-                                         enum pm_state_e newstate);
-static enum pm_state_e greedy_governor_checkstate(int domain);
+static void             greedy_governor_statechanged(int domain,
+                                                enum pm_state_e newstate);
+static enum pm_state_e  greedy_governor_checkstate(int domain);
+static void             greedy_governor_activity(int domain, int count);

Review Comment:
   I think better to get previous style without too many spaces



##########
drivers/power/pm_changestate.c:
##########
@@ -168,6 +168,35 @@ static inline void pm_changeall(int domain, enum 
pm_state_e newstate)
     }
 }
 
+#ifdef CONFIG_PM_PROCFS
+static void pm_stats(FAR struct pm_domain_s *dom, int curstate, int newstate)
+{
+  struct timespec ts;
+
+  clock_systime_timespec(&ts);
+  clock_timespec_subtract(&ts, &dom->start, &ts);
+
+  if (newstate == PM_RESTORE)
+    {
+      /* Wakeup from WFI */
+
+      clock_timespec_add(&ts, &dom->sleep[curstate], &dom->sleep[curstate]);
+    }
+  else
+    {
+      /* Sleep to WFI */
+
+      clock_timespec_add(&ts, &dom->wake[curstate], &dom->wake[curstate]);
+    }
+
+  /* Update start */
+
+  clock_systime_timespec(&dom->start);
+}
+#else
+#define pm_stats(dom, curstate, newstate)

Review Comment:
   ```suggestion
   #  define pm_stats(dom, curstate, newstate)
   ```



##########
drivers/power/pm.h:
##########
@@ -72,6 +78,14 @@ struct pm_domain_s
   /* A pointer to the PM governor instance */
 
   FAR const struct pm_governor_s *governor;
+
+  /* This semaphore manages mutually exclusive access to the domain state.
+   * It must be initialized to the value 1.
+   */
+
+  sem_t sem;
+  pid_t holder;
+  unsigned int count;

Review Comment:
   maybe better to use recursive mutex?



##########
include/nuttx/power/pm.h:
##########
@@ -85,6 +87,14 @@
  * own, custom idle loop to support board-specific IDLE time power management
  */
 
+#define PM_WAKELOCK_INITIALIZER(name, domain, state) {name, domain, state}
+
+#define PM_WAKELOCK_DECLARE(var, name, domain, state) \
+      struct pm_wakelock_s var = PM_WAKELOCK_INITIALIZER(name, domain, state)
+
+#define PM_WAKELOCK_DECLARE_STATIC(var, name, domain, state) \
+static struct pm_wakelock_s var = PM_WAKELOCK_INITIALIZER(name, domain, state)

Review Comment:
   Please move to "Private Data" section



##########
drivers/power/greedy_governor.c:
##########
@@ -46,52 +46,44 @@
  * Private Types
  ****************************************************************************/
 
+struct pm_domain_state_s
+{
+  struct wdog_s wdog;
+};
+
+struct pm_greedy_governor_s
+{
+  struct pm_domain_state_s domain_states[CONFIG_PM_NDOMAINS];
+};
+
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
 
 /* PM governor methods */
 
-static void greedy_governor_initialize(void);
-static void greedy_governor_statechanged(int domain,
-                                         enum pm_state_e newstate);
-static enum pm_state_e greedy_governor_checkstate(int domain);
+static void             greedy_governor_statechanged(int domain,
+                                                enum pm_state_e newstate);
+static enum pm_state_e  greedy_governor_checkstate(int domain);
+static void             greedy_governor_activity(int domain, int count);
 
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
 static const struct pm_governor_s g_greedy_governor_ops =
 {
-  greedy_governor_initialize,   /* initialize */
-  NULL,                         /* deinitialize */
-  greedy_governor_statechanged, /* statechanged */
-  greedy_governor_checkstate,   /* checkstate */
-  NULL,                         /* activity */
-  NULL                          /* priv */
+  .statechanged = greedy_governor_statechanged, /* statechanged */
+  .checkstate   = greedy_governor_checkstate,   /* checkstate */
+  .activity     = greedy_governor_activity,     /* activity */
 };

Review Comment:
   please get back C89 init style



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