| Message ID | 20221024142216.31273-2-neal.frager@amd.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v1,1/3] add package/versal-firmware | expand |
Hello Neal, Please note that this is not a full review. Just some comments. This PATCH 2/3 should be squashed with PATCH 3/3 into a single patch, whose commit title should be: configs/versal_vck190: new defconfig More comments below. On Mon, 24 Oct 2022 08:22:15 -0600 Neal Frager <neal.frager@amd.com> wrote: > diff --git a/board/versal/post-build.sh b/board/versal/post-build.sh > new file mode 100755 > index 0000000000..0713bd1b05 > --- /dev/null > +++ b/board/versal/post-build.sh > @@ -0,0 +1,16 @@ > +#!/bin/sh > + > +# genimage will need to find the extlinux.conf > +# in the binaries directory > + > +BOARD_DIR="$(dirname $0)" > +CONSOLE=$2 > +ROOT=$3 > + > +mkdir -p "${BINARIES_DIR}" > +cat <<-__HEADER_EOF > "${BINARIES_DIR}/extlinux.conf" > + label linux > + kernel /Image > + devicetree /system.dtb > + append console=${CONSOLE} root=/dev/${ROOT} rw rootwait > + __HEADER_EOF Meeh, I don't know if I like being that smart. What about having an extlinux.conf file per board, and simplify this? Sometimes dumb is better than smart/complicated. > diff --git a/board/versal/vck190/uboot.fragment b/board/versal/vck190/uboot.fragment > new file mode 100644 > index 0000000000..961c4239bd > --- /dev/null > +++ b/board/versal/vck190/uboot.fragment > @@ -0,0 +1 @@ > +CONFIG_DEFAULT_DEVICE_TREE="versal-vck190-rev1.1" This can be removed in favor of passing DEVICE_TREE=... in BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS. Thomas
Hello Thomas, > Please note that this is not a full review. Just some comments. > This PATCH 2/3 should be squashed with PATCH 3/3 into a single patch, whose commit title should be: > configs/versal_vck190: new defconfig Ok, no problem. > More comments below. > diff --git a/board/versal/post-build.sh b/board/versal/post-build.sh > new file mode 100755 index 0000000000..0713bd1b05 > --- /dev/null > +++ b/board/versal/post-build.sh > @@ -0,0 +1,16 @@ > +#!/bin/sh > + > +# genimage will need to find the extlinux.conf # in the binaries > +directory > + > +BOARD_DIR="$(dirname $0)" > +CONSOLE=$2 > +ROOT=$3 > + > +mkdir -p "${BINARIES_DIR}" > +cat <<-__HEADER_EOF > "${BINARIES_DIR}/extlinux.conf" > + label linux > + kernel /Image > + devicetree /system.dtb > + append console=${CONSOLE} root=/dev/${ROOT} rw rootwait > + __HEADER_EOF > Meeh, I don't know if I like being that smart. What about having an extlinux.conf file per board, and simplify this? Sometimes dumb is better than smart/complicated. I understand, and usually agree. This post_build.sh actually already exists in buildroot in the board/zynqmp directory. The reason why it was done this way was because the kria kv260 uses a different serial port and sd card device than the zynqmp zcu boards. We could be super smart and have versal use the same board/zynqmp/post-build.sh, but mixing zynqmp and versal is probably not so clean. I could also revert to dumb and easy, but that creates additional extlinux.conf files everywhere. As I will be maintaining these zynqmp and versal board files, my preference is to keep both the same style. Either both dumb with an extlinux.conf file per board or both smart/complicated. With this in mind, what is your preference? > diff --git a/board/versal/vck190/uboot.fragment > b/board/versal/vck190/uboot.fragment > new file mode 100644 > index 0000000000..961c4239bd > --- /dev/null > +++ b/board/versal/vck190/uboot.fragment > @@ -0,0 +1 @@ > +CONFIG_DEFAULT_DEVICE_TREE="versal-vck190-rev1.1" > This can be removed in favor of passing DEVICE_TREE=... in BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS. Ok, should I update all of the zynq, zynqmp and kria defconfigs as well? Thanks for your review! Best regards, Neal Frager AMD
Hi Neal, On Mon, 24 Oct 2022 08:22:15 -0600 Neal Frager <neal.frager@amd.com> wrote: > --- /dev/null > +++ b/board/versal/genimage.cfg > @@ -0,0 +1,30 @@ > +image boot.vfat { > + vfat { > + files = { > + "boot.bin", > + "system.dtb", system.dtb is stored in the FAT partition... > --- /dev/null > +++ b/board/versal/post-image.sh > @@ -0,0 +1,35 @@ > +#!/bin/sh > + > +# By default U-Boot loads DTB from a file named "system.dtb", so > +# let's use a symlink with that name that points to the *first* > +# devicetree listed in the config. > + > +FIRST_DT=$(sed -nr \ > + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > + ${BR2_CONFIG}) > + > +[ -z "${FIRST_DT}" ] || ln -fs ${FIRST_DT}.dtb ${BINARIES_DIR}/system.dtb > + > +BOARD_DIR="$(dirname $0)" > +BOARD_NAME=$4 > + > +mkdir -p "${BINARIES_DIR}" > +cat <<-__HEADER_EOF > "${BINARIES_DIR}/bootgen.bif" > + the_ROM_image: > + { > + image { > + { type=bootimage, file=${BINARIES_DIR}/${BOARD_NAME}_vpl_gen_fixed.pdi } > + { type=bootloader, file=${BINARIES_DIR}/${BOARD_NAME}_plm.elf } > + { core=psm, file=${BINARIES_DIR}/${BOARD_NAME}_psmfw.elf } > + } > + image { > + id = 0x1c000000, name=apu_subsystem > + { type=raw, load=0x00001000, file=${BINARIES_DIR}/system.dtb } ...and also in boot.bin. What's the reason for this? One copy is for U-Boot and the other for the kernel?
Hi Luca, > --- /dev/null > +++ b/board/versal/genimage.cfg > @@ -0,0 +1,30 @@ > +image boot.vfat { > + vfat { > + files = { > + "boot.bin", > + "system.dtb", > system.dtb is stored in the FAT partition... The system.dtb in the FAT partition is for the Linux kernel. > --- /dev/null > +++ b/board/versal/post-image.sh > @@ -0,0 +1,35 @@ > +#!/bin/sh > + > +# By default U-Boot loads DTB from a file named "system.dtb", so # > +let's use a symlink with that name that points to the *first* # > +devicetree listed in the config. > + > +FIRST_DT=$(sed -nr \ > + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > + ${BR2_CONFIG}) > + > +[ -z "${FIRST_DT}" ] || ln -fs ${FIRST_DT}.dtb > +${BINARIES_DIR}/system.dtb > + > +BOARD_DIR="$(dirname $0)" > +BOARD_NAME=$4 > + > +mkdir -p "${BINARIES_DIR}" > +cat <<-__HEADER_EOF > "${BINARIES_DIR}/bootgen.bif" > + the_ROM_image: > + { > + image { > + { type=bootimage, file=${BINARIES_DIR}/${BOARD_NAME}_vpl_gen_fixed.pdi } > + { type=bootloader, file=${BINARIES_DIR}/${BOARD_NAME}_plm.elf } > + { core=psm, file=${BINARIES_DIR}/${BOARD_NAME}_psmfw.elf } > + } > + image { > + id = 0x1c000000, name=apu_subsystem > + { type=raw, load=0x00001000, file=${BINARIES_DIR}/system.dtb } > ...and also in boot.bin. What's the reason for this? One copy is for U-Boot and the other for the kernel? Yes, the system.dtb packaged inside the boot.bin is for u-boot. For versal products, the bootloader is the plm (platform loader and manager) which runs on a triple-redundant microblaze. The plm does what the fsbl/spl does for zynq and zynqmp products. It parses the images in the boot.bin and boots tf-a and u-boot. Best regards, Neal Frager AMD
diff --git a/DEVELOPERS b/DEVELOPERS index ed696f4cd0..08095c9702 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -2168,6 +2168,7 @@ F: package/pkg-qmake.mk F: package/qt5/qt5opcua/ N: Neal Frager <neal.frager@amd.com> +F: board/versal/ F: board/zynq/ F: board/zynqmp/ F: board/zynqmp/kria/ diff --git a/board/versal/genimage.cfg b/board/versal/genimage.cfg new file mode 100644 index 0000000000..d994d3a2bf --- /dev/null +++ b/board/versal/genimage.cfg @@ -0,0 +1,30 @@ +image boot.vfat { + vfat { + files = { + "boot.bin", + "system.dtb", + "Image" + } + file extlinux/extlinux.conf { + image = extlinux.conf + } + } + + size = 32M +} + +image sdcard.img { + hdimage { + } + + partition boot { + partition-type = 0xC + bootable = "true" + image = "boot.vfat" + } + + partition rootfs { + partition-type = 0x83 + image = "rootfs.ext4" + } +} diff --git a/board/versal/post-build.sh b/board/versal/post-build.sh new file mode 100755 index 0000000000..0713bd1b05 --- /dev/null +++ b/board/versal/post-build.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +# genimage will need to find the extlinux.conf +# in the binaries directory + +BOARD_DIR="$(dirname $0)" +CONSOLE=$2 +ROOT=$3 + +mkdir -p "${BINARIES_DIR}" +cat <<-__HEADER_EOF > "${BINARIES_DIR}/extlinux.conf" + label linux + kernel /Image + devicetree /system.dtb + append console=${CONSOLE} root=/dev/${ROOT} rw rootwait + __HEADER_EOF diff --git a/board/versal/post-image.sh b/board/versal/post-image.sh new file mode 100755 index 0000000000..aad6813052 --- /dev/null +++ b/board/versal/post-image.sh @@ -0,0 +1,35 @@ +#!/bin/sh + +# By default U-Boot loads DTB from a file named "system.dtb", so +# let's use a symlink with that name that points to the *first* +# devicetree listed in the config. + +FIRST_DT=$(sed -nr \ + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ + ${BR2_CONFIG}) + +[ -z "${FIRST_DT}" ] || ln -fs ${FIRST_DT}.dtb ${BINARIES_DIR}/system.dtb + +BOARD_DIR="$(dirname $0)" +BOARD_NAME=$4 + +mkdir -p "${BINARIES_DIR}" +cat <<-__HEADER_EOF > "${BINARIES_DIR}/bootgen.bif" + the_ROM_image: + { + image { + { type=bootimage, file=${BINARIES_DIR}/${BOARD_NAME}_vpl_gen_fixed.pdi } + { type=bootloader, file=${BINARIES_DIR}/${BOARD_NAME}_plm.elf } + { core=psm, file=${BINARIES_DIR}/${BOARD_NAME}_psmfw.elf } + } + image { + id = 0x1c000000, name=apu_subsystem + { type=raw, load=0x00001000, file=${BINARIES_DIR}/system.dtb } + { core=a72-0, exception_level=el-3, trustzone, file=${BINARIES_DIR}/bl31.elf } + { core=a72-0, exception_level=el-2, file=${BINARIES_DIR}/u-boot } + } + } + __HEADER_EOF + +${HOST_DIR}/bin/bootgen -arch versal -image ${BINARIES_DIR}/bootgen.bif -o ${BINARIES_DIR}/boot.bin -w on +support/scripts/genimage.sh -c ${BOARD_DIR}/genimage.cfg diff --git a/board/versal/readme.txt b/board/versal/readme.txt new file mode 100644 index 0000000000..9f234be620 --- /dev/null +++ b/board/versal/readme.txt @@ -0,0 +1,54 @@ +****************************************** +Xilinx VCK190 board - Versal +****************************************** + +This document describes the Buildroot support for the VCK190 +board by Xilinx, based on Versal. It has been tested with the +VCK190 production board. + +Evaluation board features can be found here with the link below. + +VCK190: +https://www.xilinx.com/products/boards-and-kits/vck190.html + + +How to build it +=============== + +Configure Buildroot: + + $ make versal_vck190_defconfig + +Compile everything and build the rootfs image: + + $ make + +Result of the build +------------------- + +After building, you should get a tree like this: + + output/images/ + +-- boot.bin + +-- boot.vfat + +-- Image + +-- rootfs.ext2 + +-- rootfs.ext4 -> rootfs.ext2 + +-- sdcard.img + +-- system.dtb -> versal-vck190-rev1.1.dtb + `-- versal-vck190-rev1.1.dtb + +How to write the SD card +======================== + +WARNING! This will destroy all the card content. Use with care! + +The sdcard.img file is a complete bootable image ready to be written +on the boot medium. To install it, simply copy the image to an SD +card: + + # dd if=output/images/sdcard.img of=/dev/sdX + +Where 'sdX' is the device node of the SD. + +Eject the SD card, insert it in the board, and power it up. diff --git a/board/versal/vck190/uboot.fragment b/board/versal/vck190/uboot.fragment new file mode 100644 index 0000000000..961c4239bd --- /dev/null +++ b/board/versal/vck190/uboot.fragment @@ -0,0 +1 @@ +CONFIG_DEFAULT_DEVICE_TREE="versal-vck190-rev1.1"
This patch adds board support for generating images for versal boards. Signed-off-by: Neal Frager <neal.frager@amd.com> --- DEVELOPERS | 1 + board/versal/genimage.cfg | 30 +++++++++++++++++ board/versal/post-build.sh | 16 +++++++++ board/versal/post-image.sh | 35 +++++++++++++++++++ board/versal/readme.txt | 54 ++++++++++++++++++++++++++++++ board/versal/vck190/uboot.fragment | 1 + 6 files changed, 137 insertions(+) create mode 100644 board/versal/genimage.cfg create mode 100755 board/versal/post-build.sh create mode 100755 board/versal/post-image.sh create mode 100644 board/versal/readme.txt create mode 100644 board/versal/vck190/uboot.fragment