[Buildroot] [PATCH v1 2/3] add board/versal
Frager, Neal
neal.frager at amd.com
Wed Nov 2 16:56:26 UTC 2022
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
More information about the buildroot
mailing list