Dear Lukasz,
In message 20140509085203.31133238@amdc2363 you wrote:
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.
This is unsufficient. You should always verify the image on the target after the download has completed.
Participants have agreed, that we shall optionally enable crc32 (or other algorithm) calculation.
If this is the default now, it should remain the default.
- The current crc32 implementation is painfully slow (although I have
only L1 enabled on my target).
This is an unrelated problem then, which should excluded from this discussion here.
- 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.
You can still detect if the download was corrupted, report a proper error and initiate a re-download. This would at least give you a chance to react to corrupted data. Just closing the eyes and hoping no errors will ever happen has always been a bad strategy.
- 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.
Agreed. This was not my point.
What I complained about is the change in behaviour. I asked to make the existing behaviour the default, so unaware users will not be affected. Only if you intentionally want some other behaviour you can then enable this by setting the env variable.
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.
I disagree, if you pay for the speed by reduced reliability, and if you don't even inform the user about this new behaviour.
Also, I feel it might be worth to investigate why the checksumming is slow on your system.
As I've stated previously the crc32 in the current dfu implementation is only informative.
It is pretty useful information, isn't it?
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.
Ok, add this to the many deficientcies of DFU :-(
Even more important issue is that it would work only for small files (like uImage).
Why so? Can we not calculate CRC even when the transfer is broken down into several chunks?
Best regards,
Wolfgang Denk