treewide: include libfdt_env.h before fdt.h
Subsystems (3)
PATCH 1/5 treewide: include libfdt_env.h before fdt.h
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.
--- arch/arm/lib/bootm.c | 1 - arch/powerpc/lib/bootm.c | 1 - arch/x86/include/asm/arch-coreboot/sysinfo.h | 1 + common/cmd_bootm.c | 1 - common/cmd_fdt.c | 1 - common/fdt_support.c | 1 - common/image.c | 1 - include/fdt_support.h | 1 + include/image.h | 1 - tools/fdt_host.h | 1 - tools/imls/imls.c | 1 - 11 files changed, 2 insertions(+), 9 deletions(-)
arch/arm/lib/bootm.c +0 -1
@@ -30,7 +30,6 @@#include <image.h> #include <u-boot/zlib.h> #include <asm/byteorder.h> -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h> #include <asm/bootm.h>
arch/powerpc/lib/bootm.c +0 -1
@@ -36,7 +36,6 @@#include <asm/mp.h> #if defined(CONFIG_OF_LIBFDT ) -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h>
arch/x86/include/asm/arch-coreboot/sysinfo.h +1 -0
@@ -32,6 +32,7 @@#include <common.h> #include <compiler.h> +#include <libfdt_env.h> #include <fdt.h> #include <asm/arch/tables.h>
common/cmd_bootm.c +0 -1
@@ -47,7 +47,6 @@#endif #if defined(CONFIG_OF_LIBFDT ) -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h> #endif
common/cmd_fdt.c +0 -1
@@ -29,7 +29,6 @@#include <linux/ctype.h> #include <linux/types.h> #include <asm/global_data.h> -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h>
common/fdt_support.c +0 -1
@@ -28,7 +28,6 @@#include <linux/ctype.h> #include <linux/types.h> #include <asm/global_data.h> -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h> #include <exports.h>
common/image.c +0 -1
@@ -47,7 +47,6 @@#include <image.h> #if defined(CONFIG_FIT ) || defined(CONFIG_OF_LIBFDT ) -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h> #endif
include/fdt_support.h +1 -0
@@ -26,6 +26,7 @@#ifdef CONFIG_OF_LIBFDT +#include <libfdt_env.h> #include <fdt.h> u32 fdt_getprop_u32_default(const void *fdt, const char *path,
include/image.h +0 -1
@@ -52,7 +52,6 @@#endif /* USE_HOSTCC */ #if defined(CONFIG_FIT ) -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h> #define CONFIG_MD5 /* FIT images need MD5 support */
tools/fdt_host.h +0 -1
@@ -21,7 +21,6 @@#define __FDT_HOST_H__/* Make sure to include u-boot version of libfdt include files */ -#include "../include/fdt.h" #include "../include/libfdt.h" #include "../include/fdt_support.h"
tools/imls/imls.c +0 -1
@@ -38,7 +38,6 @@#endif #include <sha1.h> -#include <fdt.h> #include <libfdt.h> #include <fdt_support.h> #include <image.h>
Signature · 1 line
1.8.1.1
PATCH 1/5 treewide: include libfdt_env.h before fdt.h
Quoted 8 lines · Kim Phillips
and, if including
libfdt.hwhich includeslibfdt_env.hin the correct order, don't includefdt.hbeforelibfdt.h.this is needed to get the fdt type definitions set from the project environment before
fdt.huses them.Signed-off-by Kim Phillips <[email protected]>Cc Jerry Van Baren <[email protected]>
Maybe fdt.h should include libfdt_env.h?
Or just always use libfdt.h as the public header.
-Scott
Quoted 7 lines · On Wed, 16 Jan 2013 18:36:03 -0600 Scott Wood <[email protected]> wrote:
Quoted 6 lines · Kim Phillips
and, if including
libfdt.hwhich includeslibfdt_env.hin the correct order, don't includefdt.hbeforelibfdt.h.this is needed to get the fdt type definitions set from the project environment before
fdt.huses them.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Kim Phillips <[email protected]>Cc Jerry Van Baren <[email protected]>Maybe
fdt.hshould includelibfdt_env.h?Or just always use
libfdt.has 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?
Kim
A message that is not in the archive (sent off-list or not yet received): 20130117115456.71a38e3275230cdbb175bfe4-KZfg59tc24xl57MIdRCFDg@public.gmane.org
Hi Scott, Kim, David,
Quoted 17 lines · Kim Phillips
Quoted 7 lines · On Wed, 16 Jan 2013 18:36:03 -0600 Scott Wood…
Quoted 6 lines · Kim Phillips
and, if including
libfdt.hwhich includeslibfdt_env.hin the correct order, don't includefdt.hbeforelibfdt.h.this is needed to get the fdt type definitions set from the project environment before
fdt.huses them.Quoted 6 lines
Quoted 2 lines
Signed-off-by Kim Phillips <[email protected]>Cc Jerry Van Baren <[email protected]>Maybe
fdt.hshould includelibfdt_env.h?Or just always use
libfdt.has the public header.
Quoted 8 lines
Was just following along the same lines as the dtc commits
38ad79d3"dtc/tests: don't includefdt.hprior tolibfdt.h" and20b866a7"dtc/fdtdump: includelibfdt_env.hprior tofdt.h", acked by David G. I don't know why some only includefdt.h.devicetree-discuss/David: is there a prescribed way to go here? Change all
fdt.hincluders to just always includelibfdt.hinstead oflibfdt_env.hprior tofdt.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
A message that is not in the archive (sent off-list or not yet received): 50F843CD.1030309-He//[email protected]
Quoted 68 lines · Jerry Van Baren
Hi Scott, Kim, David,
Quoted 17 lines · Kim Phillips
Quoted 7 lines · On Wed, 16 Jan 2013 18:36:03 -0600 Scott Wood…
Quoted 6 lines · Kim Phillips
and, if including
libfdt.hwhich includeslibfdt_env.hin the correct order, don't includefdt.hbeforelibfdt.h.this is needed to get the fdt type definitions set from the project environment before
fdt.huses them.Quoted 6 lines
Quoted 2 lines
Signed-off-by Kim Phillips <[email protected]>Cc Jerry Van Baren <[email protected]>Maybe
fdt.hshould includelibfdt_env.h?Or just always use
libfdt.has the public header.Quoted 8 lines
Was just following along the same lines as the dtc commits
38ad79d3"dtc/tests: don't includefdt.hprior tolibfdt.h" and20b866a7"dtc/fdtdump: includelibfdt_env.hprior tofdt.h", acked by David G. I don't know why some only includefdt.h.devicetree-discuss/David: is there a prescribed way to go here? Change all
fdt.hincluders to just always includelibfdt.hinstead oflibfdt_env.hprior tofdt.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.has 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.his where Kim typedef'edfdt16_t,fdt32_t,fdt64_t
Quoted 10 lines
I suspect the original intent was to have <
libfdt.h> be the file that people #included. For whatever reason, most includes are (picking onfdt_ro.carbitrarily):
51 #include "libfdt_env.h"
53 #include <fdt.h>
54 #include <libfdt.h>
Sincelibfdt.h#includesfdt.handlibfdt_env.h, lines 51 and 53 (above) are redundant. It sorts out OK in dtc becauselibfdt_env.hincludesstdint.hand defines fdt*_t, but it messes me up in u-boot where (currently)libfdt_env.hdoes *not* includestdint.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.
Signature · 4 lines
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
_______________________________________________
devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
A message that is not in the archive (sent off-list or not yet received): 20130117235048.GF26321-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org
Quoted 2 lines · David Gibson
Quoted 1 line · Jerry Van Baren
Hi Scott, Kim, David,
[snip]
Quoted 11 lines
Quoted 10 lines
libfdt_env.his where Kim typedef'edfdt16_t,fdt32_t,fdt64_tI suspect the original intent was to have <
libfdt.h> be the file that people #included. For whatever reason, most includes are (picking onfdt_ro.carbitrarily): 51 #include "libfdt_env.h" 53 #include <fdt.h> 54 #include <libfdt.h> Sincelibfdt.h#includesfdt.handlibfdt_env.h, lines 51 and 53 (above) are redundant. It sorts out OK in dtc becauselibfdt_env.hincludesstdint.hand defines fdt*_t, but it messes me up in u-boot where (currently)libfdt_env.hdoes *not* includestdint.h...
Quoted 4 lines
Ok, so, the uboot
libfdt_env.hshould be fixed to defineuintXX_tandfdtXX_t(either by including stdint or my other means). The purpose oflibfdt_env.his to define the things that libfdt requires, and those types are (now) such a requirement.
I like the move, but have not done it (yet). I made a trial patch (see below) that uses libfdt.h as the interface and cleans out the (now redundant) other *fdt*.h includes. If this is in the right direction, I'll move the fdtXX_t typedefs and formally submit it.
The test suite has one failure, but it fails with or without my changes.
$ make check | grep FAIL fdtget-runtest.sh 77 121 66 111 97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 108 121 78 97 109 101 0 label01.dts.fdtget.test.dtb / compatible: FAIL Results differ from expected * FAIL: 1
********** TEST SUMMARY
* Total testcases: 1443
* PASS: 1442
* FAIL: 1
* Bad configuration: 0
* Strange test result: 0
**********
gvb
Quoted 1 line
From
1fb0193670e012072d4a9e5ac480e3201aaf30fdMon Sep 17 00:00:00 2001
From: Gerald Van Baren <[email protected]>
Date: Thu, 17 Jan 2013 21:14:01 -0500
Subject: [PATCH] Use the libfdt.h include as the interface definition
libfdt.h is the libfdt interface definition. It includes fdt.h and libfdt_env.h, so there is no reason to include those in the general code.
--- dtc.h | 3 +-- fdtdump.c | 3 +-- libfdt/fdt.c | 3 --- libfdt/fdt_empty_tree.c | 3 --- libfdt/fdt_ro.c | 3 --- libfdt/fdt_rw.c | 3 --- libfdt/fdt_strerror.c | 3 --- libfdt/fdt_sw.c | 3 --- libfdt/fdt_wip.c | 3 --- libfdt/libfdt_internal.h | 1 - 10 files changed, 2 insertions(+), 26 deletions(-)
dtc.h +1 -2
@@ -32,8 +32,7 @@#include <errno.h> #include <unistd.h> -#include <libfdt_env.h> -#include <fdt.h> +#include <libfdt.h> #include "util.h"
fdtdump.c +1 -2
@@ -8,8 +8,7 @@#include <string.h> #include <ctype.h> -#include <libfdt_env.h> -#include <fdt.h> +#include <libfdt.h> #include "util.h"
libfdt/fdt.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" int fdt_check_header (const void *fdt)
libfdt/fdt_empty_tree.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" int fdt_create_empty_tree (void *buf,int bufsize)
libfdt/fdt_ro.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" static int _fdt_nodename_eq(const void *fdt, int offset,
libfdt/fdt_rw.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" static int _fdt_blocks_misordered(const void *fdt,
libfdt/fdt_strerror.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" struct fdt_errtabent {
libfdt/fdt_sw.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" static int _fdt_sw_check_header (void *fdt)
libfdt/fdt_wip.c +0 -3
@@ -48,11 +48,8 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "libfdt_env.h" -#include <fdt.h> #include <libfdt.h> -#include "libfdt_internal.h" int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
libfdt/libfdt_internal.h +0 -1
@@ -50,7 +50,6 @@* OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include <fdt.h> #define FDT_ALIGN (x, a) (((x) + (a) - 1) & ~((a) - 1))#define FDT_TAGALIGN (x) (FDT_ALIGN((x), FDT_TAGSIZE))
Signature · 1 line
1.7.9.5
A message that is not in the archive (sent off-list or not yet received): 50F93932.60700-He//[email protected]
Quoted 36 lines · Gerald Van Baren
Quoted 2 lines · David Gibson
Quoted 1 line · Jerry Van Baren
Hi Scott, Kim, David,
[snip]
Quoted 11 lines
Quoted 10 lines
libfdt_env.his where Kim typedef'edfdt16_t,fdt32_t,fdt64_tI suspect the original intent was to have <
libfdt.h> be the file that people #included. For whatever reason, most includes are (picking onfdt_ro.carbitrarily): 51 #include "libfdt_env.h" 53 #include <fdt.h> 54 #include <libfdt.h> Sincelibfdt.h#includesfdt.handlibfdt_env.h, lines 51 and 53 (above) are redundant. It sorts out OK in dtc becauselibfdt_env.hincludesstdint.hand defines fdt*_t, but it messes me up in u-boot where (currently)libfdt_env.hdoes *not* includestdint.h...Quoted 4 lines
Ok, so, the uboot
libfdt_env.hshould be fixed to defineuintXX_tandfdtXX_t(either by including stdint or my other means). The purpose oflibfdt_env.his to define the things that libfdt requires, and those types are (now) such a requirement.I like the move, but have not done it (yet). I made a trial patch (see below) that uses
libfdt.has the interface and cleans out the (now redundant) other *fdt*.h includes. If this is in the right direction, I'll move thefdtXX_ttypedefs and formally submit it.The test suite has one failure, but it fails with or without my changes.
$ make check | grep FAIL fdtget-runtest.sh 77 121 66 111 97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 108 121 78 97 109 101 0 label01.dts.fdtget.test.dtb / compatible: FAIL Results differ from expected * FAIL: 1
Quoted 7 lines
********** TEST SUMMARY
* Total testcases: 1443
* PASS: 1442
* FAIL: 1
* Bad configuration: 0
* Strange test result: 0
**********
Yeah, Jon and I are aware of that recently introduced bug. I've had other projects to attend to and haven't had a chance to fix it yet. It's basically a testsuite bug though, so it should be harmless.
Signature · 4 lines
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
_______________________________________________
devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
Quoted 17 lines · Kim Phillips
Quoted 7 lines · On Wed, 16 Jan 2013 18:36:03 -0600 Scott Wood…
Quoted 6 lines · Kim Phillips
and, if including
libfdt.hwhich includeslibfdt_env.hin the correct order, don't includefdt.hbeforelibfdt.h.this is needed to get the fdt type definitions set from the project environment before
fdt.huses them.Quoted 6 lines
Quoted 2 lines
Signed-off-by Kim Phillips <[email protected]>Cc Jerry Van Baren <[email protected]>Maybe
fdt.hshould includelibfdt_env.h?Or just always use
libfdt.has the public header.
Quoted 8 lines
Was just following along the same lines as the dtc commits
38ad79d3"dtc/tests: don't includefdt.hprior tolibfdt.h" and20b866a7"dtc/fdtdump: includelibfdt_env.hprior tofdt.h", acked by David G. I don't know why some only includefdt.h.devicetree-discuss/David: is there a prescribed way to go here? Change all
fdt.hincluders to just always includelibfdt.hinstead oflibfdt_env.hprior tofdt.h?
Yeah, I think just including libfdt.h instead of fdt.h is the way to go. The distinction is that fdt.h contains only "passive" declarations. That is, defines and structure/type declarations but no function prorotypes or other code. In particular that means that it is strictly about the FDT tree structure only, and not any of the libfdt specific entry points.
Signature · 4 lines
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
_______________________________________________
devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
PATCH 1/5 treewide: include libfdt_env.h before fdt.h
In reply to Kim PhillipsHi Kim,
Quoted 8 lines · Kim Phillips
and, if including
libfdt.hwhich includeslibfdt_env.hin the correct order, don't includefdt.hbeforelibfdt.h.this is needed to get the fdt type definitions set from the project environment before
fdt.huses them.Signed-off-by Kim Phillips <[email protected]>Cc Jerry Van Baren <[email protected]>
Thanks, that was very helpful. I applied the series to my personal tree, will push when the dust settles.
I created a patch that uses just libfdt.h as the interface #include, will publish when the dust settles on the dtc (device tree) list discussion.
Thanks,
gvb