[riot-notifications] [RIOT-OS/RIOT] cpu/stm32: implement reset to bootloader (#14119)

Alexandre Abadie notifications at github.com
Sat May 23 14:08:05 CEST 2020


@aabadie commented on this pull request.

Nice feature, thanks for this!

The main problem I see is the fact that `pre_startup` is not used anywhere and it's not clear where and when it should be called/used.
A module (or feature) that gives the possibility to explicitly adds this feature to an application is also missing. By default, this should not be built.
Maybe move `bootloader.c` in `stm32/booloader` with a Makefile containing:
```mk
MODULE = bootloader_stm32

include $(RIOTBASE)/Makefile.base
```

and in stm32/Makefile, add:

```mk
ifneq (,bootloader_stm32,$(USEMODULE))
  DIRS += bootloader
endif
```

>From the buildsystem perspective, I'm also wondering if we want to declare the stm32 bootloader as a feature.

It would be good to have a test application to try (and build) this feature.

> @@ -28,6 +28,21 @@ extern "C" {
  */
 #define CPUID_ADDR          (0x1ffff7ac)
 
+/**
+ * @brief   Starting address of the ROM bootloader
+ *          see application note AN2606
+ */
+#if defined(CPU_LINE_STM32F030x4) || defined(CPU_LINE_STM32F030x6) || \
+    defined(CPU_LINE_STM32F030x8) || defined(CPU_LINE_STM32F031x6) || \
+    defined(CPU_LINE_STM32F051x8)
+#define STM32_LOADER_ADDR   (0x1FFFEC00)

```suggestion
#define BOOTLOADER_ADDR   (0x1FFFEC00)
```

This is a matter of taste but I find this more generic and explicit.

> +    /* remap ROM at zero */
+#if defined(SYSCFG_MEMRMP_MEM_MODE_0)
+    SYSCFG->MEMRMP = SYSCFG_MEMRMP_MEM_MODE_0;
+#elif defined(SYSCFG_CFGR1_MEM_MODE_0)
+    SYSCFG->CFGR1  = SYSCFG_CFGR1_MEM_MODE_0;
+#endif
+
+    /* load bootloader address into r0 */
+    register uint32_t dst __asm__("r0") = STM32_LOADER_ADDR;
+
+    /* jump to the bootloader */
+    __asm__ volatile("mov sp, %0" :: "r" (dst));
+    __asm__ volatile("mov pc, %0" :: "r" (dst));
+}
+
+void __attribute__((weak)) usb_board_reset_in_bootloader(void)

Why `__attribute__((weak))` ?

> + *              See application note AN2606 for which options are available on
+ *              your individual MCU.
+ *
+ * @author      Benjamin Valentin <benpicco at googlemail.com>
+ *
+ * @}
+ */
+
+#include "cpu.h"
+#include "periph_cpu.h"
+
+#define BOOTLOADER_MAGIC    0xB007AFFE
+
+static uint32_t _magic __attribute__((section(".noinit")));
+
+void pre_startup(void)

Where would you call this function ? From cpu_init ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/14119#pullrequestreview-417272835
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20200523/caabca98/attachment.htm>


More information about the notifications mailing list