Hi Heiko, > Hello Lukasz, > > Am 16.05.2014 10:58, schrieb Lukasz Majewski: > > Hi Wolfgang, Tom, > > > >> Hi Wolfgang, > >> > >>> Dear Lukasz, > >>> > >>> In message<20140515090904.32f1d13d@amdc2363> you wrote: > >>>> > >>>>>> 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. > >>>>> > >>>>> Well, looking at mainline usage of DFU, Lukasz is speaking for > >>>>> about half of the users / implementors. Since Denx is working > >>>>> with the other half, can you shed some light on actual use vs > >>>>> theoretical possibilities? > >>>> > >>>> I don't want to urge anybody on making any conclusion here :-), > >>>> but I would be very grateful if we could come up with an > >>>> agreement. > >>>> > >>>> As I've stated previously, my opinion is similar to the one > >>>> presented by Tom in this message. > >>>> > >>>> For me it would be best to not calculate any checksum on default > >>>> and only enable it when needed. > >>> > >>> I asked Heiko to run some actual tests on the boards where he has > >>> to maintain DFU for. For a 288 MiB image he did not measure any > >>> difference - with your patch applied, both with and without CRC > >>> enabled, we would get the same (slow) 1:54 minutes download time. > >> > >> As I've spoken with Heiko, am33xx uses NAND memory. I deal with > >> eMMC. Moreover, the size of "chunks" are different - 1 MiB and 32 > >> MiB. > >> > >> I must double check for the rationale for chunk size of 32 MiB at > >> Trats/Trats2 boards. I suspect, that eMMC write performance depend > >> on that. > >> > >> I will perform some performance tests with 1 MiB chunk size and > >> share the result. > > > > Unfortunately the 32 MiB is fixed for our platform. since lthor > > uses it by default. > > > >> > >>> > >>> This reinforces my speculation that you are actually addressing > >>> the wrong problem. Instead of adding new code and environment > >>> variables and making the system even more complex, we should just > >>> leave everything as is, > >> > >> During working on this patch I've replaced the crc32() method with > >> the call to hash_method(), which IMHO is welcome. > >> > >> I also don't personally like the crc32, hence I like the choice > >> which this patch gives me to use other algorithm (for which I've > >> got HW support on my platform - e.g. MD5). > >> > >>> and you should try to find out why the CRC > >>> calculation is so low for you. Checking if caches are enabled is > >>> probably among the things that should be done first. > >> > >> L1 is enabled. L2 has been disabled on purpose (power consumption > >> reduction). > > > > Regarding L2 - our platform requires SMC calls to enable and manage > > L2 cache. In my opinion support for this in u-boot is an overkill. > > > > > > Can we conclude this whole discussion? The main point was if we > > should keep calculating and displaying crc32 as default for DFU > > transfers. > > > > I'm for the option to NOT display and calculate it by default (PATCH > > v3). > > I talked with the siemens board customer, they also do not check/use > the displayed value from U-Boot ... > > So, for me it is OK to not display this value ...
Applied this patch (with no default CRC32 calculation - v3) to u-boot-dfu tree. > but we should add > to DFU such a check ... or? > > bye, > Heiko -- 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