[Buildroot] [PATCH v1 1/1] package/bootgen: clean up variable names

[Buildroot] [PATCH v1 1/1] package/bootgen: clean up variable names

[Neal Frager via buildroot]

bootgen is a host package, so the variable names should have the HOST_BOOTGEN
prefix and not just BOOTGEN.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 package/bootgen/bootgen.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/package/bootgen/bootgen.mk b/package/bootgen/bootgen.mk
index 912e2e31b3..840e89f827 100644
--- a/package/bootgen/bootgen.mk
+++ b/package/bootgen/bootgen.mk
@@ -4,11 +4,11 @@
 #
 ################################################################################
 
-BOOTGEN_VERSION = xilinx_v2024.1
-BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(BOOTGEN_VERSION))
+HOST_BOOTGEN_VERSION = xilinx_v2024.1
+HOST_BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(HOST_BOOTGEN_VERSION))
 HOST_BOOTGEN_DEPENDENCIES = host-openssl host-pkgconf
-BOOTGEN_LICENSE = Apache-2.0
-BOOTGEN_LICENSE_FILES = LICENSE
+HOST_BOOTGEN_LICENSE = Apache-2.0
+HOST_BOOTGEN_LICENSE_FILES = LICENSE
 
 define HOST_BOOTGEN_BUILD_CMDS
 	$(MAKE) $(HOST_CONFIGURE_OPTS) \
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/1] package/bootgen: clean up variable names

[Frager, Neal via buildroot]

Hi Andreas,

> Hi Neal,
> I'm happy that you introduced bootgen into the buildroot project,
> because up to now I had to call bootgen in a post-image script and use
> it from our Vivado/Vitis installation.
> Unfortunately I can't use this feature, because we have several projects
> (Xilinx/AMD based boards) which need different version. In the past I
> had problems if I don't use the same version number in the complete
> chain. Vivado, FSBL, PMU FW, ATF, uboot, Linux, bootgen, dtg, ...
> Therefore in our different projects we have to use different bootgen
> versions since a project will not be moved to a newer Vivavdo version if
> the project is in a final state.
> So my question is, if you could make HOST_BOOTGEN_VERSION configurable
> in the Config.in file? This would only be a small change and would make
> the usage of bootgen more flexible.

> What do you think?

Thank you for your email.

Yes, I am trying to make it so software developers can use buildroot for AMD /
Xilinx products without needing to install Vivado or Vitis.  We are not all
the way there yet as the conversion from XSA to DTS still requires XSCT which
is a closed source utility.

But regarding bootgen, it is already fully decoupled.  Even if you are
building your images with an older version of Vivado or Vitis, you can still
use bootgen 2024.1 to wrap these images into a boot.bin for your product.

So while you are correct that configuring the bootgen package for an older
version would be easy to do, there should no longer be any need to use an
older version of bootgen because the latest bootgen version will be
compatible with images built with older versions of AMD tools.

If you still feel you absolutely need an older version of bootgen, I would
suggest just modifying the bootgen.mk and bootgen.hash files for the version
you wish to use instead of making bootgen configurable within buildroot.

Please let me know if you have any questions.

> Regards,
> Andreas

Best regards,
Neal Frager
AMD

> bootgen is a host package, so the variable names should have the HOST_BOOTGEN
> prefix and not just BOOTGEN.
>
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>   package/bootgen/bootgen.mk | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/package/bootgen/bootgen.mk b/package/bootgen/bootgen.mk
> index 912e2e31b3..840e89f827 100644
> --- a/package/bootgen/bootgen.mk
> +++ b/package/bootgen/bootgen.mk
> @@ -4,11 +4,11 @@
>   #
>   ################################################################################
>
> -BOOTGEN_VERSION = xilinx_v2024.1
> -BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(BOOTGEN_VERSION))
> +HOST_BOOTGEN_VERSION = xilinx_v2024.1
> +HOST_BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(HOST_BOOTGEN_VERSION))
>   HOST_BOOTGEN_DEPENDENCIES = host-openssl host-pkgconf
> -BOOTGEN_LICENSE = Apache-2.0
> -BOOTGEN_LICENSE_FILES = LICENSE
> +HOST_BOOTGEN_LICENSE = Apache-2.0
> +HOST_BOOTGEN_LICENSE_FILES = LICENSE
>
>   define HOST_BOOTGEN_BUILD_CMDS
>   	$(MAKE) $(HOST_CONFIGURE_OPTS) \


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/1] package/bootgen: clean up variable names

[Thomas Petazzoni via buildroot]

Hello Neal,

On Tue, 29 Oct 2024 07:03:44 +0000
Neal Frager <neal.frager@amd.com> wrote:

> -BOOTGEN_VERSION = xilinx_v2024.1
> -BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(BOOTGEN_VERSION))
> +HOST_BOOTGEN_VERSION = xilinx_v2024.1
> +HOST_BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(HOST_BOOTGEN_VERSION))
>  HOST_BOOTGEN_DEPENDENCIES = host-openssl host-pkgconf
> -BOOTGEN_LICENSE = Apache-2.0
> -BOOTGEN_LICENSE_FILES = LICENSE
> +HOST_BOOTGEN_LICENSE = Apache-2.0
> +HOST_BOOTGEN_LICENSE_FILES = LICENSE

No, the way it is done today is correct.

Even for host packages, we expect those "generic" variables to NOT use
the HOST_ prefix, except if the value needs to be different between the
target and host package.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/1] package/bootgen: clean up variable names

[Frager, Neal via buildroot]

Hello Thomas,

> -BOOTGEN_VERSION = xilinx_v2024.1
> -BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(BOOTGEN_VERSION))
> +HOST_BOOTGEN_VERSION = xilinx_v2024.1
> +HOST_BOOTGEN_SITE = $(call github,Xilinx,bootgen,$(HOST_BOOTGEN_VERSION))
>  HOST_BOOTGEN_DEPENDENCIES = host-openssl host-pkgconf
> -BOOTGEN_LICENSE = Apache-2.0
> -BOOTGEN_LICENSE_FILES = LICENSE
> +HOST_BOOTGEN_LICENSE = Apache-2.0
> +HOST_BOOTGEN_LICENSE_FILES = LICENSE

> No, the way it is done today is correct.

> Even for host packages, we expect those "generic" variables to NOT use
> the HOST_ prefix, except if the value needs to be different between the
> target and host package.

If that is the case, then maybe these are wrong?

https://github.com/buildroot/buildroot/blob/master/package/binutils-bare-metal/binutils-bare-metal.mk
https://github.com/buildroot/buildroot/blob/master/package/gcc-bare-metal/gcc-bare-metal.mk

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/1] package/bootgen: clean up variable names

[Thomas Petazzoni via buildroot]

On Tue, 29 Oct 2024 09:08:41 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> If that is the case, then maybe these are wrong?
> 
> https://github.com/buildroot/buildroot/blob/master/package/binutils-bare-metal/binutils-bare-metal.mk
> https://github.com/buildroot/buildroot/blob/master/package/gcc-bare-metal/gcc-bare-metal.mk

"Wrong" no, as they work, but we indeed normally define those variables
without a HOST_ prefix.

One thing that should be fixed is:

define GCC_BARE_METAL_CONFIGURE_SYMLINK
	mkdir -p $(@D)/build
	ln -sf ../configure $(@D)/build/configure
endef

HOST_GCC_BARE_METAL_PRE_CONFIGURE_HOOKS += GCC_BARE_METAL_CONFIGURE_SYMLINK

GCC_BARE_METAL_CONFIGURE_SYMLINK should be
HOST_GCC_BARE_METAL_CONFIGURE_SYMLINK.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot