Message ID | 20190723013424.1827-1-danomimanchego123@gmail.com |
---|---|
State | Rejected |
Headers | show |
Series | [1/1] boot/arm-trusted-firmware: add missing host-uboot-tools select | expand |
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
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@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
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
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
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
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(+)