From 4f270ad79b2c7eb492195b2658cb921c746e131b Mon Sep 17 00:00:00 2001 From: Michael Sauer Date: Fri, 11 Sep 2026 20:53:25 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20Watchdog=20wirksam=20machen,=20Factory-R?= =?UTF-8?q?eset=20im=20App-Task,=20Resetz=C3=A4hler-Retry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - vApplicationIdleHook() füttert den Watchdog nicht mehr (sdk/Platform/Sys/wm_main.c, dokumentiert als Patch 4 in sdk/PATCHES.md). Bisher fütterte der Idle-Task den Watchdog bei jedem Leerlauf, auch wenn ein Task deadlockt (z.B. nl_app_lock() hält und nie freigibt) - der Watchdog konnte so nie greifen. - Neues Konsolenkommando `wdtest` (Parser in nl_console_cmd.c/.h, Ausführung in nl_console.c, Host-Test in test_console_cmd.c): hält nl_app_lock() für immer, damit light_task und app_task blockieren und nur der Watchdog-Reset den Chip retten kann. - nl_app_request_factory_reset() (nl_app.h) setzt nur ein volatile Flag; app_task prüft es in der Hauptschleife und ruft nl_app_factory_reset() selbst auf, da nur der App-Task die Flash-Stores besitzt. Das Konsolenkommando `factory` ruft jetzt diese Funktion statt nl_app_factory_reset() direkt aufzurufen, um die Race mit nl_state_log_append()/nl_resetcnt_clear() im App-Task zu vermeiden. Der Bootpfad (Bootzähler >= 5) ruft weiterhin direkt nl_app_factory_reset() auf. - nl_resetcnt_clear() wird nur noch als erledigt markiert, wenn es 0 zurückgibt, damit ein fehlgeschlagener Clear in der nächsten Schleifeniteration erneut versucht wird. --- sdk/PATCHES.md | 11 +++++++++++ sdk/Platform/Sys/wm_main.c | 6 ++++-- src/app_main.c | 12 ++++++++---- src/sys/nl_app.h | 7 +++++++ src/sys/nl_console.c | 7 ++++++- src/sys/nl_console_cmd.c | 5 +++-- src/sys/nl_console_cmd.h | 1 + test/test_console_cmd.c | 2 ++ 8 files changed, 42 insertions(+), 9 deletions(-) diff --git a/sdk/PATCHES.md b/sdk/PATCHES.md index f59f75c..d52e7d8 100644 --- a/sdk/PATCHES.md +++ b/sdk/PATCHES.md @@ -14,3 +14,14 @@ Lib/GNU, Bin/secboot.img, Tools/GNU/link_w600_2m.ld (→ ld/) und wm_tool.c (→ (`qconnex_sdk/src/qc_main.c`) war `pre_gpio_config()` bereits ein No-Op; `EarlyInit()` rief dort nur in die QConnex-Hardwareschicht (`QCXH_EarlyInit()`), die ninlight nicht verwendet. Board-spezifische Frühinitialisierung (falls nötig) folgt in einem späteren Plan. +4. `Platform/Sys/wm_main.c`: `vApplicationIdleHook()` hat bisher jedes Mal `tls_watchdog_clr()` + aufgerufen, wenn der Idle-Task lief - also immer dann, wenn kein anderer Task rechenbereit war. + Ein Task-Deadlock (z.B. ein Task, der `nl_app_lock()` hält und nie freigibt) hätte den Watchdog + damit nie ausgelöst, weil der Idle-Task munter weiter gefüttert hätte. Der `tls_watchdog_clr()`- + Aufruf wurde entfernt; `wfi` bleibt. Der einzige verbleibende `tls_watchdog_clr()`-Aufruf im SDK + ist in `Platform/Common/fwup/wm_fwup.c` (Firmware-Update-Task) - das ist unkritisch, da er nur + während eines laufenden OTA-Updates feuert und den `app_task`-Heartbeat in `src/app_main.c` + nicht ersetzt (der Watchdog wird sonst ausschließlich dort über den `s_light_beat`-Heartbeat + des `light_task` gefüttert). Getestet mit dem Konsolenkommando `wdtest` + (src/sys/nl_console_cmd.c, src/sys/nl_console.c): es hält `nl_app_lock()` für immer, wodurch + `light_task` und `app_task` blockieren und der Watchdog den Chip zurücksetzen muss. diff --git a/sdk/Platform/Sys/wm_main.c b/sdk/Platform/Sys/wm_main.c index 0420580..41b6fba 100644 --- a/sdk/Platform/Sys/wm_main.c +++ b/sdk/Platform/Sys/wm_main.c @@ -136,8 +136,10 @@ void tls_os_timer_init(void) void vApplicationIdleHook( void ) { - /* clear watch dog interrupt */ - tls_watchdog_clr(); + /* ninlight patch 4: do NOT feed the watchdog here. The idle task runs whenever + * no other task is ready, so clearing the watchdog from here would keep clearing + * it even while an app task is deadlocked (e.g. stuck holding nl_app_lock) and + * defeat the whole point of the watchdog. See sdk/PATCHES.md item 4. */ #if !defined(__CC_ARM) __asm volatile ("wfi"); diff --git a/src/app_main.c b/src/app_main.c index bf0b7a3..874a246 100644 --- a/src/app_main.c +++ b/src/app_main.c @@ -22,6 +22,7 @@ static tls_os_mutex_t *s_lock; static nl_settings_t s_settings; static volatile uint32_t s_light_beat; +static volatile bool s_factory_reset_requested; static u32 s_app_stack[1024]; static u32 s_light_stack[512]; @@ -44,6 +45,8 @@ int nl_app_save_settings(void) return r; } +void nl_app_request_factory_reset(void) { s_factory_reset_requested = true; } + void nl_app_factory_reset(void) { printf("factory reset\r\n"); @@ -130,10 +133,11 @@ static void app_task(void *arg) tls_os_time_delay(HZ / 10); /* 100 ms */ now = nl_app_now_ms(); - if (!counter_cleared && now - boot_ms >= NL_RESETCNT_CLEAR_MS) { - nl_resetcnt_clear(); - counter_cleared = true; - } + if (!counter_cleared && now - boot_ms >= NL_RESETCNT_CLEAR_MS) + counter_cleared = nl_resetcnt_clear() == 0; + + if (s_factory_reset_requested) + nl_app_factory_reset(); /* does not return */ nl_app_lock(); due = nl_ctrl_persist_due(now, &pending); diff --git a/src/sys/nl_app.h b/src/sys/nl_app.h index 548bf54..73b1969 100644 --- a/src/sys/nl_app.h +++ b/src/sys/nl_app.h @@ -12,4 +12,11 @@ nl_settings_t *nl_app_settings(void); int nl_app_save_settings(void); void nl_app_factory_reset(void); +/* Only sets a volatile flag; the app task (which owns the flash stores) checks it + * once per loop iteration and calls nl_app_factory_reset() itself. This avoids the + * race with the app task's own nl_state_log_append()/nl_resetcnt_clear() calls. + * The console's `factory` command and, in a later plan, REST callers must use this + * function instead of calling nl_app_factory_reset() directly from another task. */ +void nl_app_request_factory_reset(void); + #endif diff --git a/src/sys/nl_console.c b/src/sys/nl_console.c index 2b8b61f..a172489 100644 --- a/src/sys/nl_console.c +++ b/src/sys/nl_console.c @@ -96,11 +96,16 @@ static void exec(const nl_cmd_t *c) nl_app_unlock(); return; case NL_CMD_FACTORY: - nl_app_factory_reset(); + nl_app_request_factory_reset(); return; case NL_CMD_REBOOT: tls_sys_reset(); return; + case NL_CMD_WDTEST: + printf("wdtest: holding app lock forever\r\n"); + nl_app_lock(); /* deliberately never released: light_task and app_task stall on it, + * so only the watchdog reset can recover the chip */ + return; case NL_CMD_HELP: printf("%s", nl_console_help); return; diff --git a/src/sys/nl_console_cmd.c b/src/sys/nl_console_cmd.c index 5819895..42106fb 100644 --- a/src/sys/nl_console_cmd.c +++ b/src/sys/nl_console_cmd.c @@ -9,7 +9,7 @@ const char nl_console_help[] = "on [ms] | off [ms] | bri <1-100> [ms] | rgb [ms] | ct [ms]\r\n" "pob last|on|off | fade | blink [n] | pulse on|off\r\n" - "state | factory | reboot | help\r\n"; + "state | factory | reboot | wdtest | help\r\n"; static bool num(const char *s, int32_t lo, int32_t hi, int32_t *out) { @@ -104,7 +104,8 @@ nl_cmd_t nl_console_parse(const char *line) ok = n == 1 || (n == 2 && num(tok[1], 1, 10, &c.arg[0])); } else { static const struct { const char *name; nl_cmd_type_t type; } SIMPLE[] = { - {"state", NL_CMD_STATE}, {"factory", NL_CMD_FACTORY}, {"reboot", NL_CMD_REBOOT}, {"help", NL_CMD_HELP}}; + {"state", NL_CMD_STATE}, {"factory", NL_CMD_FACTORY}, {"reboot", NL_CMD_REBOOT}, + {"help", NL_CMD_HELP}, {"wdtest", NL_CMD_WDTEST}}; ok = false; for (size_t i = 0; i < sizeof SIMPLE / sizeof SIMPLE[0]; i++) if (!strcmp(tok[0], SIMPLE[i].name)) { diff --git a/src/sys/nl_console_cmd.h b/src/sys/nl_console_cmd.h index a400451..8f00bf3 100644 --- a/src/sys/nl_console_cmd.h +++ b/src/sys/nl_console_cmd.h @@ -18,6 +18,7 @@ typedef enum { NL_CMD_FACTORY, NL_CMD_REBOOT, NL_CMD_HELP, + NL_CMD_WDTEST, NL_CMD_ERROR } nl_cmd_type_t; diff --git a/test/test_console_cmd.c b/test/test_console_cmd.c index b8d1faf..aafa921 100644 --- a/test/test_console_cmd.c +++ b/test/test_console_cmd.c @@ -81,7 +81,9 @@ static void test_simple_commands(void) TEST_ASSERT_EQUAL(NL_CMD_FACTORY, nl_console_parse("factory").type); TEST_ASSERT_EQUAL(NL_CMD_REBOOT, nl_console_parse("reboot").type); TEST_ASSERT_EQUAL(NL_CMD_HELP, nl_console_parse("help").type); + TEST_ASSERT_EQUAL(NL_CMD_WDTEST, nl_console_parse("wdtest").type); TEST_ASSERT_EQUAL(NL_CMD_ERROR, nl_console_parse("state now").type); + TEST_ASSERT_EQUAL(NL_CMD_ERROR, nl_console_parse("wdtest now").type); } int main(void)