write flash on separate thread for H7 (#2490)

* write flash on separate thread for H7

* cleanup

* getting the parameters in the correct order generally helps

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/config/stm32h7ems/efifeatures.h

@@ -40,3 +40,6 @@
 #define BOARD_EXT_GPIOCHIPS			(BOARD_TLE6240_COUNT + BOARD_MC33972_COUNT + BOARD_TLE8888_COUNT + BOARD_DRV8860_COUNT + BOARD_MC33810_COUNT)
 
 #define EFI_USE_COMPRESSED_INI_MSD
+
+// H7 has dual bank, so flash on its own (low priority) thread so as to not block any other operations
+#define EFI_FLASH_WRITE_THREAD TRUE

firmware/controllers/flash_main.cpp

@@ -44,9 +44,29 @@ crc_t flashStateCrc(persistent_config_container_s *state) {
 	return calc_crc((const crc_t*) &state->persistentConfiguration, sizeof(persistent_config_s));
 }
 
+#if EFI_FLASH_WRITE_THREAD
+chibios_rt::BinarySemaphore flashWriteSemaphore(/*taken =*/ true);
+
+static THD_WORKING_AREA(flashWriteStack, UTILITY_THREAD_STACK_SIZE);
+static void flashWriteThread(void*) {
+	while (true) {
+		// Wait for a request to come in
+		flashWriteSemaphore.wait();
+
+		// Do the actual flash write operation
+		writeToFlashNow();
+	}
+}
+#endif // EFI_FLASH_WRITE_THREAD
+
 void setNeedToWriteConfiguration(void) {
 	scheduleMsg(logger, "Scheduling configuration write");
 	needToWriteConfiguration = true;
+
+#if EFI_FLASH_WRITE_THREAD
+	// Signal the flash writer thread to wake up and write at its leisure
+	flashWriteSemaphore.signal();
+#endif // EFI_FLASH_WRITE_THREAD
 }
 
 bool getNeedToWriteConfiguration(void) {
@@ -54,13 +74,15 @@ bool getNeedToWriteConfiguration(void) {
 }
 
 void writeToFlashIfPending() {
+// with a flash write thread, the schedule happens directly from
+// setNeedToWriteConfiguration, so there's nothing to do here
+#if !EFI_FLASH_WRITE_THREAD
 	if (!getNeedToWriteConfiguration()) {
 		return;
 	}
-	// todo: technically we need a lock here, realistically we should be fine.
-	needToWriteConfiguration = false;
-	scheduleMsg(logger, "Writing pending configuration");
+
 	writeToFlashNow();
+#endif
 }
 
 // Erase and write a copy of the configuration at the specified address
@@ -72,7 +94,7 @@ int eraseAndFlashCopy(flashaddr_t storageAddress, const TStorage& data)
 }
 
 void writeToFlashNow(void) {
-	scheduleMsg(logger, " !!!!!!!!!!!!!!!!!!!! BE SURE NOT WRITE WITH IGNITION ON !!!!!!!!!!!!!!!!!!!!");
+	scheduleMsg(logger, "Writing pending configuration...");
 
 	// Set up the container
 	persistentState.size = sizeof(persistentState);
@@ -94,6 +116,9 @@ void writeToFlashNow(void) {
 	assertEngineReference();
 
 	resetMaxValues();
+
+	// Write complete, clear the flag
+	needToWriteConfiguration = false;
 }
 
 static bool isValidCrc(persistent_config_container_s *state) {
@@ -201,6 +226,10 @@ void initFlash(Logging *sharedLogger) {
 #endif
 	addConsoleAction("resetconfig", doResetConfiguration);
 	addConsoleAction("rewriteconfig", rewriteConfig);
+
+#if EFI_FLASH_WRITE_THREAD
+	chThdCreateStatic(flashWriteStack, sizeof(flashWriteStack), PRIO_FLASH_WRITE, flashWriteThread, nullptr);
+#endif
 }
 
 #endif /* EFI_INTERNAL_FLASH */

firmware/controllers/thread_priority.h

@@ -29,10 +29,14 @@
 
 // Less important things
 #define PRIO_MMC (NORMALPRIO - 1)
-// USB mass storage
-#define MSD_THD_PRIO LOWPRIO
 
 // These can get starved without too much adverse effect
 #define PRIO_AUX_SERIAL NORMALPRIO
 #define PRIO_KNOCK_PROCESS (NORMALPRIO - 10)
 #define PRIO_HIP9011 (NORMALPRIO - 10)
+
+// These are intentionally low priority so they can't get in the way of anything else
+#define PRIO_FLASH_WRITE LOWPRIO
+
+// USB mass storage
+#define MSD_THD_PRIO LOWPRIO