Hi Wolfgang, > Dear Lukasz Majewski, > > In message <1399552067-31208-1-git-send-email-l.majew...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
