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

[Tom Rini]

On Thu, Apr 27, 2023 at 10:24:58AM -0600, Simon Glass wrote:
> Hi Rasmus,
> 
> On Wed, 26 Apr 2023 at 03:04, Rasmus Villemoes
> <rasmus.villemoes at prevas.dk> wrote:
> >
> > On 25/04/2023 22.09, Tom Rini wrote:
> > > On Tue, Apr 25, 2023 at 10:02:01PM +0200, Rasmus Villemoes wrote:
> > >> On 25/04/2023 21.31, Tom Rini wrote:
> > >>> On Fri, Mar 17, 2023 at 11:26:39AM +0100, Rasmus Villemoes wrote:
> > >>>
> > >>
> > >>>> Now, the only way to be really sure is to build the world
> > >>>> with/without this patch and check if any .dtb file changes, but I
> > >>>> don't have the means to do that.
> > >>>
> > >>> So, yes, this causes a bunch of fail to builds, as you noted above. The
> > >>> easiest way I think to confirm things before / after would be to make a
> > >>> quick change to tools/buildman/builderthread.py and self.CopyFiles line
> > >>> for keep_outputs to also keep the dtb or some dts files so you can diff
> > >>> before / after to make sure the end result is the same.
> > >>
> > >> Do the builds outright fail, or do they fail in the sense that some
> > >> machinery detects a change in the binary artifacts? Can you point me at
> > >> one or two CI builds that show this?
> > >
> > > They outright fail to build, mt8516_pumpkin is the one I was testing
> > > with.
> >
> > Gah, of course.
> >
> > dtb-$(CONFIG_ARCH_MEDIATEK) += \
> >         mt7622-rfb.dtb \
> >         mt7623a-unielec-u7623-02-emmc.dtb \
> >         mt7622-bananapi-bpi-r64.dtb \
> >         mt7623n-bananapi-bpi-r2.dtb \
> >         mt7629-rfb.dtb \
> >         mt7981-rfb.dtb \
> >         mt7981-emmc-rfb.dtb \
> >         mt7981-sd-rfb.dtb \
> >         mt7986a-rfb.dtb \
> >         mt7986b-rfb.dtb \
> >         mt7986a-sd-rfb.dtb \
> >         mt7986b-sd-rfb.dtb \
> >         mt7986a-emmc-rfb.dtb \
> >         mt7986b-emmc-rfb.dtb \
> >         mt8183-pumpkin.dtb \
> >         mt8512-bm1-emmc.dtb \
> >         mt8516-pumpkin.dtb \
> >         mt8518-ap1-emmc.dtb
> >
> > means that we end up building a million .dtbs that are not actually
> > relevant to the board we're building for, and the value of
> > CONFIG_SYS_BOARD==mt8516 is of course completely inappropriate for
> > mt7622-rfb.dtb, and the nodes mentioned in mt8516-u-boot.dtsi don't
> > exist in mt7622...
> >
> > [To add insult to injury, it seems that currently mt8516-u-boot.dtsi is
> > not actually being included when building mt8516-pumpkin.dtb, but it
> > seems that the intention very much is that it should be - except that
> > mt8516-u-boot.dtsi has a typo (it refers to a label topckgen_ , but the
> > trailing underscore shouldn't be there) - confirming that it does in
> > fact not get used.]
> >
> > I wonder why this isn't already a problem, but I guess that in practice
> > we never hit the CONFIG_SYS_CPU or CONFIG_SYS_VENDOR cases.
> >
> > Not sure what to do. I think it's a little counterproductive to build
> > all these .dtbs when they are not needed, and silly to have to add one's
> > .dtb to some semi-random list - which is why I pushed for 3609e1dc5f4d
> > to get in.
> >
> > And since we very much allow the .dtbs to depend on various CONFIG_
> > settings - both because different .configs can cause different
> > -u-boot-dtsi files to get included, and also because we allow direct use
> > of CONFIG_* values (or in #if, #ifdef), there's no guarantee that the
> > mt7629-rfb.dtb built with mt8516_pumpkin_defconfig is identical to the
> > one built with mt7629_rfb_defconfig. So what exactly is the point of
> > building all those irrelevant .dtbs?
> 
> Well we try to have one makefile rule for the SoC family, rather than
> one for each board, as it is a pain to deal with lots of rules. We
> sort-of follow what Linux does here.

Right, but Linux is disinclined to build targets that support a single
board and the common case is you add a new platform with "just" the DTS
files. That's not how we build tho.

> DT files are supposed to be stand-alone for the most part, although we
> do add binding includes. Since we include the autoconf you can use
> CONFIG options also, but using board-specific CONFIG options is not
> good practice, I think. After all, our goal is to upstream all these
> files, and CONFIG_SYS_BOARD won't be defined in Linux.

Right but note that the options we're talking about are the ones to set
the device tree (or trees) that are valid to the resulting binary.

-- 
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/20230427/84b99e55/attachment.sig>