[Buildroot] [PATCH v1 2/5] boot/uboot.mk: new zynqmp pmufw prebuilt option

[Frager, Neal]

Hi Brandon,

> The new BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT option will enable u-boot to
> use the xilinx-prebuilt package for downloading a pmufw.elf that gets included
> in the generated boot.bin.
> 
> If the BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT option is enabled, then the
> BR2_TARGET_UBOOT_ZYNQMP_PMUFW config for downloading a prebuilt pmufw will
> be ignored.
> 
> Signed-off-by: Neal Frager <neal.frager at amd.com>
> ---
>  boot/uboot/Config.in | 16 ++++++++++++++++
>  boot/uboot/uboot.mk  |  5 +++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> index d0901edc9d..9d275e8611 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -512,9 +512,25 @@ config BR2_TARGET_UBOOT_ZYNQMP
>  
>  if BR2_TARGET_UBOOT_ZYNQMP
>  
> +config BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT
> +	bool "xilinx-prebuilt pmufw.elf"
> +	depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG
> +	depends on BR2_TARGET_XILINX_PREBUILT

> Since this is a ZynqMP only option. Shouldn't this depend on the
> Xilinx-prebuilt ZynqMP choice?

>  depends on BR2_TARGET_XILINX_PREBUILT_ZYNQMP

The kria boards also use this u-boot feature as they are part of the zynqmp
family.  I suppose we could add this?

depends on BR2_TARGET_XILINX_PREBUILT_ZYNQMP | BR2_TARGET_XILINX_PREBUILT_KRIA

> +	help
> +	  Use xilinx-prebuilt boot package for downloading prebuilt zynqmp
> +	  pmufw.elf from https://github.com/Xilinx/soc-prebuilt-firmware repo.
> +
> +	  U-Boot build process will generate a boot.bin (to be loaded by the
> +	  ZynqMP boot ROM) containing both the U-Boot SPL and the PMU firmware
> +	  in the Xilinx-specific boot format.
> +
> +	  If this option is selected, the BR2_TARGET_UBOOT_ZYNQMP_PMUFW
> +	  prebuilt option will be ignored.
> +
>  config BR2_TARGET_UBOOT_ZYNQMP_PMUFW
>  	string "PMU firmware location"
>  	depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG
> +	depends on !BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT
>  	help
>  	  Location of a PMU firmware binary.
>  
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index b9f165f041..5d12c6283b 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -436,7 +436,12 @@ endef
>  
>  ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y)
>  
> +ifeq ($(BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT),y)
> +UBOOT_DEPENDENCIES += xilinx-prebuilt
> +UBOOT_ZYNQMP_PMUFW = $(BINARIES_DIR)/pmufw.elf
> +else
>  UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW))
> +endif #BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT
>  
>  ifneq ($(findstring ://,$(UBOOT_ZYNQMP_PMUFW)),)

> The `endif` should be moved to cover this ifneq statement. That way when
> xilinx-prebuilt is enabled it doesn't do the "://" and readlink checks.

Does this really matter?  If BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT is set,
then UBOOT_ZYNQMP_PMUFW = $(BINARIES_DIR)/pmufw.elf, so there will never be
a :// in UBOOT_ZYNQMP_PMUFW and the UBOOT_EXTRA_DOWNLOADS will not get set.

The problem with moving the endif after the ifneq is that we still need the
UBOOT_ZYNQMP_PMUFW_PATH to be set for the rest of the functionality to work.

UBOOT_ZYNQMP_PMUFW_PATH = $(shell readlink -f $(UBOOT_ZYNQMP_PMUFW))

To avoid duplicating this line of code, keeping the endif before the ifneq
seemed to be the cleaner solution to me.

>  UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW)
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

Best regards,
Neal Frager
AMD