Hi
On 05/05/13 16:15, Wolfgang Denk wrote:
Dear Tom Rini,
In message <5186723e.2080...@ti.com> you wrote:
These changes should be factored out into a separate commit. In any
case, the code should be compiled in only when needed, i. e. when
btrfs support is selected. Otherwise you just bloat the code size
and build time for all systems without need.
We should be able to make lib/crc32_c.o depend on btrfs being set,
yes. But non-command stuff should be getting garbage collected out in
most cases at last (Need to poke Albert about the patch for ARM).
Yes, it may be garbage collected, but only after compilation - which
means we add to the build time for about 1000 boards which do not use
this.
Hm... do we really need yet another crc32 implementation?
We talked about this before I believe and the answer is yes :(
Yes, I remember this discussion. But look at the code. It raises a
number of questions:
- Why do we have to calculate the crc32c_table[] at runtime? Our
regular CRC code uses a pre-calculated table; we should do the same
here.
This is part of the port. But pre-calculated table can be manually
created.
- Compare the code for crc32c_cal() in the patch with the definition
of DO_CRC(x) in "lib/crc32.c" - to me, it appears to be the same for
little endian code (it is redundant?), but different for big endian
systems - which raises the question if this code has ever been
tested on a BE machine?
My code uses lib/crc32.c and i have only tested it on
mx53loco manchine.
- The code claims to be derived from "Linux kernel crypto/crc32c.c";
but I cannot find such code in that file.
I think yes but part of part from syslinux. I have also added
SHA1 of commit so don't know.
- The implementation of crc32c_cal() suffers from a few other problems
(like not triggering the watchdog, which will cause problems on
systems that use one).
I think that is true as its not using main line crc32 code.
I think we should re-check this. My feeling is that we may eventually
end up with a different CRC table, but without need for new code.
Best regards,
Wolfgang Denk
Thanks
Adnan Ali
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
