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

[Lukasz Majewski]

Hi Heiko,

> 
> Hello Lukasz,
> 
> Am 16.05.2014 10:58, schrieb Lukasz Majewski:
> > Hi Wolfgang, Tom,
> >
> >> Hi Wolfgang,
> >>
> >>> Dear Lukasz,
> >>>
> >>> In message<20140515090904.32f1d13d at 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 ...

Ok, so we now have a consensus here.

> but we should add
> to DFU such a check ... or?

Yes, we should add a way to send the CRC32/MD5/SHA1 to our boards.

Unfortunately I'm out of office now, so it is hard for me to develop
something.

However, the rough idea would be to send the CRC32 (or any other
checksum) to a separate alt setting.
> 
> bye,
> Heiko

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140520/0e3a457a/attachment.pgp>