[Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select

[Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select

[Danomi Manchego]

The "Build BL31 U-Boot image" option uses mkimage to make atf-uboot.ub,
and has a make dependency on host-uboot-tools.  Therefore, the Config.in
option should select BR2_PACKAGE_HOST_UBOOT_TOOLS.  (Just like similar
options in linux, package/xvisor, and fs/cpio.)

Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
---
 boot/arm-trusted-firmware/Config.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
index beb95fb..2e41bd0 100644
--- a/boot/arm-trusted-firmware/Config.in
+++ b/boot/arm-trusted-firmware/Config.in
@@ -86,6 +86,7 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
 config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT
 	bool "Build BL31 U-Boot image"
 	select BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
+	select BR2_PACKAGE_HOST_UBOOT_TOOLS
 	help
 	  Generates a U-Boot image named atf-uboot.ub containing
 	  bl31.bin.  This is used for example by the Xilinx version of
-- 
2.7.4

[Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select

[Baruch Siach]

Hi Danomi,

On Mon, Jul 22, 2019 at 09:34:24PM -0400, Danomi Manchego wrote:
> The "Build BL31 U-Boot image" option uses mkimage to make atf-uboot.ub,
> and has a make dependency on host-uboot-tools.  Therefore, the Config.in
> option should select BR2_PACKAGE_HOST_UBOOT_TOOLS.  (Just like similar
> options in linux, package/xvisor, and fs/cpio.)
> 
> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
> ---
>  boot/arm-trusted-firmware/Config.in | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
> index beb95fb..2e41bd0 100644
> --- a/boot/arm-trusted-firmware/Config.in
> +++ b/boot/arm-trusted-firmware/Config.in
> @@ -86,6 +86,7 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
>  config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT
>  	bool "Build BL31 U-Boot image"
>  	select BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
> +	select BR2_PACKAGE_HOST_UBOOT_TOOLS

BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT adds host-uboot-tools to 
ARM_TRUSTED_FIRMWARE_DEPENDENCIES already. Why isn't that enough?

baruch

>  	help
>  	  Generates a U-Boot image named atf-uboot.ub containing
>  	  bl31.bin.  This is used for example by the Xilinx version of

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select

[Danomi Manchego]

Hi Baruch,

On Tue, Jul 23, 2019 at 2:32 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Danomi,
>
> On Mon, Jul 22, 2019 at 09:34:24PM -0400, Danomi Manchego wrote:
> > The "Build BL31 U-Boot image" option uses mkimage to make atf-uboot.ub,
> > and has a make dependency on host-uboot-tools.  Therefore, the Config.in
> > option should select BR2_PACKAGE_HOST_UBOOT_TOOLS.  (Just like similar
> > options in linux, package/xvisor, and fs/cpio.)
> >
> > Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
> > ---
> >  boot/arm-trusted-firmware/Config.in | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
> > index beb95fb..2e41bd0 100644
> > --- a/boot/arm-trusted-firmware/Config.in
> > +++ b/boot/arm-trusted-firmware/Config.in
> > @@ -86,6 +86,7 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
> >  config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT
> >       bool "Build BL31 U-Boot image"
> >       select BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
> > +     select BR2_PACKAGE_HOST_UBOOT_TOOLS
>
> BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT adds host-uboot-tools to
> ARM_TRUSTED_FIRMWARE_DEPENDENCIES already. Why isn't that enough?
>
> baruch

Well, why isn't the dependency enough in xvisor for the
BR2_PACKAGE_XVISOR_CREATE_UBOOT_IMAGE option?  Why isn't it enough in
cpio for the BR2_TARGET_ROOTFS_CPIO_UIMAGE option?  I don't know, but
I imagine that the answer is partially due to "truth in advertising"
with regards to the menu configuration (because "host uboot-tools" can
*appear* to be deselected but it gets built anyway if something
depends on it but does not select it).

Isn't consistency with the other places reason enough?

Danomi -

>
> >       help
> >         Generates a U-Boot image named atf-uboot.ub containing
> >         bl31.bin.  This is used for example by the Xilinx version of
>
> --
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select

[Baruch Siach]

Hi Danomi,

On Tue, Jul 23, 2019 at 08:16:45AM -0400, Danomi Manchego wrote:
> On Tue, Jul 23, 2019 at 2:32 AM Baruch Siach <baruch@tkos.co.il> wrote:
> > On Mon, Jul 22, 2019 at 09:34:24PM -0400, Danomi Manchego wrote:
> > > The "Build BL31 U-Boot image" option uses mkimage to make atf-uboot.ub,
> > > and has a make dependency on host-uboot-tools.  Therefore, the Config.in
> > > option should select BR2_PACKAGE_HOST_UBOOT_TOOLS.  (Just like similar
> > > options in linux, package/xvisor, and fs/cpio.)
> > >
> > > Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
> > > ---
> > >  boot/arm-trusted-firmware/Config.in | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
> > > index beb95fb..2e41bd0 100644
> > > --- a/boot/arm-trusted-firmware/Config.in
> > > +++ b/boot/arm-trusted-firmware/Config.in
> > > @@ -86,6 +86,7 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
> > >  config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT
> > >       bool "Build BL31 U-Boot image"
> > >       select BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31
> > > +     select BR2_PACKAGE_HOST_UBOOT_TOOLS
> >
> > BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT adds host-uboot-tools to
> > ARM_TRUSTED_FIRMWARE_DEPENDENCIES already. Why isn't that enough?
> 
> Well, why isn't the dependency enough in xvisor for the
> BR2_PACKAGE_XVISOR_CREATE_UBOOT_IMAGE option?  Why isn't it enough in
> cpio for the BR2_TARGET_ROOTFS_CPIO_UIMAGE option?  I don't know, but
> I imagine that the answer is partially due to "truth in advertising"
> with regards to the menu configuration (because "host uboot-tools" can
> *appear* to be deselected but it gets built anyway if something
> depends on it but does not select it).
> 
> Isn't consistency with the other places reason enough?

It definitely is. But usually host dependencies are not selected at the 
Kconfig level. We have 33 matches to select.*PACKAGE_HOST_. Of these, 7 are 
BR2_PACKAGE_HOST_UBOOT_TOOLS. On the other hand, we have thousands of make 
level host dependencies that are not selected.

Not sure what the rule is.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select

[Thomas Petazzoni]

Hello,

On Tue, 23 Jul 2019 16:24:37 +0300
Baruch Siach <baruch@tkos.co.il> wrote:

> > Well, why isn't the dependency enough in xvisor for the
> > BR2_PACKAGE_XVISOR_CREATE_UBOOT_IMAGE option?  Why isn't it enough in
> > cpio for the BR2_TARGET_ROOTFS_CPIO_UIMAGE option?  I don't know, but
> > I imagine that the answer is partially due to "truth in advertising"
> > with regards to the menu configuration (because "host uboot-tools" can
> > *appear* to be deselected but it gets built anyway if something
> > depends on it but does not select it).
> > 
> > Isn't consistency with the other places reason enough?  
> 
> It definitely is. But usually host dependencies are not selected at the 
> Kconfig level. We have 33 matches to select.*PACKAGE_HOST_. Of these, 7 are 
> BR2_PACKAGE_HOST_UBOOT_TOOLS. On the other hand, we have thousands of make 
> level host dependencies that are not selected.
> 
> Not sure what the rule is.

The rule is pretty much we don't care. For example, we have tons of
package that depend on host-pkgconf, but none of them select
BR2_PACKAGE_HOST_PKGCONF, even though this option exists.

All the packages using the cmake-package infrastructure use host-cmake,
but they don't select BR2_PACKAGE_HOST_CMAKE.

We have discussed several times adding in a systematic fashion
Config.in options for all host packages, and make sure they are all
selected properly, just like we do with target packages. However, last
time we discussed this, we felt the burden was way too high. For
example, all autotools-package that have AUTORECONF = YES would have to
select BR2_PACKAGE_HOST_AUTOCONF, BR2_PACKAGE_HOST_AUTOMAKE and
BR2_PACKAGE_HOST_LIBTOOL. This quickly gets pretty annoying.

So for now, we've decided to stick with what we have today: a very
clear rule for target packages, and a much fuzzier situation for host
packages.

Does this clarify the unclear situation ? :-)

Best regards,

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