On Wed, Sep 15, 2021 at 4:42 AM Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox