[U-Boot] [PATCH] fdt: Enhance dts/Makefile to be all things to all men
There are a few partially conflicting requirements in compiling the device tree, since U-Boot relies on whatever is installed on the build machine.
Some versions of dtc support -i, and this is often better than using #include since you get correct line number information in errors. Unfortunately this version must be installed manually in current Linux distributions.
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
When an device tree file has an error we want to direct the user to the right file and line number. So instead of piping the file into dts through stdin, put it in a real file so that we get a fairly sensible error message from dts. Then print out the original file details to help further.
This is based on a commit from Tom Warren. It would help if people can test it in different environments.
Signed-off-by: Tom Warren twarren@nvidia.com Signed-off-by: Simon Glass sjg@chromium.org --- dts/Makefile | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/dts/Makefile b/dts/Makefile index 03e163e..1f6fabb 100644 --- a/dts/Makefile +++ b/dts/Makefile @@ -37,11 +37,29 @@ $(if $(CONFIG_ARCH_DEVICE_TREE),,\ $(error Your architecture does not have device tree support enabled. \ Please define CONFIG_ARCH_DEVICE_TREE))
+# Provide a list of include directories for dtc +DTS_INCS-y := -i $(SRCTREE)/arch/$(ARCH)/dts + +DTS_INCS-y += -i $(SRCTREE)/board/$(VENDOR)/dts + +DTS_INCS-$(CONFIG_CHROMEOS) += -i $(SRCTREE)/cros/dts + +# Check if our dtc includes the -i option +DTS_FLAGS := $(shell if ! dtc -i 2>&1 | grep -q "invalid option"; then \ + echo $(DTS_INCS-y); fi) + # We preprocess the device tree file provide a useful define -DTS_CPPFLAGS := -x assembler-with-cpp \ +# Undefine 'linux' since it might be used in device tree files +DTS_CPPFLAGS := -x assembler-with-cpp -Ulinux \ -DARCH_CPU_DTS="$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi" \ -DBOARD_DTS="$(SRCTREE)/board/$(VENDOR)/$(BOARD)/dts/$(DEVICE_TREE).dts" \ - -I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts + -D__ASSEMBLY__ -I$(OBJTREE)/include -I$(SRCTREE)/include \ + -I$(OBJTREE)/include2 \ + -I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts \ + -include $(OBJTREE)/include/config.h + +DTS_TMP := $(OBJTREE)/include/generated/$(DEVICE_TREE).dts.in +DTS_SRC := board/$(VENDOR)/dts/$(DEVICE_TREE).dts
all: $(obj).depend $(LIB)
@@ -50,13 +68,19 @@ all: $(obj).depend $(LIB) # the filename. DT_BIN := $(obj)dt.dtb
-$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts +DTC_CMD := $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $(DTS_FLAGS) $(DTS_TMP) + +$(DT_BIN): $(TOPDIR)/$(DTS_SRC) rc=$$( \ - cat $< | $(CPP) -P $(DTS_CPPFLAGS) - | \ - { { $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} - 2>&1 ; \ + cat $< | $(CPP) -P $(DTS_CPPFLAGS) - > $(DTS_TMP); \ + { { $(DTC_CMD) 2>&1 ; \ echo $$? >&3 ; } | \ grep -v '^DTC: dts->dtb on file' ; \ } 3>&1 1>&2 ) ; \ + if [ $$rc != 0 ]; then \ + echo "Source file is $(DTS_SRC)"; \ + echo "Compiler: $(DTC_CMD)"; \ + fi; \ exit $$rc
process_lds = \
Simon,
-----Original Message----- From: Simon Glass [mailto:sjg@chromium.org] Sent: Tuesday, May 28, 2013 12:36 PM To: U-Boot Mailing List Cc: Tom Rini; Stephen Warren; Devicetree Discuss; u-boot- review@google.com; Simon Glass; Tom Warren; Jerry Van Baren Subject: [PATCH] fdt: Enhance dts/Makefile to be all things to all men
There are a few partially conflicting requirements in compiling the device tree, since U-Boot relies on whatever is installed on the build machine.
Some versions of dtc support -i, and this is often better than using #include since you get correct line number information in errors. Unfortunately this version must be installed manually in current Linux distributions.
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
When an device tree file has an error we want to direct the user to the right file and line number. So instead of piping the file into dts through stdin, put it in a real file so that we get a fairly sensible error message from dts. Then print out the original file details to help further.
This is based on a commit from Tom Warren. It would help if people can test it in different environments.
Signed-off-by: Tom Warren twarren@nvidia.com Signed-off-by: Simon Glass sjg@chromium.org
Works for me for all Tegra builds, so:
Tested-by: Tom Warren twarren@nvidia.com
Tom
dts/Makefile | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/dts/Makefile b/dts/Makefile index 03e163e..1f6fabb 100644 --- a/dts/Makefile +++ b/dts/Makefile @@ -37,11 +37,29 @@ $(if $(CONFIG_ARCH_DEVICE_TREE),,\ $(error Your architecture does not have device tree support enabled. \ Please define CONFIG_ARCH_DEVICE_TREE))
+# Provide a list of include directories for dtc DTS_INCS-y := -i +$(SRCTREE)/arch/$(ARCH)/dts
+DTS_INCS-y += -i $(SRCTREE)/board/$(VENDOR)/dts
+DTS_INCS-$(CONFIG_CHROMEOS) += -i $(SRCTREE)/cros/dts
+# Check if our dtc includes the -i option DTS_FLAGS := $(shell if ! dtc +-i 2>&1 | grep -q "invalid option"; then \
echo $(DTS_INCS-y); fi)# We preprocess the device tree file provide a useful define -DTS_CPPFLAGS := -x assembler-with-cpp \ +# Undefine 'linux' since it might be used in device tree files +DTS_CPPFLAGS := -x assembler-with-cpp -Ulinux \ - DARCH_CPU_DTS="$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVIC E_TREE).dtsi" \ - DBOARD_DTS="$(SRCTREE)/board/$(VENDOR)/$(BOARD)/dts/$(DEVICE_TR EE).dts" \
-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts
-D__ASSEMBLY__ -I$(OBJTREE)/include -I$(SRCTREE)/include\
-I$(OBJTREE)/include2 \-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts \
-include $(OBJTREE)/include/config.h+DTS_TMP := $(OBJTREE)/include/generated/$(DEVICE_TREE).dts.in +DTS_SRC := board/$(VENDOR)/dts/$(DEVICE_TREE).dts
all: $(obj).depend $(LIB)
@@ -50,13 +68,19 @@ all: $(obj).depend $(LIB) # the filename. DT_BIN := $(obj)dt.dtb
-$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts +DTC_CMD := $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $(DTS_FLAGS) +$(DTS_TMP)
+$(DT_BIN): $(TOPDIR)/$(DTS_SRC) rc=$$( \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - | \{ { $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} - 2>&1 ; \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - > $(DTS_TMP); \{ { $(DTC_CMD) 2>&1 ; \- if [ $$rc != 0 ]; then \
echo "Source file is $(DTS_SRC)"; \echo "Compiler: $(DTC_CMD)"; \- fi; \ exit $$rc
process_lds = \
1.8.2.1
-- nvpublic
On 05/28/2013 01:36 PM, Simon Glass wrote:
There are a few partially conflicting requirements in compiling the device tree, since U-Boot relies on whatever is installed on the build machine.
Some versions of dtc support -i, and this is often better than using #include since you get correct line number information in errors.
What issue is there with line numbers when using dtc? Recent dtc supports #line directives from the pre-processing results, and hence reports errors in terms of the source files, just like raw dtc without cpp.
Unfortunately this version must be installed manually in current Linux distributions.
Well, then that gets into the problem of some .dts files choosing to use /include/ and rely on -i, but others using #include and not. I don't really think it's a good idea to propagate both versions. Picking one and using it would be far better.
I really do think we should simply require a reasonably recent dtc and be done with it.
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
Linux chose to use -undef rather than undefining specific/individual defines. It'd be best to be consistent to that .dts files are more likely to be portable between the two.
diff --git a/dts/Makefile b/dts/Makefile
+# Provide a list of include directories for dtc +DTS_INCS-y := -i $(SRCTREE)/arch/$(ARCH)/dts
+DTS_INCS-y += -i $(SRCTREE)/board/$(VENDOR)/dts
That has arch/ first then board/ ... (continued a few comments below)
+DTS_INCS-$(CONFIG_CHROMEOS) += -i $(SRCTREE)/cros/dts
Is that meant to be upstream?
+# Check if our dtc includes the -i option +DTS_FLAGS := $(shell if ! dtc -i 2>&1 | grep -q "invalid option"; then \
echo $(DTS_INCS-y); fi)# We preprocess the device tree file provide a useful define -DTS_CPPFLAGS := -x assembler-with-cpp \ +# Undefine 'linux' since it might be used in device tree files +DTS_CPPFLAGS := -x assembler-with-cpp -Ulinux \
I'll repeat my request to use -undef instead.
-DARCH_CPU_DTS=\"$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi\" \ -DBOARD_DTS=\"$(SRCTREE)/board/$(VENDOR)/$(BOARD)/dts/$(DEVICE_TREE).dts\" \
-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts
-D__ASSEMBLY__ -I$(OBJTREE)/include -I$(SRCTREE)/include \-I$(OBJTREE)/include2 \
Do we really want include or include2 (what's that?) at all? The .dts files really should be standalone, and not interact with the U-Boot headers at all.
-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts \
... whereas this has board/ first then arch/. It'd be better to be consistent.
-include $(OBJTREE)/include/config.h+DTS_TMP := $(OBJTREE)/include/generated/$(DEVICE_TREE).dts.in
Hmm. This really isn't a generated header file. Can this instead be $(OBJTREE)/$(dir $@).$(notdir $@).dts or something like that?
+DTS_SRC := board/$(VENDOR)/dts/$(DEVICE_TREE).dts
all: $(obj).depend $(LIB)
@@ -50,13 +68,19 @@ all: $(obj).depend $(LIB) # the filename. DT_BIN := $(obj)dt.dtb
-$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts +DTC_CMD := $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $(DTS_FLAGS) $(DTS_TMP)
It may be better to leave $(DTS_TMP) in the make script below, so it's more obvious what file is being compiled; the re-direct to $(DTS_TMP) is left in the $(CPP) invocation below, so it'd also be consistent with that.
+$(DT_BIN): $(TOPDIR)/$(DTS_SRC) rc=$$( \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - | \{ { $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} - 2>&1 ; \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - > $(DTS_TMP); \{ { $(DTC_CMD) 2>&1 ; \
...
- if [ $$rc != 0 ]; then \
echo "Source file is $(DTS_SRC)"; \echo "Compiler: $(DTC_CMD)"; \- fi; \
Isn't that what make V=1 is for?
Hi Stephen,
On Tue, May 28, 2013 at 1:57 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/28/2013 01:36 PM, Simon Glass wrote:
There are a few partially conflicting requirements in compiling the device tree, since U-Boot relies on whatever is installed on the build machine.
Some versions of dtc support -i, and this is often better than using #include since you get correct line number information in errors.
What issue is there with line numbers when using dtc? Recent dtc supports #line directives from the pre-processing results, and hence reports errors in terms of the source files, just like raw dtc without cpp.
That's the issue. What does dtc v1.3 do?
Unfortunately this version must be installed manually in current Linux distributions.
Well, then that gets into the problem of some .dts files choosing to use /include/ and rely on -i, but others using #include and not. I don't really think it's a good idea to propagate both versions. Picking one and using it would be far better.
I really do think we should simply require a reasonably recent dtc and be done with it.
I would be happy with that, but it can be an extra barrier to getting U-Boot building. So do we need using #include at all if we are using -i ?
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
Linux chose to use -undef rather than undefining specific/individual defines. It'd be best to be consistent to that .dts files are more likely to be portable between the two.
Seems a bit extreme, but OK. Are you worried that gcc defines other things without the double underscore?
diff --git a/dts/Makefile b/dts/Makefile
+# Provide a list of include directories for dtc +DTS_INCS-y := -i $(SRCTREE)/arch/$(ARCH)/dts
+DTS_INCS-y += -i $(SRCTREE)/board/$(VENDOR)/dts
That has arch/ first then board/ ... (continued a few comments below)
+DTS_INCS-$(CONFIG_CHROMEOS) += -i $(SRCTREE)/cros/dts
Is that meant to be upstream?
+# Check if our dtc includes the -i option +DTS_FLAGS := $(shell if ! dtc -i 2>&1 | grep -q "invalid option"; then \
echo $(DTS_INCS-y); fi)# We preprocess the device tree file provide a useful define -DTS_CPPFLAGS := -x assembler-with-cpp \ +# Undefine 'linux' since it might be used in device tree files +DTS_CPPFLAGS := -x assembler-with-cpp -Ulinux \
I'll repeat my request to use -undef instead.
-DARCH_CPU_DTS=\"$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi\" \ -DBOARD_DTS=\"$(SRCTREE)/board/$(VENDOR)/$(BOARD)/dts/$(DEVICE_TREE).dts\" \
-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts
-D__ASSEMBLY__ -I$(OBJTREE)/include -I$(SRCTREE)/include \-I$(OBJTREE)/include2 \Do we really want include or include2 (what's that?) at all? The .dts files really should be standalone, and not interact with the U-Boot headers at all.
I understood that you were wanting to make use of U-Boot defines. If you want to include include/config.h then I think you need these.
-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts \... whereas this has board/ first then arch/. It'd be better to be consistent.
OK
-include $(OBJTREE)/include/config.h+DTS_TMP := $(OBJTREE)/include/generated/$(DEVICE_TREE).dts.in
Hmm. This really isn't a generated header file. Can this instead be $(OBJTREE)/$(dir $@).$(notdir $@).dts or something like that?
I didn't say header file.
The nice thing about having everything in include/generated is that it doesn't pollute the source for in-tree builds.
+DTS_SRC := board/$(VENDOR)/dts/$(DEVICE_TREE).dts
all: $(obj).depend $(LIB)
@@ -50,13 +68,19 @@ all: $(obj).depend $(LIB) # the filename. DT_BIN := $(obj)dt.dtb
-$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts +DTC_CMD := $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $(DTS_FLAGS) $(DTS_TMP)
It may be better to leave $(DTS_TMP) in the make script below, so it's more obvious what file is being compiled; the re-direct to $(DTS_TMP) is left in the $(CPP) invocation below, so it'd also be consistent with that.
+$(DT_BIN): $(TOPDIR)/$(DTS_SRC) rc=$$( \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - | \{ { $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} - 2>&1 ; \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - > $(DTS_TMP); \{ { $(DTC_CMD) 2>&1 ; \...
if [ $$rc != 0 ]; then \echo "Source file is $(DTS_SRC)"; \echo "Compiler: $(DTC_CMD)"; \fi; \Isn't that what make V=1 is for?
It produces about 800KB of other spiel though. If the build fails it is already printing stuff out - so I find this useful.
Regards, Simon
On 05/29/2013 09:59 AM, Simon Glass wrote:
Hi Stephen,
On Tue, May 28, 2013 at 1:57 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/28/2013 01:36 PM, Simon Glass wrote:
There are a few partially conflicting requirements in compiling the device tree, since U-Boot relies on whatever is installed on the build machine.
Some versions of dtc support -i, and this is often better than using #include since you get correct line number information in errors.
What issue is there with line numbers when using dtc? Recent dtc supports #line directives from the pre-processing results, and hence reports errors in terms of the source files, just like raw dtc without cpp.
That's the issue. What does dtc v1.3 do?
v1.3 doesn't include the feature, but it also doesn't support -i either.
Unfortunately this version must be installed manually in current Linux distributions.
Well, then that gets into the problem of some .dts files choosing to use /include/ and rely on -i, but others using #include and not. I don't really think it's a good idea to propagate both versions. Picking one and using it would be far better.
I really do think we should simply require a reasonably recent dtc and be done with it.
I would be happy with that, but it can be an extra barrier to getting U-Boot building.
The Linux kernel chose to solve this by bundling the required dtc source inside the kernel source tree as a tool. This seems by far the simplest way to solve the problem for U-Boot too. If not, it's not exactly hard to:
git clone make
... given that one is already building U-Boot from source anyway.
So do we need using #include at all if we are using -i ?
Well, *.dts are moving to #include (and other cpp constructs) rather than /include/ in the copies in the Linux kernel, which I think will eventually make their way into U-Boot for consistency. Equally, if/when *.dts get separated out into a separate project from the kernel, I think we'd want the same to happen for U-Boot so that the same *.dts is used. Then, there won't be a choice.
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
Linux chose to use -undef rather than undefining specific/individual defines. It'd be best to be consistent to that .dts files are more likely to be portable between the two.
Seems a bit extreme, but OK. Are you worried that gcc defines other things without the double underscore?
IIRC there were some other issues, yes. "unix" may have been one of them. The problem was reported (and I think that fix suggested) by someone else, so I don't remember the full details, although they're in the mailing list archives I suppose.
diff --git a/dts/Makefile b/dts/Makefile
-DARCH_CPU_DTS=\"$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi\" \ -DBOARD_DTS=\"$(SRCTREE)/board/$(VENDOR)/$(BOARD)/dts/$(DEVICE_TREE).dts\" \
-I$(SRCTREE)/board/$(VENDOR)/dts -I$(SRCTREE)/arch/$(ARCH)/dts
-D__ASSEMBLY__ -I$(OBJTREE)/include -I$(SRCTREE)/include \-I$(OBJTREE)/include2 \Do we really want include or include2 (what's that?) at all? The .dts files really should be standalone, and not interact with the U-Boot headers at all.
I understood that you were wanting to make use of U-Boot defines. If you want to include include/config.h then I think you need these.
I hope I didn't want to:-) The DT should really represent the HW not anything about the way U-Boot is configured.
-include $(OBJTREE)/include/config.h+DTS_TMP := $(OBJTREE)/include/generated/$(DEVICE_TREE).dts.in
Hmm. This really isn't a generated header file. Can this instead be $(OBJTREE)/$(dir $@).$(notdir $@).dts or something like that?
I didn't say header file.
The nice thing about having everything in include/generated is that it doesn't pollute the source for in-tree builds.
Well, it's in a directory that's for generated headers; it has "/include/" in the path. I don't think we should put it somewhere that C code could accidentally #include it, irrespective of how (un-)likely that is to get passed review. Also, for in-tree builds, doesn't every single other derived file get put into the source tree? I'm not sure why this file would need to be special?
+$(DT_BIN): $(TOPDIR)/$(DTS_SRC) rc=$$( \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - | \{ { $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} - 2>&1 ; \
cat $< | $(CPP) -P $(DTS_CPPFLAGS) - > $(DTS_TMP); \{ { $(DTC_CMD) 2>&1 ; \...
if [ $$rc != 0 ]; then \echo "Source file is $(DTS_SRC)"; \echo "Compiler: $(DTC_CMD)"; \fi; \Isn't that what make V=1 is for?
It produces about 800KB of other spiel though. If the build fails it is already printing stuff out - so I find this useful.
But again, why be special? I could apply the same argument to every single other C file where I might have typo'd something.
Dear Stephen,
In message 51A62F8D.9010208@wwwdotorg.org you wrote:
The Linux kernel chose to solve this by bundling the required dtc source inside the kernel source tree as a tool. This seems by far the simplest way to solve the problem for U-Boot too. If not, it's not exactly hard to:
Actually it's a horrible approach to fixing tool issues upstream. Or rather to NOT fixing issues. Instead of pushing forward that distros distribute useful, recent versions we simply copy the dtc source. Tomorrow we include the source tree for "make", and next week for "binutils" and "gcc". And in a few months we build a full distro.
It's the same with the Kconfig approach to print only short compile messages - instead of fixing this once where it belongs (in "make"), a zillion projects copy the pretty expensive code around, because it is so much easier.
It's frustrating to watch how good old methods that brought free software to the state we have today (or we had some time ago already) more and more get lost and forgotten :-(
Best regards,
Wolfgang Denk
On 05/29/2013 03:31 PM, Wolfgang Denk wrote:
Dear Stephen,
In message 51A62F8D.9010208@wwwdotorg.org you wrote:
The Linux kernel chose to solve this by bundling the required dtc source inside the kernel source tree as a tool. This seems by far the simplest way to solve the problem for U-Boot too. If not, it's not exactly hard to:
Actually it's a horrible approach to fixing tool issues upstream. Or rather to NOT fixing issues. Instead of pushing forward that distros distribute useful, recent versions we simply copy the dtc source.
I don't understand the hangup about the version of dtc that distros package.
Sure, it'd be nice if distros updated the the (currently) latest version of dtc and packaged that, so that at some time the desired version was there, and everything "just worked".
However, that's not going to outright solve the problem for a /long/ time.
What if someone wants to build U-Boot on Ubuntu 10.04 or RHEL 5. It seems quite reasonable for someone to be using those for development since they're long-term supported stable releases. Those releases don't have the (current) latest version of dtc packaged, and it's exceedingly unlikely anyone could push an update into those distros to update dtc, since they're probably in bug-fix-only mode right now, and a dtc update would be to add new features.
So, to cover that issue we must:
a) Get the latest dtc into distros right now. Wait until everyone has updated. Then, we can use the new features. This could take many many years.
b) Simply require people to install dtc from source, if their distro doesn't already package the desired version. This will immediately solve the problem, and is honestly quite simple if you're already building other things (U-Boot) from source.
I prefer option (b) here. And given that, I assert that whatever version distros package is largely irrelevant, since there's a trivial workaround if they don't have the desired version.
Longer-term distros will pick up a new version, and remove the need for build-from-source, thus streamlining the process.
To keep this process in check a bit, we could always pick a specific git commit or release version of dtc that each U-Boot version (release) will be allowed to assume. That will limit the number of times people need to update their locally-built dtc to at most once per U-Boot release. Hopefully much less often.
Dear Stephen Warren,
In message 51A67EC1.2000208@wwwdotorg.org you wrote:
To keep this process in check a bit, we could always pick a specific git commit or release version of dtc that each U-Boot version (release) will be allowed to assume. That will limit the number of times people need to update their locally-built dtc to at most once per U-Boot release. Hopefully much less often.
I think this is broken by design, in several aspects. First, U-Boot is usually not the only user of DTC. Second, assume you run a "git bisect" over a U-Boot tree - would you really want to switch DTC in- flight?
Sorry, instead we should strive to be compatible to a reasonably old, stable version of DTC, like we do for all other tools as well. As mentioned before - just because RHEL 5 ships an ancient version of - say - "make" we will NOT start building this from the sources ourself. This cannot be the way to go.
Best regards,
Wolfgang Denk
On 05/29/2013 04:36 PM, Wolfgang Denk wrote:
Dear Stephen Warren,
In message 51A67EC1.2000208@wwwdotorg.org you wrote:
To keep this process in check a bit, we could always pick a specific git commit or release version of dtc that each U-Boot version (release) will be allowed to assume. That will limit the number of times people need to update their locally-built dtc to at most once per U-Boot release. Hopefully much less often.
I think this is broken by design, in several aspects. First, U-Boot is usually not the only user of DTC. Second, assume you run a "git bisect" over a U-Boot tree - would you really want to switch DTC in- flight?
Sorry, instead we should strive to be compatible to a reasonably old, stable version of DTC, like we do for all other tools as well. As mentioned before - just because RHEL 5 ships an ancient version of - say - "make" we will NOT start building this from the sources ourself. This cannot be the way to go.
So the result of that is that we can never ever use new features in any tool, at least in any meaningful time-frame.
I think we need to differentiate between tools that are already stable and/or core to all aspects of the U-Boot build process (such as make), and tools that enable new features that are under development.
Clearly the U-Boot makefiles have to be fairly cautious in their use of any new make features, so that everyone with any reasonable version of make can build U-Boot.
However, when enabling a new feature, such as using device trees to configure U-Boot[1], for which tool support is new and evolving along with the feature itself, and which is only used on a very very few boards and even fewer SoCs right now within U-Boot, it seems entirely reasonable to demand that the people working on/with that new feature are aware that it's evolving, and that they may need to take a few extra steps to go out and get tools that support that new feature. No doubt once this feature has settled down a bit, and distros have pulled in newer versions of dtc, everthing will "just work" just like any other stable feature.
If you don't accept this, then we simply have to ban any include use in U-Boot; dtc -i isn't in distro-packaged versions of dtc, so we'd need to stop using that. We'd need to move *.dts into a single directory within U-Boot to work around that, or perhaps hard-coding relative include paths in *.dts might work. Similarly for the patches to support dtc+cpp usage, so we wouldn't be able to use named constants in DT files for many years, which would prevent sharing DT files with the kernel and/or any other standard repository of DT files, which are bound to use this feature.
[1] Which is very specifically a different feature than simply having U-Boot pass a DT to the Linux kernel during boot, and perhaps modify it a little, which clearly is not a new feature.
Hi,
On Wed, May 29, 2013 at 4:07 PM, Stephen Warren swarren@wwwdotorg.orgwrote:
On 05/29/2013 04:36 PM, Wolfgang Denk wrote:
Dear Stephen Warren,
In message 51A67EC1.2000208@wwwdotorg.org you wrote:
To keep this process in check a bit, we could always pick a specific git commit or release version of dtc that each U-Boot version (release) will be allowed to assume. That will limit the number of times people need to update their locally-built dtc to at most once per U-Boot release. Hopefully much less often.
I think this is broken by design, in several aspects. First, U-Boot is usually not the only user of DTC. Second, assume you run a "git bisect" over a U-Boot tree - would you really want to switch DTC in- flight?
Sorry, instead we should strive to be compatible to a reasonably old, stable version of DTC, like we do for all other tools as well. As mentioned before - just because RHEL 5 ships an ancient version of - say - "make" we will NOT start building this from the sources ourself. This cannot be the way to go.
So the result of that is that we can never ever use new features in any tool, at least in any meaningful time-frame.
I think we need to differentiate between tools that are already stable and/or core to all aspects of the U-Boot build process (such as make), and tools that enable new features that are under development.
Clearly the U-Boot makefiles have to be fairly cautious in their use of any new make features, so that everyone with any reasonable version of make can build U-Boot.
However, when enabling a new feature, such as using device trees to configure U-Boot[1], for which tool support is new and evolving along with the feature itself, and which is only used on a very very few boards and even fewer SoCs right now within U-Boot, it seems entirely reasonable to demand that the people working on/with that new feature are aware that it's evolving, and that they may need to take a few extra steps to go out and get tools that support that new feature. No doubt once this feature has settled down a bit, and distros have pulled in newer versions of dtc, everthing will "just work" just like any other stable feature.
If you don't accept this, then we simply have to ban any include use in U-Boot; dtc -i isn't in distro-packaged versions of dtc, so we'd need to stop using that. We'd need to move *.dts into a single directory within U-Boot to work around that, or perhaps hard-coding relative include paths in *.dts might work. Similarly for the patches to support dtc+cpp usage, so we wouldn't be able to use named constants in DT files for many years, which would prevent sharing DT files with the kernel and/or any other standard repository of DT files, which are bound to use this feature.
[1] Which is very specifically a different feature than simply having U-Boot pass a DT to the Linux kernel during boot, and perhaps modify it a little, which clearly is not a new feature.
My patch:
- doesn't require -i but uses it if available (ARCH_CPU_DTS works around this) - honours #define, #include, etc. - works with old and new versions of dtc - uses -Ulinux which we are thinking might be better done another way
Let's not throw the baby out with the bathwater. I have no problem with updating my dtc as needed, but it would certainly be nice if U-Boot didn't require this.
However, let's say Tegra needs a newer dtc than 1.3. I wonder whether we should just add a setting to tell U-Boot to not build the device tree at all? The same U-Boot binary can run on many different board times, just needing a different device tree. Rather than pick one, we could just pick none. Then if you want to use new features in dtc, just define CONFIG_OF_NO_BUILD, or similar, and you can deal with the device tree details yourself. MAKEALL/buildman will be happy with this.
Otherwise for now I think my patch is a reasonable compromise in terms of features.
Regards, Simon
On 05/29/2013 10:46 PM, Simon Glass wrote:
Hi,
On Wed, May 29, 2013 at 4:07 PM, Stephen Warren <swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org> wrote:
On 05/29/2013 04:36 PM, Wolfgang Denk wrote: > Dear Stephen Warren, > > In message <51A67EC1.2000208@wwwdotorg.org <mailto:51A67EC1.2000208@wwwdotorg.org>> you wrote: >> >> To keep this process in check a bit, we could always pick a specific git >> commit or release version of dtc that each U-Boot version (release) will >> be allowed to assume. That will limit the number of times people need to >> update their locally-built dtc to at most once per U-Boot release. >> Hopefully much less often. > > I think this is broken by design, in several aspects. First, U-Boot > is usually not the only user of DTC. Second, assume you run a "git > bisect" over a U-Boot tree - would you really want to switch DTC in- > flight? > > Sorry, instead we should strive to be compatible to a reasonably old, > stable version of DTC, like we do for all other tools as well. As > mentioned before - just because RHEL 5 ships an ancient version of - > say - "make" we will NOT start building this from the sources ourself. > This cannot be the way to go. So the result of that is that we can never ever use new features in any tool, at least in any meaningful time-frame. I think we need to differentiate between tools that are already stable and/or core to all aspects of the U-Boot build process (such as make), and tools that enable new features that are under development. Clearly the U-Boot makefiles have to be fairly cautious in their use of any new make features, so that everyone with any reasonable version of make can build U-Boot. However, when enabling a new feature, such as using device trees to configure U-Boot[1], for which tool support is new and evolving along with the feature itself, and which is only used on a very very few boards and even fewer SoCs right now within U-Boot, it seems entirely reasonable to demand that the people working on/with that new feature are aware that it's evolving, and that they may need to take a few extra steps to go out and get tools that support that new feature. No doubt once this feature has settled down a bit, and distros have pulled in newer versions of dtc, everthing will "just work" just like any other stable feature. If you don't accept this, then we simply have to ban any include use in U-Boot; dtc -i isn't in distro-packaged versions of dtc, so we'd need to stop using that. We'd need to move *.dts into a single directory within U-Boot to work around that, or perhaps hard-coding relative include paths in *.dts might work. Similarly for the patches to support dtc+cpp usage, so we wouldn't be able to use named constants in DT files for many years, which would prevent sharing DT files with the kernel and/or any other standard repository of DT files, which are bound to use this feature. [1] Which is very specifically a different feature than simply having U-Boot pass a DT to the Linux kernel during boot, and perhaps modify it a little, which clearly is not a new feature.My patch:
- doesn't require -i but uses it if available (ARCH_CPU_DTS works around
this)
If we ever need to support a dtc that doesn't implement -i, then we always need to support that. So, there's no point using -i when it's available; we should simply have a single way of writing the *.dts files that doesn't ever assume dtc -i is available. Otherwise, there will be rampant inconsistency between different *.dts files; how they're written, the flow by which they're compiled, etc.
In other words, we /always/ have to use "ARCH_CPU_DTS" in *.dts throughout the entire U-Boot source tree if we're to support not using dtc -i.
- honours #define, #include, etc.
- works with old and new versions of dtc
If we are forced to rely only upon features in old versions of dtc, we specifically shouldn't allow use of features that will only work with newer dtc. If we don't actively enforce this rule, we haven't achieved the goal of not relying upon new versions of dtc.
...
However, let's say Tegra needs a newer dtc than 1.3. I wonder whether we
This shouldn't be about whether a specific Soc's DT requires some feature or not. All the U-Boot *.dts should work the same way.
should just add a setting to tell U-Boot to not build the device tree at all? The same U-Boot binary can run on many different board times, just needing a different device tree.
Unfortunately, that's not remotely true yet for any Tegra system at least; there's still a bunch of C code specific to each board. The amount is reducing as things get migrated to DT, but we certainly aren't there yet.
Rather than pick one, we could just pick none. Then if you want to use new features in dtc, just define CONFIG_OF_NO_BUILD, or similar, and you can deal with the device tree details yourself. MAKEALL/buildman will be happy with this.
So you're saying: move the *.dts files out of U-Boot into some separate project. It'd be nice to do that; hopefully that separate project could be the one where the kernel's *.dts files move to. The big issue here is that the DT bindings U-Boot uses are often different to the standard bindings, so U-Boot currently contains U-Boot-specific *.dts so there wouldn't be much point moving them out of U-Boot into some common DT repository. Again, hopefully that's changing over time, but we're still not there yet.
Otherwise for now I think my patch is a reasonable compromise in terms of features.
It seems to be aimed specifically at enabling use of new dtc features when present. That seems to be specifically against Wolfgang's goal of not requiring new dtc features. There's no point allowing use of new dtc features if we can't require them, since there's no fallback when they aren't there to use.
Hi Stephen,
On Wed, May 29, 2013 at 10:11 PM, Stephen Warren swarren@wwwdotorg.orgwrote:
On 05/29/2013 10:46 PM, Simon Glass wrote:
Hi,
On Wed, May 29, 2013 at 4:07 PM, Stephen Warren <swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org> wrote:
On 05/29/2013 04:36 PM, Wolfgang Denk wrote: > Dear Stephen Warren, > > In message <51A67EC1.2000208@wwwdotorg.org <mailto:51A67EC1.2000208@wwwdotorg.org>> you wrote: >> >> To keep this process in check a bit, we could always pick a specific git >> commit or release version of dtc that each U-Boot version (release) will >> be allowed to assume. That will limit the number of times people need to >> update their locally-built dtc to at most once per U-Boot release. >> Hopefully much less often. > > I think this is broken by design, in several aspects. First,U-Boot
> is usually not the only user of DTC. Second, assume you run a "git > bisect" over a U-Boot tree - would you really want to switch DTCin-
> flight? > > Sorry, instead we should strive to be compatible to a reasonablyold,
> stable version of DTC, like we do for all other tools as well. As > mentioned before - just because RHEL 5 ships an ancient version of
> say - "make" we will NOT start building this from the sourcesourself.
> This cannot be the way to go. So the result of that is that we can never ever use new features inany
tool, at least in any meaningful time-frame. I think we need to differentiate between tools that are alreadystable
and/or core to all aspects of the U-Boot build process (such asmake),
and tools that enable new features that are under development. Clearly the U-Boot makefiles have to be fairly cautious in their useof
any new make features, so that everyone with any reasonable versionof
make can build U-Boot. However, when enabling a new feature, such as using device trees to configure U-Boot[1], for which tool support is new and evolving along with the feature itself, and which is only used on a very very few boards and even fewer SoCs right now within U-Boot, it seems entirely reasonable to demand that the people working on/with that new feature are aware that it's evolving, and that they may need to take a fewextra
steps to go out and get tools that support that new feature. No doubt once this feature has settled down a bit, and distros have pulled in newer versions of dtc, everthing will "just work" just like any other stable feature. If you don't accept this, then we simply have to ban any include usein
U-Boot; dtc -i isn't in distro-packaged versions of dtc, so we'dneed to
stop using that. We'd need to move *.dts into a single directorywithin
U-Boot to work around that, or perhaps hard-coding relative include paths in *.dts might work. Similarly for the patches to supportdtc+cpp
usage, so we wouldn't be able to use named constants in DT files for many years, which would prevent sharing DT files with the kerneland/or
any other standard repository of DT files, which are bound to usethis
feature. [1] Which is very specifically a different feature than simply having U-Boot pass a DT to the Linux kernel during boot, and perhaps modifyit
a little, which clearly is not a new feature.My patch:
- doesn't require -i but uses it if available (ARCH_CPU_DTS works around
this)
I had to remove all sharp objects from the room half-way through reading your email :-) Gosh, things aren't that bad!
If we ever need to support a dtc that doesn't implement -i, then we always need to support that. So, there's no point using -i when it's available; we should simply have a single way of writing the *.dts files that doesn't ever assume dtc -i is available. Otherwise, there will be rampant inconsistency between different *.dts files; how they're written, the flow by which they're compiled, etc.
In other words, we /always/ have to use "ARCH_CPU_DTS" in *.dts throughout the entire U-Boot source tree if we're to support not using dtc -i.
Well realistically at some point there will be a new dtc release, and perhaps a year or so after that we can maybe start deprecating this and requiring -i.
- honours #define, #include, etc.
- works with old and new versions of dtc
If we are forced to rely only upon features in old versions of dtc, we specifically shouldn't allow use of features that will only work with newer dtc. If we don't actively enforce this rule, we haven't achieved the goal of not relying upon new versions of dtc.
As you know I'm uncomfortable with the idea of using CPP to do things that I feel dtc should do for itself, but yes if dtc does not grow these features, we have little choice. But as an example, both #include and symbols can be syntactically very similar whether CPP is used or not.
...
However, let's say Tegra needs a newer dtc than 1.3. I wonder whether we
This shouldn't be about whether a specific Soc's DT requires some feature or not. All the U-Boot *.dts should work the same way.
should just add a setting to tell U-Boot to not build the device tree at all? The same U-Boot binary can run on many different board times, just needing a different device tree.
Unfortunately, that's not remotely true yet for any Tegra system at least; there's still a bunch of C code specific to each board. The amount is reducing as things get migrated to DT, but we certainly aren't there yet.
OK, but that wasn't quite my point....
Rather than pick one, we could just pick none. Then if you want to use new features in dtc, just define CONFIG_OF_NO_BUILD, or similar, and you can deal with the device tree details yourself. MAKEALL/buildman will be happy with this.
So you're saying: move the *.dts files out of U-Boot into some separate project. It'd be nice to do that; hopefully that separate project could be the one where the kernel's *.dts files move to. The big issue here is that the DT bindings U-Boot uses are often different to the standard bindings, so U-Boot currently contains U-Boot-specific *.dts so there wouldn't be much point moving them out of U-Boot into some common DT repository. Again, hopefully that's changing over time, but we're still not there yet.
a. I didn't say move them out of U-Boot b. If there are differences between the U-Boot and kernel bindings, we should fix that c. My point was that 'building nothing' fudges the 'failed build' problem and might be an acceptable compromise. Then Tegra can use the whiz-bang new features without breaking the build.
Otherwise for now I think my patch is a reasonable compromise in terms of features.
It seems to be aimed specifically at enabling use of new dtc features when present. That seems to be specifically against Wolfgang's goal of not requiring new dtc features. There's no point allowing use of new dtc features if we can't require them, since there's no fallback when they aren't there to use.
My patch is specifically designed to provide a fallback when the are not there to use. I'm not sure how feasible this is long term, but it works for now.
I have no problem with including dtc in with U-Boot if that is the chosen approach. It solves some problems. In fact my Kbuild patch set could make that quite easy. However I do understand Wolfgang's POV that it is philosophically a dangerous path.
You know my strong support for some sort of symbolic support in dtc, and I share your frustration that it hasn't happened yet.
Is my patch better than what is there, or is it just making things worse?
Regards, Simon
Dear Stephen,
In message 51A6DF7C.30903@wwwdotorg.org you wrote:
It seems to be aimed specifically at enabling use of new dtc features when present. That seems to be specifically against Wolfgang's goal of not requiring new dtc features. There's no point allowing use of new dtc
Please stop planting statements on me that I did not make.
Best regards,
Wolfgang Denk
On 05/30/2013 01:56 AM, Wolfgang Denk wrote:
Dear Stephen,
In message 51A6DF7C.30903@wwwdotorg.org you wrote:
It seems to be aimed specifically at enabling use of new dtc features when present. That seems to be specifically against Wolfgang's goal of not requiring new dtc features. There's no point allowing use of new dtc
Please stop planting statements on me that I did not make.
Sorry, but that's what I believe you meant. If you didn't mean that, well then let's clear this up, and the rest of the conversation will be a lot simpler:
I believe that for building the *.dts in U-Boot we should simply require the user to have a version of dtc that supports the recently added features such as:
* -i directive. * Ability to parse the output of cpp well (e.g. #line directives, emit useful error/warning messages in this case). * Cell expression support.
Those are all present in the latest git repo for dtc.
If we do that, then we won't need any conditional logic in the U-Boot makefiles. At least parts of Simon's patch won't be necessary.
I'm quite happy to achieve this requirement by having the user install that dtc into the $PATH prior to running any U-Boot make, either manually or via distro packages once they're available.
It is my understanding that you object to requiring such a new version of dtc, irrespective of the means by which it's provided. If this is not true, then please do let me know; it would vastly simplify matters.
Dear Stephen Warren,
In message 51A68A4C.4060505@wwwdotorg.org you wrote:
Sorry, instead we should strive to be compatible to a reasonably old, stable version of DTC, like we do for all other tools as well. As mentioned before - just because RHEL 5 ships an ancient version of - say - "make" we will NOT start building this from the sources ourself. This cannot be the way to go.
So the result of that is that we can never ever use new features in any tool, at least in any meaningful time-frame.
I wrote "we should strive", not "this is the only way".
However, when enabling a new feature, such as using device trees to configure U-Boot[1], for which tool support is new and evolving along with the feature itself, and which is only used on a very very few boards and even fewer SoCs right now within U-Boot, it seems entirely reasonable to demand that the people working on/with that new feature are aware that it's evolving, and that they may need to take a few extra steps to go out and get tools that support that new feature. No doubt once this feature has settled down a bit, and distros have pulled in newer versions of dtc, everthing will "just work" just like any other stable feature.
Agreed. And nothing prevents you to installl on your system another, more recent version of dtc or any other tools needed for specific purposes.
If you don't accept this, then we simply have to ban any include use in U-Boot; dtc -i isn't in distro-packaged versions of dtc, so we'd need to
This has never been my intention. I object only against including the dtc source into U-Boot, and against automatic methods to build dtc as part of U-Boot, even in a way that depends on the U-Boot version.
dtc is a tool, like gcc or others. We may test against specific versions of the tools in the Makefiles, and even abort a build if an incompatible version is found. But we will not include tool sources, nor build / install rules for tools.
Best regards,
Wolfgang Denk
Dear Simon Glass,
In message 1369769778-12455-1-git-send-email-sjg@chromium.org you wrote:
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
I think this is not a good way to address this issue. The GCC documentation (section "System-specific Predefined Macros" [1]) desribes how this should be handled. The "correct" (TM) way to fix this is by adding "-ansi" or any "-std" option that requests strict conformance to the compiler/preprocessor command line.
[1] http://gcc.gnu.org/onlinedocs/cpp/System_002dspecific-Predefined-Macros.html...
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Tue, May 28, 2013 at 2:08 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1369769778-12455-1-git-send-email-sjg@chromium.org you wrote:
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
I think this is not a good way to address this issue. The GCC documentation (section "System-specific Predefined Macros" [1]) desribes how this should be handled. The "correct" (TM) way to fix this is by adding "-ansi" or any "-std" option that requests strict conformance to the compiler/preprocessor command line.
[1] http://gcc.gnu.org/onlinedocs/cpp/System_002dspecific-Predefined-Macros.html...
Stephen suggested a slightly more extreme version too - I was worried that all the typing stuff in U-Boot headers would break in this case, but I didn't actually test it, so perhaps it is fine.
Regards, Simon
Dear Simon,
In message CAPnjgZ2a+qrsPWTz5Y=48m_LCRqAikY0-seJudW8AY5asdwmxw@mail.gmail.com you wrote:
I think this is not a good way to address this issue. The GCC documentation (section "System-specific Predefined Macros" [1]) desribes how this should be handled. The "correct" (TM) way to fix this is by adding "-ansi" or any "-std" option that requests strict conformance to the compiler/preprocessor command line.
[1] http://gcc.gnu.org/onlinedocs/cpp/System_002dspecific-Predefined-Macros.html...
Stephen suggested a slightly more extreme version too - I was worried that all the typing stuff in U-Boot headers would break in this case, but I didn't actually test it, so perhaps it is fine.
Yes, I've seen it. I think either approach is better that manually undef-ing specific variables. From my personal point of view I think the -undef approach is a bit of overkill, but I would not protest.
Best regards,
Wolfgang Denk
On 05/28/2013 03:08 PM, Wolfgang Denk wrote:
Dear Simon Glass,
In message 1369769778-12455-1-git-send-email-sjg@chromium.org you wrote:
Some device tree files use the word 'linux' which gets replaced with '1' by many version of gcc, including version 4.7. So undefine this.
I think this is not a good way to address this issue. The GCC documentation (section "System-specific Predefined Macros" [1]) desribes how this should be handled. The "correct" (TM) way to fix this is by adding "-ansi" or any "-std" option that requests strict conformance to the compiler/preprocessor command line.
[1] http://gcc.gnu.org/onlinedocs/cpp/System_002dspecific-Predefined-Macros.html...
-ansi at least was considered when the Linux kernel patches for dtc+cpp support were being developed, but it was rejected. While it possibly does solve this specific issue fine, there were other more general problems; IIRC (and I might not) it completely changes the way macro expansion happens, which results in it being pretty useless. Hence, "-x assembler-with-cpp" was chosen over e.g. "-ansi".
Dear Stephen,
In message 51A634B5.5060309@wwwdotorg.org you wrote:
I think this is not a good way to address this issue. The GCC documentation (section "System-specific Predefined Macros" [1]) desribes how this should be handled. The "correct" (TM) way to fix this is by adding "-ansi" or any "-std" option that requests strict conformance to the compiler/preprocessor command line.
[1] http://gcc.gnu.org/onlinedocs/cpp/System_002dspecific-Predefined-Macros.html...
-ansi at least was considered when the Linux kernel patches for dtc+cpp support were being developed, but it was rejected. While it possibly
Can you provide references? I'd like to understand why it was rejected - it seems to be the "official" approach to the problem.
does solve this specific issue fine, there were other more general problems; IIRC (and I might not) it completely changes the way macro expansion happens, which results in it being pretty useless. Hence, "-x assembler-with-cpp" was chosen over e.g. "-ansi".
Again, do you have any reference? "completely changes the way macro expansion happens" sounds terribly dangerous, so it would be better to know about that exactly...
Best regards,
Wolfgang Denk
On 05/29/2013 03:33 PM, Wolfgang Denk wrote:
Dear Stephen,
In message 51A634B5.5060309@wwwdotorg.org you wrote:
I think this is not a good way to address this issue. The GCC documentation (section "System-specific Predefined Macros" [1]) desribes how this should be handled. The "correct" (TM) way to fix this is by adding "-ansi" or any "-std" option that requests strict conformance to the compiler/preprocessor command line.
[1] http://gcc.gnu.org/onlinedocs/cpp/System_002dspecific-Predefined-Macros.html...
-ansi at least was considered when the Linux kernel patches for dtc+cpp support were being developed, but it was rejected. While it possibly
Can you provide references? I'd like to understand why it was rejected - it seems to be the "official" approach to the problem.
does solve this specific issue fine, there were other more general problems; IIRC (and I might not) it completely changes the way macro expansion happens, which results in it being pretty useless. Hence, "-x assembler-with-cpp" was chosen over e.g. "-ansi".
Again, do you have any reference? "completely changes the way macro expansion happens" sounds terribly dangerous, so it would be better to know about that exactly...
Sorry, I was thinking about -traditional-cpp, not -ansi. We had considered -traditional-cpp as a workaround because DT uses property names that start with #, and this triggered cpp to treat them as macro names, which went wrong. However, due to -traditional-cpp being quite different from ISO cpp, we selected -x assembler-with-cpp instead, which both implements an ISO cpp, but also only handles pre-processing directives with the # in column 1.
Now, let's discuss -ansi vs. -undef some more.
Since DT is supposed to be a HW description, it shouldn't be using cpp's built-in macros to compile in different ways; there really isn't a concept of the "target arch of compilation"; a .dts file should simply compile the same way everywhere. Different DTs represent different HW; we don't use a single DT with conditional compilation to represent different HW. This led Rob Herring (one of the kernel device tree maintainers) to propose the following rules for cpp usage on device trees:
https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-October/020831.ht...
One of which was:
- No gcc built-in define usage
I don't see any disagreement with that bullet point in the thread, and indeed it makes sense to me for the reasons I outlined above.
Some history on why we needed to not-define, or un-define, some macros (such as the linux macro we're discussing here) can be found in:
http://linux-kernel.2935.n7.nabble.com/PATCH-V7-kbuild-create-a-rule-to-run-...
The last few messages there end up mentioning -undef, which we/I ended up using. It looks like -ansi wasn't mentioned at all when discussing this issue.
However, I assert that -undef is better, since we end up without any pre-defined macros, which DT files shouldn't be using anyway. By my reading of the cpp manual link you send, -ansi simply stops non-conforming pre-defined macros from being defined, but doesn't stop them all being defined as -undef does. Hence, I prefer -undef.
Oh, and another of Rob's proposed rules:
- No kernel kconfig option usage
Would also imply that U-Boots config headers shouldn't be accessible when compiling device trees. The last few kernel patches I sent to enable dtc+cpp usage were specifically aimed at limiting the set of headers that DT files can use to those specifically part of the DT bindings, and not general Linux headers, For example, see the following Linux kernel commit:
========== kbuild: create an "include chroot" for DT bindings
The recent dtc+cpp support allows header files and C pre-processor defines/macros to be used when compiling device tree files. These headers will typically define various constants that are part of the device tree bindings.
The original patch which set up the dtc+cpp include path only considered using those headers from device tree files. However, most are also useful for kernel code which needs to interpret the device tree.
In both the DT files and the kernel, I'd like to include the DT-related headers in the same way, for example, <dt-bindings/gpio/tegra-gpio.h>. That will simplify any text which discusses the DT header locations.
Creating a <dt-bindings/> for kernel source to use is as simple as placing files into include/dt-bindings/.
However, when compiling DT files, the include path should be restricted so that only the dt-bindings path is available; arbitrary kernel headers shouldn't be exposed. For this reason, create a specific include directory for use by dtc+cpp, and symlink dt-bindings from there to the actual location of include/dt-bindings/. For want of a better location, place this "include chroot" into the existing dts/ directory.
arch/*/boot/dts/include/dt-bindings -> ../../../../../include/dt-bindings
Some headers used by device tree files may not be useful to the kernel; they may be used simply to aid in constructing the DT file (e.g. macros to create a node), but not define any information that the kernel needs to share. These may be placed directly into arch/*/boot/dts/ along with the DT files themselves. ==========
Dear Stephen,
In message 51A6869F.1020004@wwwdotorg.org you wrote:
Since DT is supposed to be a HW description, it shouldn't be using cpp's built-in macros to compile in different ways; there really isn't a concept of the "target arch of compilation"; a .dts file should simply compile the same way everywhere. Different DTs represent different HW;
I am not 100% sure of that. First, it's not correct to say "compiled compile the same way everywhere" - it's not that "where" it compiled, butthe "for which machine", i. e. which exact cross tool chain is selected.
And I can well imagine DTs that include parts for specific IP blocks, where the exact details might depend on the target architecture. We see an incresing number of cases where the same IP blocks are being used in different architectures, even of different endianess, etc. So even if this appears not useful or needed today, I think we should not intentionally barr the way unless that would realy cause problems to us.
Or do you see a real potentiol of conflict with the classed "reserved namespaces" in GCC with double underscores?
we don't use a single DT with conditional compilation to represent different HW. This led Rob Herring (one of the kernel device tree maintainers) to propose the following rules for cpp usage on device trees:
https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-October/020831.ht...
One of which was:
- No gcc built-in define usage
I understand that was part of some discussion then. But has it been generally agreed on, and eventually even turned into a DT policy document?
Some history on why we needed to not-define, or un-define, some macros (such as the linux macro we're discussing here) can be found in:
http://linux-kernel.2935.n7.nabble.com/PATCH-V7-kbuild-create-a-rule-to-run-...
The last few messages there end up mentioning -undef, which we/I ended up using. It looks like -ansi wasn't mentioned at all when discussing this issue.
"-x assembler-with-cpp" makes sense for other reasons, i. e. being able to mark related lines incommonly used header files with __ASSEMBLER__ and making sure that '#' gets only processed when in column 1. It does NOT solve the indetifier problem, though, as GCC will still define "unix" and :linux" (and eventually more):
-> gcc -E -dM -x assembler-with-cpp - < /dev/null | grep -v _ #define unix 1 #define linux 1
However, I assert that -undef is better, since we end up without any pre-defined macros, which DT files shouldn't be using anyway. By my reading of the cpp manual link you send, -ansi simply stops non-conforming pre-defined macros from being defined, but doesn't stop them all being defined as -undef does. Hence, I prefer -undef.
Why do you want to stop something which doesn't hurt you, but might eventually be useful in other cases?
No, I do not have any real such use cases at hand, but I can see that it might be for example useful to auto-detect the certain properties of the system we're building for.
Oh, and another of Rob's proposed rules:
Agreed, another propsal...
shouldn't be exposed. For this reason, create a specific include directory for use by dtc+cpp, and symlink dt-bindings from there to the actual location of include/dt-bindings/. For want of a better location, place this "include chroot" into the existing dts/ directory.
arch/*/boot/dts/include/dt-bindings -> ../../../../../include/dt-bindings
OT here, but: Don't you see a risk that such arch specific things might become a problem soon?
Best regards,
Wolfgang Denk
participants (4)
-
Simon Glass -
Stephen Warren -
Tom Warren -
Wolfgang Denk