Re: [U-Boot] [PATCH 1/5] treewide: include libfdt_env.h before fdt.h

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3834 bytes --]

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-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 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-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> >>>Cc: Jerry Van Baren <gvb.uboot-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>
> >>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.

-- 
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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss