Re: [PATCH RFC 26/26] dts: support building all dtb files for a specific vendor

On 05/03/2024 12:35, Sumit Garg wrote:
> Hi Caleb,
> 
> On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <[email protected]> wrote:
>>
>> This adjusts OF_UPSTREAM to behave more like the kernel by allowing for
>> all the devicetree files for a given vendor to be compiled. This is
>> useful for Qualcomm in particular as most boards are supported by a
>> single U-Boot build just provided with a different DT.
>>
>> Signed-off-by: Caleb Connolly <[email protected]>
>> ---
>>  dts/Kconfig          | 24 ++++++++++++++++++++++++
>>  scripts/Makefile.dts | 17 ++++++++++++++++-
>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/dts/Kconfig b/dts/Kconfig
>> index b9b6367154ef..67d9dc489856 100644
>> --- a/dts/Kconfig
>> +++ b/dts/Kconfig
>> @@ -100,8 +100,32 @@ config OF_UPSTREAM
>>           However, newer boards whose devicetree source files haven't landed in
>>           the dts/upstream subtree, they can override this option to have the
>>           DT build from existing U-Boot tree location instead.
>>
>> +config OF_UPSTREAM_BUILD_VENDOR
>> +       bool "Build all devicetree files for a particular vendor"
>> +       depends on OF_UPSTREAM
>> +       help
>> +         Enable building all devicetree files for a particular vendor. This
> 
> Do we really want to build all the DTBs even if many of those aren't
> supported by U-Boot at all? I would have rather added Makefile targets
> for boards which really supports a single defconfig eg.
> qcom_defconfig.

Yes, for the 4 Qualcomm SoCs currently supported there are 51 dts
targets that ought to be able to run U-Boot to some extent

$ ls -l dts/upstream/src/arm6/qcom/{msm8916,sdm845,msm8996,qcs404}*.dts\
	| wc -l
51

What do you mean by a "makefile target"? Like copying
arch/arm64/boot/dts/qcom/Makefile from Linux? I guess my concern here
would be keeping it in sync, and introducing additional busywork when
porting.

We do have a lot of Qualcomm DTS files, it takes maybe 10 seconds to
compile them all on my machine, but that's only once. With incremental
builds this becomes largely irrelevant.
> 
> -Sumit
> 
>> +         is useful for generic U-Boot configurations where many boards can
>> +         be supported with a single binary.
>> +
>> +         This is only available for platforms using upstream devicetree.
>> +
>> +config OF_UPSTREAM_VENDOR
>> +       string "Vendor to build all upstream devicetree files for"
>> +       depends on OF_UPSTREAM_BUILD_VENDOR
>> +       default "qcom" if ARCH_SNAPDRAGON
>> +       default "rockchip" if ARCH_ROCKCHIP
>> +       default "amlogic" if ARCH_MESON
>> +       default "allwinner" if ARCH_SUNXI
>> +       default "mediatek" if ARCH_MEDIATEK
>> +       default "marvell" if ARCH_MVEBU
>> +       default "xilinx" if ARCH_VERSAL || ARCH_ZYNQ
>> +       default "nvidia" if ARCH_TEGRA
>> +       help
>> +         Select the vendor to build all devicetree files for.
>> +
>>  choice
>>         prompt "Provider of DTB for DT control"
>>         depends on OF_CONTROL
>>
>> diff --git a/scripts/Makefile.dts b/scripts/Makefile.dts
>> index 5e2429c6170c..8005527f3df7 100644
>> --- a/scripts/Makefile.dts
>> +++ b/scripts/Makefile.dts
>> @@ -1,3 +1,18 @@
>>  # SPDX-License-Identifier: GPL-2.0+
>>
>> -dtb-y += $(patsubst %,%.dtb,$(subst ",,$(CONFIG_DEFAULT_DEVICE_TREE) $(CONFIG_OF_LIST) $(CONFIG_SPL_OF_LIST)))
>> +dtb-y += $(patsubst %,%.dtb,\
>> +       $(subst ",,$(CONFIG_DEFAULT_DEVICE_TREE) $(CONFIG_OF_LIST) $(CONFIG_SPL_OF_LIST)))
>> +
>> +ifeq ($(CONFIG_OF_UPSTREAM_BUILD_VENDOR),y)
>> +ifeq ($(CONFIG_ARM64),y)
>> +dt_dir := $(srctree)/dts/upstream/src/arm64
>> +else
>> +dt_dir := $(srctree)/dts/upstream/src/$(ARCH)
>> +endif
>> +
>> +dtb-vendor_dts := $(patsubst %.dts,%.dtb, \
>> +       $(wildcard $(dt_dir)/$(subst ",,$(CONFIG_OF_UPSTREAM_VENDOR))/*.dts))
>> +
>> +dtb-y += $(subst $(dt_dir)/,,$(dtb-vendor_dts))
>> +
>> +endif
>>
>> --
>> 2.44.0
>>

-- 
// Caleb (they/them)