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. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
