[PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi

[Simon Glass]

Hi Rasmus,

On Wed, 3 May 2023 at 02:25, Rasmus Villemoes
<rasmus.villemoes at prevas.dk> wrote:
>
> On 01/05/2023 18.32, Simon Glass wrote:
> > Hi Rasmus,
> >
> > On Mon, 1 May 2023 at 02:49, Rasmus Villemoes
> > <rasmus.villemoes at prevas.dk> wrote:
> >>
> >> On 27/04/2023 19.31, Tom Rini wrote:
> >>>>
> >>>> Well, I'm not sure there's a use case for building all of the extra
> >>>> device trees. I think what I'll do right now is fire off a CI run (or a
> >>>> few, in the event of problems) where we just use the logic of
> >>>> 3609e1dc5f4d and see what falls down.
> >>>
> >>> So this gets us a few failures.  You can see them on
> >>> https://source.denx.de/u-boot/u-boot/-/jobs/618127 but one type of
> >>> failure seems to be the case where CONFIG_DEFAULT_DEVICE_TREE isn't
> >>> contained in CONFIG_OF_LIST (ls1088aqds_tfa for example) and the other
> >>> case is where CONFIG_OF_LIST != CONFIG_SPL_OF_LIST and this fails
> >>> because fdtgrep runs NOT on spl/arch/.../foo.dtb but rather
> >>> arch/.../foo.dtb and so we don't have the dtb file around.
> >>>
> >>
> >> Hm, the former sounds like a bug in the defconfig, the second sounds
> >> like a legit use case (or why would we have SPL_OF_LIST). Anyway, both
> >> should be fixable by just changing the logic of scripts/Makefile.dts a
> >> little; say add the union of DEFAULT_DEVICE_TREE, OF_LIST and
> >> SPL_OF_LIST to dtb-y. Something like
> >>
> >> diff --git a/scripts/Makefile.dts b/scripts/Makefile.dts
> >> index 2561025da8..5e2429c617 100644
> >> --- a/scripts/Makefile.dts
> >> +++ b/scripts/Makefile.dts
> >> @@ -1,3 +1,3 @@
> >>  # SPDX-License-Identifier: GPL-2.0+
> >>
> >> -dtb-y += $(patsubst %,%.dtb,$(subst ",,$(CONFIG_$(SPL_)OF_LIST)))
> >> +dtb-y += $(patsubst %,%.dtb,$(subst ",,$(CONFIG_DEFAULT_DEVICE_TREE)
> >> $(CONFIG_OF_LIST) $(CONFIG_SPL_OF_LIST)))
> >
> > I think we should require that all the DTs in the list are already
> > built using the standard rule.
> >
> > i.e. Makefile.dts should just have a check that OF_LIST doesn't bring
> > in anything new. If it does, then the build should fail.
>
> I disagree.
>
> IMO, having those enourmous lists is unmaintainable, and
> as I've pointed out, actively misleading because (like it or not), the
> result of building foo.dtb depends (or to be pedantically correct, _may
> depend_) on whether we're building foo_defconfig or bar_defconfig,
> despite both foo and bar being members of the same SOC family or vendor
> or however foo.dtb and bar.dtb have been categorized.

That's actually not how it is supposed to work, though.

>
> > The list is really about which ones should be put into the FIT. We
> > shouldn't be putting in things that don't exist normally for that SoC.
>
> Yes, and I'm not talking about changing any of _that_. I'm just saying:
> The board, in the form of the defconfig, already provides information on
> which dtbs can be relevant for any bootphase, so let's just build the
> union of those, _but nothing else_.

But then we end up with lots of TARGET rules. Plus the Makefile
no-longer describes which DTs are built?? Perhaps I just misunderstand
where you are heading.

>
> > Meanwhile I think we should move towards prohibiting CONFIG_TARGET_...
> > in Makefile rules,
>
> I think we can get rid of all of it, or perhaps with some exceptions for
> sandbox and the like. I mean, Tom's quick test didn't show that many
> problems from just nuking almost all of arch/*/dts/Makefile.
>  since this is creating some of this problem. i.e.
> > we should do what Linux does.
>
> I don't think so. Linux (and in general an OS kernel) is in a completely
> different situation than a bootloader. It's possible to build one kernel
> that will boot on many different boards with just an appropriate dtb
> being passed. A bootloader will always need to be built for the specific
> target (or for a very close family of targets); you can't really imagine
> building an imx_defconfig u-boot binary and expect that to run on all
> imx-boards.

Actually that's really not true, at least not as broadly as you have
said it. The original purpose of bringing DT into U-Boot was to allow
an exynos5 build to run on 3-4 different boards, just with the DT. As
you have seen we have boards that provide an SPL_OF_LIST to deal with
differences relevant to U-Boot. The DEFAULT_DEVICE_TREE for a board is
really just that. It should be possible to use a different DT with the
same U-Boot binary and have it work on a different board (with the
same SoC).

If you think about the implications of that, it means that we need to
use compatible strings for board features, not #ifdefs and special C
files.

What sort of things are ending up in the DT that make it build
differently from other boards? Is it Kconfig options? I am aware of
this on x86, since there is a common u-boot.dtsi for all boards.

We definitely take a risk by including Kconfig options in the DT, but
I would hope that DT differences are in the board's .dts, not in
shared .dtsi files.

>
> That said, there's another thing "Linux does", at least right now for
> arm64 and soonish also for arm32, namely putting dts files in individual
> vendor directories. _That_ I'd really love to see happen in U-Boot as
> well, because it's not just the 1400 line Makefile that's unmaintanable,
> it's also the 2600+ files in one directory.

I never list the directory, but yes it would be great to move to how
Linux does this too.

Regards,
Simon