Hi Wolfgang, On Tue, Apr 16, 2013 at 10:40 PM, Wolfgang Denk <w...@denx.de> wrote: > Dear Simon Glass, > > In message > <capnjgz2jrvpq56_epvkumh8e1l2lj0hgzncs-grn9bxfosx...@mail.gmail.com> you > wrote: >> >> >> +#ifdef USE_HOSTCC >> >> + crc = htobe32(crc); >> >> memcpy(output, &crc, sizeof(crc)); >> >> +#else >> >> + put_unaligned_be32(crc, output); >> >> +#endif >> > >> > Why is this depending on USE_HOSTCC, and not on the endianess? >> >> We always want big-endian in this case, since the bytes have to date >> been written that way by the crc32 command. > > Let me rephrase. Why do we need an #ifdef here, and why depends this > on USE_HOSTCC? > >> > And why do we need the #ifdef? Can we not always use htobe32() and >> > put_unaligned_be32() ? >> >> Well I don't think put_unaligned_be32 is available to user space, >> which is the environment that the tools are built under. It is >> available in the kernel, but that's not our environment. > > It appears put_unaligned_be32() is a widely unknown, pretty exotic > function that so far has been used in ony two source files: > > drivers/usb/gadget/f_mass_storage.c > lib/tpm.c > > The implementation (in "include/linux/unaligned/generic.h") is ugly > and pretty expensive in terms of run time and memory footprint. > > I would like to avoid it's usage all together.
It ends up producing this on ARMv7: 43e2c1d8: e1a03820 lsr r3, r0, #16 43e2c1dc: e5c40003 strb r0, [r4, #3] 43e2c1e0: e5c43001 strb r3, [r4, #1] 43e2c1e4: e1a02423 lsr r2, r3, #8 43e2c1e8: e7e73450 ubfx r3, r0, #8, #8 43e2c1ec: e5c42000 strb r2, [r4] 43e2c1f0: e5c43002 strb r3, [r4, #2] In fact with unaligned support we could optimise this. > >> I'm not happy with this solution and would be pleased to find a better >> way, but I'm not sure what it is. > > Does the htobe32() + memcpy() approach not work everywhere? But htobe32() isn't available in U-Boot. I tried using: #ifdef USE_HOSTCC crc = htobe32(crc); #else crc = cpu_to_be32(crc); #endif memcpy(output, &crc, sizeof(crc)); This is one instruction (4 bytes, 16%) smaller, but I suspect quite a lot slower due to the overhead of a very small memcpy(). 43e2c1d8: e28d1008 add r1, sp, #8 43e2c1dc: e3a02004 mov r2, #4 43e2c1e0: e6bf0f30 rev r0, r0 43e2c1e4: e5210004 str r0, [r1, #-4]! 43e2c1e8: e1a00004 mov r0, r4 43e2c1ec: eb001af7 bl 43e32dd0 <memcpy> > >> But this patch does fix a real bug which we should sort out before the >> release, one way or another. > > Agreed. Let me know what you prefer and I will update the patch. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
