Re: [Buildroot] [PATCH v4 2/2] configs/versal_vck190: new defconfig

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

On Wed, 9 Nov 2022 15:10:27 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> Hi Luca,
> 
> > Le 9 nov. 2022 à 15:56, Luca Ceresoli <luca.ceresoli@bootlin.com> a écrit :
> > 
> > On Wed, 9 Nov 2022 02:12:14 -0700
> > Neal Frager <neal.frager@amd.com> wrote:
> >   
> >> This patch adds support for Xilinx Versal VCK190 evaluation board.
> >> 
> >> VCK190 features can be found here:
> >> https://www.xilinx.com/products/boards-and-kits/vck190.html
> >> 
> >> The VCK190 is based on the Xilinx Versal family:
> >> https://www.xilinx.com/products/silicon-devices/acap/versal.html
> >> 
> >> The VC1902 included with the VCK190 evaluation board has Xilinx
> >> AI Engine acclerators designed for accelerating machine learning
> >> applications.  Also included is an upgrade from prior Zynq and
> >> ZynqMP families to ARM Cortex-A72 cores.
> >> 
> >> While the Linux kernel for Versal is quite similar to ZynqMP,
> >> the boot process has significantly changed.
> >> 
> >> Triple-redundant MicroBlaze cores are used to boot and setup
> >> Versal devices.  For this reason, current buildroot support
> >> will download pre-built firmware images and use Xilinx bootgen
> >> to generate the boot.bin for the vck190.
> >> 
> >> Signed-off-by: Neal Frager <neal.frager@amd.com>  
> > 
> > [...]
> >   
> >> diff --git a/configs/versal_vck190_defconfig b/configs/versal_vck190_defconfig
> >> new file mode 100644
> >> index 0000000000..8c5d391b97
> >> --- /dev/null
> >> +++ b/configs/versal_vck190_defconfig
> >> @@ -0,0 +1,38 @@
> >> +BR2_aarch64=y
> >> +BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_15=y
> >> +BR2_ROOTFS_POST_BUILD_SCRIPT="board/versal/post-build.sh"
> >> +BR2_ROOTFS_POST_IMAGE_SCRIPT="board/versal/post-image.sh"
> >> +BR2_ROOTFS_POST_SCRIPT_ARGS="ttyAMA0,115200 mmcblk0p2 vck190"
> >> +BR2_LINUX_KERNEL=y
> >> +BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
> >> +BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,Xilinx,linux-xlnx,xlnx_rebase_v5.15_LTS_2022.2)/xlnx_rebase_v5.15_LTS_2022.2.tar.gz"
> >> +BR2_LINUX_KERNEL_DEFCONFIG="xilinx_versal"
> >> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
> >> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/versal-vck190-rev1.1"
> >> +BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
> >> +BR2_TARGET_ROOTFS_EXT2=y
> >> +BR2_TARGET_ROOTFS_EXT2_4=y
> >> +# BR2_TARGET_ROOTFS_TAR is not set
> >> +BR2_TARGET_ARM_TRUSTED_FIRMWARE=y
> >> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_TARBALL=y
> >> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_TARBALL_LOCATION="$(call github,Xilinx,arm-trusted-firmware,xlnx_rebase_v2.6_2022.2)/xlnx_rebase_v2.6_2022.2.tar.gz"
> >> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="versal"
> >> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT=y
> >> +BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
> >> +BR2_TARGET_UBOOT_NEEDS_ATF_BL31_ELF=y
> >> +BR2_TARGET_UBOOT=y
> >> +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
> >> +BR2_TARGET_UBOOT_CUSTOM_TARBALL=y
> >> +BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call github,Xilinx,u-boot-xlnx,xlnx_rebase_v2022.01_2022.2)/xlnx_rebase_v2022.01_2022.2.tar.gz"
> >> +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="xilinx_versal_virt"
> >> +BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="DEVICE_TREE=versal-vck190-rev1.1"
> >> +BR2_TARGET_UBOOT_NEEDS_DTC=y
> >> +BR2_TARGET_UBOOT_NEEDS_OPENSSL=y
> >> +BR2_TARGET_UBOOT_FORMAT_ELF=y
> >> +BR2_PACKAGE_VERSAL_FIRMWARE=y
> >> +BR2_PACKAGE_VERSAL_FIRMWARE_VERSION="v2022.2"
> >> +BR2_PACKAGE_VERSAL_FIRMWARE_BOARD="vck190"  
> > 
> > These two lines should be removed. They hold the default values after
> > it has been added to patch 1.  
> 
> While functionally, you are correct that I can remove these two lines, I am not sure it is the right thing to do.
> 
> I designed these patches with the idea of supporting additional versal boards in the future.  When a second board gets supported, it would make more sense for each defconfig be designed for the specific board.
> 
> I only created a default, so that the package would always build, even with tests where these parameters are not defined.  But in reality, I would like users to always specify these parameters to get the right boot firmware files for their board.
> 
> One thing I could do is get rid of the version definition in the defconfig file and let that just be default (latest) unless specified otherwise by a user.

Thinking twice about that I think you have a good point here, at least
for the version whose default is likely to change in the future. So you
can leave it as is, forget about my comment.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

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