[U-Boot] [PATCH 2/2 v12] Introduced btrfs file-system with btrload command

[Adnan Ali <adnan.ali@codethink.co.uk>]

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