[PATCH] kbuild: clean up code duplication in cmd_fdtoverlay

Masahiro Yamada posted 1 patch 1 month, 2 weeks ago
scripts/Makefile.lib | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
[PATCH] kbuild: clean up code duplication in cmd_fdtoverlay
Posted by Masahiro Yamada 1 month, 2 weeks ago
When resolving a merge conflict, Linus noticed the fdtoverlay command
duplication introduced by commit 49636c5680b9 ("kbuild: verify dtoverlay
files against schema"). He suggested a clean-up.

I eliminated the duplication and refactored the code a little further.

No functional changes are intended, except for the short logs.

The log will look as follows:

  $ make ARCH=arm64 defconfig dtbs_check
      [ snip ]
    DTC [C] arch/arm64/boot/dts/freescale/imx93-tqma9352-mba93xxca.dtb
    DTC [C] arch/arm64/boot/dts/freescale/imx93-tqma9352-mba93xxla.dtb
    DTC [C] arch/arm64/boot/dts/freescale/imx93-var-som-symphony.dtb
    DTC [C] arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb
    DTC     arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-imx219.dtbo
    OVL [C] arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-imx219.dtb

The tag [C] indicates that the schema check is executed.

Link: https://lore.kernel.org/lkml/CAHk-=wiF3yeWehcvqY-4X7WNb8n4yw_5t0H1CpEpKi7JMjaMfw@mail.gmail.com/#t
Requested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.lib | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index fe3668dc4954..207325eaf1d1 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -400,26 +400,23 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
 	$(call if_changed,wrap_S_dtb)
 
-quiet_cmd_dtc = DTC     $@
+quiet_dtb_check_tag = $(if $(dtb-check-enabled),[C],   )
+cmd_dtb_check = $(if $(dtb-check-enabled),; $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true)
+
+quiet_cmd_dtc = DTC $(quiet_dtb_check_tag) $@
 cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
 	$(DTC) -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
-	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
-
-DT_CHECK_CMD = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA)
+	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) \
+	$(cmd_dtb_check)
 
 # NOTE:
 # Do not replace $(filter %.dtb %.dtbo, $^) with $(real-prereqs). When a single
 # DTB is turned into a multi-blob DTB, $^ will contain header file dependencies
 # recorded in the .*.cmd file.
-ifneq ($(CHECK_DTBS),)
-quiet_cmd_fdtoverlay = DTOVLCH $@
-      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) ; $(DT_CHECK_CMD) $@ || true
-else
-quiet_cmd_fdtoverlay = DTOVL   $@
-      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^)
-endif
+quiet_cmd_fdtoverlay = OVL $(quiet_dtb_check_tag) $@
+      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) $(cmd_dtb_check)
 
 $(multi-dtb-y): FORCE
 	$(call if_changed,fdtoverlay)
@@ -430,16 +427,11 @@ DT_CHECKER ?= dt-validate
 DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
 DT_BINDING_DIR := Documentation/devicetree/bindings
 DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json
-
-quiet_cmd_dtb =	DTC_CHK $@
-      cmd_dtb =	$(cmd_dtc) ; $(DT_CHECK_CMD) $@ || true
-else
-quiet_cmd_dtb = $(quiet_cmd_dtc)
-      cmd_dtb = $(cmd_dtc)
+dtb-check-enabled = $(if $(filter %.dtb, $@),y)
 endif
 
 $(obj)/%.dtb: $(obj)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
-	$(call if_changed_dep,dtb)
+	$(call if_changed_dep,dtc)
 
 $(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
 	$(call if_changed_dep,dtc)
-- 
2.43.0
Re: [PATCH] kbuild: clean up code duplication in cmd_fdtoverlay
Posted by Nicolas Schier 1 month ago
On Fri, Jul 26, 2024 at 04:23:14AM +0900, Masahiro Yamada wrote:
> When resolving a merge conflict, Linus noticed the fdtoverlay command
> duplication introduced by commit 49636c5680b9 ("kbuild: verify dtoverlay
> files against schema"). He suggested a clean-up.
> 
> I eliminated the duplication and refactored the code a little further.
> 
> No functional changes are intended, except for the short logs.
> 
> The log will look as follows:
> 
>   $ make ARCH=arm64 defconfig dtbs_check
>       [ snip ]
>     DTC [C] arch/arm64/boot/dts/freescale/imx93-tqma9352-mba93xxca.dtb
>     DTC [C] arch/arm64/boot/dts/freescale/imx93-tqma9352-mba93xxla.dtb
>     DTC [C] arch/arm64/boot/dts/freescale/imx93-var-som-symphony.dtb
>     DTC [C] arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb
>     DTC     arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-imx219.dtbo
>     OVL [C] arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-imx219.dtb
> 
> The tag [C] indicates that the schema check is executed.
> 
> Link: https://lore.kernel.org/lkml/CAHk-=wiF3yeWehcvqY-4X7WNb8n4yw_5t0H1CpEpKi7JMjaMfw@mail.gmail.com/#t
> Requested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.lib | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index fe3668dc4954..207325eaf1d1 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -400,26 +400,23 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>  $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
>  	$(call if_changed,wrap_S_dtb)
>  
> -quiet_cmd_dtc = DTC     $@
> +quiet_dtb_check_tag = $(if $(dtb-check-enabled),[C],   )
> +cmd_dtb_check = $(if $(dtb-check-enabled),; $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true)
> +
> +quiet_cmd_dtc = DTC $(quiet_dtb_check_tag) $@
>  cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
>  	$(DTC) -o $@ -b 0 \
>  		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
>  		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
> -	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
> -
> -DT_CHECK_CMD = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA)
> +	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) \
> +	$(cmd_dtb_check)
>  
>  # NOTE:
>  # Do not replace $(filter %.dtb %.dtbo, $^) with $(real-prereqs). When a single
>  # DTB is turned into a multi-blob DTB, $^ will contain header file dependencies
>  # recorded in the .*.cmd file.
> -ifneq ($(CHECK_DTBS),)
> -quiet_cmd_fdtoverlay = DTOVLCH $@
> -      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) ; $(DT_CHECK_CMD) $@ || true
> -else
> -quiet_cmd_fdtoverlay = DTOVL   $@
> -      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^)
> -endif
> +quiet_cmd_fdtoverlay = OVL $(quiet_dtb_check_tag) $@
> +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) $(cmd_dtb_check)
>  
>  $(multi-dtb-y): FORCE
>  	$(call if_changed,fdtoverlay)
> @@ -430,16 +427,11 @@ DT_CHECKER ?= dt-validate
>  DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
>  DT_BINDING_DIR := Documentation/devicetree/bindings
>  DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json
> -
> -quiet_cmd_dtb =	DTC_CHK $@
> -      cmd_dtb =	$(cmd_dtc) ; $(DT_CHECK_CMD) $@ || true
> -else
> -quiet_cmd_dtb = $(quiet_cmd_dtc)
> -      cmd_dtb = $(cmd_dtc)
> +dtb-check-enabled = $(if $(filter %.dtb, $@),y)
>  endif
>  
>  $(obj)/%.dtb: $(obj)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
> -	$(call if_changed_dep,dtb)
> +	$(call if_changed_dep,dtc)
>  
>  $(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
>  	$(call if_changed_dep,dtc)
> -- 
> 2.43.0
> 

Looks good to me, thanks.

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>