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