diff mbox series

[RFC] dfu: handle short frame result of UPLOAD in state_dfu_idle

Message ID 20210615094718.RFC.1.I1158bd6d095c996f2dbd4b0aa9327e4eee202331@changeid
State New
Delegated to: Lukasz Majewski
Headers show
Series [RFC] dfu: handle short frame result of UPLOAD in state_dfu_idle | expand

Commit Message

Patrick Delaunay June 15, 2021, 7:47 a.m. UTC
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 RFC 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>
---
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.

Patrick.


 drivers/usb/gadget/f_dfu.c | 2 ++
 1 file changed, 2 insertions(+)
diff mbox series

Patch

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? */