Re: [PATCH] kbuild: dtc: Allow adding device tree fragments via config

[Trent Piepho <trent.piepho@igorinstitute.com>]

On Wed, Sep 15, 2021 at 4:42 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> On 08.09.21 20:59, Trent Piepho wrote:
> > This introduces config variables that allow adding additional fragments
> > to the Barbox device tree(s).
> >
> > Example uses are adjusting the flash partition layout, adding barebox
> > state varibles, or adding an I2C device.  These can be now be done with
> > build configuration only, without needing to patch the existing dts
> > files in the Barebox source.
> >
>
> I don't see the utility of using this new mechanism for in-tree boards.
> I suggest dropping EXTRA_DTS_FRAGMENTS.

I wasn't sure if this was useful or not.  I didn't have a use case,
but thought somebody might think of one.

> > Preprocessing the dts file gains another layer, where a generated dts
> > source consisting of an include directive for the original dts source is
> > followed by more includes for each fragment.  This is piped to the
> > existing preprocessor call on stdin to avoid another temporary file.
> > cpp/dtc will correctly identify errors in the source files they occur
> > in.  The -MT option is used so the cpp auto-dependencies reference the
> > original dts source and not the generated code passed on stdin.
>
> If you go this route wouldn't you want to apply device tree overlays?

Can the Barebox apply overlays to the *Barebox dtb* when it starts?
of_register_overlay() applies the overlay to the Linux dtb if I'm not
mistaken.  The Barebox dtb is passed to the entry function from the
pbl.  By the time board code runs in the main Barebox it's too late to
modify the Barebox device tree.

> Blindly applying fragments doesn't mesh well with multi-image.

It is working well for me, but it was not done blindly.  I think the
use case was not clear.  This is for Barebox when integrated into a
build system using something like Yocto or Buildroot for someone
creating a complete finished FW for a real product..  If one was only
working on Barebox alone, then you would probably just modify existing
dts files or add a new image build for your board+dts combo.

Since one is configuring Barebox for their specific product, they do
not want to build every board Barebox supports.  They just want their
board built.  So one will not create specific NAND flash partitions
for their exact firmware design and then want those partitions in
every board Barebox supports.  That doesn't make sense.

I have one product I am writing FW for, but I still use multi-image
build because there are different Barebox images for mutually
exclusive choices on how to configure the hardware.

I want to use RAUC and for that I need to create barebox-state
variables in the Barebox device tree.

With this patch, I can create a dts fragment with a barebox-state
node.  Then I configure buildroot to add this to Barebox.  All my
different Barebox images now have the barebox-state node, since the
fragment works for all them.

I commit to my FW repository (which would be like a product specific
Yocto meta-layer) the barebox-state dts fragment and the RAUC system
config that references those state variables.  They are both in the
same repository and if the variables change, both the dts fragment and
the RAUC config can be updated in the same atomic commit.

So now I have added rauc support entirely from the Buildroot
configuration menu with added dts fragment and rauc config files in my
FW repository.  *I did not need to apply any patches to the Barebox
code*

That is the use case here. So I can use rauc with just the standard
Barebox source without needing to patch it.

Of course there are many other things one would probably also do this
way, like changing an expansion header from I2C to UART or
partitioning NAND flash.

I do not see how one could do this with overlays.  Even if it was
possible to apply an overlay in the pbl, one would still need to patch
the lowlevel board code to find and apply those overlays, which
defeats the whole point of not needing to patch the Barebox source.

I'll also mention that Linux can do this too in a way.  If you need to
modify your board dts for Linux, what you do is add an external dts
file in Buildroot/Yocto and have it make a dtb from that.  You don't
need to patch the dts files in the kernel source.  But the Barebox dtb
is not supplied to Barebox by a bootloader like Linux.  The way dtb
generation is integrated into the image Makefile and even the lowlevel
code makes it not practical to add new images to Barebox with only a
make command line argument.

> I assume with overlays, you could skip an overlay if it has a differing
> compatible. If we don't use overlays, you should at least define
> a symbol with the name of the device tree file, so fragments have
> a chance of being multi-image compatible via preprocessor logic.

Certainly one could use the preprocessor.  It would be much easier to
just define what you need in the dts itself.  But one would need to
patch the Barebox dts and the point was to avoid that.  I suppose the
makefile could construct a valid C macro name from the filename and
define it.

But I wonder if one needs this?  If I was building two different
boards under yocto, I would have a machine specific override in my
barebox bbappend to add the only dts fragment for board I was building
for.  Yocto builds a different copy of barebox for each
target/machine.  I do not need barebox to use the preprocessor to turn
off the fragment I am not using.  I would not have yocto give it the
unused fragment in the first place.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox