laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42176?usp=email )
Change subject: dfu: protect USB_DFU_STATE_DFU_MANIFEST by a critical section
......................................................................
dfu: protect USB_DFU_STATE_DFU_MANIFEST by a critical section
In the MANIFEST state, the IRQ won't move the state, but
to prevent incosistency between dfu_manifestation_complete &
dfu_state, use a critical section.
Change-Id: Idf5fb7d55b4051ba7e235dfa409a4de18a8f208c
---
M usb_start.c
1 file changed, 2 insertions(+), 0 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
diff --git a/usb_start.c b/usb_start.c
index 78711f1..0b99746 100644
--- a/usb_start.c
+++ b/usb_start.c
@@ -189,12 +189,14 @@
case USB_DFU_STATE_DFU_MANIFEST: // we can start manifestation (finish flashing)
// in theory every DFU files should have a suffix to with a CRC to check the data
// in practice most downloaded files are just the raw binary with DFU suffix
+ CRITICAL_SECTION_ENTER();
dfu_manifestation_complete = true; // we completed flashing and all checks
if (usb_dfu_func_desc->bmAttributes & USB_DFU_ATTRIBUTES_MANIFEST_TOLERANT) {
dfu_state = USB_DFU_STATE_DFU_MANIFEST_SYNC;
} else {
dfu_state = USB_DFU_STATE_DFU_MANIFEST_WAIT_RESET;
}
+ CRITICAL_SECTION_LEAVE();
break;
case USB_DFU_STATE_DFU_MANIFEST_WAIT_RESET:
if (usb_dfu_func_desc->bmAttributes & USB_DFU_ATTRIBUTES_WILL_DETACH) {
--
To view, visit https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42176?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-Change-Id: Idf5fb7d55b4051ba7e235dfa409a4de18a8f208c
Gerrit-Change-Number: 42176
Gerrit-PatchSet: 6
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42321?usp=email )
Change subject: dfu: flash: protect parsing of rc of flashing
......................................................................
dfu: flash: protect parsing of rc of flashing
Otherwise it is not guaranteed that both dfu_status and dfu_state
are in sync when the IRQ handler is running.
Change-Id: Ifc0d56d779ec31382855d6c367478104bc04e1e7
---
M usb_start.c
1 file changed, 2 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
Hoernchen: Looks good to me, but someone else must approve
diff --git a/usb_start.c b/usb_start.c
index cb2bc51..43b61b0 100644
--- a/usb_start.c
+++ b/usb_start.c
@@ -184,6 +184,7 @@
dfu_download_data, dfu_download_length);
}
+ CRITICAL_SECTION_ENTER();
if (ERR_NONE == rc) {
dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE; // indicate flashing this block has been completed
} else { // there has been a programming error
@@ -196,6 +197,7 @@
dfu_status = USB_DFU_STATUS_ERR_PROG;
}
}
+ CRITICAL_SECTION_LEAVE();
} else { // there was no data to flash
// this case should not happen, but it's not a critical error
dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE; // indicate flashing can continue
--
To view, visit https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42321?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-Change-Id: Ifc0d56d779ec31382855d6c367478104bc04e1e7
Gerrit-Change-Number: 42321
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42322?usp=email )
Change subject: dfu: download: make variables between IRQ and main loop volatile
......................................................................
dfu: download: make variables between IRQ and main loop volatile
Both are access by IRQ and mainloop and are written by the IRQ handler.
Change-Id: Ic3dccd77eff7feb164f9f07047680eef3f7c2516
---
M usb/class/dfu/device/dfudf.c
M usb/class/dfu/device/dfudf.h
2 files changed, 4 insertions(+), 4 deletions(-)
Approvals:
Jenkins Builder: Verified
Hoernchen: Looks good to me, approved
diff --git a/usb/class/dfu/device/dfudf.c b/usb/class/dfu/device/dfudf.c
index 389f07f..7388060 100644
--- a/usb/class/dfu/device/dfudf.c
+++ b/usb/class/dfu/device/dfudf.c
@@ -43,8 +43,8 @@
volatile enum usb_dfu_status dfu_status = USB_DFU_STATUS_OK;
uint8_t dfu_download_data[512];
-uint16_t dfu_download_length = 0;
-size_t dfu_download_offset = 0;
+volatile uint16_t dfu_download_length = 0;
+volatile size_t dfu_download_offset = 0;
/* buffer the first block, to write it last */
uint8_t dfu_download_data_first[512];
diff --git a/usb/class/dfu/device/dfudf.h b/usb/class/dfu/device/dfudf.h
index 390aace..15d6ad3 100644
--- a/usb/class/dfu/device/dfudf.h
+++ b/usb/class/dfu/device/dfudf.h
@@ -48,9 +48,9 @@
*/
extern uint8_t dfu_download_data[512];
/** Length of downloaded data in bytes */
-extern uint16_t dfu_download_length;
+extern volatile uint16_t dfu_download_length;
/** Offset of where the downloaded data should be flashed in bytes */
-extern size_t dfu_download_offset;
+extern volatile size_t dfu_download_offset;
extern uint8_t dfu_download_data_first[512];
extern volatile uint16_t dfu_download_length_first;
--
To view, visit https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42322?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-Change-Id: Ic3dccd77eff7feb164f9f07047680eef3f7c2516
Gerrit-Change-Number: 42322
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42273?usp=email )
Change subject: dfu: download: flash the first block in manifest phase
......................................................................
dfu: download: flash the first block in manifest phase
To prevent half flashed applications, erase the first page
when dfu downloading starts and save the first block for later.
In manifest stage, flash the first block.
If the first 4 byte are 0xffffffff, the board won't boot
into application and go into the dfu bootloader.
Change-Id: I894f3ee71587ccb287e92d7025039954991c631f
---
M usb/class/dfu/device/dfudf.c
M usb/class/dfu/device/dfudf.h
M usb_start.c
3 files changed, 58 insertions(+), 10 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/usb/class/dfu/device/dfudf.c b/usb/class/dfu/device/dfudf.c
index f931260..389f07f 100644
--- a/usb/class/dfu/device/dfudf.c
+++ b/usb/class/dfu/device/dfudf.c
@@ -45,6 +45,11 @@
uint8_t dfu_download_data[512];
uint16_t dfu_download_length = 0;
size_t dfu_download_offset = 0;
+
+/* buffer the first block, to write it last */
+uint8_t dfu_download_data_first[512];
+volatile uint16_t dfu_download_length_first = 0;
+
bool dfu_manifestation_complete = false;
/**
diff --git a/usb/class/dfu/device/dfudf.h b/usb/class/dfu/device/dfudf.h
index 818252d..390aace 100644
--- a/usb/class/dfu/device/dfudf.h
+++ b/usb/class/dfu/device/dfudf.h
@@ -51,6 +51,10 @@
extern uint16_t dfu_download_length;
/** Offset of where the downloaded data should be flashed in bytes */
extern size_t dfu_download_offset;
+
+extern uint8_t dfu_download_data_first[512];
+extern volatile uint16_t dfu_download_length_first;
+
/** If manifestation (firmware flash and check) is complete */
extern bool dfu_manifestation_complete;
diff --git a/usb_start.c b/usb_start.c
index 0b99746..cb2bc51 100644
--- a/usb_start.c
+++ b/usb_start.c
@@ -157,6 +157,7 @@
uint32_t application_start_address = BL_SIZE_BYTE;
ASSERT(application_start_address > 0);
+ int rc;
while (true) { // main DFU infinite loop
enum usb_dfu_state last_dfu_state = dfu_state;
@@ -167,7 +168,22 @@
case USB_DFU_STATE_DFU_DNBUSY: // there is some data to be flashed
LED_SYSTEM_off(); // switch LED off to indicate we are flashing
if (dfu_download_length > 0) { // there is some data to be flashed
- int32_t rc = flash_write(&FLASH_0, application_start_address + dfu_download_offset, dfu_download_data, dfu_download_length); // write downloaded data chunk to flash
+ /* The first block will be only erased, to ensure the
+ * full firmware has been written.
+ * In case of a power loss, the bootloader won't boot applications
+ * if the first 4 byte are 0xffffffff
+ */
+ if (dfu_download_offset == 0) {
+ dfu_download_length_first = dfu_download_length;
+ memcpy(dfu_download_data_first, dfu_download_data, dfu_download_length);
+ /* erasing the first page is enough, flash_write will keep care of the remaining ones */
+ rc = flash_erase(&FLASH_0, application_start_address, 1);
+ } else {
+ // write downloaded data chunk to flash
+ rc = flash_write(&FLASH_0, application_start_address + dfu_download_offset,
+ dfu_download_data, dfu_download_length);
+ }
+
if (ERR_NONE == rc) {
dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE; // indicate flashing this block has been completed
} else { // there has been a programming error
@@ -186,16 +202,39 @@
}
LED_SYSTEM_on(); // switch LED on to indicate USB DFU can resume
break;
- case USB_DFU_STATE_DFU_MANIFEST: // we can start manifestation (finish flashing)
- // in theory every DFU files should have a suffix to with a CRC to check the data
- // in practice most downloaded files are just the raw binary with DFU suffix
- CRITICAL_SECTION_ENTER();
- dfu_manifestation_complete = true; // we completed flashing and all checks
- if (usb_dfu_func_desc->bmAttributes & USB_DFU_ATTRIBUTES_MANIFEST_TOLERANT) {
- dfu_state = USB_DFU_STATE_DFU_MANIFEST_SYNC;
- } else {
- dfu_state = USB_DFU_STATE_DFU_MANIFEST_WAIT_RESET;
+ case USB_DFU_STATE_DFU_MANIFEST:
+ /* finish flashing by writing the first block */
+ rc = flash_write(&FLASH_0, application_start_address, dfu_download_data_first,
+ dfu_download_length_first);
+ /* try to clear it, to ensure it doesn't boot into a broken application */
+ if (rc != ERR_NONE) {
+ flash_erase(&FLASH_0, application_start_address, 1);
}
+
+ CRITICAL_SECTION_ENTER();
+ switch (rc) {
+ case ERR_NONE:
+ if (usb_dfu_func_desc->bmAttributes & USB_DFU_ATTRIBUTES_MANIFEST_TOLERANT) {
+ dfu_state = USB_DFU_STATE_DFU_MANIFEST_SYNC;
+ } else {
+ dfu_state = USB_DFU_STATE_DFU_MANIFEST_WAIT_RESET;
+ }
+ dfu_status = USB_DFU_STATUS_OK;
+ break;
+ case ERR_BAD_ADDRESS:
+ dfu_state = USB_DFU_STATE_DFU_ERROR;
+ dfu_status = USB_DFU_STATUS_ERR_ADDRESS;
+ break;
+ case ERR_DENIED:
+ dfu_state = USB_DFU_STATE_DFU_ERROR;
+ dfu_status = USB_DFU_STATUS_ERR_WRITE;
+ break;
+ default:
+ dfu_state = USB_DFU_STATE_DFU_ERROR;
+ dfu_status = USB_DFU_STATUS_ERR_PROG;
+ break;
+ }
+ dfu_manifestation_complete = true; // we completed flashing and all checks
CRITICAL_SECTION_LEAVE();
break;
case USB_DFU_STATE_DFU_MANIFEST_WAIT_RESET:
--
To view, visit https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42273?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-Change-Id: I894f3ee71587ccb287e92d7025039954991c631f
Gerrit-Change-Number: 42273
Gerrit-PatchSet: 7
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42175?usp=email )
(
4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: dfu: MANIFEST_SYNC: stay in MANIFEST_SYNC when manifestationIntolerant & completed
......................................................................
dfu: MANIFEST_SYNC: stay in MANIFEST_SYNC when manifestationIntolerant & completed
In theory a device which is manifestintolerant and completed the manifestation should
not reach MANIFEST_SYNC again.
Remove the state transistion to WAIT-RESET and stay in the current state and
wait for the main loop to change the state is safer.
Change-Id: I8c34a18e2336731126a8c01070d86e2547578e3d
---
M usb/class/dfu/device/dfudf.c
1 file changed, 8 insertions(+), 5 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/usb/class/dfu/device/dfudf.c b/usb/class/dfu/device/dfudf.c
index 76dcc9b..f931260 100644
--- a/usb/class/dfu/device/dfudf.c
+++ b/usb/class/dfu/device/dfudf.c
@@ -167,12 +167,15 @@
dfu_state = USB_DFU_STATE_DFU_DNBUSY; // switch to busy state
break;
case USB_DFU_STATE_DFU_MANIFEST_SYNC:
- if (!dfu_manifestation_complete) {
+ if (dfu_manifestation_complete) {
+ if (usb_dfu_func_desc->bmAttributes & USB_DFU_ATTRIBUTES_MANIFEST_TOLERANT) {
+ dfu_state = USB_DFU_STATE_DFU_IDLE; // go back to idle mode
+ }
+ /* Otherwise stay in MANIFEST_SYNC. Hower this his should not happen (after manifestation
+ * the state should be dfuMANIFEST-WAIT-RESET if we are not manifest tolerant)
+ */
+ } else {
dfu_state = USB_DFU_STATE_DFU_MANIFEST; // go to manifest mode
- } else if (usb_dfu_func_desc->bmAttributes & USB_DFU_ATTRIBUTES_MANIFEST_TOLERANT) {
- dfu_state = USB_DFU_STATE_DFU_IDLE; // go back to idle mode
- } else { // this should not happen (after manifestation the state should be dfuMANIFEST-WAIT-RESET if we are not manifest tolerant)
- dfu_state = USB_DFU_STATE_DFU_MANIFEST_WAIT_RESET; // wait for reset
}
break;
default:
--
To view, visit https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42175?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-Change-Id: I8c34a18e2336731126a8c01070d86e2547578e3d
Gerrit-Change-Number: 42175
Gerrit-PatchSet: 6
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>