Re: [Buildroot] [PATCH v1 1/1] boot/xilinx-prebuilt: simplify family logic

[Luca Ceresoli via buildroot <buildroot@buildroot.org>]

Hello Neal,

On Mon, 29 Sep 2025 08:00:52 +0100
Neal Frager <neal.frager@amd.com> wrote:

> Currently, the xilinx-prebuilt package uses an if-then-else statement for
> deciding which files need to be installed from the xilinx-prebuilt repo. This
> works fine for now because there are really only two options, either versal or
> zynqmp/kria.
> 
> Starting with the xilinx_v2025.2 release, the versal2 family along with the
> vek385 board will be added to Buildroot and thus the xilinx-prebuilt package
> as well. In the future, additional families will also probably be added to
> this package.
> 
> To avoid creating a long if-then-else-then-else statement which will only grow
> in complexity over time, simplify the logic of the xilinx-prebuilt package by
> giving each xilinx family its own if statement. In the long term, this will
> make the xilinx-prebuilt package easier to understand and maintain.

OK for splitting the if/then construct which is not very readable as of
now.

> For this reason, this patch moves the zynqmp/kria families from the "else"
> statement to having their own if statement.

Not sure about this, see below.

> --- a/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> +++ b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> @@ -103,21 +103,32 @@ define XILINX_PREBUILT_INSTALL_VERSAL_PSMFW
>  		$(BINARIES_DIR)/psmfw.elf
>  endef
>  endif # !BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PSMFW
> +endif # BR2_TARGET_XILINX_PREBUILT_VERSAL
> +
> +ifeq ($(BR2_TARGET_XILINX_PREBUILT_KRIA),y)
> +ifneq ($(BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW),y)
> +define XILINX_PREBUILT_INSTALL_KRIA_PMUFW
> +	$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/pmufw.elf \
> +		$(BINARIES_DIR)/pmufw.elf
> +endef
> +endif # !BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW
> +endif # BR2_TARGET_XILINX_PREBUILT_KRIA
>  
> -else # BR2_TARGET_XILINX_PREBUILT_VERSAL
> +ifeq ($(BR2_TARGET_XILINX_PREBUILT_ZYNQMP),y)
>  ifneq ($(BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW),y)
>  define XILINX_PREBUILT_INSTALL_ZYNQMP_PMUFW
>  	$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/pmufw.elf \
>  		$(BINARIES_DIR)/pmufw.elf
>  endef
>  endif # !BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW
> -endif # BR2_TARGET_XILINX_PREBUILT_VERSAL
> +endif # BR2_TARGET_XILINX_PREBUILT_ZYNQMP

zynqmp and kria are doing the same thing (normal, as kria is a zynqmp),
so those are duplicated function. Unless you expect them to become
different, then I'd leave them as a single function. One of these
should work:

ifeq ($(BR2_TARGET_XILINX_PREBUILT_ZYNQMP)$(BR2_TARGET_XILINX_PREBUILT_KRIA),y)
...
endif

ifneq ($(BR2_TARGET_XILINX_PREBUILT_ZYNQMP)$(BR2_TARGET_XILINX_PREBUILT_KRIA),)
...
endif

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot