| Message ID | 20230811162546.8088-2-robertoxmed@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | onfigs/roc_rk3399_pc: Bring back and update the configuration for the rockchip rk3399 built by firefly. | expand |
Hello Roberto, here are a few things, which could be improved: [--SNIP--] > diff --git a/DEVELOPERS b/DEVELOPERS > index 6ffa3ee693..5ebabd01cb 100644 > --- a/DEVELOPERS > +++ b/DEVELOPERS > @@ -2546,6 +2546,10 @@ F: package/mhz/ > N: Robert Rose <robertroyrose@gmail.com> > F: package/grpc/ > > +N: Roberto Medina <robertoxmed@gmail.com> > +F: configs/roc_pc_rk3399_defconfig > +F: board/firefly/roc-rk3399 I think when it comes to repositories (i.e., board/firefly/roc-rk3399) it is best practice to add a trailing space. I guess this way one can easily distinguish directories from single files. [--SNIP--] In the defconfig: > +BR2_TARGET_UBOOT_CUSTOM_VERSION=y > +BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2022.04" I think 2022.04 is actually the current default version of buildroot. So unless you have reason to freeze it to this version on purpose (e.g., because older and newer versions are known to be not working with this board - which I don't believe is the case) you could simply omit these two lines. [--SNIP--] Best regards, Kilian Zinnecker
Hello Roberto, > > diff --git a/DEVELOPERS b/DEVELOPERS > > index 6ffa3ee693..5ebabd01cb 100644 > > --- a/DEVELOPERS > > +++ b/DEVELOPERS > > @@ -2546,6 +2546,10 @@ F: package/mhz/ > > > > N: Robert Rose <robertroyrose@gmail.com> > > F: package/grpc/ > > > > +N: Roberto Medina <robertoxmed@gmail.com> > > +F: configs/roc_pc_rk3399_defconfig > > +F: board/firefly/roc-rk3399 > > I think when it comes to repositories (i.e., board/firefly/roc-rk3399) it is > best practice to add a trailing space. I guess this way one can easily > distinguish directories from single files. ... a trailing slash instead of a trailing space, of course ... Best regards, Kilian
Hi Kilian, Thanks for reviewing as usual. On 8/11/23 21:02, Kilian Zinnecker wrote: >> diff --git a/DEVELOPERS b/DEVELOPERS >> index 6ffa3ee693..5ebabd01cb 100644 >> --- a/DEVELOPERS >> +++ b/DEVELOPERS >> @@ -2546,6 +2546,10 @@ F: package/mhz/ >> N: Robert Rose <robertroyrose@gmail.com> >> F: package/grpc/ >> >> +N: Roberto Medina <robertoxmed@gmail.com> >> +F: configs/roc_pc_rk3399_defconfig >> +F: board/firefly/roc-rk3399 > > I think when it comes to repositories (i.e., board/firefly/roc-rk3399) it is > best practice to add a trailing space. I guess this way one can easily > distinguish directories from single files. Noted for the trailing slash. >> +BR2_TARGET_UBOOT_CUSTOM_VERSION=y >> +BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2022.04" > > I think 2022.04 is actually the current default version of buildroot. So > unless you have reason to freeze it to this version on purpose (e.g., because > older and newer versions are known to be not working with this board - which I > don't believe is the case) you could simply omit these two lines. Actually in v2 of this patch, Thomas requested to set a fixed version for U-Boot (mail from 08/08). Which made sense to me as TFA and Linux have a fixed version as well. Best, Roberto
Hello Robert, [--SNIP--] > >> +BR2_TARGET_UBOOT_CUSTOM_VERSION=y > >> +BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2022.04" > > > > I think 2022.04 is actually the current default version of buildroot. So > > unless you have reason to freeze it to this version on purpose (e.g., > > because older and newer versions are known to be not working with this > > board - which I don't believe is the case) you could simply omit these > > two lines. > > Actually in v2 of this patch, Thomas requested to set a fixed version > for U-Boot (mail from 08/08). Which made sense to me as TFA and Linux > have a fixed version as well. Ah, I see, makes sense ... then never mind my comment :) Best regards, Kilian
Hello Roberto, I've applied your patch to our next branch, with some very minor updates. First, I simplified the commit title to: configs/roc_rk3388_pc: new defconfig See below for more comments. On Fri, 11 Aug 2023 18:25:38 +0200 Roberto Medina <robertoxmed@gmail.com> wrote: > * Uses a version 2.9 of TF-A which solves the building issue for this > board. > * Increase the size of the ext2 partition to be able to create the final > image. > * Bump and set the U-Boot version that was used for this configuration. > * Bump and set the version of Linux to 6.1.45. > * Some minor fixes in the post-build.sh script using `shellcheck`. These details are the details of the changes between the v2 and v3 of your patch, so they do not belong to the commit log, but they should go... > > Signed-off-by: Roberto Medina <robertoxmed@gmail.com> > Tested-by: Kilian Zinnecker <kilian.zinnecker@mail.de> > Reviewed-by: Kilian Zinnecker <kilian.zinnecker@mail.de> > --- .... here. > DEVELOPERS | 4 +++ > board/firefly/roc-rk3399-pc/extlinux.conf | 4 +++ > board/firefly/roc-rk3399-pc/genimage.cfg | 22 ++++++++++++ > board/firefly/roc-rk3399-pc/post-build.sh | 5 +++ > board/firefly/roc-rk3399-pc/readme.txt | 40 +++++++++++++++++++++ > configs/roc_pc_rk3399_defconfig | 42 +++++++++++++++++++++++ > 6 files changed, 117 insertions(+) > create mode 100644 board/firefly/roc-rk3399-pc/extlinux.conf > create mode 100644 board/firefly/roc-rk3399-pc/genimage.cfg > create mode 100755 board/firefly/roc-rk3399-pc/post-build.sh > create mode 100644 board/firefly/roc-rk3399-pc/readme.txt > create mode 100644 configs/roc_pc_rk3399_defconfig > > diff --git a/DEVELOPERS b/DEVELOPERS > index 6ffa3ee693..5ebabd01cb 100644 > --- a/DEVELOPERS > +++ b/DEVELOPERS > @@ -2546,6 +2546,10 @@ F: package/mhz/ > N: Robert Rose <robertroyrose@gmail.com> > F: package/grpc/ > > +N: Roberto Medina <robertoxmed@gmail.com> > +F: configs/roc_pc_rk3399_defconfig > +F: board/firefly/roc-rk3399 Alphabetic ordering was not correct here. And I added the final slash for the directory, as suggested by Kilian. Best regards, Thomas
On Fri, 11 Aug 2023 23:59:11 +0200 Kilian Zinnecker via buildroot <buildroot@buildroot.org> wrote: > > Actually in v2 of this patch, Thomas requested to set a fixed version > > for U-Boot (mail from 08/08). Which made sense to me as TFA and Linux > > have a fixed version as well. > > Ah, I see, makes sense ... then never mind my comment :) Correct, our convention in defconfigs is that the version of the Linux kernel and bootloader/firmware should be fixed. Indeed, we want the defconfigs to be known working configurations, that have been tested on hardware. If we let them the default/latest Linux/U-Boot version, it means this version gets auto-updated, without the defconfig maintainer necessarily checking that it is still works OK on HW. So: policy is to use fixed version for Linux kernel and bootloaders in defconfigs. Best regards, Thomas
Hi Thomas,
On 8/12/23 21:58, Thomas Petazzoni wrote:
> I've applied your patch to our next branch, with some very minor updates.
Woops, I was just working on a v4 when you applied I believe. Thank you
for the changes.
Best,
Roberto
diff --git a/DEVELOPERS b/DEVELOPERS index 6ffa3ee693..5ebabd01cb 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -2546,6 +2546,10 @@ F: package/mhz/ N: Robert Rose <robertroyrose@gmail.com> F: package/grpc/ +N: Roberto Medina <robertoxmed@gmail.com> +F: configs/roc_pc_rk3399_defconfig +F: board/firefly/roc-rk3399 + N: Rodrigo Rebello <rprebello@gmail.com> F: package/chocolate-doom/ F: package/irssi/ diff --git a/board/firefly/roc-rk3399-pc/extlinux.conf b/board/firefly/roc-rk3399-pc/extlinux.conf new file mode 100644 index 0000000000..50a358fadc --- /dev/null +++ b/board/firefly/roc-rk3399-pc/extlinux.conf @@ -0,0 +1,4 @@ +label RK3399RocPC linux + kernel /boot/Image + devicetree /boot/rk3399-roc-pc.dtb + append earlycon=uart8250,mmio32,0xff1a0000 root=/dev/mmcblk0p1 rootwait diff --git a/board/firefly/roc-rk3399-pc/genimage.cfg b/board/firefly/roc-rk3399-pc/genimage.cfg new file mode 100644 index 0000000000..966c869273 --- /dev/null +++ b/board/firefly/roc-rk3399-pc/genimage.cfg @@ -0,0 +1,22 @@ +image sdcard.img { + hdimage { + } + + partition u-boot-tpl-spl-dtb { + in-partition-table = "no" + image = "idbloader.img" + offset = 32K + } + + partition u-boot-dtb { + in-partition-table = "no" + image = "u-boot.itb" + offset = 8M + size = 30M + } + + partition rootfs { + partition-type = 0x83 + image = "rootfs.ext4" + } +} diff --git a/board/firefly/roc-rk3399-pc/post-build.sh b/board/firefly/roc-rk3399-pc/post-build.sh new file mode 100755 index 0000000000..d84a5da0b0 --- /dev/null +++ b/board/firefly/roc-rk3399-pc/post-build.sh @@ -0,0 +1,5 @@ +#!/bin/sh + +BOARD_DIR="$(dirname "$0")" + +install -m 0644 -D "$BOARD_DIR"/extlinux.conf "$TARGET_DIR"/boot/extlinux/extlinux.conf diff --git a/board/firefly/roc-rk3399-pc/readme.txt b/board/firefly/roc-rk3399-pc/readme.txt new file mode 100644 index 0000000000..8a6f1cdee6 --- /dev/null +++ b/board/firefly/roc-rk3399-pc/readme.txt @@ -0,0 +1,40 @@ +Libre Computer Board ROC-RK3399-PC +=================================== + +Build: + + $ make roc_pc_rk3399_defconfig + $ make + +Files created in output directory +================================= + +output/images + +├── bl31.elf +├── idbloader.img +├── Image +├── rk3399-roc-pc.dtb +├── rootfs.ext2 +├── rootfs.ext4 -> rootfs.ext2 +├── rootfs.tar +├── sdcard.img +├── u-boot.bin +└── u-boot.itb + +Creating bootable SD card: +========================== + +Simply invoke (as root) + +sudo dd if=output/images/sdcard.img of=/dev/sdX && sync + +Where X is your SD card device + +Serial console +-------------- + +Baudrate for this board is 1500000 + +Wiki link: +https://wiki.amarulasolutions.com/bsp/rockchip/rk3399/roc-rk3399-pc.html diff --git a/configs/roc_pc_rk3399_defconfig b/configs/roc_pc_rk3399_defconfig new file mode 100644 index 0000000000..3cc64be176 --- /dev/null +++ b/configs/roc_pc_rk3399_defconfig @@ -0,0 +1,42 @@ +BR2_aarch64=y +BR2_cortex_a72_a53=y +BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_6_1=y +BR2_TARGET_GENERIC_HOSTNAME="roc-rk3399-pc" +BR2_TARGET_GENERIC_ISSUE="Welcome to ROC-RK3399-PC!" +BR2_ROOTFS_POST_BUILD_SCRIPT="board/firefly/roc-rk3399-pc/post-build.sh" +BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh" +BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/firefly/roc-rk3399-pc/genimage.cfg" +BR2_LINUX_KERNEL=y +BR2_LINUX_KERNEL_CUSTOM_VERSION=y +BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="6.1.45" +BR2_LINUX_KERNEL_USE_ARCH_DEFAULT_CONFIG=y +BR2_LINUX_KERNEL_DTS_SUPPORT=y +BR2_LINUX_KERNEL_INTREE_DTS_NAME="rockchip/rk3399-roc-pc" +BR2_LINUX_KERNEL_INSTALL_TARGET=y +BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y +BR2_TARGET_ROOTFS_EXT2=y +BR2_TARGET_ROOTFS_EXT2_4=y +BR2_TARGET_ROOTFS_EXT2_SIZE="100M" +BR2_TARGET_ARM_TRUSTED_FIRMWARE=y +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git" +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="v2.9" +BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="rk3399" +BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGES="" +BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_ARM32_TOOLCHAIN=y +BR2_TARGET_UBOOT=y +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y +BR2_TARGET_UBOOT_CUSTOM_VERSION=y +BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2022.04" +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="roc-pc-rk3399" +BR2_TARGET_UBOOT_NEEDS_DTC=y +BR2_TARGET_UBOOT_NEEDS_PYTHON3=y +BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y +BR2_TARGET_UBOOT_NEEDS_ATF_BL31_ELF=y +BR2_TARGET_UBOOT_FORMAT_CUSTOM=y +BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot.itb" +BR2_TARGET_UBOOT_SPL=y +BR2_TARGET_UBOOT_SPL_NAME="idbloader.img" +BR2_PACKAGE_HOST_DOSFSTOOLS=y +BR2_PACKAGE_HOST_GENIMAGE=y +BR2_PACKAGE_HOST_MTOOLS=y