diff mbox series

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

Message ID 20240304-b4-upstream-dt-headers-v1-26-b7ff41925f92@linaro.org
State Superseded
Headers show
Series Drop DT upstream compatible dt-binding headers | expand

Commit Message

Caleb Connolly March 4, 2024, 4:51 p.m. UTC
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 <caleb.connolly@linaro.org>
---
 dts/Kconfig          | 24 ++++++++++++++++++++++++
 scripts/Makefile.dts | 17 ++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

Comments

Tony Dinh March 4, 2024, 11:02 p.m. UTC | #1
Hi Caleb,

On Mon, Mar 4, 2024 at 9:24 AM Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
> ---
>  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
> +         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

Perhaps here it should be:
default "marvell" if ARCH_MVEBU || ARCH_KIRKWOOD

All the best,
Tony

> +       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
>
Sumit Garg March 5, 2024, 12:35 p.m. UTC | #2
Hi Caleb,

On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
> ---
>  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.

-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
>
Tom Rini March 5, 2024, 12:42 p.m. UTC | #3
On Tue, Mar 05, 2024 at 06:05:52PM +0530, Sumit Garg wrote:
> Hi Caleb,
> 
> On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
> > ---
> >  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, I think this target is useful as it addresses some of Simon's
earlier concerns with the feature.
Caleb Connolly March 5, 2024, 12:51 p.m. UTC | #4
On 05/03/2024 12:35, Sumit Garg wrote:
> Hi Caleb,
> 
> On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
>> ---
>>  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
>>
Sumit Garg March 5, 2024, 1:14 p.m. UTC | #5
On Tue, 5 Mar 2024 at 18:21, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>
>
> On 05/03/2024 12:35, Sumit Garg wrote:
> > Hi Caleb,
> >
> > On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
> >> ---
> >>  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

Have you tested U-Boot on all of them? IMO, it would be good to make
people aware about supported boards via listing their DTs at least.

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

qcom_defconfig currently only supports sdm845 and qcs404.

> 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.

See following diff:

diff --git a/dts/upstream/src/arm64/Makefile b/dts/upstream/src/arm64/Makefile
index 9a8f6aa35846..ecc15021cb08 100644
--- a/dts/upstream/src/arm64/Makefile
+++ b/dts/upstream/src/arm64/Makefile
@@ -2,6 +2,10 @@

 include $(srctree)/scripts/Makefile.dts

+dtb-$(CONFIG_ARCH_SNAPDRAGON) += qcom/sdm845-db845c.dtb \
+       qcom/sdm845-samsung-starqltechn.dtb \
+       qcom/qcs404-evb-4000.dtb
+
 targets += $(dtb-y)

>
> 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.

Maybe someone cares about build time too but that's not my primary
concern. We shouldn't be giving the false impression that all the DTs
present in the vendor directory are supported by U-Boot.

-Sumit
Caleb Connolly March 5, 2024, 2:15 p.m. UTC | #6
[trimmed CC list a bit as this is getting offtopic for the original thread]

On 05/03/2024 13:14, Sumit Garg wrote:
> On Tue, 5 Mar 2024 at 18:21, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>>
>>
>> On 05/03/2024 12:35, Sumit Garg wrote:
>>> Hi Caleb,
>>>
>>> On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
>>>> ---
>>>>  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
> 
> Have you tested U-Boot on all of them? IMO, it would be good to make
> people aware about supported boards via listing their DTs at least.

Well the "ideal" goal is that every SoC in upstream is supported. All of
the changes I've introduced so far work towards that end, so this is
just another step in that direction. Obviously it's a lofty one, but I
see no reason to intentionally make things harder for ourselves by
gatekeeping what DTB files we build.

I have additional features planned that help here, and plenty more
ideas... But I can confirm that most of the phones (which are pretty
much identical to the reference boards) do indeed "just work" provided
the SoC is supported.

It makes sense to use board/qualcomm/<soc-codename>/MAINTAINERS for this
imo, there we can reference the specific dts files so device maintainers
can be CC'd if there are relevant changes when deviceree-rebasing is
updated.

I would like to update the Qualcomm docs to describe the general
approach here and help guide new contributors. But (as is hopefully
obvious by this email) I'm still very much learning as I go. What do you
think?
> 
>>
>> $ ls -l dts/upstream/src/arm6/qcom/{msm8916,sdm845,msm8996,qcs404}*.dts\
>>         | wc -l
>> 51
>>
> 
> qcom_defconfig currently only supports sdm845 and qcs404.
> 
>> 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.
> 
> See following diff:
> 
> diff --git a/dts/upstream/src/arm64/Makefile b/dts/upstream/src/arm64/Makefile
> index 9a8f6aa35846..ecc15021cb08 100644
> --- a/dts/upstream/src/arm64/Makefile
> +++ b/dts/upstream/src/arm64/Makefile
> @@ -2,6 +2,10 @@
> 
>  include $(srctree)/scripts/Makefile.dts
> 
> +dtb-$(CONFIG_ARCH_SNAPDRAGON) += qcom/sdm845-db845c.dtb \
> +       qcom/sdm845-samsung-starqltechn.dtb \
> +       qcom/qcs404-evb-4000.dtb
> +
>  targets += $(dtb-y)
> 
>>
>> 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.
> 
> Maybe someone cares about build time too but that's not my primary
> concern. We shouldn't be giving the false impression that all the DTs
> present in the vendor directory are supported by U-Boot.
> 
> -Sumit
Sumit Garg March 6, 2024, 11:50 a.m. UTC | #7
On Tue, 5 Mar 2024 at 19:45, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> [trimmed CC list a bit as this is getting offtopic for the original thread]
>
> On 05/03/2024 13:14, Sumit Garg wrote:
> > On Tue, 5 Mar 2024 at 18:21, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> >>
> >>
> >>
> >> On 05/03/2024 12:35, Sumit Garg wrote:
> >>> Hi Caleb,
> >>>
> >>> On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
> >>>> ---
> >>>>  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
> >
> > Have you tested U-Boot on all of them? IMO, it would be good to make
> > people aware about supported boards via listing their DTs at least.
>
> Well the "ideal" goal is that every SoC in upstream is supported. All of
> the changes I've introduced so far work towards that end, so this is
> just another step in that direction. Obviously it's a lofty one, but I
> see no reason to intentionally make things harder for ourselves by
> gatekeeping what DTB files we build.
>
> I have additional features planned that help here, and plenty more
> ideas... But I can confirm that most of the phones (which are pretty
> much identical to the reference boards) do indeed "just work" provided
> the SoC is supported.
>
> It makes sense to use board/qualcomm/<soc-codename>/MAINTAINERS for this
> imo, there we can reference the specific dts files so device maintainers
> can be CC'd if there are relevant changes when deviceree-rebasing is
> updated.
>
> I would like to update the Qualcomm docs to describe the general
> approach here and help guide new contributors. But (as is hopefully
> obvious by this email) I'm still very much learning as I go. What do you
> think?

I am still not convinced that we should be building all the DTBs if
all of them aren't supported. The docs are there to help people about
what they have built rather than the opposite being what you have
built isn't currently supported.

However, I will let others chime in too. Maybe I am missing something.

-Sumit

> >
> >>
> >> $ ls -l dts/upstream/src/arm6/qcom/{msm8916,sdm845,msm8996,qcs404}*.dts\
> >>         | wc -l
> >> 51
> >>
> >
> > qcom_defconfig currently only supports sdm845 and qcs404.
> >
> >> 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.
> >
> > See following diff:
> >
> > diff --git a/dts/upstream/src/arm64/Makefile b/dts/upstream/src/arm64/Makefile
> > index 9a8f6aa35846..ecc15021cb08 100644
> > --- a/dts/upstream/src/arm64/Makefile
> > +++ b/dts/upstream/src/arm64/Makefile
> > @@ -2,6 +2,10 @@
> >
> >  include $(srctree)/scripts/Makefile.dts
> >
> > +dtb-$(CONFIG_ARCH_SNAPDRAGON) += qcom/sdm845-db845c.dtb \
> > +       qcom/sdm845-samsung-starqltechn.dtb \
> > +       qcom/qcs404-evb-4000.dtb
> > +
> >  targets += $(dtb-y)
> >
> >>
> >> 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.
> >
> > Maybe someone cares about build time too but that's not my primary
> > concern. We shouldn't be giving the false impression that all the DTs
> > present in the vendor directory are supported by U-Boot.
> >
> > -Sumit
>
> --
> // Caleb (they/them)
Caleb Connolly March 21, 2024, 8:20 p.m. UTC | #8
Hi Sumit,

On 06/03/2024 11:50, Sumit Garg wrote:
> On Tue, 5 Mar 2024 at 19:45, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> [trimmed CC list a bit as this is getting offtopic for the original thread]
>>
>> On 05/03/2024 13:14, Sumit Garg wrote:
>>> On Tue, 5 Mar 2024 at 18:21, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 05/03/2024 12:35, Sumit Garg wrote:
>>>>> Hi Caleb,
>>>>>
>>>>> On Mon, 4 Mar 2024 at 22:22, Caleb Connolly <caleb.connolly@linaro.org> 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 <caleb.connolly@linaro.org>
>>>>>> ---
>>>>>>  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
>>>
>>> Have you tested U-Boot on all of them? IMO, it would be good to make
>>> people aware about supported boards via listing their DTs at least.
>>
>> Well the "ideal" goal is that every SoC in upstream is supported. All of
>> the changes I've introduced so far work towards that end, so this is
>> just another step in that direction. Obviously it's a lofty one, but I
>> see no reason to intentionally make things harder for ourselves by
>> gatekeeping what DTB files we build.
>>
>> I have additional features planned that help here, and plenty more
>> ideas... But I can confirm that most of the phones (which are pretty
>> much identical to the reference boards) do indeed "just work" provided
>> the SoC is supported.
>>
>> It makes sense to use board/qualcomm/<soc-codename>/MAINTAINERS for this
>> imo, there we can reference the specific dts files so device maintainers
>> can be CC'd if there are relevant changes when deviceree-rebasing is
>> updated.
>>
>> I would like to update the Qualcomm docs to describe the general
>> approach here and help guide new contributors. But (as is hopefully
>> obvious by this email) I'm still very much learning as I go. What do you
>> think?
> 
> I am still not convinced that we should be building all the DTBs if
> all of them aren't supported. The docs are there to help people about
> what they have built rather than the opposite being what you have
> built isn't currently supported.
> 
> However, I will let others chime in too. Maybe I am missing something.

Hi Sumit,

I guess nobody else has very strong opinions on this... I've thought on
it some more and I definitely get your concern about not having a
specific list of supported devices.

However, I think the potential for a device to be supported without
having to send any patches to U-Boot (just upstream the DTS!) is pretty
awesome... Would it be a suitable middleground to maintain a list of
supported Qualcomm devices (and features) in the documentation?

Kind regards,
> 
> -Sumit
> 
>>>
>>>>
>>>> $ ls -l dts/upstream/src/arm6/qcom/{msm8916,sdm845,msm8996,qcs404}*.dts\
>>>>         | wc -l
>>>> 51
>>>>
>>>
>>> qcom_defconfig currently only supports sdm845 and qcs404.
>>>
>>>> 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.
>>>
>>> See following diff:
>>>
>>> diff --git a/dts/upstream/src/arm64/Makefile b/dts/upstream/src/arm64/Makefile
>>> index 9a8f6aa35846..ecc15021cb08 100644
>>> --- a/dts/upstream/src/arm64/Makefile
>>> +++ b/dts/upstream/src/arm64/Makefile
>>> @@ -2,6 +2,10 @@
>>>
>>>  include $(srctree)/scripts/Makefile.dts
>>>
>>> +dtb-$(CONFIG_ARCH_SNAPDRAGON) += qcom/sdm845-db845c.dtb \
>>> +       qcom/sdm845-samsung-starqltechn.dtb \
>>> +       qcom/qcs404-evb-4000.dtb
>>> +
>>>  targets += $(dtb-y)
>>>
>>>>
>>>> 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.
>>>
>>> Maybe someone cares about build time too but that's not my primary
>>> concern. We shouldn't be giving the false impression that all the DTs
>>> present in the vendor directory are supported by U-Boot.
>>>
>>> -Sumit
>>
>> --
>> // Caleb (they/them)
diff mbox series

Patch

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
+	  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