Hi
On 05/05/13 16:15, Wolfgang Denk wrote:
Dear Tom Rini,
In message 5186723E.2080503@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