On Thu, Jan 17, 2013 at 01:32:45PM -0500, Jerry Van Baren wrote:
Hi Scott, Kim, David,
On 01/17/2013 12:54 PM, Kim Phillips wrote:
On Wed, 16 Jan 2013 18:36:03 -0600 Scott Wood scottwood@freescale.com wrote:
On 01/16/2013 05:59:04 PM, Kim Phillips wrote:
and, if including libfdt.h which includes libfdt_env.h in the correct order, don't include fdt.h before libfdt.h.
this is needed to get the fdt type definitions set from the project environment before fdt.h uses them.
Signed-off-by: Kim Phillips kim.phillips@freescale.com Cc: Jerry Van Baren gvb.uboot@gmail.com
Maybe fdt.h should include libfdt_env.h?
Or just always use libfdt.h as the public header.
Was just following along the same lines as the dtc commits 38ad79d3 "dtc/tests: don't include fdt.h prior to libfdt.h" and 20b866a7 "dtc/fdtdump: include libfdt_env.h prior to fdt.h", acked by David G. I don't know why some only include fdt.h.
devicetree-discuss/David: is there a prescribed way to go here? Change all fdt.h includers to just always include libfdt.h instead of libfdt_env.h prior to fdt.h?
I started applying Kim's "sparse" patches to the u-boot source and ran into this issue pretty hard.
In u-boot, there is an added complexity that the "tools" (host-based u-boot support tools) support flattened device trees, but explicitly include the u-boot version of libfdt declarations so they don't fall out of sync if the host has a non-compatible libfdt version. Having them out of sync would be a *horrible* situation to sort out - everything would build OK but nothing would work right, probably with no useful diagnostic information. This originated in 2008, so life may be better nowadays. Or maybe not.
I would be in favor of explicitly including all the *fdt* headers in the sources. Alternately, Scott's suggestion of just including libfdt.h as the public header seems good, but I'm pretty sure it will mess me up with the explicit #including in the host-based "tools" build, leaving us with nasty work-arounds or a risk of hard to identify incompatible host vs. u-boot fdt versions.
Who Includes Who
fdt.h - no includes
fdt_support.h - (u-boot only file) 29 #include <fdt.h>
libfdt.h 54 #include <libfdt_env.h> 55 #include <fdt.h>
libfdt_env.h
- u-boot version is minimal, uses pre-existing macros for byte swapping
- dtc version implements byte swapping, includes: 4 #include <stddef.h> 5 #include <stdint.h> 6 #include <string.h>
libfdt_env.h is where Kim typedef'ed fdt16_t, fdt32_t, fdt64_t
I suspect the original intent was to have <libfdt.h> be the file that people #included. For whatever reason, most includes are (picking on fdt_ro.c arbitrarily): 51 #include "libfdt_env.h" 53 #include <fdt.h> 54 #include <libfdt.h> Since libfdt.h #includes fdt.h and libfdt_env.h, lines 51 and 53 (above) are redundant. It sorts out OK in dtc because libfdt_env.h includes stdint.h and defines fdt*_t, but it messes me up in u-boot where (currently) libfdt_env.h does *not* include stdint.h...
Ok, so, the uboot libfdt_env.h should be fixed to define uintXX_t and fdtXX_t (either by including stdint or my other means). The purpose of libfdt_env.h is to define the things that libfdt requires, and those types are (now) such a requirement.