lynxis lazus has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42174?usp=email )
Change subject: dfu: rewrite firmware downloading
......................................................................
dfu: rewrite firmware downloading
Improve handling of dfu_state by moving more state changing towards the
IRQ handler. Having both the main loop and IRQ changes dfu_state within
the same state could lead to races.
The main loop is now only a simple worker which reports back via dfu_flash_done
&
dfu_flash_status.
Change-Id: I345d5948455b25cd8a2efb1abfd9d0986ebd8cef
---
M usb/class/dfu/device/dfudf.c
M usb/class/dfu/device/dfudf.h
M usb_start.c
3 files changed, 78 insertions(+), 34 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-asf4-dfu refs/changes/74/42174/1
diff --git a/usb/class/dfu/device/dfudf.c b/usb/class/dfu/device/dfudf.c
index c66454b..0f5ec0e 100644
--- a/usb/class/dfu/device/dfudf.c
+++ b/usb/class/dfu/device/dfudf.c
@@ -42,9 +42,15 @@
volatile enum usb_dfu_state dfu_state = USB_DFU_STATE_DFU_IDLE;
volatile enum usb_dfu_status dfu_status = USB_DFU_STATUS_OK;
+/* flashed the last given block */
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;
+
+/* only when flash done is true, flash rc is valid */
+volatile bool dfu_flash_done = false;
+volatile enum usb_dfu_status dfu_flash_status = USB_DFU_STATUS_ERR_UNKNOWN;
+
bool dfu_manifestation_complete = false;
/**
@@ -159,10 +165,20 @@
dfu_state = USB_DFU_STATE_DFU_ERROR; // unsupported class
request
to_return = ERR_UNSUPPORTED_OP; // stall control pipe (don't
reply to the request)
break;
- case USB_DFU_GETSTATUS: // get status
+ case USB_DFU_GETSTATUS:
switch (dfu_state) {
- case USB_DFU_STATE_DFU_DNLOAD_SYNC: // download has not
completed
- dfu_state = USB_DFU_STATE_DFU_DNBUSY; // switch to busy
state
+ case USB_DFU_STATE_DFU_DNLOAD_SYNC:
+ case USB_DFU_STATE_DFU_DNBUSY:
+ if (!dfu_flash_done) {
+ dfu_state = USB_DFU_STATE_DFU_DNBUSY;
+ break;
+ }
+
+ dfu_status = dfu_flash_status;
+ if (dfu_status == USB_DFU_STATUS_OK)
+ dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE;
+ else
+ dfu_state = USB_DFU_STATE_DFU_ERROR;
break;
case USB_DFU_STATE_DFU_MANIFEST_SYNC:
if (!dfu_manifestation_complete) {
@@ -230,36 +246,49 @@
if (!(usb_dfu_func_desc->bmAttributes & USB_REQ_DFU_DNLOAD)) {
// download is not enabled
dfu_state = USB_DFU_STATE_DFU_ERROR; // unsupported
class request
to_return = ERR_UNSUPPORTED_OP; // stall control pipe
(don't reply to the request)
+ break;
} else if (USB_DFU_STATE_DFU_IDLE != dfu_state &&
USB_DFU_STATE_DFU_DNLOAD_IDLE != dfu_state) { // wrong state to request download
// warn about programming error
dfu_status = USB_DFU_STATUS_ERR_PROG;
dfu_state = USB_DFU_STATE_DFU_ERROR;
to_return = ERR_INVALID_ARG; // stall control pipe to
indicate error
+ break;
} else if (USB_DFU_STATE_DFU_IDLE == dfu_state && (0 ==
req->wLength)) { // download request should not start empty
// warn about programming error
dfu_status = USB_DFU_STATUS_ERR_PROG;
dfu_state = USB_DFU_STATE_DFU_ERROR;
to_return = ERR_INVALID_ARG; // stall control pipe to
indicate error
+ break;
} else if (USB_DFU_STATE_DFU_DNLOAD_IDLE == dfu_state && (0 ==
req->wLength)) { // download completed
dfu_manifestation_complete = false; // clear
manifestation status
+ to_return = usbdc_xfer(ep, NULL, 0, false); // send ack
to the setup request to get the data
dfu_state = USB_DFU_STATE_DFU_MANIFEST_SYNC; // prepare
for manifestation phase
- to_return = usbdc_xfer(ep, NULL, 0, false); // send ACK
+ break;
} else if (req->wLength > sizeof(dfu_download_data)) { // there
is more data to be flash then our buffer (the USB control buffer size should be
less or equal)
// warn about programming error
dfu_status = USB_DFU_STATUS_ERR_PROG;
dfu_state = USB_DFU_STATE_DFU_ERROR;
to_return = ERR_INVALID_ARG; // stall control pipe to
indicate error
- } else { // there is data to be flash
- if (USB_SETUP_STAGE == stage) { // there will be data
to be flash
- to_return = usbdc_xfer(ep, dfu_download_data,
req->wLength, false); // send ack to the setup request to get the data
- } else { // now there is data to be flashed
- dfu_download_offset = req->wValue *
sizeof(dfu_download_data); // remember which block to flash
- dfu_download_length = req->wLength; // remember
the data size to be flash
- dfu_state = USB_DFU_STATE_DFU_DNLOAD_SYNC; //
go to sync state
- to_return = usbdc_xfer(ep, NULL, 0, false); //
ACK the data
- // we let the main application flash the data
because this can be long and would stall the USB ISR
- }
+ break;
}
+
+ /* The error cases are handled */
+ if (USB_SETUP_STAGE == stage) { // there will be data to be
flash
+ to_return = usbdc_xfer(ep, dfu_download_data,
req->wLength, false); // send ack to the setup request to get the data
+ break;
+ }
+
+ // now there is data to be flashed
+ if (USB_DFU_STATE_DFU_IDLE == dfu_state) {
+ /* first packet */
+ dfu_download_offset = 0;
+ }
+
+ /* main loop will increment offset after flashing */
+ dfu_download_length = req->wLength;
+ dfu_state = USB_DFU_STATE_DFU_DNLOAD_SYNC;
+ to_return = usbdc_xfer(ep, NULL, 0, false);
+ dfu_flash_done = false;
break;
default: // all other DFU class OUT request
dfu_state = USB_DFU_STATE_DFU_ERROR; // unknown class request
diff --git a/usb/class/dfu/device/dfudf.h b/usb/class/dfu/device/dfudf.h
index 818252d..b9db46f 100644
--- a/usb/class/dfu/device/dfudf.h
+++ b/usb/class/dfu/device/dfudf.h
@@ -48,9 +48,14 @@
*/
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;
+
+/** when flash done is true, flash status is valid */
+extern volatile bool dfu_flash_done;
+/** Result of the last blocked flashed. */
+extern volatile enum usb_dfu_status dfu_flash_status;
/** If manifestation (firmware flash and check) is complete */
extern bool dfu_manifestation_complete;
diff --git a/usb_start.c b/usb_start.c
index 78711f1..bf06596 100644
--- a/usb_start.c
+++ b/usb_start.c
@@ -164,25 +164,35 @@
// run the second part of the USB DFU state machine handling
non-USB aspects
switch (last_dfu_state) {
case USB_DFU_STATE_DFU_DNLOAD_SYNC:
- case USB_DFU_STATE_DFU_DNBUSY: // there is some data to be
flashed
+ case USB_DFU_STATE_DFU_DNBUSY:
+ if (dfu_flash_done)
+ break;
+
+ /* FIXME: check if dfu_download_offset / length are
valid */
+
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
- 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
- dfu_state = USB_DFU_STATE_DFU_ERROR;
- if (ERR_BAD_ADDRESS == rc) {
- dfu_status =
USB_DFU_STATUS_ERR_ADDRESS;
- } else if (ERR_DENIED == rc) {
- dfu_status =
USB_DFU_STATUS_ERR_WRITE;
- } else {
- dfu_status =
USB_DFU_STATUS_ERR_PROG;
- }
+ 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
+ CRITICAL_SECTION_ENTER();
+ switch (rc) {
+ case ERR_NONE:
+ dfu_flash_status = USB_DFU_STATUS_OK;
+ dfu_download_offset +=
dfu_download_length;
+ break;
+ case ERR_BAD_ADDRESS:
+ dfu_flash_status =
USB_DFU_STATUS_ERR_ADDRESS;
+ break;
+ case ERR_DENIED:
+ dfu_flash_status =
USB_DFU_STATUS_ERR_WRITE;
+ break;
+ default:
+ dfu_flash_status =
USB_DFU_STATUS_ERR_PROG;
+ break;
}
- } 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
+ dfu_flash_done = true;
+ CRITICAL_SECTION_LEAVE();
}
LED_SYSTEM_on(); // switch LED on to indicate USB DFU
can resume
break;
--
To view, visit https://gerrit.osmocom.org/c/osmo-asf4-dfu/+/42174?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-Change-Id: I345d5948455b25cd8a2efb1abfd9d0986ebd8cef
Gerrit-Change-Number: 42174
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <[email protected]>
