usb: dfu: introduce dfuMANIFEST state

on nand flash using ubi, after the download of the new image into the flash, the "rest" of the nand sectors get erased while flushing the medium. With current u-boot version dfu-util may show:

Starting download: [##################################################] finished! state(7) = dfuMANIFEST, status(0) = No error condition is present unable to read DFU status

as dfu_get_status is not answered while erasing sectors, if erasing needs some time.

So do the following changes to prevent this:

- introduce dfuManifest state
According to dfu specification [1] section 7:
"the device enters the dfuMANIFEST-SYNC state and awaits the solicitation
of the status report by the host. Upon receipt of the anticipated DFU_GETSTATUS, the device enters the dfuMANIFEST state, where it completes its reprogramming operations."

- when stepping into dfuManifest state, sending a PollTimeout DFU_MANIFEST_POLL_TIMEOUT in ms, to the host, so the host (dfu-util) waits the PollTimeout before sending a get_status again.

Patch 0002-usb-dfu-introduce-dfuMANIFEST-state.patch shows following checkpatch errors, but I think they are OK, as the hole file uses CamelCase for the statenames as this in sync with [1]

CHECK: Avoid CamelCase: <DFU_STATE_dfuMANIFEST>
#103: FILE: drivers/usb/gadget/f_dfu.c:190:
+ f_dfu->dfu_state = DFU_STATE_dfuMANIFEST;

CHECK: Avoid CamelCase: <DFU_STATE_dfuIDLE>
#156: FILE: drivers/usb/gadget/f_dfu.c:491:
+ f_dfu->dfu_state = DFU_STATE_dfuIDLE;

CHECK: Avoid CamelCase: <DFU_STATE_dfuERROR>
#166: FILE: drivers/usb/gadget/f_dfu.c:501:
+ f_dfu->dfu_state = DFU_STATE_dfuERROR;

total: 0 errors, 0 warnings, 3 checks, 133 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE USLEEP_RANGE

[1]:
http://www.usb.org/developers/devclass_docs/usbdfu10.pdf

- changes for v3:
- remove "{" after "if" as Marek Vasut suggested
- changes for v2:
- add Pantelis Antoniou to Cc
- add comment from Marek Vasut
- move comment and "if" back to dfu_write()
- add comment from Lukasz Majewski:
- remove unneccessary comment "end?"
- add Readme entry for DFU_DEFAULT_POLL_TIMEOUT
- remove unneeded flag setpolltimeout

- ToDo:
- look if it is possible to get rid of the constant DFU_MANIFEST_POLL_TIMEOUT for all dfu entities. Maybe as Lukasz suggested:
Add a callback - called e.g. (*poll_timeout) to the struct [mmc|nand]_internal_data. Then some helper functions would be defined at dfu_[mmc|nand].c and used at f_dfu.c so we can decide on a per medium/partition base, how long a PollTimeout would be for a dfu entity ... This can be done in a seperate
step.

Heiko Schocher (3):
usb, dfu: extract flush code into seperate function
usb: dfu: introduce dfuMANIFEST state
am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards

 README                                 | 10 +++++++
 drivers/dfu/dfu.c                      | 42 +++++++++++++++--------------
 drivers/usb/gadget/f_dfu.c             | 48 +++++++++++++++++++++++++++++-----
 include/configs/siemens-am33x-common.h |  1 +
 include/dfu.h                          |  4 +++
 5 files changed, 79 insertions(+), 26 deletions(-)

1.8.3.1

move the flushing code into an extra function dfu_flush(), so it can be used from other code.

---
- changes for v3:
- remove "{" after "if" as Marek Vasut suggested
- changes for v2
- add comment from Marek Vasut
- move comment and "if" back to dfu_write()
- add comment from Lukasz Majewski:
- remove unneccessary comment
- add Pantelis to Cc

---
 drivers/dfu/dfu.c | 42 ++++++++++++++++++++++++------------------
 include/dfu.h     |  1 +
 2 files changed, 25 insertions(+), 18 deletions(-)

index 56e69fd..5232986 100644
@@ -127,6 +127,28 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu)
 	return ret;
 }
 
+int dfu_flush(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
+{
+	int ret = 0;
+
+	if (dfu->flush_medium)
+		ret = dfu->flush_medium(dfu);
+
+	printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
+
+	/* clear everything */
+	dfu_free_buf();
+	dfu->crc = 0;
+	dfu->offset = 0;
+	dfu->i_blk_seq_num = 0;
+	dfu->i_buf_start = dfu_buf;
+	dfu->i_buf_end = dfu_buf;
+	dfu->i_buf = dfu->i_buf_start;
+	dfu->inited = 0;
+
+	return ret;
+}
+
 int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 {
 	int ret = 0;

@@ -198,24 +220,8 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 	}
 
 	/* end? */
-	if (size == 0) {
-		/* Now try and flush to the medium if needed. */
-		if (dfu->flush_medium)
-			ret = dfu->flush_medium(dfu);
-		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
-
-		/* clear everything */
-		dfu_free_buf();
-		dfu->crc = 0;
-		dfu->offset = 0;
-		dfu->i_blk_seq_num = 0;
-		dfu->i_buf_start = dfu_buf;
-		dfu->i_buf_end = dfu_buf;
-		dfu->i_buf = dfu->i_buf_start;
-
-		dfu->inited = 0;
-
-	}
+	if (size == 0)
+		ret = dfu_flush(dfu, buf, size, blk_seq_num);
 
 	return ret = 0 ? size : ret;
 }

index f973426..272a245 100644
@@ -138,6 +138,7 @@ unsigned long dfu_get_buf_size(void);
 
 int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
 int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
+int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
 /* Device specific */
 #ifdef CONFIG_DFU_MMC
 extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s);

1.8.3.1

on nand flash using ubi, after the download of the new image into the flash, the "rest" of the nand sectors get erased while flushing the medium. With current u-boot version dfu-util may show:

Starting download: [##################################################] finished! state(7) = dfuMANIFEST, status(0) = No error condition is present unable to read DFU status

as get_status is not answered while erasing sectors, if erasing needs some time.

So do the following changes to prevent this:

- introduce dfuManifest state
According to dfu specification
( http://www.usb.org/developers/devclass_docs/usbdfu10.pdf ) section 7:
"the device enters the dfuMANIFEST-SYNC state and awaits the solicitation
of the status report by the host. Upon receipt of the anticipated DFU_GETSTATUS, the device enters the dfuMANIFEST state, where it completes its reprogramming operations."

- when stepping into dfuManifest state, sending a PollTimeout DFU_MANIFEST_POLL_TIMEOUT in ms, to the host, so the host (dfu-util) waits the PollTimeout before sending a get_status again.

---
- changes for v3:
Fixed patch, as patch 1/3 changed
- changes for v2:
- reworked as patch 01 of this series changed as Marek Vasut commented

    -> setting req->length = 0 before calling dfu_flush() not necessary
  - add comment from Lukasz Majewski:
    - add Pantelis to Cc
    - add Readme entry for DFU_DEFAULT_POLL_TIMEOUT
    - remove unneeded flag setpolltimeout
---
 README                     | 10 ++++++++++
 drivers/dfu/dfu.c          |  4 ----
 drivers/usb/gadget/f_dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++------
 include/dfu.h              |  3 +++
 4 files changed, 55 insertions(+), 10 deletions(-)

index 216f0c7..7cb7c4f 100644
@@ -1525,6 +1525,16 @@ The following options need to be configured:
 		this to the maximum filesize (in bytes) for the buffer.
 		Default is 4 MiB if undefined.
 
+		DFU_DEFAULT_POLL_TIMEOUT
+		Poll timeout [ms], is the timeout a device can send to the
+		host. The host must wait for this timeout before sending
+		a subsequent DFU_GET_STATUS request to the device.
+
+		DFU_MANIFEST_POLL_TIMEOUT
+		Poll timeout [ms], which the device sends to the host when
+		entering dfuMANIFEST state. Host waits this timeout, before
+		sending again an USB request to the device.
+
 - Journaling Flash filesystem support:
 		CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF, CONFIG_JFFS2_NAND_SIZE,
 		CONFIG_JFFS2_NAND_DEV

index 5232986..8a09aaf 100644
@@ -219,10 +219,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 			ret = tret;
 	}
 
-	/* end? */
-	if (size == 0)
-		ret = dfu_flush(dfu, buf, size, blk_seq_num);
-
 	return ret = 0 ? size : ret;
 }
 

index 272a245..6c71ecb 100644
@@ -80,6 +80,9 @@ static inline unsigned int get_mmc_blk_size(int dev)
 #ifndef DFU_DEFAULT_POLL_TIMEOUT
 #define DFU_DEFAULT_POLL_TIMEOUT 0
 #endif
+#ifndef DFU_MANIFEST_POLL_TIMEOUT
+#define DFU_MANIFEST_POLL_TIMEOUT	DFU_DEFAULT_POLL_TIMEOUT
+#endif
 
 struct dfu_entity {
 	char			name[DFU_NAME_SIZE];

1.8.3.1

as the siemens boards use dfu for updating a nand ubi partition add DFU_MANIFEST_POLL_TIMEOUT to them, so dfu host waits after complete transfer of the new image for DFU_MANIFEST_POLL_TIMEOUT ms before sending again an usb request. So the board have enough time to erase rest of the nand sectors.

---
- no changes for v3
- changes for v2:
- add Reviewed-by from Lukasz Majewski
- add Pantelis Antoniou to Cc

---
 include/configs/siemens-am33x-common.h | 1 +
 1 file changed, 1 insertion(+)

index 98b6e72..721c4e6 100644
@@ -265,6 +265,7 @@
 #define CONFIG_DFU_NAND
 #define CONFIG_CMD_DFU
 #define CONFIG_SYS_DFU_DATA_BUF_SIZE	(1 << 20)
+#define DFU_MANIFEST_POLL_TIMEOUT	25000
 
 #endif /* CONFIG_SPL_BUILD */
 

1.8.3.1