Change in simtrace2[master]: DFU: use central DFU override check
Harald Welte has submitted this change and it was merged. (
https://gerrit.osmocom.org/9913 )
Change subject: DFU: use central DFU override check
..
DFU: use central DFU override check
TRACE_INFO will also provide the DFU start reason, but TRACE_INFO
(TRACE_LEVEL >= 4) should only be used for debugging.
WARNING: With TRACE_LEVEL >= 4 the DFU binary is over the maximum of
16 kiB allocated for the DFU bootloader.
Thus make combined will not boot the main application because its
start if after the expecte 16 kiB address; and flashing using DFU
will overwrite the DFU bootloader itself.
Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa
---
M firmware/apps/dfu/main.c
M firmware/libboard/common/source/board_cstartup_gnu.c
2 files changed, 51 insertions(+), 28 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/firmware/apps/dfu/main.c b/firmware/apps/dfu/main.c
index 26cbb8c..467b7dd 100644
--- a/firmware/apps/dfu/main.c
+++ b/firmware/apps/dfu/main.c
@@ -159,22 +159,30 @@
int USBDFU_OverrideEnterDFU(void)
{
uint32_t *app_part = (uint32_t *)FLASH_ADDR(0);
+ /* at the first call we are before the text segment has been relocated,
+* so g_dfu is not initialized yet */
+ g_dfu = &_g_dfu;
+ if (USB_DFU_MAGIC == g_dfu->magic) {
+ return 1;
+ }
/* If the loopback jumper is set, we enter DFU mode */
- if (board_override_enter_dfu())
- return 1;
+ if (board_override_enter_dfu()) {
+ return 2;
+ }
/* if the first word of the application partition doesn't look
* like a stack pointer (i.e. point to RAM), enter DFU mode */
- if ((app_part[0] < IRAM_ADDR) ||
- ((uint8_t *)app_part[0] > IRAM_END))
- return 1;
+ if ((app_part[0] < IRAM_ADDR) || ((uint8_t *)app_part[0] > IRAM_END)) {
+ return 3;
+ }
/* if the second word of the application partition doesn't look
* like a function from flash (reset vector), enter DFU mode */
if (((uint32_t *)app_part[1] < app_part) ||
- ((uint8_t *)app_part[1] > IFLASH_END))
- return 1;
+ ((uint8_t *)app_part[1] > IFLASH_END)) {
+ return 4;
+ }
return 0;
}
@@ -218,7 +226,7 @@
EEFC_ReadUniqueID(g_unique_id);
-printf("\n\r\n\r"
+ printf("\n\r\n\r"
"=\n\r"
"DFU bootloader %s for board %s (C) 2010-2017 by Harald
Welte\n\r"
"=\n\r",
@@ -230,6 +238,35 @@
g_unique_id[2], g_unique_id[3]);
TRACE_INFO("Reset Cause: 0x%x\n\r", reset_cause);
+#if (TRACE_LEVEL >= TRACE_LEVEL_INFO)
+ /* Find out why we are in the DFU bootloader, and not the main
application */
+ TRACE_INFO("DFU bootloader start reason: ");
+ switch (USBDFU_OverrideEnterDFU()) {
+ case 0:
+ /* 0 normally means that there is no override, but we are in
the bootloader,
+* thus the first check in board_cstartup_gnu did return
something else than 0.
+* this can only be g_dfu->magic which is erased when the
segment are
+* relocated, which happens in board_cstartup_gnu just after
USBDFU_OverrideEnterDFU.
+* no static variable can be used to store this case since this
will also be overwritten
+*/
+ case 1:
+ TRACE_INFO_WP("DFU switch requested by main application\n\r");
+ break;
+ case 2:
+ TRACE_INFO_WP("bootloader forced (button pressed or jumper
set)\n\r");
+ break;
+ case 3:
+ TRACE_INFO_WP("stack pointer (first application word) does no
point in RAM\n\r");
+ break;
+ case 4: // the is no reason
+ TRACE_INFO_WP("reset vector (second application word) does no
point in flash\n\r");
+ break;
+ default:
+ TRACE_INFO_WP("unknown\n\r");
+ break;
+ }
+#endif
+
/* clear g_dfu on power-up reset */
if (reset_cause == 0)
memset(g_dfu, 0, sizeof(*g_dfu));
diff --git a/firmware/libboard/common/source/board_cstartup_gnu.c
b/firmware/libboard/common/source/board_cstartup_gnu.c
index 42231fb..bfe59ad 100644
--- a/firmware/libboard/common/source/board_cstartup_gnu.c
+++ b/firmware/libboard/common/source/board_cstartup_gnu.c
@@ -159,26 +159,12 @@
#if defined(BOARD_USB_DFU) && defined(APPLICATION_dfu)
-/* we are before the text segment has been relocated, so g_dfu is
- * not initialized yet */
-g_dfu = &_g_dfu;
-if ((g_dfu->magic != USB_DFU_MAGIC) &&
Change in simtrace2[master]: DFU: use central DFU override check
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/9913 ) Change subject: DFU: use central DFU override check .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/9913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa Gerrit-Change-Number: 9913 Gerrit-PatchSet: 4 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Wed, 11 Jul 2018 19:50:41 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in simtrace2[master]: DFU: use central DFU override check
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/9913 to look at the new patch set (#4). Change subject: DFU: use central DFU override check .. DFU: use central DFU override check TRACE_INFO will also provide the DFU start reason, but TRACE_INFO (TRACE_LEVEL >= 4) should only be used for debugging. WARNING: With TRACE_LEVEL >= 4 the DFU binary is over the maximum of 16 kiB allocated for the DFU bootloader. Thus make combined will not boot the main application because its start if after the expecte 16 kiB address; and flashing using DFU will overwrite the DFU bootloader itself. Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa --- M firmware/apps/dfu/main.c M firmware/libboard/common/source/board_cstartup_gnu.c 2 files changed, 51 insertions(+), 28 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/13/9913/4 -- To view, visit https://gerrit.osmocom.org/9913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa Gerrit-Change-Number: 9913 Gerrit-PatchSet: 4 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Jenkins Builder
Change in simtrace2[master]: DFU: use central DFU override check
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/9913 to look at the new patch set (#3). Change subject: DFU: use central DFU override check .. DFU: use central DFU override check TRACE_INFO will also provide the DFU start reason, but TRACE_INFO (TRACE_LEVEL >= 4) should only be used for debugging. WARNING: With TRACE_LEVEL >= 4 the DFU binary is over the maximum of 16 kiB allocated for the DFU bootloader. Thus make combined will not boot the main application because its start if after the expecte 16 kiB address; and flashing using DFU will overwrite the DFU bootloader itself. Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa --- M firmware/apps/dfu/main.c M firmware/libboard/common/source/board_cstartup_gnu.c 2 files changed, 51 insertions(+), 28 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/13/9913/3 -- To view, visit https://gerrit.osmocom.org/9913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa Gerrit-Change-Number: 9913 Gerrit-PatchSet: 3 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Jenkins Builder
Change in simtrace2[master]: DFU: use central DFU override check
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/9913 to look at the new patch set (#2). Change subject: DFU: use central DFU override check .. DFU: use central DFU override check TRACE_INFO will also provide the DFU start reason, but TRACE_INFO (TRACE_LEVEL >= 4) should only be used for debugging. WARNING: With TRACE_LEVEL >= 4 the DFU binary is over the maximum of 16 kiB allocated for the DFU bootloader. Thus make combined will not boot the main application because its start if after the expecte 16 kiB address; and flashing using DFU will overwrite the DFU bootloader itself. Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa --- M firmware/apps/dfu/main.c M firmware/libboard/common/source/board_cstartup_gnu.c 2 files changed, 49 insertions(+), 28 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/13/9913/2 -- To view, visit https://gerrit.osmocom.org/9913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa Gerrit-Change-Number: 9913 Gerrit-PatchSet: 2 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Jenkins Builder
Change in simtrace2[master]: DFU: use central DFU override check
Kévin Redon has uploaded this change for review. (
https://gerrit.osmocom.org/9913
Change subject: DFU: use central DFU override check
..
DFU: use central DFU override check
TRACE_INFO will also provide the DFU start reason, but TRACE_INFO
(TRACE_LEVEL >= 4) should only be used for debugging.
WARNING: With TRACE_LEVEL >= 4 the DFU binary is over the maximum of
16 kiB allocated for the DFU bootloader.
Thus make combined will not boot the main application because its
start if after the expecte 16 kiB address; and flashing using DFU
will overwrite the DFU bootloader itself.
Change-Id: I82323e0f76c03f67df7dc8f2b6783166cc25f3aa
---
M firmware/apps/dfu/main.c
M firmware/libboard/common/source/board_cstartup_gnu.c
2 files changed, 49 insertions(+), 28 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/13/9913/1
diff --git a/firmware/apps/dfu/main.c b/firmware/apps/dfu/main.c
index 26cbb8c..796a13f 100644
--- a/firmware/apps/dfu/main.c
+++ b/firmware/apps/dfu/main.c
@@ -159,22 +159,32 @@
int USBDFU_OverrideEnterDFU(void)
{
uint32_t *app_part = (uint32_t *)FLASH_ADDR(0);
+ /* at the first call we are before the text segment has been relocated,
+* so g_dfu is not initialized yet */
+g_dfu = &_g_dfu;
+ static bool dfu_magic = false;
+ if (dfu_magic || (!dfu_magic && USB_DFU_MAGIC == g_dfu->magic)) {
+ dfu_magic = true;
+ return 1;
+ }
/* If the loopback jumper is set, we enter DFU mode */
- if (board_override_enter_dfu())
- return 1;
+ if (board_override_enter_dfu()) {
+ return 2;
+ }
/* if the first word of the application partition doesn't look
* like a stack pointer (i.e. point to RAM), enter DFU mode */
- if ((app_part[0] < IRAM_ADDR) ||
- ((uint8_t *)app_part[0] > IRAM_END))
- return 1;
+ if ((app_part[0] < IRAM_ADDR) || ((uint8_t *)app_part[0] > IRAM_END)) {
+ return 3;
+ }
/* if the second word of the application partition doesn't look
* like a function from flash (reset vector), enter DFU mode */
if (((uint32_t *)app_part[1] < app_part) ||
- ((uint8_t *)app_part[1] > IFLASH_END))
- return 1;
+ ((uint8_t *)app_part[1] > IFLASH_END)) {
+ return 4;
+ }
return 0;
}
@@ -218,7 +228,7 @@
EEFC_ReadUniqueID(g_unique_id);
-printf("\n\r\n\r"
+ printf("\n\r\n\r"
"=\n\r"
"DFU bootloader %s for board %s (C) 2010-2017 by Harald
Welte\n\r"
"=\n\r",
@@ -230,6 +240,31 @@
g_unique_id[2], g_unique_id[3]);
TRACE_INFO("Reset Cause: 0x%x\n\r", reset_cause);
+#if (TRACE_LEVEL >= TRACE_LEVEL_INFO)
+ /* Find out why we are in the DFU bootloader, and not the main
application */
+ TRACE_INFO("DFU bootloader start reason: ");
+ switch (USBDFU_OverrideEnterDFU()) {
+ case 0:
+ TRACE_INFO_WP("none\n\r");
+ break;
+ case 1:
+ TRACE_INFO_WP("DFU switch requested by main application\n\r");
+ break;
+ case 2:
+ TRACE_INFO_WP("bootloader forced (button pressed or jumper
set)\n\r");
+ break;
+ case 3:
+ TRACE_INFO_WP("stack pointer (first application word) does no
point in RAM\n\r");
+ break;
+ case 4: // the is no reason
+ TRACE_INFO_WP("reset vector (second application word) does no
point in flash\n\r");
+ break;
+ default:
+ TRACE_INFO_WP("unknown\n\r");
+ break;
+ }
+#endif
+
/* clear g_dfu on power-up reset */
if (reset_cause == 0)
memset(g_dfu, 0, sizeof(*g_dfu));
diff --git a/firmware/libboard/common/source/board_cstartup_gnu.c
b/firmware/libboard/common/source/board_cstartup_gnu.c
index 42231fb..bfe59ad 100644
--- a/firmware/libboard/common/source/board_cstartup_gnu.c
+++ b/firmware/libboard/common/source/board_cstartup_gnu.c
@@ -159,26 +159,12 @@
#if defined(BOARD_USB_DFU) && defined(APPLICATION_dfu)
-/* we are before the text segment has been relocated, so g_dfu is
- * not initialized yet */
-g_dfu = &_g_dfu;
-if ((g_dfu->magic != USB_DFU_MAGIC) && !USBDFU_OverrideEnterDFU()) {
-/* start application if valid
- * the application starts with the vector table
- * the first entry in the vector table is the initial stack pointer
(SP) address
- * the stack will be placed in RAM, which begins at 0x2000
- * there is up to 48 KB of RAM (0xc000)
- * since the stack
