<p>Kévin Redon has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/12595">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">avoid mutli-packet USB transfer<br><br>the control endpoint packet size is 64 bytes for USB full speed.<br>it is possible to make larger data transfer by transferring<br>multiple packets which can then by re-assembled.<br>this feature is also supported by the SAM E54 USB stack, but for<br>an unknown reason is the transfer size is larger than 64 bytes but<br>not a multiple of it, the transfer cannot complete (it is not<br>ACKed).<br><br>reducing the transfer size to 64 bytes removes this issue.<br>the data needs to be re-assembled in software to fill a flash page.<br><br>sadly a new issue has been uncovered using this method.<br>the size of the last packet (e.g. request bLength) is kept by the<br>USB stack for the next packet. Thus is the last packet is less<br>than 6 bytes, the status request after the download request fails.<br><br>Change-Id: Icb4c5f4bc06095f5f962152b8d8247054ef6a520<br>---<br>M usb/class/dfu/device/dfudf.c<br>M usb/class/dfu/device/dfudf.h<br>M usb/class/dfu/device/dfudf_desc.h<br>M usb_start.c<br>4 files changed, 53 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-asf4-dfu refs/changes/95/12595/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/usb/class/dfu/device/dfudf.c b/usb/class/dfu/device/dfudf.c</span><br><span>index 8232979..545f57c 100644</span><br><span>--- a/usb/class/dfu/device/dfudf.c</span><br><span>+++ b/usb/class/dfu/device/dfudf.c</span><br><span>@@ -42,7 +42,7 @@</span><br><span> enum usb_dfu_state dfu_state = USB_DFU_STATE_DFU_IDLE;</span><br><span> enum usb_dfu_status dfu_status = USB_DFU_STATUS_OK;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-uint8_t dfu_download_data[512];</span><br><span style="color: hsl(120, 100%, 40%);">+uint8_t dfu_download_data[64];</span><br><span> uint16_t dfu_download_length = 0;</span><br><span> size_t dfu_download_offset = 0;</span><br><span> bool dfu_manifestation_complete = false;</span><br><span>@@ -244,12 +244,12 @@</span><br><span> to_return = ERR_INVALID_ARG; // stall control pipe to indicate error</span><br><span> } else { // there is data to be flash</span><br><span> if (USB_SETUP_STAGE == stage) { // there will be data to be flash</span><br><span style="color: hsl(0, 100%, 40%);">- to_return = usbdc_xfer(ep, dfu_download_data, req->wLength, false); // send ack to the setup request to get the data</span><br><span style="color: hsl(120, 100%, 40%);">+ to_return = usbdc_xfer(ep, dfu_download_data, req->wLength, req->wLength < 64); // send ack to the setup request to get the data</span><br><span> } else { // now there is data to be flashed</span><br><span> dfu_download_offset = req->wValue * sizeof(dfu_download_data); // remember which block to flash</span><br><span> dfu_download_length = req->wLength; // remember the data size to be flash</span><br><span> dfu_state = USB_DFU_STATE_DFU_DNLOAD_SYNC; // go to sync state</span><br><span style="color: hsl(0, 100%, 40%);">- to_return = usbdc_xfer(ep, NULL, 0, false); // ACK the data</span><br><span style="color: hsl(120, 100%, 40%);">+ //to_return = usbdc_xfer(ep, NULL, 0, true); // send ACK</span><br><span> // we let the main application flash the data because this can be long and would stall the USB ISR</span><br><span> }</span><br><span> }</span><br><span>diff --git a/usb/class/dfu/device/dfudf.h b/usb/class/dfu/device/dfudf.h</span><br><span>index cee5845..a7143f9 100644</span><br><span>--- a/usb/class/dfu/device/dfudf.h</span><br><span>+++ b/usb/class/dfu/device/dfudf.h</span><br><span>@@ -43,10 +43,9 @@</span><br><span> extern enum usb_dfu_status dfu_status;</span><br><span> </span><br><span> /** Downloaded data to be programmed in flash</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * 512 is the flash page size of the SAM D5x/E5x</span><br><span style="color: hsl(120, 100%, 40%);">+ * 64 bytes is the control buffer size</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">-extern uint8_t dfu_download_data[512];</span><br><span style="color: hsl(120, 100%, 40%);">+extern uint8_t dfu_download_data[64];</span><br><span> /** Length of downloaded data in bytes */</span><br><span> extern uint16_t dfu_download_length;</span><br><span> /** Offset of where the downloaded data should be flashed in bytes */</span><br><span>diff --git a/usb/class/dfu/device/dfudf_desc.h b/usb/class/dfu/device/dfudf_desc.h</span><br><span>index cd6ba41..68880c7 100644</span><br><span>--- a/usb/class/dfu/device/dfudf_desc.h</span><br><span>+++ b/usb/class/dfu/device/dfudf_desc.h</span><br><span>@@ -75,9 +75,14 @@</span><br><span> CONF_USB_DFUD_BMATTRI, \</span><br><span> CONF_USB_DFUD_BMAXPOWER)</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/** \remark ideally the transfer size should be the same size as flash pages (512 byte for SAM E5x/D5x).</span><br><span style="color: hsl(120, 100%, 40%);">+ the control endpoint transfer size is 64 byte for full speed device.</span><br><span style="color: hsl(120, 100%, 40%);">+ thus 512 transfers must be split in a mutli-packet transfer.</span><br><span style="color: hsl(120, 100%, 40%);">+ 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</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span> #define DFUD_IFACE_DESCB USB_DFU_FUNC_DESC_BYTES(USB_DFU_ATTRIBUTES_CAN_DOWNLOAD | USB_DFU_ATTRIBUTES_WILL_DETACH, \</span><br><span> 0, /**< detaching makes only sense in run-time mode */ \</span><br><span style="color: hsl(0, 100%, 40%);">- 512, /**< transfer size corresponds to page size for optimal flash writing */ \</span><br><span style="color: hsl(120, 100%, 40%);">+ 64, /**< transfer size corresponds to the control endpoint packet size */ \</span><br><span> 0x0110 /**< DFU specification version 1.1 used */ )</span><br><span> </span><br><span> #define DFUD_IFACE_DESCES \</span><br><span>diff --git a/usb_start.c b/usb_start.c</span><br><span>index ad91840..487e672 100644</span><br><span>--- a/usb_start.c</span><br><span>+++ b/usb_start.c</span><br><span>@@ -42,8 +42,14 @@</span><br><span> /** USB DFU functional descriptor (with DFU attributes) */</span><br><span> static const uint8_t usb_dfu_func_desc_bytes[] = {DFUD_IFACE_DESCB};</span><br><span> static const usb_dfu_func_desc_t* usb_dfu_func_desc = (usb_dfu_func_desc_t*)&usb_dfu_func_desc_bytes;</span><br><span style="color: hsl(0, 100%, 40%);">-/** Ctrl endpoint buffer */</span><br><span style="color: hsl(120, 100%, 40%);">+/** Control endpoint buffer */</span><br><span> static uint8_t ctrl_buffer[64];</span><br><span style="color: hsl(120, 100%, 40%);">+/** Buffer to store downloaded data before flashing a complete page */</span><br><span style="color: hsl(120, 100%, 40%);">+static uint8_t flash_page[512];</span><br><span style="color: hsl(120, 100%, 40%);">+/** Address offset where the flash page should be flashed */</span><br><span style="color: hsl(120, 100%, 40%);">+static uint32_t flash_offset = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+/** If the page has already been flashed */</span><br><span style="color: hsl(120, 100%, 40%);">+static bool flash_programmed = true;</span><br><span> </span><br><span> /**</span><br><span> * \brief USB DFU Init</span><br><span>@@ -55,6 +61,11 @@</span><br><span> </span><br><span> usbdc_start(single_desc);</span><br><span> usbdc_attach();</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ // initialise flash page with empty flash</span><br><span style="color: hsl(120, 100%, 40%);">+ for (uint16_t i = 0; i < ARRAY_SIZE(flash_page); i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ flash_page[i] = 0xff;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> /**</span><br><span>@@ -91,21 +102,38 @@</span><br><span> if (USB_DFU_STATE_DFU_DNLOAD_SYNC == dfu_state || USB_DFU_STATE_DFU_DNBUSY == dfu_state) { // there is some data to be flashed</span><br><span> gpio_set_pin_level(LED_SYSTEM, true); // switch LED off to indicate we are flashing</span><br><span> if (dfu_download_length > 0) { // there is some data to be flashed</span><br><span style="color: hsl(0, 100%, 40%);">- 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</span><br><span style="color: hsl(0, 100%, 40%);">- if (ERR_NONE == rc) {</span><br><span style="color: hsl(0, 100%, 40%);">- dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE; // indicate flashing this block has been completed</span><br><span style="color: hsl(0, 100%, 40%);">- } else { // there has been a programming error</span><br><span style="color: hsl(0, 100%, 40%);">- dfu_state = USB_DFU_STATE_DFU_ERROR;</span><br><span style="color: hsl(0, 100%, 40%);">- if (ERR_BAD_ADDRESS == rc) {</span><br><span style="color: hsl(0, 100%, 40%);">- dfu_status = USB_DFU_STATUS_ERR_ADDRESS;</span><br><span style="color: hsl(0, 100%, 40%);">- } else if (ERR_DENIED == rc) {</span><br><span style="color: hsl(0, 100%, 40%);">- dfu_status = USB_DFU_STATUS_ERR_WRITE;</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- dfu_status = USB_DFU_STATUS_ERR_PROG;</span><br><span style="color: hsl(120, 100%, 40%);">+/*</span><br><span style="color: hsl(120, 100%, 40%);">+ if (dfu_download_offset >= flash_offset + sizeof(flash_page)) { // we start a new page</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!flash_programmed) { // the previous page needs to be flashed</span><br><span style="color: hsl(120, 100%, 40%);">+ 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</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ERR_NONE == rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ // nothing to do for now</span><br><span style="color: hsl(120, 100%, 40%);">+ } else { // there has been a programming error</span><br><span style="color: hsl(120, 100%, 40%);">+ dfu_state = USB_DFU_STATE_DFU_ERROR;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ERR_BAD_ADDRESS == rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ dfu_status = USB_DFU_STATUS_ERR_ADDRESS;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (ERR_DENIED == rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ dfu_status = USB_DFU_STATUS_ERR_WRITE;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ dfu_status = USB_DFU_STATUS_ERR_PROG;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ // initialise flash page with empty flash</span><br><span style="color: hsl(120, 100%, 40%);">+ for (uint16_t i = 0; i < ARRAY_SIZE(flash_page); i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ flash_page[i] = 0xff;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ flash_programmed = true; // remember we programmed the flash page</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+ flash_offset = (dfu_download_offset / sizeof(flash_page)) * sizeof(flash_page); // remember new page offset</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- } else { // there was no data to flash</span><br><span style="color: hsl(0, 100%, 40%);">- // this case should not happen, but it's not a critical error</span><br><span style="color: hsl(120, 100%, 40%);">+ // copy data from download buffer to our flash page</span><br><span style="color: hsl(120, 100%, 40%);">+ 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++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ flash_page[dfu_download_offset % ARRAY_SIZE(flash_page) + i] = dfu_download_data[i];</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ flash_programmed = false; // remember there is data to be flashed</span><br><span style="color: hsl(120, 100%, 40%);">+*/</span><br><span style="color: hsl(120, 100%, 40%);">+ 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)</span><br><span style="color: hsl(120, 100%, 40%);">+ } else { // there was no data to be copied/flashed</span><br><span> dfu_state = USB_DFU_STATE_DFU_DNLOAD_IDLE; // indicate flashing can continue</span><br><span> }</span><br><span> gpio_set_pin_level(LED_SYSTEM, false); // switch LED on to indicate USB DFU can resume</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12595">change 12595</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/12595"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-asf4-dfu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Icb4c5f4bc06095f5f962152b8d8247054ef6a520 </div>
<div style="display:none"> Gerrit-Change-Number: 12595 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Kévin Redon <kredon@sysmocom.de> </div>