Re: [PATCH v5 1/2] kbuild: Convert %.dtb(o) targets to single target rules

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kbuild: single-target .dtb and .dtbo rules fail to descend into boot/dts
- [Medium] kbuild: unconditional dtstree prefixing corrupts non-dtstree .dtb targets
--

commit 7e93223635a0ece771bb2b9e12840b3751ca5a16
Author: Rob Herring (Arm) <robh@kernel.org>

kbuild: Convert %.dtb(o) targets to single target rules

This commit replaces the dedicated top-level %.dtb and %.dtbo rules with the
generic kbuild single-target infrastructure, rewriting relative DTB paths to
include $(dtstree) and using dtbs_prepare as the prerequisite for DT-only
single builds.

> diff --git a/Makefile b/Makefile
> index 4ad67b737af7a..c307ee9293bcd 100644
> --- a/Makefile
> +++ b/Makefile

[ ... ]

> @@ -2212,8 +2208,20 @@ ifneq ($(KBUILD_MODPOST_NOFINAL),1)
>  endif
>  	$(Q)rm -f modules.order
>  
> +# For compatibility with existing %.dtb targets without full path
> +single-dtb := $(filter-out $(dtstree)/%, $(filter %.dtb %.dtbo, $(single-no-ko)))
> +single-no-ko := $(filter-out $(single-dtb), $(single-no-ko)) $(addprefix $(dtstree)/, $(single-dtb))

[Severity: Medium]
What happens here when building an in-tree .dtb or .dtbo target located
outside $(dtstree), such as make drivers/of/empty_root.dtb or
make drivers/of/unittest-data/testcases.dtbo, or when $(dtstree) is empty?

Because single-dtb matches any %.dtb or %.dtbo target that does not begin
with $(dtstree)/, targets under drivers/of/ or drivers/clk/ have
$(dtstree)/ unconditionally prepended to their path, rewriting them to
non-existent paths under arch/$(SRCARCH)/boot/dts/drivers/....

In addition, unlike the single-dt-patterns check just below, this rewrite is
not guarded by ifneq ($(dtstree),). On architectures without a boot/dts
directory or during external module M= builds where $(dtstree) is empty,
$(addprefix $(dtstree)/, $(single-dtb)) prepends a leading / to the target.

Should this rewrite be guarded by ifneq ($(dtstree),) and restricted to
paths that actually exist under $(dtstree) (or do not already exist at their
given path)?

> +
>  single-goals := $(addprefix $(build-dir)/, $(single-no-ko))
>  
> +# DT-only targets need host tools, but no architecture-specific preparation.
> +ifneq ($(dtstree),)
> +single-dt-patterns := $(addprefix $(dtstree)/,%.dtb %.dtbo)
> +ifeq ($(filter-out $(single-dt-patterns),$(single-no-ko)),)
> +single-build-prep := dtbs_prepare
> +endif
> +endif
> +
>  KBUILD_MODULES := y
>  
>  endif
> @@ -2225,7 +2233,7 @@ prepare: outputmakefile
>  # make menuconfig etc.
>  # Error messages still appears in the original language
>  PHONY += $(build-dir)
> -$(build-dir): prepare
> +$(build-dir): $(single-build-prep)
>  	$(Q)$(MAKE) $(build)=$@ need-builtin=1 need-modorder=1 $(single-goals)

[Severity: High]
Does routing single .dtb and .dtbo targets through $(build-dir) allow
scripts/Makefile.build to descend into arch/$(SRCARCH)/boot/dts?

Previously, the %.dtb and %.dtbo rules invoked
$(MAKE) $(build)=$(dtstree) $(dtstree)/$@, starting scripts/Makefile.build
directly at obj=arch/$(SRCARCH)/boot/dts. With this change, $(build-dir)
starts at obj=. and descends into obj=arch/$(SRCARCH), which includes
arch/$(SRCARCH)/Kbuild. However, in arch/$(SRCARCH)/Kbuild (for example in
arch/arm64/Kbuild):

    # for cleaning
    subdir- += boot

neither boot nor boot/dts is in obj-y, obj-m, subdir-y, or subdir-m. When
scripts/Makefile.build evaluates single-subdirs at obj=arch/$(SRCARCH):

    single-subdirs := $(foreach d, $(subdir-ym), $(if $(filter $d/%, $(MAKECMDGOALS)), $d))
    single-subdir-goals := $(filter $(addsuffix /%, $(single-subdirs)), $(MAKECMDGOALS))

single-subdirs is empty, so Kbuild stops descending at obj=arch/$(SRCARCH)
and places arch/$(SRCARCH)/boot/dts/<vendor>/<target>.dtb(o) directly into
$(targets) at obj=arch/$(SRCARCH) without ever including
arch/$(SRCARCH)/boot/dts/Makefile or the vendor subdirectory Makefile.

This causes three regressions when building single targets at this commit:

1. Building any single .dtbo target (e.g. make <vendor>/<overlay>.dtbo)
   fails with "No rule to make target", because at obj=arch/$(SRCARCH),
   need-dtbslist, dtb-y, and dtb- are empty, and scripts/Makefile.build:

       ifneq ($(need-dtbslist)$(dtb-y)$(dtb-)$(filter %.dtb %.dtb.o %.dtbo.o,$(targets)),)
       include $(srctree)/scripts/Makefile.dtbs
       endif

   omits %.dtbo from the $(targets) filter, so scripts/Makefile.dtbs is
   never included.

2. Building any composite .dtb target defined via <name>-dtbs := base.dtb
   overlay.dtbo (such as make qcom/apq8016-sbc-usb-host.dtb) fails with
   "No rule to make target" because the vendor Makefile is not included and
   multi-dtb-y remains empty.

3. Building simple .dtb targets matches $(obj)/%.dtb: $(obj)/%.dts at
   obj=arch/$(SRCARCH) with target-stem set to boot/dts/<vendor>/<board>
   instead of <board>, silently dropping per-directory DTC_FLAGS,
   per-target DTC_FLAGS_<stem>, and base-dtb-y automatic -@ symbol
   generation.

Note that a subsequent commit in this series, commit 02ed329e5679 ("kbuild:
Support directory targets for building DTBs"), partially addresses traversal
by passing need-dtbslist=1 for DT builds and adding subdir-y += boot/dts in
scripts/Makefile.build when need-dtbslist is set, though this commit is
broken on its own during bisection (and the %.dtbo filter omission in
scripts/Makefile.build remains if need-dtbslist, dtb-y, and dtb- are empty).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-dtb-dirs-v3-v5-0-dfe13d47aa64@kernel.org?part=1