fix: Flash-Lock im SDK, Store-Lock, Flush vor Reboot

sdk/PATCHES.md

@@ -25,3 +25,5 @@ Lib/GNU, Bin/secboot.img, Tools/GNU/link_w600_2m.ld (→ ld/) und wm_tool.c (→
    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.
+5. `Platform/Drivers/internalflash/wm_internal_fls.c`: Semaphor `s_fls2_lock` serialisiert den Flash-Pfad
+   oberhalb von 1 MB (Pin-Umschaltung via `tls_spifls_cs/ck/di/do_switch` war ungeschützt).

sdk/Platform/Drivers/internalflash/wm_internal_fls.c

@@ -19,6 +19,12 @@
 #include "wm_gpio.h"
 #include "wm_io.h"
 
+/* ninlight patch 5: the >1 MB (second flash) path switches the SPI pins without any lock;
+ * serialise it for all callers (SDK params, fwup, ninlight). See sdk/PATCHES.md. */
+static tls_os_sem_t *s_fls2_lock = NULL;
+#define FLS2_LOCK()   do { if (s_fls2_lock) tls_os_sem_acquire(s_fls2_lock, 0); } while (0)
+#define FLS2_UNLOCK() do { if (s_fls2_lock) tls_os_sem_release(s_fls2_lock); } while (0)
+
 void tls_spifls_cs_switch(enum tls_io_name flashcs, int openflag);
 void tls_spifls_di_switch(enum tls_io_name flashdi, int openflag);
 void tls_spifls_do_switch(enum tls_io_name flashdo, int openflag);
@@ -611,6 +617,7 @@ int tls_fls_read(u32 addr, u8 *buf, u32 len)
         {
             lenfor2M  = len - lenfor1M;
             lenfor2M = (addrfor2M + lenfor2M) <= (FLASH_BASE_ADDR | flashtotalsize) ? lenfor2M : ((FLASH_BASE_ADDR | flashtotalsize) - addrfor2M);
+            FLS2_LOCK();
             tls_spifls_cs_switch(WM_IO_PA_02, 1);
             tls_spifls_ck_switch(WM_IO_PA_11, 1);
             tls_spifls_di_switch(WM_IO_PA_03, 1);
@@ -620,6 +627,7 @@ int tls_fls_read(u32 addr, u8 *buf, u32 len)
             tls_spifls_do_switch(WM_IO_PA_09, 0);
             tls_spifls_ck_switch(WM_IO_PA_11, 0);
             tls_spifls_cs_switch(WM_IO_PA_02, 0);
+            FLS2_UNLOCK();
 
             return err;
         }
@@ -756,6 +764,7 @@ int tls_fls_write(u32 addr, u8 *buf, u32 len)
         {
             lenfor2M  = len - lenfor1M;
             lenfor2M = (addrfor2M + lenfor2M) <= (FLASH_BASE_ADDR | flashtotalsize) ? lenfor2M : ((FLASH_BASE_ADDR | flashtotalsize) - addrfor2M);
+            FLS2_LOCK();
             tls_spifls_cs_switch(WM_IO_PA_02, 1);
             tls_spifls_ck_switch(WM_IO_PA_11, 1);
             tls_spifls_di_switch(WM_IO_PA_03, 1);
@@ -765,6 +774,7 @@ int tls_fls_write(u32 addr, u8 *buf, u32 len)
             tls_spifls_do_switch(WM_IO_PA_09, 0);
             tls_spifls_ck_switch(WM_IO_PA_11, 0);
             tls_spifls_cs_switch(WM_IO_PA_02, 0);
+            FLS2_UNLOCK();
 
 
             return err;
@@ -805,6 +815,7 @@ int tls_fls_erase(u32 sector)
     }
     else if (sector < (flashtotalsize / INSIDE_FLS_SECTOR_SIZE + INSIDE_FLS_BASE_ADDR / INSIDE_FLS_SECTOR_SIZE))
     {
+        FLS2_LOCK();
         tls_spifls_cs_switch(WM_IO_PA_02, 1);
         tls_spifls_ck_switch(WM_IO_PA_11, 1);
         tls_spifls_di_switch(WM_IO_PA_03, 1);
@@ -814,6 +825,7 @@ int tls_fls_erase(u32 sector)
         tls_spifls_do_switch(WM_IO_PA_09, 0);
         tls_spifls_ck_switch(WM_IO_PA_11, 0);
         tls_spifls_cs_switch(WM_IO_PA_02, 0);
+        FLS2_UNLOCK();
         return err;
     }
 
@@ -854,6 +866,7 @@ static void tls_fls_flush_sector(void)
     else if (gsSector < (flashtotalsize / INSIDE_FLS_SECTOR_SIZE + INSIDE_FLS_BASE_ADDR / INSIDE_FLS_SECTOR_SIZE))
     {
         addr = gsSector * INSIDE_FLS_SECTOR_SIZE;
+        FLS2_LOCK();
         tls_spifls_cs_switch(WM_IO_PA_02, 1);
         tls_spifls_ck_switch(WM_IO_PA_11, 1);
         tls_spifls_di_switch(WM_IO_PA_03, 1);
@@ -863,6 +876,7 @@ static void tls_fls_flush_sector(void)
         tls_spifls_do_switch(WM_IO_PA_09, 0);
         tls_spifls_ck_switch(WM_IO_PA_11, 0);
         tls_spifls_cs_switch(WM_IO_PA_02, 0);
+        FLS2_UNLOCK();
     }
     //gsSecOffset = 0;
 
@@ -1042,6 +1056,7 @@ int tls_fls_chip_erase(void)
 
     if (inner2flashsize)
     {
+        FLS2_LOCK();
         tls_spifls_cs_switch(WM_IO_PA_02, 1);
         tls_spifls_ck_switch(WM_IO_PA_11, 1);
         tls_spifls_di_switch(WM_IO_PA_03, 1);
@@ -1051,6 +1066,7 @@ int tls_fls_chip_erase(void)
         tls_spifls_do_switch(WM_IO_PA_09, 0);
         tls_spifls_ck_switch(WM_IO_PA_11, 0);
         tls_spifls_cs_switch(WM_IO_PA_02, 0);
+        FLS2_UNLOCK();
     }
 
     tls_mem_free(cache);
@@ -1157,6 +1173,8 @@ int tls_fls_init(void)
         TLS_DBGPRT_ERR("create semaphore @fls_lock fail!\n");
         return TLS_FLS_STATUS_ENOMEM;
     }
+    tls_os_sem_create(&s_fls2_lock, 1);
+
     inside_fls = fls;
 
     inner1flashsize = getFlashDensity();

src/app_main.c

@@ -20,6 +20,7 @@
 #define NL_APP_PRIO   40
 
 static tls_os_mutex_t *s_lock;
+static tls_os_mutex_t *s_store_lock; /* serialises all nl_settings_* / nl_state_log_* / nl_resetcnt_* calls */
 static nl_settings_t s_settings;
 static volatile uint32_t s_light_beat;
 static volatile bool s_factory_reset_requested;
@@ -29,6 +30,9 @@ static u32 s_light_stack[512];
 void nl_app_lock(void) { tls_os_mutex_acquire(s_lock, 0); }
 void nl_app_unlock(void) { tls_os_mutex_release(s_lock); }
 
+static void store_lock(void) { tls_os_mutex_acquire(s_store_lock, 0); }
+static void store_unlock(void) { tls_os_mutex_release(s_store_lock); }
+
 uint32_t nl_app_now_ms(void)
 {
     return (uint32_t)((uint64_t)tls_os_get_time() * 1000u / HZ);
@@ -38,13 +42,41 @@ nl_settings_t *nl_app_settings(void) { return &s_settings; }
 
 int nl_app_save_settings(void)
 {
+    static nl_settings_t copy; /* guarded by the store lock */
     int r;
+
+    store_lock();
     nl_app_lock();
-    r = nl_settings_save(&s_settings);
+    copy = s_settings;
     nl_app_unlock();
+    r = nl_settings_save(&copy);
+    store_unlock();
     return r;
 }
 
+void nl_app_flush(void)
+{
+    nl_light_state_t st;
+
+    store_lock();
+    nl_app_lock();
+    nl_ctrl_get(&st);
+    nl_app_unlock();
+    if (nl_state_log_append(&st) == 0) { /* no write when unchanged */
+        nl_app_lock();
+        nl_ctrl_persisted(&st);
+        nl_app_unlock();
+    }
+    store_unlock();
+}
+
+void nl_app_reboot(void)
+{
+    printf("reboot\r\n");
+    nl_app_flush();
+    tls_sys_reset();
+}
+
 void nl_app_request_factory_reset(void) { s_factory_reset_requested = true; }
 
 void nl_app_factory_reset(void)
@@ -54,9 +86,11 @@ void nl_app_factory_reset(void)
     nl_engine_blink(3);
     nl_app_unlock();
     tls_os_time_delay(2 * HZ); /* 3 blinks = 1.8 s, the light task keeps running */
+    store_lock();
     nl_settings_erase();
     nl_state_log_erase();
     nl_resetcnt_clear();
+    store_unlock();
     tls_sys_reset();
 }
 
@@ -95,12 +129,14 @@ static void app_task(void *arg)
     (void)arg;
     nl_flash_init();
     nl_led_init(board); /* hardware init first (spec 3.4): LEDs dark before any flash/state scan */
+    store_lock();
     nl_resetcnt_init(base + NL_FL_RESETCNT);
     boots = nl_resetcnt_boot();
     nl_settings_init(base + NL_FL_SETTINGS_A, base + NL_FL_SETTINGS_B);
     nl_settings_load(&s_settings);
     nl_state_log_init(base + NL_FL_STATE_0, base + NL_FL_STATE_1);
     have_last = nl_state_log_load(&last);
+    store_unlock();
     printf("boot #%u, settings: pob=%u fade=%u/%u, last state: %s\r\n", boots,
            s_settings.power_on_behaviour, s_settings.fade_in_ms, s_settings.fade_out_ms,
            have_last ? "yes" : "no");
@@ -133,8 +169,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)
+        if (!counter_cleared && now - boot_ms >= NL_RESETCNT_CLEAR_MS) {
+            store_lock();
             counter_cleared = nl_resetcnt_clear() == 0;
+            store_unlock();
+        }
 
         if (s_factory_reset_requested)
             nl_app_factory_reset(); /* does not return */
@@ -142,10 +181,16 @@ static void app_task(void *arg)
         nl_app_lock();
         due = nl_ctrl_persist_due(now, &pending);
         nl_app_unlock();
-        if (due && nl_state_log_append(&pending) == 0) {
+        if (due) {
-            nl_app_lock();
+            int r;
-            nl_ctrl_persisted(&pending);
+            store_lock();
-            nl_app_unlock();
+            r = nl_state_log_append(&pending);
+            store_unlock();
+            if (r == 0) {
+                nl_app_lock();
+                nl_ctrl_persisted(&pending);
+                nl_app_unlock();
+            }
         }
 
         if (s_light_beat != beat_seen) {
@@ -166,5 +211,9 @@ void UserMain(void)
         printf("FATAL: cannot create app lock\r\n");
         return;
     }
+    if (tls_os_mutex_create(0, &s_store_lock) != TLS_OS_SUCCESS) {
+        printf("FATAL: cannot create store lock\r\n");
+        return;
+    }
     tls_os_task_create(NULL, "nl_app", app_task, NULL, (u8 *)s_app_stack, sizeof(s_app_stack), NL_APP_PRIO, 0);
 }

src/sys/nl_app.h

@@ -9,7 +9,13 @@ void nl_app_lock(void);
 void nl_app_unlock(void);
 uint32_t nl_app_now_ms(void);
 nl_settings_t *nl_app_settings(void);
+/* Copies the settings under the app lock and writes them under the store lock; never call
+ * while holding the app lock. */
 int nl_app_save_settings(void);
+/* Writes a pending light state immediately (no write when unchanged). */
+void nl_app_flush(void);
+/* Flushes pending state, then resets the chip. Does not return. */
+void nl_app_reboot(void);
 void nl_app_factory_reset(void);
 
 /* Only sets a volatile flag; the app task (which owns the flash stores) checks it

src/sys/nl_console.c

@@ -99,7 +99,7 @@ static void exec(const nl_cmd_t *c)
         nl_app_request_factory_reset();
         return;
     case NL_CMD_REBOOT:
-        tls_sys_reset();
+        nl_app_reboot();
         return;
     case NL_CMD_WDTEST:
         printf("wdtest: holding app lock forever\r\n");