From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adnan Ali <adnan.ali@codethink.co.uk>
Date: Thu, 09 May 2013 15:55:03 +0100
Subject: [U-Boot] [PATCH 2/2 v12] Introduced btrfs file-system with
 btrload command
In-Reply-To: <20130505151509.4D9E5380E6D@gemini.denx.de>
References: <1367660512-10489-1-git-send-email-adnan9901@yahoo.com>
	<1367660512-10489-3-git-send-email-adnan9901@yahoo.com>
	<20130505072819.5DDBA380E6C@gemini.denx.de>
	<5186723E.2080503@ti.com>
	<20130505151509.4D9E5380E6D@gemini.denx.de>
Message-ID: <518BB8C7.3090704@codethink.co.uk>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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