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

[Heiko Schocher <hs@denx.de>]

Hello Tom,

Am 05.06.2013 14:43, schrieb 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:
[...]
>>>> +		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 :)

CONFIG_SYS_DFU_MAX_FILE_SIZE is used in ./drivers/dfu/dfu_mmc.c ...

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

Hmm.. the CONFIG_SYS_DFU_DATA_BUF_SIZE limits not the size of
a file that could be loaded into a partition. It specifies
only the size of one chunk, that get burned into the raw nand ...

And this should be a configurable size ...

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

Timeout in libusb_control_transfer while the target writes
the 8MiB into the nand ... ?

I try to find out something ...

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

Yeah, but why is the file size < buffer size as default?

In dfu_mmc:
If raw partition, if dfu_buf (size of CONFIG_SYS_DFU_DATA_BUF_SIZE)
full -> write it to the mmc. Same for nand.

If FAT or EXT4 partition (mmc only), write the dfu_buffer through
mmc_file_buffer() to dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE] ...
this seems buggy to me, but maybe I oversee something, I could not
try it ... and if the hole file is transfered, the dfu_file_buf
gets flushed to the partition ...

The CONFIG_SYS_DFU_MAX_FILE_SIZE is only needed as we can only
write a complete image to FAT, EXT4 (also UBI) partitions, I think.

So we have in the dfu subsystem following problems:

a) we have no common API to add image types. Currently
   all dfu_{device_type} has to program it.
b) we have no possibility to write image types (FAT, EXT4 or
   UBI) in chunks -> CONFIG_SYS_DFU_MAX_FILE_SIZE introduced
c) CONFIG_SYS_DFU_DATA_BUF_SIZE > CONFIG_SYS_DFU_MAX_FILE_SIZE
   which is in my eyes buggy ...
d) sporadic problems with CONFIG_SYS_DFU_DATA_BUF_SIZE = 8MiB
   Currently i get always an error ... try to find out why ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany