| Message ID | 20211013170053.1.I1158bd6d095c996f2dbd4b0aa9327e4eee202331@changeid |
|---|---|
| State | Accepted |
| Commit | 86b6a38863bebb70a65a53f93a1ffafc4a472169 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | dfu: handle short frame result of UPLOAD in state_dfu_idle | expand |
On Wed, Oct 13, 2021 at 05:01:37PM +0200, Patrick Delaunay wrote: > In DFU v1.1 specification [1] the DFU_UPLOAD (Short Frame) > is handled only in dfuUPLOADIDLE state: > > - Figure A.1 Interface state transition diagram > > - the state description in chapter A.2 > > A.2.3 State 2 dfuIDLE > on Receipt of the DFU_UPLOAD request,and bitCanUpload = 1 > the Next State is dfuUPLOADIDLE > > A.2.10 State 9 dfuUPLOAD-IDLE > When the length of the data transferred by the device in response > to a DFU_UPLOAD request is less than wLength. (Short frame) > the Next State is dfuIDLE > > In current code, when an UPLOAD is completely performed after the first > request (for example with wLength=200 and data read = 9), the DFU state > stay at dfuUPLOADIDLE until receiving a DFU_UPLOAD or a DFU_ABORT request > even it is unnecessary as the previous DFU_UPLOAD request already reached > the EOF. > > This patch proposes to finish the DFU uploading (don't go to dfuUPLOADIDLE) > and completes the control-read operation (go to DFU_STATE_dfuIDLE) when > the first UPLOAD response has a short frame as an end of file (EOF) > indicator even if it is not explicitly allowed in the DFU specification > but this seems logical. > > [1] https://www.usb.org/sites/default/files/DFU_1.1.pdf > > Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com> Applied to u-boot/master, thanks!
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 4bedc7d3a1..e9340ff5cb 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -336,6 +336,8 @@ static int state_dfu_idle(struct f_dfu *f_dfu, f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE; f_dfu->blk_seq_num = 0; value = handle_upload(req, len); + if (value >= 0 && value < len) + f_dfu->dfu_state = DFU_STATE_dfuIDLE; break; case USB_REQ_DFU_ABORT: /* no zlp? */
In DFU v1.1 specification [1] the DFU_UPLOAD (Short Frame) is handled only in dfuUPLOADIDLE state: - Figure A.1 Interface state transition diagram - the state description in chapter A.2 A.2.3 State 2 dfuIDLE on Receipt of the DFU_UPLOAD request,and bitCanUpload = 1 the Next State is dfuUPLOADIDLE A.2.10 State 9 dfuUPLOAD-IDLE When the length of the data transferred by the device in response to a DFU_UPLOAD request is less than wLength. (Short frame) the Next State is dfuIDLE In current code, when an UPLOAD is completely performed after the first request (for example with wLength=200 and data read = 9), the DFU state stay at dfuUPLOADIDLE until receiving a DFU_UPLOAD or a DFU_ABORT request even it is unnecessary as the previous DFU_UPLOAD request already reached the EOF. This patch proposes to finish the DFU uploading (don't go to dfuUPLOADIDLE) and completes the control-read operation (go to DFU_STATE_dfuIDLE) when the first UPLOAD response has a short frame as an end of file (EOF) indicator even if it is not explicitly allowed in the DFU specification but this seems logical. [1] https://www.usb.org/sites/default/files/DFU_1.1.pdf Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com> --- Hi Lukasz, This case is correctly handle in dfu-util, see dfu_load.c dfuload_do_upload() { .... while (1) { ... rc = dfu_upload(dif->dev_handle, dif->interface, xfer_size, transaction++, buf); .... dfu_file_write_crc(fd, 0, buf, rc); total_bytes += rc; if (total_bytes < 0) errx(EX_SOFTWARE, "\nReceived too many bytes (wraparound)"); if (rc < xfer_size) { /* last block, return */ ret = 0; break; } } } In the upload loop the code doesn't make difference for the first request and the next one: the last block is detected as soon as the received data < requested size. So it is safe to do the same in U-Boot's DFU stack, skip the dfuUPLOADIDLE state when the upload operation is finished after the first request. This patch avoid to ABORT the unfinished UPLOAD request before the next command. This patch was previously sent as RFC = [RFC] dfu: handle short frame result of UPLOAD in state_dfu_idle http://patchwork.ozlabs.org/project/uboot/list/?series=248838&state=* Patrick. drivers/usb/gadget/f_dfu.c | 2 ++ 1 file changed, 2 insertions(+)