Merge pull request #1329 from spark/feature/bootloader-flash-retry

[Original #1324] Verify the bootloader was flashed correctly and retry if not

hal/src/photon/core_hal.c

@@ -122,14 +122,14 @@ void HAL_Core_Setup_finalize(void)
     // in the eeprom region.
     const module_info_t* app_info = FLASH_ModuleInfo(FLASH_INTERNAL, app_backup);
     if (app_info->module_start_address==(void*)0x80A0000) {
-    		LED_SetRGBColor(RGB_COLOR_GREEN);
-    		uint32_t length = app_info->module_end_address-app_info->module_start_address+4;
-    		if (length < 80*1024 && FLASH_CopyMemory(FLASH_INTERNAL, app_backup, FLASH_INTERNAL, 0x80E0000, length, MODULE_FUNCTION_USER_PART, MODULE_VERIFY_CRC|MODULE_VERIFY_FUNCTION)) {
-        		FLASH_CopyMemory(FLASH_INTERNAL, app_backup, FLASH_INTERNAL, 0x80A0000, length, MODULE_FUNCTION_USER_PART, MODULE_VERIFY_CRC|MODULE_VERIFY_DESTINATION_IS_START_ADDRESS|MODULE_VERIFY_FUNCTION);
-    			FLASH_EraseMemory(FLASH_INTERNAL, app_backup, length);
-    		}
-    		LED_SetRGBColor(RGB_COLOR_WHITE);
-    	}
+            LED_SetRGBColor(RGB_COLOR_GREEN);
+            uint32_t length = app_info->module_end_address-app_info->module_start_address+4;
+            if (length < 80*1024 && FLASH_CopyMemory(FLASH_INTERNAL, app_backup, FLASH_INTERNAL, 0x80E0000, length, MODULE_FUNCTION_USER_PART, MODULE_VERIFY_CRC|MODULE_VERIFY_FUNCTION) == FLASH_ACCESS_RESULT_OK) {
+                FLASH_CopyMemory(FLASH_INTERNAL, app_backup, FLASH_INTERNAL, 0x80A0000, length, MODULE_FUNCTION_USER_PART, MODULE_VERIFY_CRC|MODULE_VERIFY_DESTINATION_IS_START_ADDRESS|MODULE_VERIFY_FUNCTION);
+                FLASH_EraseMemory(FLASH_INTERNAL, app_backup, length);
+            }
+            LED_SetRGBColor(RGB_COLOR_WHITE);
+        }
 #endif
 }
 

hal/src/stm32f2xx/bootloader.cpp

@@ -7,19 +7,19 @@
 #include "bootloader_hal.h"
 
 #ifdef HAL_REPLACE_BOOTLOADER_OTA
-bool bootloader_update(const void* bootloader_image, unsigned length)
+int bootloader_update(const void* bootloader_image, unsigned length)
 {
     HAL_Bootloader_Lock(false);
-    bool result =  (FLASH_CopyMemory(FLASH_INTERNAL, (uint32_t)bootloader_image,
+    int result = (FLASH_CopyMemory(FLASH_INTERNAL, (uint32_t)bootloader_image,
         FLASH_INTERNAL, 0x8000000, length, MODULE_FUNCTION_BOOTLOADER,
         MODULE_VERIFY_DESTINATION_IS_START_ADDRESS|MODULE_VERIFY_CRC|MODULE_VERIFY_FUNCTION));
     HAL_Bootloader_Lock(true);
     return result;
 }
 #else
-bool bootloader_update(const void*, unsigned)
+int bootloader_update(const void*, unsigned)
 {
-    return false;
+    return FLASH_ACCESS_RESULT_ERROR;
 }
 #endif // HAL_REPLACE_BOOTLOADER_OTA
 

hal/src/stm32f2xx/bootloader.h

@@ -17,7 +17,7 @@ extern "C" {
 bool bootloader_requires_update(const uint8_t* bootloader_image, uint32_t length);
 bool bootloader_update_if_needed();
 
-bool bootloader_update(const void* bootloader_image, unsigned length);
+int bootloader_update(const void* bootloader_image, unsigned length);
 
 
 #ifdef __cplusplus

hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp

@@ -36,8 +36,16 @@
 #include "hal_platform.h"
 #include "hal_event.h"
 #include "service_debug.h"
+#include "spark_wiring_random.h"
+#include "delay_hal.h"
+// For ATOMIC_BLOCK
+#include "spark_wiring_interrupts.h"
 
-#define OTA_CHUNK_SIZE          512
+#define OTA_CHUNK_SIZE                 (512)
+#define BOOTLOADER_RANDOM_BACKOFF_MIN  (200)
+#define BOOTLOADER_RANDOM_BACKOFF_MAX  (1000)
+
+static hal_update_complete_t flash_bootloader(hal_module_t* mod, uint32_t moduleLength);
 
 /**
  * Finds the location where a given module is stored. The module is identified
@@ -239,6 +247,44 @@ int HAL_FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t length,
     return FLASH_Update(pBuffer, address, length);
 }
 
+static hal_update_complete_t flash_bootloader(hal_module_t* mod, uint32_t moduleLength)
+{
+    hal_update_complete_t result = HAL_UPDATE_ERROR;
+    uint32_t attempt = 0;
+    do {
+        int fres = FLASH_ACCESS_RESULT_ERROR;
+        if (attempt++ > 0) {
+            ATOMIC_BLOCK() {
+                // If it's not the first flashing attempt, try with interrupts disabled
+                bootloader_update((const void*)mod->bounds.start_address, moduleLength + 4);
+            }
+        } else {
+            bootloader_update((const void*)mod->bounds.start_address, moduleLength + 4);
+        }
+        if (fres == FLASH_ACCESS_RESULT_OK) {
+            // Validate bootloader
+            hal_module_t module;
+            bool module_fetched = fetch_module(&module, &module_bootloader, true, MODULE_VALIDATION_INTEGRITY | MODULE_VALIDATION_DEPENDENCIES_FULL);
+            if (module_fetched && (module.validity_checked == module.validity_result)) {
+                result = HAL_UPDATE_APPLIED;
+                break;
+            }
+        }
+
+        if (fres == FLASH_ACCESS_RESULT_BADARG) {
+            // The bootloader is still intact, for some reason the module being copied has failed some checks.
+            result = HAL_UPDATE_ERROR;
+            break;
+        }
+
+        // Random backoff
+        system_tick_t period = random(BOOTLOADER_RANDOM_BACKOFF_MIN, BOOTLOADER_RANDOM_BACKOFF_MAX);
+        LOG_DEBUG(WARN, "Failed to flash bootloader. Retrying in %lu ms", period);
+        HAL_Delay_Milliseconds(period);
+    } while ((result == HAL_UPDATE_ERROR));
+    return result;
+}
+
 hal_update_complete_t HAL_FLASH_End(hal_module_t* mod)
 {
     hal_module_t module;
@@ -253,8 +299,7 @@ hal_update_complete_t HAL_FLASH_End(hal_module_t* mod)
 
         // bootloader is copied directly
         if (function==MODULE_FUNCTION_BOOTLOADER) {
-            if (bootloader_update((const void*)module_ota.start_address, moduleLength+4))
-                result = HAL_UPDATE_APPLIED;
+            result = flash_bootloader(&module, moduleLength);
         }
         else
         {

hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.h

@@ -23,6 +23,7 @@
 extern const module_bounds_t* module_bounds[];
 extern const unsigned module_bounds_length;
 extern const module_bounds_t module_ota;
+extern const module_bounds_t module_bootloader;
 extern const module_bounds_t module_user;
 
 

platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c

@@ -253,18 +253,18 @@ bool FLASH_EraseMemory(flash_device_t flashDeviceID, uint32_t startAddress, uint
     return false;
 }
 
-bool FLASH_CheckCopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
+int FLASH_CheckCopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
                       flash_device_t destinationDeviceID, uint32_t destinationAddress,
                       uint32_t length, uint8_t module_function, uint8_t flags)
 {
     if (!FLASH_CheckValidAddressRange(sourceDeviceID, sourceAddress, length))
     {
-        return false;
+        return FLASH_ACCESS_RESULT_BADARG;
     }
 
     if (!FLASH_CheckValidAddressRange(destinationDeviceID, destinationAddress, length))
     {
-        return false;
+        return FLASH_ACCESS_RESULT_BADARG;
     }
 
 #ifndef USE_SERIAL_FLASH    // this predates the module system (early P1's using external flash for storage)
@@ -274,33 +274,33 @@ bool FLASH_CheckCopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress
 
         if((flags & (MODULE_VERIFY_LENGTH|MODULE_VERIFY_CRC)) && (length < moduleLength+4))
         {
-            return false;
+            return FLASH_ACCESS_RESULT_BADARG;
         }
 
         const module_info_t* info = FLASH_ModuleInfo(sourceDeviceID, sourceAddress);
         if ((info->module_function != MODULE_FUNCTION_RESOURCE) && (info->platform_id != PLATFORM_ID))
         {
-            return false;
+            return FLASH_ACCESS_RESULT_BADARG;
         }
 
         // verify destination address
         if ((flags & MODULE_VERIFY_DESTINATION_IS_START_ADDRESS) && (((uint32_t)info->module_start_address) != destinationAddress))
         {
-            return false;
+            return FLASH_ACCESS_RESULT_BADARG;
         }
 
         if ((flags & MODULE_VERIFY_FUNCTION) && (info->module_function != module_function))
         {
-            return false;
+            return FLASH_ACCESS_RESULT_BADARG;
         }
 
         if ((flags & MODULE_VERIFY_CRC) && !FLASH_VerifyCRC32(sourceDeviceID, sourceAddress, moduleLength))
         {
-            return false;
+            return FLASH_ACCESS_RESULT_BADARG;
         }
     }
 #endif
-    return true;
+    return FLASH_ACCESS_RESULT_OK;
 }
 
 bool CopyFlashBlock(flash_device_t sourceDeviceID, uint32_t sourceAddress, flash_device_t destinationDeviceID, uint32_t destinationAddress, uint32_t length)
@@ -390,13 +390,13 @@ bool CopyFlashBlock(flash_device_t sourceDeviceID, uint32_t sourceAddress, flash
 	return success;
 }
 
-bool FLASH_CopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
-                      flash_device_t destinationDeviceID, uint32_t destinationAddress,
-                      uint32_t length, uint8_t module_function, uint8_t flags)
+int FLASH_CopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
+                     flash_device_t destinationDeviceID, uint32_t destinationAddress,
+                     uint32_t length, uint8_t module_function, uint8_t flags)
 {
-    if (!FLASH_CheckCopyMemory(sourceDeviceID, sourceAddress, destinationDeviceID, destinationAddress, length, module_function, flags))
+    if (FLASH_CheckCopyMemory(sourceDeviceID, sourceAddress, destinationDeviceID, destinationAddress, length, module_function, flags) != FLASH_ACCESS_RESULT_OK)
     {
-        return false;
+        return FLASH_ACCESS_RESULT_BADARG;
     }
 
     if (sourceDeviceID == FLASH_SERIAL)
@@ -414,12 +414,12 @@ bool FLASH_CopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
     		if (blockLength>length)
     			blockLength = length;
     		if (!CopyFlashBlock(sourceDeviceID, sourceAddress, destinationDeviceID, destinationAddress, blockLength))
-    			return false;
+    			return FLASH_ACCESS_RESULT_ERROR;
     		length -= blockLength;
     		sourceAddress += blockLength;
     		destinationAddress += blockLength;
     }
-    return true;
+    return FLASH_ACCESS_RESULT_OK;
 }
 
 bool FLASH_CompareMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
@@ -571,10 +571,10 @@ bool FLASH_ClearFactoryResetModuleSlot(void)
     return (dct_write_app_data(&magic_num, magic_num_offs, sizeof(magic_num)) == 0);
 }
 
-bool FLASH_ApplyFactoryResetImage(copymem_fn_t copy)
+int FLASH_ApplyFactoryResetImage(copymem_fn_t copy)
 {
     platform_flash_modules_t flash_module;
-    bool restoreFactoryReset = false;
+    int restoreFactoryReset = FLASH_ACCESS_RESULT_ERROR;
 
     //Read the flash modules info from the dct area
     dct_read_app_data_copy(DCT_FLASH_MODULES_OFFSET + (FAC_RESET_SLOT * sizeof(flash_module)), &flash_module, sizeof(flash_module));
@@ -603,12 +603,12 @@ bool FLASH_ApplyFactoryResetImage(copymem_fn_t copy)
 
 bool FLASH_IsFactoryResetAvailable(void)
 {
-    return FLASH_ApplyFactoryResetImage(FLASH_CheckCopyMemory);
+    return !FLASH_ApplyFactoryResetImage(FLASH_CheckCopyMemory);
 }
 
 bool FLASH_RestoreFromFactoryResetModuleSlot(void)
 {
-    return FLASH_ApplyFactoryResetImage(FLASH_CopyMemory);
+    return !FLASH_ApplyFactoryResetImage(FLASH_CopyMemory);
 }
 
 //This function called in bootloader to perform the memory update process

platform/MCU/shared/STM32/inc/flash_access.h

@@ -7,6 +7,11 @@
 extern "C" {
 #endif
 
+typedef enum {
+  FLASH_ACCESS_RESULT_OK             = 0,
+  FLASH_ACCESS_RESULT_BADARG         = 1,
+  FLASH_ACCESS_RESULT_ERROR          = 2
+} flash_access_result_t;
 
 /* MAL access layer for Internal/Serial Flash Routines */
 //New routines specific for BM09/BM14 flash usage
@@ -16,24 +21,24 @@ bool FLASH_CheckValidAddressRange(flash_device_t flashDeviceID, uint32_t startAd
 bool FLASH_WriteProtectMemory(flash_device_t flashDeviceID, uint32_t startAddress, uint32_t length, bool protect);
 bool FLASH_EraseMemory(flash_device_t flashDeviceID, uint32_t startAddress, uint32_t length);
 
-typedef bool (*copymem_fn_t)(flash_device_t sourceDeviceID, uint32_t sourceAddress,
-                      flash_device_t destinationDeviceID, uint32_t destinationAddress,
-                      uint32_t length, uint8_t module_function, uint8_t flags);
+typedef int (*copymem_fn_t)(flash_device_t sourceDeviceID, uint32_t sourceAddress,
+                            flash_device_t destinationDeviceID, uint32_t destinationAddress,
+                            uint32_t length, uint8_t module_function, uint8_t flags);
 
 
 /**
  * Determines if the memory copy can be performed.
  */
-bool FLASH_CheckCopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
-                      flash_device_t destinationDeviceID, uint32_t destinationAddress,
-                      uint32_t length, uint8_t module_function, uint8_t flags);
+int FLASH_CheckCopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
+                          flash_device_t destinationDeviceID, uint32_t destinationAddress,
+                          uint32_t length, uint8_t module_function, uint8_t flags);
 
 /**
  * @param validateDestinationAddress checks if the destination address corresponds with the start address in the module
  */
-bool FLASH_CopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
-                      flash_device_t destinationDeviceID, uint32_t destinationAddress,
-                      uint32_t length, uint8_t module_function, uint8_t flags);
+int FLASH_CopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
+                     flash_device_t destinationDeviceID, uint32_t destinationAddress,
+                     uint32_t length, uint8_t module_function, uint8_t flags);
 
 bool FLASH_CompareMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
                          flash_device_t destinationDeviceID, uint32_t destinationAddress,