Merge pull request #377 from ejtagle/master

Try to gracefully handle FLASH operations queue becoming full, instead of silently failing and allow OTA DFU process to be recoverable, in case of a communication error in the DUAL_BANK configuration

Author: hathach

CMakeLists.txt

@@ -28,6 +28,7 @@ set(MBR_HEX ${SOFTDEVICE_DIR}/mbr/hex/mbr_nrf52_2.4.1_mbr.hex)
 set(TINYUSB_DIR ${CMAKE_CURRENT_LIST_DIR}/lib/tinyusb/src)
 set(TCRYPT_DIR ${CMAKE_CURRENT_LIST_DIR}/lib/tinycrypt/lib)
 
+option(DEFAULT_TO_OTA_DFU "Default to OTA DFU instead of Serial DFU" OFF)
 option(SIGNED_FW "Enable signed firmware verification" OFF)
 option(DUALBANK_FW "Enable dual bank DFU support" OFF)
 option(FORCE_UF2 "Force UF2 support even with SIGNED_FW" OFF)
@@ -196,6 +197,10 @@ if (FORCE_UF2)
   target_compile_definitions(bootloader PUBLIC FORCE_UF2)
 endif ()
 
+if (DEFAULT_TO_OTA_DFU)
+  target_compile_definitions(bootloader PUBLIC DEFAULT_TO_OTA_DFU)
+endif ()
+
 if (TRACE_ETM STREQUAL "1")
   # ENABLE_TRACE will cause system_nrf5x.c to set up ETM trace
   target_compile_definitions(bootloader PUBLIC ENABLE_TRACE)

Makefile

@@ -2,14 +2,15 @@
 # CONFIGURE
 # - SDK_PATH     : path to SDK directory
 #
-# - SD_NAME      : e.g s132, s140
-# - SD_VERSION   : SoftDevice version e.g 6.0.0
-# - SD_HEX       : to bootloader hex binary
-# - SIGNED_FW    : if bootloader will ONLY accept signed firmware
-# - SIGNED_FW_QX : Qx for signed firmware verification
-# - SIGNED_FW_QY : Qy for signed firmware verification
-# - DUALBANK_FW  : If bootloader will implement a dual bank feature to allow autorecover from failed
-# - FORCE_UF2    : if SIGNED_FW is 1, will force to include UF2 support (UNSECURE, UF2 does NOT validate signature!)
+# - SD_NAME            : e.g s132, s140
+# - SD_VERSION         : SoftDevice version e.g 6.0.0
+# - SD_HEX             : to bootloader hex binary
+# - SIGNED_FW          : if bootloader will ONLY accept signed firmware
+# - SIGNED_FW_QX       : Qx for signed firmware verification
+# - SIGNED_FW_QY       : Qy for signed firmware verification
+# - DUALBANK_FW        : If bootloader will implement a dual bank feature to allow autorecover from failed
+# - FORCE_UF2          : if SIGNED_FW is 1, will force to include UF2 support (UNSECURE, UF2 does NOT validate signature!)
+# - DEFAULT_TO_OTA_DFU : if entering DFU, by default enter OTA DFU instead of Serial DFU
 #------------------------------------------------------------------------------
 
 PYTHON = python
@@ -371,6 +372,10 @@ ifeq ($(FORCE_UF2), 1)
 CFLAGS += -DFORCE_UF2
 endif
 
+ifeq ($(DEFAULT_TO_OTA_DFU), 1)
+CFLAGS += -DDEFAULT_TO_OTA_DFU
+endif
+
 _VER = $(subst ., ,$(word 1, $(subst -, ,$(GIT_VERSION))))
 CFLAGS += -DMK_BOOTLOADER_VERSION='($(word 1,$(_VER)) << 16) + ($(word 2,$(_VER)) << 8) + $(word 3,$(_VER))'
 

lib/sdk11/components/libraries/bootloader_dfu/bootloader.c

@@ -44,7 +44,8 @@ typedef enum
     BOOTLOADER_SETTINGS_SAVING,                         /**< Bootloader status for indicating that saving of bootloader settings is in progress. */
     BOOTLOADER_COMPLETE,                                /**< Bootloader status for indicating that all operations for the update procedure has completed and it is safe to reset the system. */
     BOOTLOADER_TIMEOUT,                                 /**< Bootloader status field for indicating that a timeout has occured and current update process should be aborted. */
-    BOOTLOADER_RESET,                                   /**< Bootloader status field for indicating that a reset has been requested and current update process should be aborted. */
+    BOOTLOADER_SYS_RESET,                               /**< Bootloader status field for indicating that a reset has been requested and current update process should be aborted. */
+    BOOTLOADER_RESET_TO_SELF,                           /**< Bootloader status field for indicating that a reset has been requested and current update process should be aborted and the bootloader must be reentered. */
 } bootloader_status_t;
 
 static pstorage_handle_t        m_bootsettings_handle;  /**< Pstorage handle to use for registration and identifying the bootloader module on subsequent calls to the pstorage module for load and store of bootloader setting in flash. */
@@ -136,9 +137,10 @@ static void wait_for_events(void)
 
     if ((m_update_status == BOOTLOADER_COMPLETE) ||
         (m_update_status == BOOTLOADER_TIMEOUT) ||
-        (m_update_status == BOOTLOADER_RESET) )
+        (m_update_status == BOOTLOADER_SYS_RESET) ||
+        (m_update_status == BOOTLOADER_RESET_TO_SELF))
     {
-      // When update has completed or a timeout/reset occured we will return.
+      // When update has completed or a timeout/reset occurred we will return.
       return;
     }
   }
@@ -186,10 +188,36 @@ static void bootloader_settings_save(bootloader_settings_t * p_settings)
 {
   if ( is_ota() )
   {
-    uint32_t err_code = pstorage_clear(&m_bootsettings_handle, sizeof(bootloader_settings_t));
+    uint32_t err_code;
+    while(1) {
+      err_code = pstorage_clear(&m_bootsettings_handle, sizeof(bootloader_settings_t));
+      if (err_code != NRF_ERROR_NO_MEM) {
+        break;
+      }
+      // This means the write/erase queue of commands was completely full - Should not
+      //  happen, but better safe than sorry, wait until space becomes available -
+      //  the pstorage event handler is only run from the main loop, and we are also 
+      //  running from a BLE event on the main loop: This means if we wait here, the FLASH 
+      //  completion events will never be handled (will be queued by app_scheduler, but
+      //  not handled, and that means this wait will never end... So, process SOC events
+      //  from the SD (but NOT BLE events!) - This will free entries on the pstorage queue
+      //  as soon as operations are complete, allowing this op to be queued
+      while (NRF_ERROR_NOT_FOUND != proc_soc()) {
+        // nothing
+      }
+    }
     APP_ERROR_CHECK(err_code);
 
-    err_code = pstorage_store(&m_bootsettings_handle, (uint8_t *) p_settings, sizeof(bootloader_settings_t), 0);
+    while(1) {
+      err_code = pstorage_store(&m_bootsettings_handle, (uint8_t *) p_settings, sizeof(bootloader_settings_t), 0);
+      if (err_code != NRF_ERROR_NO_MEM) {
+        break;
+      }
+      // No space, wait until an entry in the queue is freed
+      while (NRF_ERROR_NOT_FOUND != proc_soc()) {
+        // nothing
+      }
+    }
     APP_ERROR_CHECK(err_code);
   }
   else
@@ -296,14 +324,19 @@ void bootloader_dfu_update_process(dfu_update_status_t update_status)
   }
   else if (update_status.status_code == DFU_RESET)
   {
-    m_update_status = BOOTLOADER_RESET;
+    m_update_status =
+      (update_status.restart_into_bootloader == false ? BOOTLOADER_SYS_RESET : BOOTLOADER_RESET_TO_SELF);
   }
   else
   {
     // No implementation needed.
   }
 }
 
+bool bootloader_must_reset_to_self(void)
+{
+  return m_update_status == BOOTLOADER_RESET_TO_SELF;
+}
 
 uint32_t bootloader_init(void)
 {

lib/sdk11/components/libraries/bootloader_dfu/bootloader.h

@@ -32,6 +32,13 @@
  */
 uint32_t bootloader_init(void);
 
+/**@brief Function to check if bootloader must be reentered after a reset
+ * @details: This function can only be called after bootloader_dfu_start returns
+ * @retval     true          If device must reboot to the bootloader
+ * @retval     false         If device must reboot to the application, if possible
+ */
+bool bootloader_must_reset_to_self(void);
+
 /**@brief Function for validating application region in flash.
  * @retval     true          If Application region is valid.
  * @retval     false         If Application region is not valid.
@@ -92,6 +99,8 @@ uint32_t bootloader_dfu_sd_update_finalize(void);
 
 void bootloader_mbr_addrs_populate(void);
 
+extern uint32_t proc_soc(void);
+
 #endif // BOOTLOADER_H__
 
 /**@} */

lib/sdk11/components/libraries/bootloader_dfu/bootloader_util.c

@@ -32,6 +32,12 @@
 #if defined ( __CC_ARM )
 __asm static void bootloader_util_reset(uint32_t start_addr)
 {
+    MOVS  R5, #0x00             ; R5 = 0
+    MSR   CONTROL, R5           ; Clear CONTROL
+    MSR   PRIMASK, R5           ; Clear PRIMASK
+    MSR   BASEPRI, R5           ; Clear BASEPRI
+    MSR   FAULTMASK, R5         ; Clear FAULTMASK
+
     LDR   R5, [R0]              ; Get App initial MSP for bootloader.
     MSR   MSP, R5               ; Set the main stack pointer to the applications MSP.
     LDR   R0, [R0, #0x04]       ; Load Reset handler into R0. This will be first argument to branch instruction (BX).
@@ -69,6 +75,12 @@ static inline void bootloader_util_reset (uint32_t start_addr) __attribute__ ((o
 static inline void bootloader_util_reset(uint32_t start_addr)
 {
     __asm volatile(
+        "movs  r5, #0x00\t\n"           // R5 = 0
+        "msr   control, r5\t\n"         // Clear CONTROL
+        "msr   primask, r5\t\n"         // Clear PRIMASK
+        "msr   basepri, r5\t\n"         // Clear BASEPRI
+        "msr   faultmask, r5\t\n"       // Clear FAULTMASK
+
         "ldr   r0, [%0]\t\n"            // Get App initial MSP for bootloader.
         "msr   msp, r0\t\n"             // Set the main stack pointer to the applications MSP.
         "ldr   r0, [%0, #0x04]\t\n"     // Load Reset handler into R0.
@@ -108,7 +120,13 @@ static inline void bootloader_util_reset(uint32_t start_addr)
 #elif defined ( __ICCARM__ )
 static inline void bootloader_util_reset(uint32_t start_addr)
 {
-    asm("ldr   r5, [%0]\n"                    // Get App initial MSP for bootloader.
+    asm("movs  r5, #0x00\n"                   // R5 = 0
+        "msr   control, r5\n"                 // Clear CONTROL
+        "msr   primask, r5\n"                 // Clear PRIMASK
+        "msr   basepri, r5\n"                 // Clear BASEPRI
+        "msr   faultmask, r5\n"               // Clear FAULTMASK
+
+        "ldr   r5, [%0]\n"                    // Get App initial MSP for bootloader.
         "msr   msp, r5\n"                     // Set the main stack pointer to the applications MSP.
         "ldr   r0, [%0, #0x04]\n"             // Load Reset handler into R0.