On Tue, Oct 30, 2012 at 04:57:54PM -0500, Kim Phillips wrote:

Projects such as linux and u-boot run sparse on libfdt.  libfdt
contains the notion of endianness via usage of endian conversion
functions such as fdt32_to_cpu.  As such, in order to pass endian
checks, libfdt has to annotate its fdt variables as big endian.
This patch does that ifdef __CHECKER__ (a symbol sparse defines),
for two new fdt types: fdt32_t and fdt64_t, and subsequently
silences warnings emitted by sparse when parsing libfdt.

Signed-off-by: Kim Phillips <redacted>
---
note: wasn't sure whether to introduce the new fdt32 types, or
just have libfdt use __be32 directly.

I prefer having the fdt32 types, to match the cpu_to_fdt32() and so
forth functions we already use.

So I like this in principle, but a couple of nits.

First, I'd really like to see an accompanying patch that adds targets
to the dtc makefiles to run sparse over the sources.  I couldn't
really test this, because I couldn't figure out quite what options I
needed to invoke sparse with to get it to work properly.

And I'd also like to see the default libfdt_env.h updated to supply
the necessary sparse stuff - including the necessary __force casts in
its byteswap functions.

[snip]

So, I'd prefer not to use __CHECKER__ directly here.  I'd rather we
defined a new specific symbol, that libfdt_env.h can set based on
__CHECKER__ if it wants.  Let's say _FDT_SPARSE The reason is
because..

+typedef __be32 fdt32_t;
+typedef __be64 fdt64_t;

..just running sparse does *not* immediately give you __be32 and
__be64 types - those have to be defined in terms of
__attribute__((bitwise)) and whatnot.  Your uboot env probably does
that already, but the default libfdt_env.h certainly doesn't.
Effectively _FDT_SPARSE is sayint two things, first that we're
compiling under sparse, but also that we have suitably defined endian
types in the environment.

Actually, given that libfdt_env.h is already required to provide the
cpu_to_fdt32() and so forth macros, I think it's slightly neater to
just require it to directly supply the fdt32_t etc. types when it
defines _FDT_SPARSE as well, rather than defining them in terms of
__beXX here then in terms of the attributes in the environment.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson