Hi Kyungmin, On Thu, Aug 9, 2018 at 4:02 PM Bin Meng <bmeng...@gmail.com> wrote: > > Hi Kyungmin, > > On Mon, Aug 6, 2018 at 1:56 PM, Bin Meng <bmeng...@gmail.com> wrote: > > On Wed, Aug 1, 2018 at 9:50 PM, Heinrich Schuchardt <xypron.g...@gmx.de> > > wrote: > >> > >> > >> On 08/01/2018 02:13 PM, Bin Meng wrote: > >>> Hi, > >>> > >>> Currently it seems that we have two CRC32 implementation in U-Boot. > >>> Two headers files are provided. > >>> > >>> 1. include/linux/crc32.h > >>> The implementation is drivers/mtd/ubi/crc32.c. > >>> Codes that use this implementation include: > >>> > >>> drivers/mtd/ubi/* > >>> drivers/mtd/ubispl/* > >>> fs/ubifs/* > >>> > >>> 2. include/u-boot/crc.h > >>> The implementation is lib/crc32.c > >>> Codes that use this implementation include: > >>> > >>> fs/btrfs/hash.c > >>> tools/* > >>> common/hash.c > >>> common/image.c > >>> common/image-fit.c > >>> lib/efi_loader/efi_boottime.c > >>> > >>> It looks that include/linux/crc32.h was originally imported from Linux > >>> kernel's include/linux/crc32.h, but the implementation in Linux > >>> kernel's lib/crc32.c was not imported to U-Boot's lib/crc32.c but to > >>> drivers/mtd/ubi/crc32.c. Why? > >>> > >>> Somehow U-Boot lib/crc32.c uses another different implementation from > >>> zlib. > >>> > >>> This is a mess. For example if I include both headers in one C file, > >>> it won't compile. > >>> > >>> Can we clean this up? > >> > >> Thanks for pointing this out. > >> > >> The drivers/mtd/ubi/crc32.c is based on an elder version of Linux. > >> > >> When looking at the function signatures I am not happy with > >> include/u-boot/crc.h > >> uint32_t crc32 (uint32_t, const unsigned char *, uint) > >> The last parameter should be size_t. Otherwise the CRC may be wrong on > >> 64bit systems. > >> > >> The two crc32 implementations do not have the same result on a > >> low-endian system: > >> > >> crc32_le(0, 'U-Boot', 6) = a289ac17 > >> crc32(0, 'U-Boot', 6) = 134b0db4. > >> > >> According to the comments in in include/linux/crc32.h the result of > >> crc32_le is in bit reversed order. > >> > >> Conflicting definitions could be avoided by removing #define crc32() in > >> include/linux/crc32.h and adjusting the ubi code accordingly. > >> > > > > I would like to see one CRC32 implementation to support all use cases > > in U-Boot. Allowing two different implementation just confuses people. > > Since UBI seems to be the only user of the Linux CRC32 implementation > but the header files are a mess, I would expect some comments or plan > from you.
Any comments? Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
