[U-Boot] [PATCH] dfu: fix some issues with reads/uploads

[Stephen Warren]

On 05/22/2014 04:20 AM, Lukasz Majewski wrote:
> Hi Stephen,
> 
>> From: Stephen Warren <swarren at nvidia.com>
>>
>> DFU read support appears to rely upon dfu->read_medium() updating the
>> passed-by-reference len parameter to indicate the remaining size
>> available for reading.
>>
>> dfu_read_medium_mmc() never does this, and the implementation of
>> dfu_read_medium_nand() will only work if called just once; it
>> hard-codes the value to the total size of the NAND device
>> irrespective of read offset.
>>
>> I believe that overloading dfu->read_medium() is confusing. As such,
>> this patch introduces a new function dfu->get_medium_size() which can
>> be used to explicitly find out the medium size, and nothing else.
>> dfu_read() is modified to use this function to set the initial value
>> for dfu->r_left, rather than attempting to use the side-effects of
>> dfu->read_medium() for this purpose.
>>
>> Due to this change, dfu_read() must initially set dfu->b_left to 0,
>> since no data has been read.
>>
>> dfu_read_buffer_fill() must also be modified not to adjust dfu->r_left
>> when simply copying data from dfu->i_buf_start to the upload request
>> buffer. r_left represents the amount of data left to be read from HW.
>> That value is not affected by the memcpy(), but only by calls to
>> dfu->read_medium().
>>
>> After this change, I can read from either a 4MB or 1.5MB chunk of a
>> 4MB eMMC boot partion with CONFIG_SYS_DFU_DATA_BUF_SIZE==1MB. Without
>> this change, attempting to do that would result in DFU read returning
>> no data at all due to r_left never being set.
...
> I've tested it with trats2. It introduces regression with my tests
> setup.
> 
> I think that it is the highest time to share my test setup for DFU.
> Proper patches would be prepared ASAP.

Ah, your test scripts imply you're testing using files on a filesystem,
whereas I've only tested with raw MMC partitions:

setenv dfu_alt_info boot0 raw 0 0x2000 mmcpart 1\;boot1 raw 0 0x2000
mmcpart 2; dfu 0 mmc 0

(I test on alt info "boot1")

Can you re-run your tests on a raw partition and validate they work OK
for you? That would at least prove the concept of the patches. I'm not
convinced tests on raw partitions would work with or without my patches.

I can see why my patches made files rather than raw partitions fail; I
only wrote dfu_get_medium_size_mmc() to support raw partitions, so it
needs extra code to work for files. Solving the same problem for files
rather than raw partitions might be painful; I guess I'll see!

As an aside, mmc_file_op() doesn't seem to support files larger than the
DFU buffer, size no offset information is ever passed to the
fatload/fatwrite commands. Is that intentional?

I'm also puzzled why the file IO code is part of dfu_mmc.c; commands
like fat_load (or the internal U-Boot filesystem APIs) support more than
just MMC. Wouldn't it be better to have the following separate
"backends" to DFU:

* MMC (raw IO only)
* NAND (raw IO only)
* RAM (raw IO only)
* Filesystem (anything that fs/fat/ext* load/write can support)

That way, DFU could be applied to e.g. USB mass storage, IDE, SATA, ...
devices too.