public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Jerry Van Baren <vanbaren@cideas.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/5] treewide: include libfdt_env.h before fdt.h
Date: Thu, 17 Jan 2013 13:32:45 -0500	[thread overview]
Message-ID: <50F843CD.1030309@cideas.com> (raw)
In-Reply-To: <20130117115456.71a38e3275230cdbb175bfe4@freescale.com>

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... and then things 
get really wonky in the u-boot "tools" directory due to the need to use 
the u-boot version of the *fdt*.h headers, not random stuff installed on 
the computer.

HTH,
gvb

  reply	other threads:[~2013-01-17 18:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-16 23:59 [U-Boot] [PATCH 1/5] treewide: include libfdt_env.h before fdt.h Kim Phillips
2013-01-17  0:36 ` Scott Wood
2013-01-17 17:54   ` Kim Phillips
2013-01-17 18:32     ` Jerry Van Baren [this message]
2013-01-17 23:50       ` David Gibson
2013-01-18 11:59         ` Gerald Van Baren
2013-01-21  0:17           ` David Gibson
2013-01-17 23:48     ` David Gibson
2013-01-18 12:02 ` Jerry Van Baren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50F843CD.1030309@cideas.com \
    --to=vanbaren@cideas.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox