chore(watchdog): init right after device driver

use sys_init to initialize watchdog callback cannot be set dynamically anymore, but weakly defined instead so that it can be overridden by user at compile time. Signed-off-by: Cyril Fougeray <[email protected]>
worldcoin · Nov 29, 2024 · 99ae45c · 99ae45c
1 parent eca1892
commit 99ae45c
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 19 deletions.
diff --git a/lib/errors/errors_tests.c b/lib/errors/errors_tests.c
@@ -100,13 +100,6 @@ trigger_z_oops(void)
 }
 #endif
 
-__maybe_unused static bool
-watchdog_feed_callback(void)
-{
-    /* Deliberately prevent feeding the watchdog */
-    return false;
-}
-
 void
 fatal_errors_trigger(enum error_case_e type)
 {
@@ -160,7 +153,7 @@ fatal_errors_trigger(enum error_case_e type)
 #endif
 #ifdef CONFIG_ORB_LIB_WATCHDOG
     case FATAL_WATCHDOG:
-        (void)watchdog_init(watchdog_feed_callback);
+        watchdog_stop_feed();
         break;
 #endif
 

diff --git a/lib/watchdog/Kconfig b/lib/watchdog/Kconfig
@@ -4,6 +4,18 @@ config ORB_LIB_WATCHDOG
 
 if ORB_LIB_WATCHDOG
 
+config ORB_LIB_WATCHDOG_SYS_INIT
+    bool "Initialize watchdog in sys_init, when disabled (default) `watchdog_init()` must be explicitly called"
+    default n
+
+if ORB_LIB_WATCHDOG_SYS_INIT
+
+config ORB_LIB_WATCHDOG_INIT_PRIORITY
+    int "Watchdog initialization priority"
+    default 51 # KERNEL_INIT_PRIORITY_DEVICE + 1
+
+endif
+
 config ORB_LIB_THREAD_PRIORITY_WATCHDOG
     int "Watchdog thread priority"
     default 10

diff --git a/lib/watchdog/include/watchdog.h b/lib/watchdog/include/watchdog.h
@@ -1,6 +1,23 @@
 #pragma once
 
 #include <stdbool.h>
+
+/**
+ * @brief Perform checks to determine if watchdog should be fed
+ *
+ * Weakly defined function in `watchdog.c` that can be overridden by the user
+ */
+bool
+watchdog_perform_checks(void);
+
+/**
+ * @brief Stop feeding the watchdog
+ */
+void
+watchdog_stop_feed(void);
+
+#if !defined(CONFIG_ORB_LIB_WATCHDOG_SYS_INIT)
+
 /**
  * Setup watchdog & spawn low-priority thread to reload the watchdog
  *
@@ -14,3 +31,5 @@
  */
 int
 watchdog_init(bool (*callback)(void));
+
+#endif
diff --git a/lib/watchdog/watchdog.c b/lib/watchdog/watchdog.c
@@ -15,8 +15,6 @@ static volatile int wdt_channel_id = -1;
 static const struct device *const watchdog_dev =
     DEVICE_DT_GET(DT_ALIAS(watchdog0));
 
-static bool (*watchdog_callback)(void) = NULL;
-
 #ifndef WATCHDOG_RELOAD_MS
 #define WATCHDOG_RELOAD_MS CONFIG_ORB_LIB_WATCHDOG_RELOAD_MS
 #endif
@@ -25,13 +23,27 @@ BUILD_ASSERT(CONFIG_ORB_LIB_WATCHDOG_RELOAD_MS <
                  CONFIG_ORB_LIB_WATCHDOG_TIMEOUT_MS,
              "Watchdog reload time must be less than watchdog timeout");
 
+static bool feed = true;
+
+__WEAK bool
+watchdog_perform_checks(void)
+{
+    return true;
+}
+
+void
+watchdog_stop_feed(void)
+{
+    feed = false;
+}
+
 static void
 watchdog_thread()
 {
     while (wdt_channel_id >= 0) {
         // Allow null callback,
         // Don't rearrange due to short circuit rules, NULL check must be first
-        if ((watchdog_callback == NULL) || (watchdog_callback() == true)) {
+        if (feed && watchdog_perform_checks() == true) {
             wdt_feed(watchdog_dev, wdt_channel_id);
         }
         k_sleep(K_MSEC(WATCHDOG_RELOAD_MS));
@@ -41,7 +53,7 @@ watchdog_thread()
 }
 
 int
-watchdog_init(bool (*callback)(void))
+watchdog_init(void)
 {
     int err_code;
 
@@ -81,8 +93,6 @@ watchdog_init(bool (*callback)(void))
         return RET_ERROR_NOT_INITIALIZED;
     }
 
-    watchdog_callback = callback;
-
     k_thread_create(&watchdog_thread_data, stack_area,
                     K_THREAD_STACK_SIZEOF(stack_area),
                     (k_thread_entry_t)watchdog_thread, NULL, NULL, NULL,
@@ -91,3 +101,7 @@ watchdog_init(bool (*callback)(void))
 
     return RET_SUCCESS;
 }
+
+#if CONFIG_ORB_LIB_WATCHDOG_SYS_INIT
+SYS_INIT(watchdog_init, POST_KERNEL, CONFIG_ORB_LIB_WATCHDOG_INIT_PRIORITY);
+#endif
diff --git a/main_board/debug.conf b/main_board/debug.conf
@@ -54,5 +54,3 @@ CONFIG_POWER_SEQUENCE_LOG_LEVEL_DBG=y
 # thread awareness
 # openOCD with Zephyr patch is needed
 CONFIG_DEBUG_THREAD_INFO=y
-
-CONFIG_ORB_LIB_WATCHDOG=n
diff --git a/main_board/prj.conf b/main_board/prj.conf
@@ -86,8 +86,9 @@ CONFIG_ORB_LIB_UART_MESSAGING=y
 CONFIG_ORB_LIB_ERRORS=y
 CONFIG_ORB_LIB_DFU=y
 CONFIG_ORB_LIB_STORAGE=y
-CONFIG_ORB_LIB_WATCHDOG=y
 CONFIG_WATCHDOG=y
+CONFIG_ORB_LIB_WATCHDOG=y
+CONFIG_ORB_LIB_WATCHDOG_SYS_INIT=y
 CONFIG_ORB_LIB_HEALTH_MONITORING=y
 CONFIG_ORB_LIB_LOGS_CAN=y
 # Same priority as CAN RX and TX threads to make sure other threads cannot block DFU

diff --git a/main_board/src/main.c b/main_board/src/main.c
@@ -205,8 +205,8 @@ initialize(void)
 
     app_assert_init(app_assert_cb);
 
-#if CONFIG_ORB_LIB_WATCHDOG
-    err_code = watchdog_init(NULL);
+#if CONFIG_ORB_LIB_WATCHDOG && !(CONFIG_ORB_LIB_WATCHDOG_SYS_INIT)
+    err_code = watchdog_init();
     ASSERT_SOFT(err_code);
 #endif
 

diff --git a/main_board/src/power/boot/boot.c b/main_board/src/power/boot/boot.c
@@ -630,6 +630,12 @@ app_init_state(void)
     return ret;
 }
 
+#if CONFIG_ORB_LIB_WATCHDOG_SYS_INIT
+BUILD_ASSERT(CONFIG_ORB_LIB_WATCHDOG_INIT_PRIORITY <
+                 SYS_INIT_WAIT_FOR_BUTTON_PRESS_PRIORITY,
+             "Watchdog must be initialized before waiting for button press");
+#endif
+
 SYS_INIT(app_init_state, POST_KERNEL, SYS_INIT_WAIT_FOR_BUTTON_PRESS_PRIORITY);
 
 #if defined(CONFIG_BOARD_DIAMOND_MAIN)