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

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

[Trent Piepho]

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.

The advantage is greater when an external build system, such as Yocto or
Buildroot, is being used to build Barebox.  The build system can drop in
a dts fragment to partition flash and build from unaltered Barebox
source.  This avoids the need for cumbersome maintenance of patch files
to modify Barebox's source for each flash partition layout.

Two variables are added, which function identically.  The idea is that
one can be used in Barebox in-tree or out-of-tree defconfig files and
the other injected by an external build system.  Injecting the latter
variable will not clobber the value of the former.

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.

Signed-off-by: Trent Piepho <trent.piepho@igorinstitute.com>
---
 common/Kconfig       | 20 ++++++++++++++++++++
 scripts/Makefile.lib |  7 ++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/common/Kconfig b/common/Kconfig
index a9feae2ae..1b95ded87 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1091,6 +1091,26 @@ config SYSTEMD_OF_WATCHDOG
 	  in the kernel device tree. If the kernel is booted without a device
 	  tree or with one that lacks aliases, nothing is added.
 
+config EXTRA_DTS_FRAGMENTS
+	string "additional dts file fragments"
+	depends on OFTREE
+	help
+	  List of dts fragment files that will be appended to Barebox's device
+	  tree(s) source when building the dtb file(s).  If multiple files are
+	  listed, they will be appended in order.  Relative filenames will use
+	  the dtc include search path.
+
+config EXTERNAL_DTS_FRAGMENTS
+	string "external dts file fragments"
+	depends on OFTREE
+	help
+	  This is like EXTRA_DTS_FRAGMENTS, but it's intended that it not be put
+	  into Barebox defconfig files and instead used by an external build
+	  system, like Yocto or buildroot, to add dts fragments from outside the
+	  Barebox source tree into the Barebox build.  The build system can
+	  override this variable without modifying anything set by the Barebox
+	  defconfig.
+
 menu "OP-TEE loading"
 
 config OPTEE_SIZE
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 80d76b177..093dd3cdd 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -201,6 +201,7 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(__cpp_flags)
 ld_flags       = $(KBUILD_LDFLAGS) $(ldflags-y)
 
 dtc_cpp_flags  = -Wp,-MD,$(depfile).pre -nostdinc                        \
+		 -Wp,-MT,$(basename $(notdir $@)).o                      \
 		 -I$(srctree)/arch/$(SRCARCH)/dts/include		 \
 		 -I$(srctree)/dts/include                                \
 		 -I$(srctree)/include                                    \
@@ -335,8 +336,12 @@ cmd_dt_S_dtb = $(srctree)/scripts/gen-dtb-s $(subst -,_,$(*F)) $< $(CONFIG_IMD)
 $(obj)/%.dtb.S: $(obj)/%.dtb $(srctree)/scripts/gen-dtb-s FORCE
 	$(call if_changed,dt_S_dtb)
 
+dts-frags = $(subst $(quote),,$(CONFIG_EXTRA_DTS_FRAGMENTS) $(CONFIG_EXTERNAL_DTS_FRAGMENTS))
 quiet_cmd_dtc = DTC     $@
-cmd_dtc = $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
+# For compatibility between make 4.2 and 4.3
+H := \#
+cmd_dtc = /bin/echo -e $(foreach f,$< $(dts-frags),'$(H)include "$(f)"\n') | \
+	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
 	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
 		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
 		-i $(srctree)/dts/src/$(SRCARCH) \
-- 
2.31.1


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

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

[Ahmad Fatoum]

Hello Trent,

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.
> 
> The advantage is greater when an external build system, such as Yocto or
> Buildroot, is being used to build Barebox.  The build system can drop in
> a dts fragment to partition flash and build from unaltered Barebox
> source.  This avoids the need for cumbersome maintenance of patch files
> to modify Barebox's source for each flash partition layout.> 
> Two variables are added, which function identically.  The idea is that
> one can be used in Barebox in-tree or out-of-tree defconfig files and
> the other injected by an external build system.  Injecting the latter
> variable will not clobber the value of the former.

I don't see the utility of using this new mechanism for in-tree boards.
I suggest dropping EXTRA_DTS_FRAGMENTS.
 
> 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?
Blindly applying fragments doesn't mesh well with multi-image.
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.

Cheers,
Ahmad

> 
> Signed-off-by: Trent Piepho <trent.piepho@igorinstitute.com>
> ---
>  common/Kconfig       | 20 ++++++++++++++++++++
>  scripts/Makefile.lib |  7 ++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index a9feae2ae..1b95ded87 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1091,6 +1091,26 @@ config SYSTEMD_OF_WATCHDOG
>  	  in the kernel device tree. If the kernel is booted without a device
>  	  tree or with one that lacks aliases, nothing is added.
>  
> +config EXTRA_DTS_FRAGMENTS
> +	string "additional dts file fragments"
> +	depends on OFTREE
> +	help
> +	  List of dts fragment files that will be appended to Barebox's device
> +	  tree(s) source when building the dtb file(s).  If multiple files are
> +	  listed, they will be appended in order.  Relative filenames will use
> +	  the dtc include search path.
> +
> +config EXTERNAL_DTS_FRAGMENTS
> +	string "external dts file fragments"
> +	depends on OFTREE
> +	help
> +	  This is like EXTRA_DTS_FRAGMENTS, but it's intended that it not be put
> +	  into Barebox defconfig files and instead used by an external build
> +	  system, like Yocto or buildroot, to add dts fragments from outside the
> +	  Barebox source tree into the Barebox build.  The build system can
> +	  override this variable without modifying anything set by the Barebox
> +	  defconfig.
> +
>  menu "OP-TEE loading"
>  
>  config OPTEE_SIZE
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 80d76b177..093dd3cdd 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -201,6 +201,7 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(__cpp_flags)
>  ld_flags       = $(KBUILD_LDFLAGS) $(ldflags-y)
>  
>  dtc_cpp_flags  = -Wp,-MD,$(depfile).pre -nostdinc                        \
> +		 -Wp,-MT,$(basename $(notdir $@)).o                      \
>  		 -I$(srctree)/arch/$(SRCARCH)/dts/include		 \
>  		 -I$(srctree)/dts/include                                \
>  		 -I$(srctree)/include                                    \
> @@ -335,8 +336,12 @@ cmd_dt_S_dtb = $(srctree)/scripts/gen-dtb-s $(subst -,_,$(*F)) $< $(CONFIG_IMD)
>  $(obj)/%.dtb.S: $(obj)/%.dtb $(srctree)/scripts/gen-dtb-s FORCE
>  	$(call if_changed,dt_S_dtb)
>  
> +dts-frags = $(subst $(quote),,$(CONFIG_EXTRA_DTS_FRAGMENTS) $(CONFIG_EXTERNAL_DTS_FRAGMENTS))
>  quiet_cmd_dtc = DTC     $@
> -cmd_dtc = $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> +# For compatibility between make 4.2 and 4.3
> +H := \#
> +cmd_dtc = /bin/echo -e $(foreach f,$< $(dts-frags),'$(H)include "$(f)"\n') | \
> +	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
>  	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
>  		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
>  		-i $(srctree)/dts/src/$(SRCARCH) \
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

[Trent Piepho]

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

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

[Ahmad Fatoum]

Hello Trent,

On 19.09.21 23:51, Trent Piepho wrote:
> 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.

I think we should actively discourage use of this for in-tree configs,
so I prefer removing it.

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

I meant overlay application at compile time. I have no experience with
that and was interested to hear your opinion on it.

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

Early runtime application is possible as well, see
of_overlay_apply_tree(). But that's not what I want to advocate here.

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

In my bubble, barebox is occasionally configured to support multiple
board variants via multi-image when it's not feasible to differentiate
at runtime.

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

So the bubbles do overlap :-)

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

New features shouldn't break multi-image. And adding a new mechanism
that applies a fragment to all device trees can introduce such breakage.

There is an easy way out, define the (sanitized) board name as a macro
and let users worry about it.

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

I see the utility when using e.g. evaluation kits barebox already
supports. Could be useful for DistroKit if RAUC support were to
be added.

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

Yes, upstream code should take multi-image into account.
I guess sticking some

  #define $(subst -,_,$<) into the echo'ed string should be enough

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

I usually avoid Yocto MACHINEs for board variants. Build same rootfs
for both variants, ship two device trees and reference them either
from FIT configurations or bootloader specs and let the bootloader worry
about selecting the correct DT to boot.

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

[Trent Piepho]

On Mon, Sep 20, 2021 at 2:02 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> >>> 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?
>
> I meant overlay application at compile time. I have no experience with
> that and was interested to hear your opinion on it.

This could be done, but I think overall it is worse.  Additional
fragments can be made into dtbo files and then fdtoverlay from dtc
package can apply them to the main dtb at compile time.  But there are
drawbacks:
dtb must be compiled with symbols, which makes it larger.  And
non-fragment users get a different dtb than before for no reason.
One can not use the preprocessor to control overlays per board.  Such
as changing a node name from <&nand> to <&emmc> based on board or
enabling/disable the entire fragment.
The overlay can not delete nodes or properties, i.e. /delete-property/
and /delete-node/ are not useful.

A possible benefit of overlays is if an overlay is used in many
different images, then it could be compiled only once.  But dtc is not
any slower to compile a dts fragment append to the main dts than
fdoverlay is to apply an already compiled dtbo to the main dtb, so
there is not even a build time improvement here.

So, I can see only disadvantages and not advantages for compile time overlays.

Maybe one could ship a firmware update with a pool of overlays, then
firmware update installation could construct a final dtb from hardware
variant appropriate overlays.  That would be a way to make use of
non-dynamic overlays.  Otherwise I think there is no advantage to
non-runtime applied overlays over appended dts fragments.

>
> There is an easy way out, define the (sanitized) board name as a macro
> and let users worry about it.

Ok, I can add that.  I think the same identifier as constructed for
the C symbol of the compiled in dtb bytes will work.


> >
> > That is the use case here. So I can use rauc with just the standard
> > Barebox source without needing to patch it.
>
> I see the utility when using e.g. evaluation kits barebox already
> supports. Could be useful for DistroKit if RAUC support were to
> be added.

I used this originally for a devkit.  But it is Variscite's DART-6UL,
which is not supported in Barebox.  So I added support.  But not just
support for the one thing my FW does, support as a proper Barebox
devkit target.

Now I want to change the flash partitions for my specific FW design
and also want RAUC and barebox-state.  Should I include this in my
Barebox devkit support?  No one else wants it, at least not as I have
implemented it.   Should other users submit patches to Barebox to
change the partitions to what they want?  Of course not.

My answer is I should not have to patch Barebox to do this.  I do not
patch Barebox to change the defconfig I'm building with.  I do not
patch RAUC to configure it.  I do not patch wic to change my SD card
partitions.

Now we have made a custom board for the next phase and I have added a
new board to Barebox for this.  But still I use external dts fragments
despite having my FW specific Barebox git repo.  Because I do not want
FW system configuration to be done inside the bootloader codebase.  It
is much better if it is part of my FW git repo that configures
everything else in the system too.

In the old days, we had to patch a header in U-Boot to change *any*
bootloader configuration. Yuck! Then we had Barebox and it used the
kernel config system and we could have an external defconfig.  So much
better!  And Linux had board code with compiled in driver
configuration data, but then we got OF device trees and it was much
better.

So I want external dts files with Barebox.  Complete external dts is
really hard to do with the way Barebox images work.  But really, I
never write a complete dts from scatch. It is always a modification of
a base dts.  This patch to Barebox is not very complex and it lets me
do anything I have ever wanted to do with external Linux dts files in
Barebox.

>
> > 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.
>
> I usually avoid Yocto MACHINEs for board variants. Build same rootfs
> for both variants, ship two device trees and reference them either
> from FIT configurations or bootloader specs and let the bootloader worry
> about selecting the correct DT to boot.

When I have done one FW for multiple boards, I've always been able to
use the same bootloader and have it, or a preloader, determine board,
usually by resistor strapping on a GPIO or ADC, so it is easy to use
without drivers and to keep consistent on each board variant.  But if
one simply could not do that, then I see that multiple images from a
single Barebox machine target would be useful.  Also useful for
Buildroot, which is not as sophisticated as Yocto when it comes to
building a package in different ways.  One gets one target or one
host.  Not target-arm, target-arm-machineA, target-arm-machineB, etc.
like Yocto.

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

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

[Ahmad Fatoum]

Hi,

On 20.09.21 22:43, Trent Piepho wrote:
> On Mon, Sep 20, 2021 at 2:02 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>>>>> 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?
>>
>> I meant overlay application at compile time. I have no experience with
>> that and was interested to hear your opinion on it.
> 
> This could be done, but I think overall it is worse.  Additional
> fragments can be made into dtbo files and then fdtoverlay from dtc
> package can apply them to the main dtb at compile time.  But there are
> drawbacks:
> dtb must be compiled with symbols, which makes it larger.  And
> non-fragment users get a different dtb than before for no reason.
> One can not use the preprocessor to control overlays per board.  Such
> as changing a node name from <&nand> to <&emmc> based on board or
> enabling/disable the entire fragment.
> The overlay can not delete nodes or properties, i.e. /delete-property/
> and /delete-node/ are not useful.
> 
> A possible benefit of overlays is if an overlay is used in many
> different images, then it could be compiled only once.  But dtc is not
> any slower to compile a dts fragment append to the main dts than
> fdoverlay is to apply an already compiled dtbo to the main dtb, so
> there is not even a build time improvement here.
> 
> So, I can see only disadvantages and not advantages for compile time overlays.
> 
> Maybe one could ship a firmware update with a pool of overlays, then
> firmware update installation could construct a final dtb from hardware
> variant appropriate overlays.  That would be a way to make use of
> non-dynamic overlays.  Otherwise I think there is no advantage to
> non-runtime applied overlays over appended dts fragments.

Thanks for the clarification. Doesn't sound appropriate indeed.

>> There is an easy way out, define the (sanitized) board name as a macro
>> and let users worry about it.
> 
> Ok, I can add that.  I think the same identifier as constructed for
> the C symbol of the compiled in dtb bytes will work.

Ye, that would be nice.

>>> That is the use case here. So I can use rauc with just the standard
>>> Barebox source without needing to patch it.
>>
>> I see the utility when using e.g. evaluation kits barebox already
>> supports. Could be useful for DistroKit if RAUC support were to
>> be added.
> 
> I used this originally for a devkit.  But it is Variscite's DART-6UL,
> which is not supported in Barebox.  So I added support.  But not just
> support for the one thing my FW does, support as a proper Barebox
> devkit target.
> 
> Now I want to change the flash partitions for my specific FW design
> and also want RAUC and barebox-state.  Should I include this in my
> Barebox devkit support?  No one else wants it, at least not as I have
> implemented it.   Should other users submit patches to Barebox to
> change the partitions to what they want?  Of course not.
> 
> My answer is I should not have to patch Barebox to do this.  I do not
> patch Barebox to change the defconfig I'm building with.  I do not
> patch RAUC to configure it.  I do not patch wic to change my SD card
> partitions.
> 
> Now we have made a custom board for the next phase and I have added a
> new board to Barebox for this.  But still I use external dts fragments
> despite having my FW specific Barebox git repo.  Because I do not want
> FW system configuration to be done inside the bootloader codebase.  It
> is much better if it is part of my FW git repo that configures
> everything else in the system too.

If all you need to change is config/DT/environment, I guess this works.

We often do need customer-specific board code (detect optional USB stick,
apply device tree fixups, address some hardware quirk...), which is why
this approach here was a bit new to me.

> In the old days, we had to patch a header in U-Boot to change *any*
> bootloader configuration. Yuck! Then we had Barebox and it used the
> kernel config system and we could have an external defconfig.  So much
> better!  And Linux had board code with compiled in driver
> configuration data, but then we got OF device trees and it was much
> better.
> 
> So I want external dts files with Barebox.  Complete external dts is
> really hard to do with the way Barebox images work.  But really, I
> never write a complete dts from scatch. It is always a modification of
> a base dts.  This patch to Barebox is not very complex and it lets me
> do anything I have ever wanted to do with external Linux dts files in
> Barebox.

I see.

>>> 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.
>>
>> I usually avoid Yocto MACHINEs for board variants. Build same rootfs
>> for both variants, ship two device trees and reference them either
>> from FIT configurations or bootloader specs and let the bootloader worry
>> about selecting the correct DT to boot.
> 
> When I have done one FW for multiple boards, I've always been able to
> use the same bootloader and have it, or a preloader, determine board,
> usually by resistor strapping on a GPIO or ADC, so it is easy to use
> without drivers and to keep consistent on each board variant.  But if
> one simply could not do that, then I see that multiple images from a
> single Barebox machine target would be useful.  Also useful for
> Buildroot, which is not as sophisticated as Yocto when it comes to
> building a package in different ways.  One gets one target or one
> host.  Not target-arm, target-arm-machineA, target-arm-machineB, etc.
> like Yocto.
Thanks,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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