kbuild: use separate commands for generating .dtb and .dt.yaml

PATCH In review Masahiro Yamada 2 participants 5 messages May 4, 2020 – May 5, 2020

Subsystems (2)

Kbuild Device tree

lore

Share & tools

lore

Masahiro Yamada

author

2020-05-04 02:07 UTC #0 lore raw

PATCH 1/2 kbuild: use separate commands for generating *.dtb and *.dt.yaml

Currently, cmd_dtc takes $(2) to select the target format. The additional parameter complicates things when it is used with cmd_and_fixdep and if_changed_rule.

To make it easier to understand, use a separate cmd_dtc_yaml.

One visible change is, 'DTCYAML' is used for the log when compiling *.dts to *.dt.yaml.

Signed-off-by Masahiro Yamada <[email protected]>

---

 scripts/Makefile.lib | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/Makefile.lib +12 -7

index 4b799737722c..8f9f2abf3d67 100644
@@ -284,32 +284,37 @@ cmd_dt_S_dtb=						\
 $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 	$(call if_changed,dt_S_dtb)
 
-quiet_cmd_dtc = DTC     $@
-cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
+__cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
 	$(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-	$(DTC) -O $(2) -o $@ -b 0 \
+	$(DTC) -O $(1) -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
+quiet_cmd_dtc = DTC     $@
+      cmd_dtc = $(call __cmd_dtc,dtb)
+
 $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
-	$(call if_changed_dep,dtc,dtb)
+	$(call if_changed_dep,dtc)
 
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
 # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
 DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
 
+quiet_cmd_dtc_yaml = DTCYAML $@
+      cmd_dtc_yaml = $(call __cmd_dtc,yaml)
+
 quiet_cmd_dtb_check =	CHECK   $@
       cmd_dtb_check =	$(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
 
-define rule_dtc
-	$(call cmd_and_fixdep,dtc,yaml)
+define rule_dtc_yaml
+	$(call cmd_and_fixdep,dtc_yaml)
 	$(call cmd,dtb_check)
 endef
 
 $(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
-	$(call if_changed_rule,dtc,yaml)
+	$(call if_changed_rule,dtc_yaml)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)

Signature · 1 line

2.25.1

Masahiro Yamada

author

2020-05-04 02:08 UTC #1 lore raw

PATCH 2/2 scripts/dtc: compile separate dtc-yaml

Marek Behún reported a case where pkg-config fails to detect the libyaml-dev package, which is presumably a bug of the distro.

Irrespective of that, I am not a big fan of pkg-config in the Makefile parse stage. The cost of pkg-config is quite small, but it is evaluated everytime we run make, even when we do 'make mrproper'. This commit changes the Makefile to not rely on pkg-config at all.

The normal build should not require libyaml-dev while we need to compile dtc with libyaml for the schema check.

Build two dtc variants:

scripts/dtc/dtc for *.dts -> *.dtb
scripts/dtc/dtc-yaml for *.dts -> *.dt.yaml

'make dtbs_check' or 'make dt_binding_check' without libyaml-dev installed will fail like this:

scripts/dtc/yamltree.c:9:10: fatal error: yaml.h: No such file or directory

I hope people will notice what to do.

Signed-off-by Masahiro Yamada <[email protected]>

---

 scripts/Makefile.lib      |  9 +++++----
 scripts/dtc/.gitignore    |  1 +
 scripts/dtc/Makefile      | 28 +++++++++++-----------------
 scripts/dtc/dtc-no-yaml.c |  3 +++
 4 files changed, 20 insertions(+), 21 deletions(-)
 create mode 100644 scripts/dtc/dtc-no-yaml.c

scripts/Makefile.lib +5 -4

index 8f9f2abf3d67..ca12412304b8 100644
@@ -246,6 +246,7 @@ quiet_cmd_gzip = GZIP    $@
 # DTC
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc
+DTC_YAML ?= $(objtree)/scripts/dtc/dtc-yaml
 
 # Disable noisy checks by default
 ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)

@@ -286,13 +287,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 
 __cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
 	$(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-	$(DTC) -O $(1) -o $@ -b 0 \
+	$(1) -O $(2) -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
 quiet_cmd_dtc = DTC     $@
-      cmd_dtc = $(call __cmd_dtc,dtb)
+      cmd_dtc = $(call __cmd_dtc,$(DTC),dtb)
 
 $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 	$(call if_changed_dep,dtc)

@@ -303,7 +304,7 @@ DT_BINDING_DIR := Documentation/devicetree/bindings
 DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
 
 quiet_cmd_dtc_yaml = DTCYAML $@
-      cmd_dtc_yaml = $(call __cmd_dtc,yaml)
+      cmd_dtc_yaml = $(call __cmd_dtc,$(DTC_YAML),yaml)
 
 quiet_cmd_dtb_check =	CHECK   $@
       cmd_dtb_check =	$(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@

@@ -313,7 +314,7 @@ define rule_dtc_yaml
 	$(call cmd,dtb_check)
 endef
 
-$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
+$(obj)/%.dt.yaml: $(src)/%.dts $(DTC_YAML) $(DT_TMP_SCHEMA) FORCE
 	$(call if_changed_rule,dtc_yaml)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)

scripts/dtc/.gitignore +1 -0

index b814e6076bdb..e0cf8222c137 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 dtc
+dtc-yaml

scripts/dtc/Makefile +11 ~~-17~~

index ef85f8b7d4a7..c6d7e2b70f08 100644
@@ -1,28 +1,22 @@
 # SPDX-License-Identifier: GPL-2.0
 # scripts/dtc makefile
 
-hostprogs			:= dtc
-always-$(CONFIG_DTC)		+= $(hostprogs)
-always-$(CHECK_DT_BINDING)	+= $(hostprogs)
+# Build the yaml variant for DT schema validation, which requires libyaml
+# development package. The normal build does not need it.
+hostprogs			:= dtc dtc-yaml
+always-$(CONFIG_DTC)		+= dtc
+always-$(CHECK_DTBS)		+= dtc-yaml
+always-$(CHECK_DT_BINDING)	+= dtc-yaml
 
-dtc-objs	:= dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
-		   srcpos.o checks.o util.o
-dtc-objs	+= dtc-lexer.lex.o dtc-parser.tab.o
+common-objs	:= flattree.o fstree.o data.o livetree.o treesource.o \
+		   srcpos.o checks.o util.o dtc-lexer.lex.o dtc-parser.tab.o
+dtc-objs	:= dtc-no-yaml.o $(common-objs)
+dtc-yaml-objs	:= dtc.o yamltree.o $(common-objs)
+HOSTLDLIBS_dtc-yaml := -lyaml
 
 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS := -I $(srctree)/$(src)/libfdt
 
-ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
-ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
-$(error dtc needs libyaml for DT schema validation support. \
-	Install the necessary libyaml development package.)
-endif
-HOST_EXTRACFLAGS += -DNO_YAML
-else
-dtc-objs	+= yamltree.o
-HOSTLDLIBS_dtc	:= $(shell pkg-config yaml-0.1 --libs)
-endif
-
 # Generated files need one more search path to include headers in source tree
 HOSTCFLAGS_dtc-lexer.lex.o := -I $(srctree)/$(src)
 HOSTCFLAGS_dtc-parser.tab.o := -I $(srctree)/$(src)

scripts/dtc/dtc-no-yaml.c new file +3 -0

new file mode 100644
index 000000000000..f3459e45835f
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#define NO_YAML
+#include "dtc.c"

Signature · 1 line

2.25.1

Rob Herring

2020-05-04 19:15 UTC #2 lore raw

Re: [PATCH 2/2] scripts/dtc: compile separate dtc-yaml

Quoted 8 lines · Masahiro Yamada

Marek Behún reported a case where pkg-config fails to detect the libyaml-dev package, which is presumably a bug of the distro.
Irrespective of that, I am not a big fan of pkg-config in the Makefile parse stage. The cost of pkg-config is quite small, but it is evaluated everytime we run make, even when we do 'make mrproper'. This commit changes the Makefile to not rely on pkg-config at all.

I don't really love the solution here... I'm inclined to just make libyaml always required. Anyone building dtbs should care about validating them. However, there's some dts files sprinkled in the tree such as DT unittests and I don't want to break allmodconfig for CI, 0-day, etc. Though eventually we may have to.

Quoted 7 lines

The normal build should not require libyaml-dev while we need to compile dtc with libyaml for the schema check.
Build two dtc variants:
scripts/dtc/dtc for *.dts -> *.dtb
scripts/dtc/dtc-yaml for *.dts -> *.dt.yaml

My longer term plan is to integrate the schema checks into dtc. This would be some sort of plugin to dtc found or specified at run-time. It would eliminate the need for 2 passes of dtc and the 2nd case will go away.

Quoted 22 lines

'make dtbs_check' or 'make dt_binding_check' without libyaml-dev installed will fail like this:
scripts/dtc/yamltree.c:9:10: fatal error: yaml.h: No such file or directory
I hope people will notice what to do.
Signed-off-by Masahiro Yamada <[email protected]>
---
 scripts/Makefile.lib      |  9 +++++----
 scripts/dtc/.gitignore    |  1 +
 scripts/dtc/Makefile      | 28 +++++++++++-----------------
 scripts/dtc/dtc-no-yaml.c |  3 +++
 4 files changed, 20 insertions(+), 21 deletions(-)
 create mode 100644 scripts/dtc/dtc-no-yaml.c
scripts/Makefile.lib +0 -0
index 8f9f2abf3d67..ca12412304b8 100644

scripts/Makefile.lib quiet_cmd_gzip = GZIP $@ · line 249

Quoted 4 lines

scripts/Makefile.lib +1 -0

@@ -246,6 +246,7 @@ quiet_cmd_gzip = GZIP    $@
 # DTC
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc
+DTC_YAML ?= $(objtree)/scripts/dtc/dtc-yaml

Can we make 'DTC' override both and keep this an internal detail.

Quoted 60 lines

scripts/Makefile.lib +4 -4

 # Disable noisy checks by default
 ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)

@@ -286,13 +287,13 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 
 __cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
        $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-       $(DTC) -O $(1) -o $@ -b 0 \
+       $(1) -O $(2) -o $@ -b 0 \
                $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
                -d $(depfile).dtc.tmp $(dtc-tmp) ; \
        cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
 quiet_cmd_dtc = DTC     $@
-      cmd_dtc = $(call __cmd_dtc,dtb)
+      cmd_dtc = $(call __cmd_dtc,$(DTC),dtb)
 
 $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
        $(call if_changed_dep,dtc)

@@ -303,7 +304,7 @@ DT_BINDING_DIR := Documentation/devicetree/bindings
 DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
 
 quiet_cmd_dtc_yaml = DTCYAML $@
-      cmd_dtc_yaml = $(call __cmd_dtc,yaml)
+      cmd_dtc_yaml = $(call __cmd_dtc,$(DTC_YAML),yaml)
 
 quiet_cmd_dtb_check =  CHECK   $@
       cmd_dtb_check =  $(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@

@@ -313,7 +314,7 @@ define rule_dtc_yaml
        $(call cmd,dtb_check)
 endef
 
-$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
+$(obj)/%.dt.yaml: $(src)/%.dts $(DTC_YAML) $(DT_TMP_SCHEMA) FORCE
        $(call if_changed_rule,dtc_yaml)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)

scripts/dtc/.gitignore +1 -0

index b814e6076bdb..e0cf8222c137 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 dtc
+dtc-yaml

scripts/dtc/Makefile +5 -3

index ef85f8b7d4a7..c6d7e2b70f08 100644
@@ -1,28 +1,22 @@
 # SPDX-License-Identifier: GPL-2.0
 # scripts/dtc makefile
 
-hostprogs                      := dtc
-always-$(CONFIG_DTC)           += $(hostprogs)
-always-$(CHECK_DT_BINDING)     += $(hostprogs)
+# Build the yaml variant for DT schema validation, which requires libyaml
+# development package. The normal build does not need it.
+hostprogs                      := dtc dtc-yaml
+always-$(CONFIG_DTC)           += dtc
+always-$(CHECK_DTBS)           += dtc-yaml

scripts/dtc/Makefile line 15

Quoted 10 lines

scripts/dtc/Makefile +6 -3

+always-$(CHECK_DT_BINDING)     += dtc-yaml
 
-dtc-objs       := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
-                  srcpos.o checks.o util.o
-dtc-objs       += dtc-lexer.lex.o dtc-parser.tab.o
+common-objs    := flattree.o fstree.o data.o livetree.o treesource.o \
+                  srcpos.o checks.o util.o dtc-lexer.lex.o dtc-parser.tab.o
+dtc-objs       := dtc-no-yaml.o $(common-objs)
+dtc-yaml-objs  := dtc.o yamltree.o $(common-objs)
+HOSTLDLIBS_dtc-yaml := -lyaml

Does this work for yocto? As we had this before commit 067c650c456e. Not clear if this changed for any reason or just 'let's use pkg-config everywhere'.

Or is there another way to fix yocto issue and we can just check the header exists. I assume yocto needs some prefix in front of '/usr/include/yaml.h'?

Quoted 29 lines

scripts/dtc/Makefile +0 ~~-11~~

 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS := -I $(srctree)/$(src)/libfdt
 
-ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
-ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
-$(error dtc needs libyaml for DT schema validation support. \
-       Install the necessary libyaml development package.)
-endif
-HOST_EXTRACFLAGS += -DNO_YAML
-else
-dtc-objs       += yamltree.o
-HOSTLDLIBS_dtc := $(shell pkg-config yaml-0.1 --libs)
-endif
-
 # Generated files need one more search path to include headers in source tree
 HOSTCFLAGS_dtc-lexer.lex.o := -I $(srctree)/$(src)
 HOSTCFLAGS_dtc-parser.tab.o := -I $(srctree)/$(src)

scripts/dtc/dtc-no-yaml.c new file +3 -1

new file mode 100644
index 000000000000..f3459e45835f
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#define NO_YAML
+#include "dtc.c"
--

2.25.1

Masahiro Yamada

author

2020-05-05 03:05 UTC #3 lore raw

Re: [PATCH 2/2] scripts/dtc: compile separate dtc-yaml

Quoted 22 lines · Rob Herring

Quoted 8 lines · Masahiro Yamada
Marek Behún reported a case where pkg-config fails to detect the libyaml-dev package, which is presumably a bug of the distro.
Irrespective of that, I am not a big fan of pkg-config in the Makefile parse stage. The cost of pkg-config is quite small, but it is evaluated everytime we run make, even when we do 'make mrproper'. This commit changes the Makefile to not rely on pkg-config at all.
I don't really love the solution here... I'm inclined to just make libyaml always required. Anyone building dtbs should care about validating them. However, there's some dts files sprinkled in the tree such as DT unittests and I don't want to break allmodconfig for CI, 0-day, etc. Though eventually we may have to.
Quoted 5 lines
The normal build should not require libyaml-dev while we need to compile dtc with libyaml for the schema check.
Build two dtc variants:

Quoted 7 lines

Quoted 2 lines
scripts/dtc/dtc for *.dts -> *.dtb
scripts/dtc/dtc-yaml for *.dts -> *.dt.yaml
My longer term plan is to integrate the schema checks into dtc. This would be some sort of plugin to dtc found or specified at run-time. It would eliminate the need for 2 passes of dtc and the 2nd case will go away.

OK.

Quoted 10 lines

scripts/Makefile.lib quiet_cmd_gzip = GZIP $@ · line 249
Quoted 7 lines
scripts/Makefile.lib +1 -0
@@ -246,6 +246,7 @@ quiet_cmd_gzip = GZIP    $@
 # DTC
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc
+DTC_YAML ?= $(objtree)/scripts/dtc/dtc-yaml
Can we make 'DTC' override both and keep this an internal detail.

No. For parallel building *.dtb and *.dt.yaml,
they must be separate instances.

Quoted 10 lines

scripts/Makefile.lib quiet_cmd_gzip = GZIP $@ · line 250
Quoted 1 line
scripts/Makefile.lib +1 -0
+HOSTLDLIBS_dtc-yaml := -lyaml
Does this work for yocto? As we had this before commit 067c650c456e. Not clear if this changed for any reason or just 'let's use pkg-config everywhere'.
Or is there another way to fix yocto issue and we can just check the header exists. I assume yocto needs some prefix in front of '/usr/include/yaml.h'?

My bad - I missed 067c650c456e

We need pkg-config to deal with yocto.

Sorry, I take back this series.

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

author

2020-05-05 08:24 UTC #4 lore raw

Re: [PATCH 2/2] scripts/dtc: compile separate dtc-yaml

Quoted 63 lines · Masahiro Yamada

Quoted 22 lines · Rob Herring
Quoted 8 lines · Masahiro Yamada
Marek Behún reported a case where pkg-config fails to detect the libyaml-dev package, which is presumably a bug of the distro.
Irrespective of that, I am not a big fan of pkg-config in the Makefile parse stage. The cost of pkg-config is quite small, but it is evaluated everytime we run make, even when we do 'make mrproper'. This commit changes the Makefile to not rely on pkg-config at all.
I don't really love the solution here... I'm inclined to just make libyaml always required. Anyone building dtbs should care about validating them. However, there's some dts files sprinkled in the tree such as DT unittests and I don't want to break allmodconfig for CI, 0-day, etc. Though eventually we may have to.
Quoted 5 lines
The normal build should not require libyaml-dev while we need to compile dtc with libyaml for the schema check.
Build two dtc variants:
Quoted 7 lines
Quoted 2 lines
scripts/dtc/dtc for *.dts -> *.dtb
scripts/dtc/dtc-yaml for *.dts -> *.dt.yaml
My longer term plan is to integrate the schema checks into dtc. This would be some sort of plugin to dtc found or specified at run-time. It would eliminate the need for 2 passes of dtc and the 2nd case will go away.
OK.
Quoted 10 lines
scripts/Makefile.lib quiet_cmd_gzip = GZIP $@ · line 249
Quoted 7 lines
scripts/Makefile.lib +1 -0
@@ -246,6 +246,7 @@ quiet_cmd_gzip = GZIP    $@
 # DTC
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc
+DTC_YAML ?= $(objtree)/scripts/dtc/dtc-yaml
Can we make 'DTC' override both and keep this an internal detail.
No. For parallel building *.dtb and *.dt.yaml,
they must be separate instances.
Quoted 10 lines
scripts/Makefile.lib quiet_cmd_gzip = GZIP $@ · line 250
Quoted 1 line
scripts/Makefile.lib +1 -0
+HOSTLDLIBS_dtc-yaml := -lyaml
Does this work for yocto? As we had this before commit 067c650c456e. Not clear if this changed for any reason or just 'let's use pkg-config everywhere'.
Or is there another way to fix yocto issue and we can just check the header exists. I assume yocto needs some prefix in front of '/usr/include/yaml.h'?

Quoted 6 lines

My bad - I missed 067c650c456e
We need pkg-config to deal with yocto.
Sorry, I take back this series.

On my second thought, we can search libyaml
in non-standard path without pkg-config.

Kbuild provides a way to specify additional flags
for building host tools.

make HOSTCFLAGS=<install-dir-in-yocto>/include
HOSTLDFLAGS=<install-dir-in-yocto>/lib

Signature · 2 lines

Best Regards
Masahiro Yamada