[U-Boot] [PATCH 2/2 v12] Introduced btrfs file-system with btrload command
Adnan Ali
adnan.ali at codethink.co.uk
Thu May 9 16:55:03 CEST 2013
Hi
On 05/05/13 16:15, Wolfgang Denk wrote:
> Dear Tom Rini,
>
> In message <5186723E.2080503 at 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
More information about the U-Boot
mailing list