[U-Boot] [PATCH 1/2] dfu: usb: f_dfu: Set deferred call for dfu_flush() function

[Heiko Schocher]

Hello Lukasz,

Am 28.01.2016 um 17:46 schrieb Lukasz Majewski:
> This patch fixes situation when one would like to write large file into
> medium with the file system (fat, ext4, etc).
> This change sets file size limitation to the DFU internal buffer size.
>
> Since u-boot is not supporting interrupts and seek on file systems, it
> becomes challenging to store large file appropriately.
>
> To reproduce this error - create large file (around 26 MiB) and sent it
> to the target board.
>
> Lets examine the flow of USB transactions:
>
> 0. DFU uses EP0 with 64B MPS [Max Packet Size]
>
> 1. Send file - OUT (PC->target) - dat_26MiB.img is sent with 4096 B transactions
>
> 2. Get status - OUT (PC->target) - wait for DFU_STATE_dfuDNLOAD_IDLE (0x05) sent
> 				   from target board - IN transaction
> 				   (target->PC)
>
> 3. The whole file content is sent to target - OUT (PC->target) with ZLP [Zero
> 					      Length Packet]
>
> Now the interesting part starts:
>
> 4. OUT (PC->target) Setup transaction (request to share DFU state)
>
> 5. IN (target->PC) - reply the current DFU state
> 	- In the UDC driver the req->completion (with dfu_flush) is called
> 	  after successful IN transfer.
> 	- The dfu_flush() (called from req->completion callback) saves the
> 	  whole file at once (u-boot doesn't support seek on fs).
> 	  Such operation takes considerable time. When the file
> 	  is large - e.g. 26MiB - this time may be more than 5 seconds.
>
> 6. OUT (PC->target) - ZLP, is send in the same time when dfu_flush()
>   writes data to eMMC memory.
>   The dfu-util application has hard coded timeout on USB transaction
>   completion set to 5 seconds (it uses libusb calls).
>
> When the file to store is large (e.g. 26 MiB) the time needed to write it
> may excess the dfu-util timeout and following error message will be displayed:
> "unable to read DFU status" on the HOST PC console.
>
> This change is supposed to leverage DFU's part responsible for storing files
> on file systems. Other DFU operations - i.e. raw/partition write to NAND and
> eMMC should work as before.
>
> The only functional change is the error reporting. When dfu_flush() fails
> the u-boot prompt will exit with error information and dfu-util application
> exits afterwards as well.
>
> Test HW:
> - Odroid XU3 (Exynos5433) - test with large file
> - Trats (Exynos4210) - test for regression - eMMC, raw,
>
> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> Reported-by: Alex Gdalevich <agdalevich at axion-biosystems.com>
> ---
>   common/cmd_dfu.c           | 20 ++++++++++++++++++++
>   drivers/usb/gadget/f_dfu.c | 11 +++--------
>   include/dfu.h              | 25 +++++++++++++++++++++++++
>   3 files changed, 48 insertions(+), 8 deletions(-)

Tested on the dxr2 board with an etamin module containing a 4GiB DDP nand
with a download size from 400MiB ... worked.
(This test is not yet in my nightly builds ...)

Tested-by: Heiko Schocher <hs at denx.de>

bye,
Heiko
BTW:
I used not the "dfu_nand" module, instead I am just developing
a "dfu_mtd" for accessing mtd partitions. So mtd partitions with
underlying mtd concatenated nand devices for example can be programmed
with dfu ... seems first version works  now ...

>
> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> index 6d95ce9..d8aae26 100644
> --- a/common/cmd_dfu.c
> +++ b/common/cmd_dfu.c
> @@ -79,6 +79,26 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		if (ctrlc())
>   			goto exit;
>
> +		if (dfu_get_defer_flush()) {
> +			/*
> +			 * Call to usb_gadget_handle_interrupts() is necessary
> +			 * to act on ZLP OUT transaction from HOST PC after
> +			 * transmitting the whole file.
> +			 *
> +			 * If this ZLP OUT packet is NAK'ed, the HOST libusb
> +			 * function fails after timeout (by default it is set to
> +			 * 5 seconds). In such situation the dfu-util program
> +			 * exits with error message.
> +			 */
> +			usb_gadget_handle_interrupts(controller_index);
> +			ret = dfu_flush(dfu_get_defer_flush(), NULL, 0, 0);
> +			dfu_set_defer_flush(NULL);
> +			if (ret) {
> +				error("Deferred dfu_flush() failed!");
> +				goto exit;
> +			}
> +		}
> +
>   		WATCHDOG_RESET();
>   		usb_gadget_handle_interrupts(controller_index);
>   	}
> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> index 77a1567..7d88008 100644
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -44,6 +44,8 @@ struct f_dfu {
>   	unsigned int                    poll_timeout;
>   };
>
> +struct dfu_entity *dfu_defer_flush;
> +
>   typedef int (*dfu_state_fn) (struct f_dfu *,
>   			     const struct usb_ctrlrequest *,
>   			     struct usb_gadget *,
> @@ -167,14 +169,7 @@ static void dnload_request_complete(struct usb_ep *ep, struct usb_request *req)
>   static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req)
>   {
>   	struct f_dfu *f_dfu = req->context;
> -	int ret;
> -
> -	ret = dfu_flush(dfu_get_entity(f_dfu->altsetting), req->buf,
> -			req->length, f_dfu->blk_seq_num);
> -	if (ret) {
> -		f_dfu->dfu_status = DFU_STATUS_errUNKNOWN;
> -		f_dfu->dfu_state = DFU_STATE_dfuERROR;
> -	}
> +	dfu_set_defer_flush(dfu_get_entity(f_dfu->altsetting));
>   }
>
>   static inline int dfu_get_manifest_timeout(struct dfu_entity *dfu)
> diff --git a/include/dfu.h b/include/dfu.h
> index 6118dc2..f39d3f1 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -163,6 +163,31 @@ 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);
>
> +/*
> + * dfu_defer_flush - pointer to store dfu_entity for deferred flashing.
> + *		     It should be NULL when not used.
> + */
> +extern struct dfu_entity *dfu_defer_flush;
> +/**
> + * dfu_get_defer_flush - get current value of dfu_defer_flush pointer
> + *
> + * @return - value of the dfu_defer_flush pointer
> + */
> +static inline struct dfu_entity *dfu_get_defer_flush(void)
> +{
> +	return dfu_defer_flush;
> +}
> +
> +/**
> + * dfu_set_defer_flush - set the dfu_defer_flush pointer
> + *
> + * @param dfu - pointer to the dfu_entity, which should be written
> + */
> +static inline void dfu_set_defer_flush(struct dfu_entity *dfu)
> +{
> +	dfu_defer_flush = dfu;
> +}
> +
>   /**
>    * dfu_write_from_mem_addr - write data from memory to DFU managed medium
>    *
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany