Hello Tom,
Am 06.06.2013 17:55, schrieb Tom Rini:
> On Wed, Jun 05, 2013 at 04:04:46PM +0200, Heiko Schocher wrote:
>
> [snip]
>>>> 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 ...
>
> Ah yes, right.
>
>>>> 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 ...
>
> Or a saner, smaller size? I think we've got a few things being done in
> a confusing and/or incorrect manner, see below.
Yes, smaller size is maybe the way to go ... see below
>>>> 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 ...
>
> Well, it ought to be fine, but we're pushing some boundaries here, and
> I don't know if we have a good reason for it. We aren't changing the
> size of the chunks being passed along.
My suspicion that the problem is a timeout was the right idea!
I tried following patch in the current dfu-util sources:
(git clone git://gitorious.org/dfu-util/dfu-util.git
current head: f66634406e9397e67c34033c3c0c26dde486528c)
diff --git a/src/main.c b/src/main.c
index 2aea134..12f6f1d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -290,7 +290,8 @@ int main(int argc, char **argv)
exit(0);
}
- dfu_init(5000);
+ printf("Using 20 sec timeout\n");
+ dfu_init(20000);
num_devs = count_dfu_devices(ctx, dif);
if (num_devs == 0) {
and I see no longer the above error! So I see two solutions
for my problem:
- make DFU_DATA_BUF_SIZE in U-Boot smaller or configurable
- make the timeout in dfu-util bigger or configurable
>>>>>> -#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?
>
> A bug, I'm pretty sure. The change that made buffer > file was with the
> comment to allow for bigger files. I think it might not have been
> completely tested :)
Maybe. I don't know.
>> 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.
>
> Right. Since we want to buffer up some amount of data, flush, repeat
> until done.
Yep.
>> 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 need here is that since we can't append to files, transfer the whole
> file, then write. We will not be told the filesize ahead of time, so we
> have to transfer up to the buffer and if we would exceed, error out at
> that point, otherwise continue. Now I'm pretty sure, but not 100% sure
> that there's a reason we can't use dfu_buf in both places. That needs
> re-checking.
Ok.
>> 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.
>
> It'll be needed for any file write, rather than block write. The
> question is, can it ever be valid for MAX_FILE_SIZE to be smaller than
> BUF_SIZE ? Maybe.
I would say no ...
>> 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.
>
> We also have no common image types.
Then we should introduce it ... oh, maybe we have them:
drivers/dfu/dfu_mmc.c in dfu_write_medium_mmc()i
switch(dfu->layout)
DFU_RAW_ADDR:
DFU_FS_FAT:
DFU_FS_EXT4:
?
>> b) we have no possibility to write image types (FAT, EXT4 or
>> UBI) in chunks -> CONFIG_SYS_DFU_MAX_FILE_SIZE introduced
>
> Correct.
>
>> c) CONFIG_SYS_DFU_DATA_BUF_SIZE > CONFIG_SYS_DFU_MAX_FILE_SIZE
>> which is in my eyes buggy ...
>
> Or at least very odd, given the current defaults for both.
>
>> d) sporadic problems with CONFIG_SYS_DFU_DATA_BUF_SIZE = 8MiB
>> Currently i get always an error ... try to find out why ...
>
> Maybe we don't need to go so large for the buffer.
Yep, with 1 or 2 MiB I see no problems with current dfu-util.
> If you've got a beaglebone (and I know there's one in denx :)) or am335x
> gp evm, you can test filesystem writes on SD cards with DFU.
Ah! Ok.
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
