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

[Tom Rini]

On Wed, May 03, 2023 at 04:56:18PM -0600, Simon Glass wrote:
> 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.

Er, what do you mean? The Makefiles are supposed to produce functional
binaries.  Which they mostly only do not due to the logic in
arch/*/dts/Makefile but the logic in scripts/Makefile.dts that ensure
the dtbs the config file says it needs are built.

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

Yes, what we're doing is getting rid of all of the TARGET rules, and all
of the SOC rules and so forth, as the config file needs (and almost
always does) say what dtbs are going to be used.

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

In theory, yes, we can support multiple boards with a single binary, and
even do that when they're non-trivial variants.  Many of the TI
defconfigs do this (with some "fun" code at run time to read the EEPROM
and see what we're really on).  But to do that we don't rely on the
Makefile to have some explicit rule for the dtbs to generate but instead
OF_LIST and DEFAULT_DEVICE_TREE.

Take a look at
https://source.denx.de/u-boot/u-boot/-/commit/a05e0d0e6b9103542a1076f9cab0005f400fa072
to see what arch/*/dts/Makefile will look like, once this is done.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230503/c57829cc/attachment.sig>