[U-Boot] dfu: make data buffer size configurable

[Tom Rini]

On Wed, Jun 05, 2013 at 06:53:53AM +0200, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 04.06.2013 22:04, schrieb Tom Rini:
> > On Tue, Jun 04, 2013 at 11:22:54AM +0200, Heiko Schocher wrote:
> > 
> >> Dfu transfer uses a buffer before writing data to the
> >> raw storage device. Make the size (in bytes) of this buffer
> >> configurable.
> > 
> > NAK.
> 
> :-(
> 
> >> +		CONFIG_SYS_DFU_DATA_BUF_SIZE
> >> +		Dfu transfer uses a buffer before writing data to the
> >> +		raw storage device. Make the size (in bytes) of this buffer
> >> +		configurable.
> >> +
> >>  		CONFIG_SYS_DFU_MAX_FILE_SIZE
> >>  		When updating files rather than the raw storage device,
> >>  		we use a static buffer to copy the file into and then write
> > 
> > The point of the buffer being configurable is to allow for larger files,
> > right?  We need to fix CONFIG_SYS_DFU_MAX_FILE_SIZE so that..
> 
> In current code CONFIG_SYS_DFU_MAX_FILE_SIZE is not used in dfu_nand.c,

Nor anywhere else.  As I said in the DFU + UBI thread, there's a bug
here :)

> as if buffer is full, it is immediately flushed to nand.
> Also default value from CONFIG_SYS_DFU_MAX_FILE_SIZE is smaller (4MiB)
> as default value of CONFIG_SYS_DFU_DATA_BUF_SIZE (8MiB) ...

Right, and the commit that did it was about increasing the size of the
kernel that could be sent over.

> I used on my upcoming board port a CONFIG_SYS_DFU_DATA_BUF_SIZE from
> 1MiB and that worked perfectly, when transferring a file > 200MB.
> The default value from 8MiB sometimes caused an error on the host:
> 
> []# date;dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi;date
> Di 28. Mai 14:20:44 CEST 2013
> dfu-util 0.5
> [...]
> Copying data from PC to DFU device
> Starting download: [#############################################dfu_download: libusb_control_transfer returned -7
> Error during download
> 
> Why we have a buffersize from 8MiB for raw writes, but a max file size
> from 4MiB only?

Then we need to poke around the code here a bit more and see what's
going on, and fix things so that we can both do larger (say, 8MiB)
filesystem transfers and not have dfu-util get mad sometimes.

> >> -#define DFU_DATA_BUF_SIZE		(1024*1024*8)	/* 8 MiB */
> >> +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE
> >> +#define CONFIG_SYS_DFU_DATA_BUF_SIZE		(1024*1024*8)	/* 8 MiB */
> >> +#endif
> >>  #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE
> >>  #define CONFIG_SYS_DFU_MAX_FILE_SIZE	(4 << 20)	/* 4 MiB */
> >>  #endif
> > 
> > We use one variable for both spots.  Or is there some case I'm missing
> > where we need to buffer 8MiB at a time for raw writes?  In which case we
> > still need to make CONFIG_SYS_DFU_MAX_FILE_SIZE be used :)
> 
> I do not really know, why we have 2 defines here!

File size vs buffer size?  I'm not quite certain it was the right way to
go either.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130605/bc2e7e74/attachment.pgp>