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

[Lukasz Majewski]

Hi Heiko,

> 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>

Thanks for testing!

> 
> 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 ...

Very nice to hear. I'm looking forward for your patches for review.

BTW. I do plan to setup tbot for my private Beagle Bone Black in the
weekend.

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



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group