Change in osmo-asf4-dfu[master]: avoid mutli-packet USB transfer

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Kévin Redon gerrit-no-reply at lists.osmocom.org
Wed Jan 16 17:42:26 UTC 2019


Kévin Redon has uploaded this change for review. ( https://gerrit.osmocom.org/12595


Change subject: avoid mutli-packet USB transfer
......................................................................

avoid mutli-packet USB transfer

the control endpoint packet size is 64 bytes for USB full speed.
it is possible to make larger data transfer by transferring
multiple packets which can then by re-assembled.
this feature is also supported by the SAM E54 USB stack, but for
an unknown reason is the transfer size is larger than 64 bytes but
not a multiple of it, the transfer cannot complete (it is not
ACKed).

reducing the transfer size to 64 bytes removes this issue.
the data needs to be re-assembled in software to fill a flash page.

sadly a new issue has been uncovered using this method.
the size of the last packet (e.g. request bLength) is kept by the
USB stack for the next packet. Thus is the last packet is less
than 6 bytes, the status request after the download request fails.

Change-Id: Icb4c5f4bc06095f5f962152b8d8247054ef6a520
---
M usb/class/dfu/device/dfudf.c
M usb/class/dfu/device/dfudf.h
M usb/class/dfu/device/dfudf_desc.h
M usb_start.c
4 files changed, 53 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-asf4-dfu refs/changes/95/12595/1

diff --git a/usb/class/dfu/device/dfudf.c b/usb/class/dfu/device/dfudf.c
index 8232979..545f57c 100644
--- a/usb/class/dfu/device/dfudf.c
+++ b/usb/class/dfu/device/dfudf.c
@@ -42,7 +42,7 @@
 enum usb_dfu_state dfu_state = USB_DFU_STATE_DFU_IDLE;
 enum usb_dfu_status dfu_status = USB_DFU_STATUS_OK;
 
-uint8_t dfu_download_data[512];
+uint8_t dfu_download_data[64];
 uint16_t dfu_download_length = 0;
 size_t dfu_download_offset = 0;
 bool dfu_manifestation_complete = false;
@@ -244,12 +244,12 @@
 			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
+				to_return = usbdc_xfer(ep, dfu_download_data, req->wLength, req->wLength < 64); // 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
+				//to_return = usbdc_xfer(ep, NULL, 0, true); // send ACK
 				// we let the main application flash the data because this can be long and would stall the USB ISR
 			}
 		}
diff --git a/usb/class/dfu/device/dfudf.h b/usb/class/dfu/device/dfudf.h
index cee5845..a7143f9 100644
--- a/usb/class/dfu/device/dfudf.h
+++ b/usb/class/dfu/device/dfudf.h
@@ -43,10 +43,9 @@
 extern enum usb_dfu_status dfu_status;
 
 /** Downloaded data to be programmed in flash
- *
- *  512 is the flash page size of the SAM D5x/E5x
+ *  64 bytes is the control buffer size
  */
-extern uint8_t dfu_download_data[512];
+extern uint8_t dfu_download_data[64];
 /** Length of downloaded data in bytes */
 extern uint16_t dfu_download_length;
 /** Offset of where the downloaded data should be flashed in bytes */
diff --git a/usb/class/dfu/device/dfudf_desc.h b/usb/class/dfu/device/dfudf_desc.h
index cd6ba41..68880c7 100644
--- a/usb/class/dfu/device/dfudf_desc.h
+++ b/usb/class/dfu/device/dfudf_desc.h
@@ -75,9 +75,14 @@
 	                           CONF_USB_DFUD_BMATTRI, \
 	                           CONF_USB_DFUD_BMAXPOWER)
 
+/** \remark ideally the transfer size should be the same size as flash pages (512 byte for SAM E5x/D5x).
+            the control endpoint transfer size is 64 byte for full speed device.
+            thus 512 transfers must be split in a mutli-packet transfer.
+            this should be supported by the hardware, but I did not manage to have complete/successful multi-packet transfer is the transfer is larger than 64 bytes but not a multiple of 64
+ */
 #define DFUD_IFACE_DESCB USB_DFU_FUNC_DESC_BYTES(USB_DFU_ATTRIBUTES_CAN_DOWNLOAD | USB_DFU_ATTRIBUTES_WILL_DETACH, \
 	                     	                     0, /**< detaching makes only sense in run-time mode */ \
-	                     	                     512, /**< transfer size corresponds to page size for optimal flash writing */ \
+	                     	                     64, /**< transfer size corresponds to the control endpoint packet size */ \
 	                     	                     0x0110 /**< DFU specification version 1.1 used */ )
 
 #define DFUD_IFACE_DESCES \
diff --git a/usb_start.c b/usb_start.c
index ad91840..487e672 100644
--- a/usb_start.c
+++ b/usb_start.c
@@ -42,8 +42,14 @@
 /** USB DFU functional descriptor (with DFU attributes) */
 static const uint8_t usb_dfu_func_desc_bytes[] = {DFUD_IFACE_DESCB};
 static const usb_dfu_func_desc_t* usb_dfu_func_desc = (usb_dfu_func_desc_t*)&usb_dfu_func_desc_bytes;
-/** Ctrl endpoint buffer */
+/** Control endpoint buffer */
 static uint8_t ctrl_buffer[64];
+/** Buffer to store downloaded data before flashing a complete page */
+static uint8_t flash_page[512];
+/** Address offset where the flash page should be flashed */
+static uint32_t flash_offset = 0;
+/** If the page has already been flashed */
+static bool flash_programmed = true;
 
 /**
  * \brief USB DFU Init
@@ -55,6 +61,11 @@
 
 	usbdc_start(single_desc);
 	usbdc_attach();
+
+	// initialise flash page with empty flash
+	for (uint16_t i = 0; i < ARRAY_SIZE(flash_page); i++) {
+		flash_page[i] = 0xff;
+	}
 }
 
 /**
@@ -91,21 +102,38 @@
 		if (USB_DFU_STATE_DFU_DNLOAD_SYNC == dfu_state || USB_DFU_STATE_DFU_DNBUSY == dfu_state) { // there is some data to be flashed
 			gpio_set_pin_level(LED_SYSTEM, true); // 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;
+/*
+				if (dfu_download_offset >= flash_offset + sizeof(flash_page)) { // we start a new page
+					if (!flash_programmed) { // the previous page needs to be flashed
+						int32_t rc = flash_write(&FLASH_0, application_start_address + flash_offset, flash_page, sizeof(flash_page)); // write re-assembled downloaded data chunks to flash
+						if (ERR_NONE == rc) {
+							// nothing to do for now
+						} 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;
+							}
+						}
+						// initialise flash page with empty flash
+						for (uint16_t i = 0; i < ARRAY_SIZE(flash_page); i++) {
+							flash_page[i] = 0xff;
+						}
+						flash_programmed = true; // remember we programmed the flash page
 					}
+					flash_offset = (dfu_download_offset / sizeof(flash_page)) * sizeof(flash_page); // remember new page offset
 				}
-			} else { // there was no data to flash
-				// this case should not happen, but it's not a critical error
+				// copy data from download buffer to our flash page
+				for (uint16_t i = 0; i < dfu_download_length && i < ARRAY_SIZE(dfu_download_data) && i + (dfu_download_offset % ARRAY_SIZE(flash_page)) < ARRAY_SIZE(flash_page); i++) {
+					flash_page[dfu_download_offset % ARRAY_SIZE(flash_page) + i] = dfu_download_data[i];
+				}
+				flash_programmed = false; // remember there is data to be flashed
+*/
+				dfu_download_length = 0; // remember we copied the data (in the next turn the state will be idle again if there was no error before)
+			} else { // there was no data to be copied/flashed
 				dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE; // indicate flashing can continue
 			}
 			gpio_set_pin_level(LED_SYSTEM, false); // switch LED on to indicate USB DFU can resume

-- 
To view, visit https://gerrit.osmocom.org/12595
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-asf4-dfu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb4c5f4bc06095f5f962152b8d8247054ef6a520
Gerrit-Change-Number: 12595
Gerrit-PatchSet: 1
Gerrit-Owner: Kévin Redon <kredon at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190116/00e3351e/attachment.htm>


More information about the gerrit-log mailing list