On Thu, 15 Nov 2012 15:43:40 +1100 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Wed, Nov 14, 2012 at 06:59:58PM -0600, Kim Phillips wrote: > > +#define EXTRACT_BYTE(x, n) ((unsigned long long)((uint8_t *)&x)[n]) > > +#define __SWAB16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1)) > > +#define __SWAB32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << > > 16) | \ > > + (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3)) > > +#define __SWAB64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << > > 48) | \ > > + (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \ > > + (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \ > > + (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7)) > > This is not right, or at least very misleading. "swab" usually refers > to an unconditional byteswap. But the macros above are nops on a > big-endian machine. but they don't get called on a big endian system. This is the name linux uses. If you want them renamed, please suggest names - I can't read your mind. > > -static inline uint32_t fdt32_to_cpu(uint32_t x) > > -{ > > - return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | > > (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3); > > +/* > > + * determine host endianness: > > + * *__first_byte is 0x11 on big endian systems > > + * *__first_byte is 0x44 on little endian systems > > + */ > > +static const uint32_t __native = 0x11223344u; > > +static const uint8_t *__first_byte = (const uint8_t *)&__native; > > + > > +#define DEF_FDT_TO_CPU(bits) \ > > +static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \ > > +{ \ > > + if (*__first_byte == 0x11) \ > > + return (__force uint##bits##_t)x; \ > > + else \ > > + return (__force uint##bits##_t)__SWAB##bits(x); \ > > } > > -#define cpu_to_fdt32(x) fdt32_to_cpu(x) > > +DEF_FDT_TO_CPU(16) > > +DEF_FDT_TO_CPU(32) > > +DEF_FDT_TO_CPU(64) > > In fact, I really don't see why you're rewriting the byteswapper > functions as part of this patch. The existing versions aren't very > nice, but if you want to rewrite those, please do it in a separate > patch. This patchseries is about silencing sparse warnings in linux, u-boot, and libfdt. Sparse is intelligent in that if you mismatch a native type of value 0 to a bitwise restricted type, it won't call a warning. The existing short-circuiting of the byteswapper functions, i.e., defining cpu_to_fdt32(x) to fdt32_to_cpu(x) and vice versa doesn't allow for correct attribution propagation. Therefore I chose to allow sparse to see the actual conversion. If you have any other ideas on how to silence sparse in libfdt, let me know. Kim _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot