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

[Tom Rini]

On Wed, Apr 26, 2023 at 11:04:24AM +0200, Rasmus Villemoes 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.]

... ugh.

> 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?
> 
> So obviously my patch cannot go in as-is. But I do think there are some
> things that need to be rethought in our build system.
> 
> Now that all of CONFIG_OF_LIST gets built automatically, couldn't we
> delete most if not all of the dtb-$(CONFIG_SOMETHING) += stanzas?

So yes, I think with 3609e1dc5f4d we should be able to massively delete
arch/arm/dts/Makefile (and all of the rest, too).

> Or to make the build of those extra dtbs a little more deterministic and
> fix the issue with a random/wrong/inapplicable -u-boot.dtsi being picked
> up, perhaps change the %.dtb rule so that the the magic *-u-boot.dtsi
> file (whichever one applies) is only included when $@ is in
> CONFIG_$(SPL_)OF_LIST?

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.

-- 
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/efdd4983/attachment.sig>