Re: [Buildroot] [PATCH v6 2/5] boot/xilinx-embeddedsw: only allow apps for device family

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

On Thu, 14 Aug 2025 06:26:28 +0100
Neal Frager <neal.frager@amd.com> wrote:

> This patch adds an architecture cpu dependency to each application to make
> sure that users can only build applications which are applicable to their
> target device family.

Generally speaking, git commit messages should use imperative tenses:
"This patch adds" -> "Add", etc.

I won't be nacking this patch just for this, but it would be nice for
the future.

> The versal_plm and versal_psmfw applications are specific to versal devices
> which are based on BR2_cortex_a72.
> 
> The zynqmp_pmufw application is specific to zynqmp devices which are based on
> BR2_cortex_a53.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
> V1->V2:
> - Replaced new family variant config option with an architecture cpu
>   dependency, so no new configs are needed.
> - Updated patch title and commit message accordingly.
> V2->V3:
> - Changed package dependency to only appear if the cpu is BR2_cortex_a53
>   or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
>   appear with zero application options if another BR2_aarch64 cpu is
>   selected.
> V3->V4:
> - Added BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS, so that all arch
>   dependencies are in one place
> V4->V5:
> - Replace leading space copy paste error with tabs
> V5->V6:
> - Take advantage of BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS from
>   boot/Config.in to remove dependencies on BR2_aarch64.
> ---
>  boot/xilinx-embeddedsw/Config.in | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
> index 0dd6433608..3826f2e18e 100644
> --- a/boot/xilinx-embeddedsw/Config.in
> +++ b/boot/xilinx-embeddedsw/Config.in
> @@ -1,11 +1,9 @@
>  comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-buildroot-elf"
>  	depends on BR2_TOOLCHAIN_BARE_METAL_BUILDROOT_ARCH != "microblazeel-buildroot-elf"
> -	depends on BR2_aarch64
>  	depends on BR2_TOOLCHAIN_BARE_METAL_BUILDROOT
>  
>  menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
>  	bool "xilinx-embeddedsw"
> -	depends on BR2_aarch64
>  	depends on BR2_TOOLCHAIN_BARE_METAL_BUILDROOT
>  	help
>  	  Build boot firmware applications from source for Xilinx

Shouldn't these two line removals be in patch 1?

Otherwise looks good.

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