[Buildroot] [PATCH] boot/uboot: add support for custom DT name

[Buildroot] [PATCH] boot/uboot: add support for custom DT name

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

Some u-boot default configuration files could be shared between
targets and used for building images for multiple board types.
The only difference between such builds is the DTB embedded in
in the boot image for each specific platform.
This approach is widely used by Marvell, having the same u-boot
configuration file for the entire SoC family, but allowing builds
of multiple target flavors by supplying the device tree name through
make command parameter DEVICE_TREE=xxx
This patch adds such capability to uboot module of the buildroot.
The custome DT name could be  defined by
BR2_TARGET_UBOOT_CUSTOM_DTS_NAME entry.

Change-Id: I69e193339b0369a736bdf98491b9914d24a54e17
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
---
 boot/uboot/Config.in | 8 ++++++++
 boot/uboot/uboot.mk  | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
index ac6f8bc8c1..c67994229a 100644
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -529,4 +529,12 @@ config BR2_TARGET_UBOOT_CUSTOM_DTS_PATH
 
 endif
 
+config BR2_TARGET_UBOOT_CUSTOM_DTS_NAME
+	string "Device Tree Source name"
+	help
+	  The value of u-boot DEVICE_TREE for usage with the selected board
+	  configuration file. Allows sharing of the same u-boot configuration
+	  for building images for different platforms/boards.
+
+
 endif # BR2_TARGET_UBOOT
diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index cbdfee6ac3..a0342ec6c8 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -135,6 +135,11 @@ UBOOT_MAKE_OPTS += \
 	HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \
 	HOSTLDFLAGS="$(HOST_LDFLAGS)"
 
+UBOOT_CUSTOM_DTS_NAME = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_DTS_NAME))
+ifneq ($(UBOOT_CUSTOM_DTS_NAME),)
+UBOOT_MAKE_OPTS += DEVICE_TREE=$(UBOOT_CUSTOM_DTS_NAME)
+endif
+
 ifeq ($(BR2_TARGET_UBOOT_NEEDS_ATF_BL31),y)
 UBOOT_DEPENDENCIES += arm-trusted-firmware
 UBOOT_MAKE_OPTS += BL31=$(BINARIES_DIR)/bl31.bin
-- 
2.17.1

[Buildroot] [PATCH] boot/uboot: add support for custom DT name

[Thomas Petazzoni]

Hello Kostya,

+Clemens Gruber in Cc, who posted a related U-Boot patch.

On Mon, 24 Dec 2018 15:19:04 +0200, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Some u-boot default configuration files could be shared between
> targets and used for building images for multiple board types.
> The only difference between such builds is the DTB embedded in
> in the boot image for each specific platform.
> This approach is widely used by Marvell, having the same u-boot
> configuration file for the entire SoC family, but allowing builds
> of multiple target flavors by supplying the device tree name through
> make command parameter DEVICE_TREE=xxx
> This patch adds such capability to uboot module of the buildroot.
> The custome DT name could be  defined by
> BR2_TARGET_UBOOT_CUSTOM_DTS_NAME entry.
> 
> Change-Id: I69e193339b0369a736bdf98491b9914d24a54e17
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>

Reading this again, in fact it is very similar to patch
http://patchwork.ozlabs.org/patch/881197/ we already have in the queue.
With a very important distinction: your patch passes DEVICE_TREE=,
while the existing pending patch passes EXT_DTB=. The obvious question
that comes up is: which one is right ? Or are these two orthogonal
things ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [EXT] Re: [PATCH] boot/uboot: add support for custom DT name

[Kostya Porotchkin]

Hi, Thomas,

> -----Original Message-----
> From: buildroot <buildroot-bounces@busybox.net> On Behalf Of Thomas
> Petazzoni
> Sent: Monday, December 24, 2018 15:48
> To: Kostya Porotchkin <kostap@marvell.com>
> Cc: Clemens Gruber <clemens.gruber@pqgruber.com>;
> buildroot at buildroot.org
> Subject: [EXT] Re: [Buildroot] [PATCH] boot/uboot: add support for custom
> DT name
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hello Kostya,
> 
> +Clemens Gruber in Cc, who posted a related U-Boot patch.
> 
> On Mon, 24 Dec 2018 15:19:04 +0200, kostap at marvell.com wrote:
> > From: Konstantin Porotchkin <kostap@marvell.com>
> >
> > Some u-boot default configuration files could be shared between
> > targets and used for building images for multiple board types.
> > The only difference between such builds is the DTB embedded in in the
> > boot image for each specific platform.
> > This approach is widely used by Marvell, having the same u-boot
> > configuration file for the entire SoC family, but allowing builds of
> > multiple target flavors by supplying the device tree name through make
> > command parameter DEVICE_TREE=xxx This patch adds such capability to
> > uboot module of the buildroot.
> > The custome DT name could be  defined by
> > BR2_TARGET_UBOOT_CUSTOM_DTS_NAME entry.
> >
> > Change-Id: I69e193339b0369a736bdf98491b9914d24a54e17
> > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> 
> Reading this again, in fact it is very similar to patch
> http://patchwork.ozlabs.org/patch/881197/ we already have in the queue.
> With a very important distinction: your patch passes DEVICE_TREE=, while
> the existing pending patch passes EXT_DTB=. The obvious question that
> comes up is: which one is right ? Or are these two orthogonal things ?
[KP] As far as I understand, the EXT_DTP is used for pointing to out of the tree pre-compiled DTB file, while DEVICE_TREE allows selection between DTS files existing in u-boot tree. So the first one is for customers that do not want to share their DT details and the second one for re-using the same configuration with different DTs.
Does it makes sense?
Kosta
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH] boot/uboot: add support for custom DT name

[Clemens Gruber]

Hi Thomas, Kostya, Matthew,

On Mon, Dec 24, 2018 at 02:47:59PM +0100, Thomas Petazzoni wrote:
> Hello Kostya,
> 
> +Clemens Gruber in Cc, who posted a related U-Boot patch.
> 
> On Mon, 24 Dec 2018 15:19:04 +0200, kostap at marvell.com wrote:
> > From: Konstantin Porotchkin <kostap@marvell.com>
> > 
> > Some u-boot default configuration files could be shared between
> > targets and used for building images for multiple board types.
> > The only difference between such builds is the DTB embedded in
> > in the boot image for each specific platform.
> > This approach is widely used by Marvell, having the same u-boot
> > configuration file for the entire SoC family, but allowing builds
> > of multiple target flavors by supplying the device tree name through
> > make command parameter DEVICE_TREE=xxx
> > This patch adds such capability to uboot module of the buildroot.
> > The custome DT name could be  defined by
> > BR2_TARGET_UBOOT_CUSTOM_DTS_NAME entry.
> > 
> > Change-Id: I69e193339b0369a736bdf98491b9914d24a54e17
> > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> 
> Reading this again, in fact it is very similar to patch
> http://patchwork.ozlabs.org/patch/881197/ we already have in the queue.
> With a very important distinction: your patch passes DEVICE_TREE=,
> while the existing pending patch passes EXT_DTB=. The obvious question
> that comes up is: which one is right ? Or are these two orthogonal
> things ?

They solve different problems.

The reason for the EXT_DTB patch was to allow passing pre-built dtbs
with metadata used for verified boot in U-Boot.
(It's a simple device tree blob which stores the public key for the FIT
image with the kernel, initrd etc. and is generated by U-Boot mkimage)

Matthew Weber suggested alternatives like appending this blob to
uboot.bin or decompiling the dtb and including the resulting dts via
BR2_TARGET_UBOOT_CUSTOM_DTS_PATH.

Afaik, DEVICE_TREE is for user-specified "normal" DTs, e.g. for
configuration sharing / to override CONFIG_DEFAULT_DEVICE_TREE.
EXT_DTB is used for these extra DT blobs in verified boot scenarios.

Happy holidays!
Clemens

[Buildroot] [PATCH] boot/uboot: add support for custom DT name

[Thomas Petazzoni]

Hello Clemens, Kostya,

On Mon, 24 Dec 2018 17:59:31 +0100
Clemens Gruber <clemens.gruber@pqgruber.com> wrote:

> They solve different problems.
> 
> The reason for the EXT_DTB patch was to allow passing pre-built dtbs
> with metadata used for verified boot in U-Boot.
> (It's a simple device tree blob which stores the public key for the FIT
> image with the kernel, initrd etc. and is generated by U-Boot mkimage)
> 
> Matthew Weber suggested alternatives like appending this blob to
> uboot.bin or decompiling the dtb and including the resulting dts via
> BR2_TARGET_UBOOT_CUSTOM_DTS_PATH.
> 
> Afaik, DEVICE_TREE is for user-specified "normal" DTs, e.g. for
> configuration sharing / to override CONFIG_DEFAULT_DEVICE_TREE.
> EXT_DTB is used for these extra DT blobs in verified boot scenarios.

I thought a bit about this. U-Boot in fact has tons of environment
variables to tweak stuff. Instead of adding explicit options for each
of them, perhaps we should simply add an option that allows to pass
arbitrary environment variables to the U-Boot build. Something long the
lines of:

config BR2_TARGET_UBOOT_CUSTOM_ENVVARS
	string "additional environment variables"
	help
	  ...

UBOOT_MAKE_OPTS += $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVVARS))

Not only this would allow to support more than EXT_DTB and DEVICE_TREE,
but other variables. In addition, what worries me about EXT_DTB and
DEVICE_TREE as individual options is that the semantic is hard to
explain. Both are device trees, but they don't serve the same purpose.

Thoughts ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] boot/uboot: add support for custom DT name

[Clemens Gruber]

Hi Thomas,

On Sun, Feb 03, 2019 at 08:49:50PM +0100, Thomas Petazzoni wrote:
> Hello Clemens, Kostya,
> 
> On Mon, 24 Dec 2018 17:59:31 +0100
> Clemens Gruber <clemens.gruber@pqgruber.com> wrote:
> 
> > They solve different problems.
> > 
> > The reason for the EXT_DTB patch was to allow passing pre-built dtbs
> > with metadata used for verified boot in U-Boot.
> > (It's a simple device tree blob which stores the public key for the FIT
> > image with the kernel, initrd etc. and is generated by U-Boot mkimage)
> > 
> > Matthew Weber suggested alternatives like appending this blob to
> > uboot.bin or decompiling the dtb and including the resulting dts via
> > BR2_TARGET_UBOOT_CUSTOM_DTS_PATH.
> > 
> > Afaik, DEVICE_TREE is for user-specified "normal" DTs, e.g. for
> > configuration sharing / to override CONFIG_DEFAULT_DEVICE_TREE.
> > EXT_DTB is used for these extra DT blobs in verified boot scenarios.
> 
> I thought a bit about this. U-Boot in fact has tons of environment
> variables to tweak stuff. Instead of adding explicit options for each
> of them, perhaps we should simply add an option that allows to pass
> arbitrary environment variables to the U-Boot build. Something long the
> lines of:
> 
> config BR2_TARGET_UBOOT_CUSTOM_ENVVARS
> 	string "additional environment variables"
> 	help
> 	  ...
> 
> UBOOT_MAKE_OPTS += $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVVARS))
> 
> Not only this would allow to support more than EXT_DTB and DEVICE_TREE,
> but other variables. In addition, what worries me about EXT_DTB and
> DEVICE_TREE as individual options is that the semantic is hard to
> explain. Both are device trees, but they don't serve the same purpose.
> 
> Thoughts ?

Sounds good to me! And it's future-proof.

Thanks,
Clemens