[U-Boot] [PATCH v3] dfu: Introduction of the "dfu_hash_algo" env variable for checksum method setting

[Lukasz Majewski]

Hi Wolfgang,

> Dear Lukasz Majewski,
> 
> In message <1399552067-31208-1-git-send-email-l.majewski at samsung.com>
> you wrote:
> > Up till now the CRC32 of received data was calculated
> > unconditionally. The standard crc32 implementation causes long
> > delay when large images were uploaded.
> > 
> > The "dfu_hash_algo" environment variable gives the opportunity to
> > enable on demand (when e.g. debugging) the hash (crc32) calculation.
> > It can be done without need to recompile the u-boot binary and
> > reuses the generic hash framework.
> > 
> > By default the crc32 is NOT calculated anymore.
> 
> I consider this a VARY BAD idea, as it causes a significant decrease
> of reliability and robustness of the systems.  Please do not do this.

I do understand that reliability is very important, but please
consider following arguments:

1. Now calculated crc32 is only used for debugging. 

For automated tests I use MD5 and compare this value before sending
data to target via DFU and after I read it. This testing is done purely
on HOST machine.

Please refer to the discussion which we had at previous version of this
patch:

http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/183311/focus=183455

Participants have agreed, that we shall optionally enable crc32 (or
other algorithm) calculation. 

2. The current crc32 implementation is painfully slow (although I have
only L1 enabled on my target). 

3. With large files (like rootfs images) we store data (to medium) with
32 MiB chunks, which means that when we calculate complete crc32 the
image is already written to its final destination.

Of course we could store the rootfs to some "free" space on the eMMC,
then calculate crc32 and store it to the final position. This however
would take considerable time and require wrapping our binaries to
special headers (as described below). 

4. This patch also allows some flexibility: by setting the env variable
we can decide which algorithm to use (crc32, sha1, etc). It is
appealing since we use the hash_* code anyway.

> 
> In any case, if you introduce this, the behaviour should be
> documented, and the default setting should be such as to keep the
> previous behaviour, i. e. CRC checking should remain on by default.
> then people who are willing to trade reliability for a little speed

I would not touch the code if the speedup wouldn't be so significant.
Reducing flashing time of 400 MiB file from 65 s to 25 s is worth its
effort.

> can still switch it off, but the unawarerest of the users will not
> suffer.

As I've stated previously the crc32 in the current dfu implementation
is only informative.

To take the full advantage of it, we would need to modify the dfu-util
to wrap the sent file to some kind of header or locally write some
script to do that. However, this is not specified by the standard and
would be u-boot's extension of the DFU. 

Even more important issue is that it would work only for small files
(like uImage).

> 
> 
> Best regards,
> 
> Wolfgang Denk
> 

-- 
Best regards,

Lukasz Majewski

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