Re: [Buildroot] [PATCH v2] configs/imxrt1050-evk: New defconfig

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Jesse,

Thanks for your contribution!

On Sat, 30 Jul 2022 02:12:19 -0400
Jesse Taube <mr.bossman075@gmail.com> wrote:

> Add defconfig for imxrt1050-evk is a development board from NXP.
> 
> The i.MXRTxxxx family spreads from i.MXRT1020 to i.MXRT1170 with the
> first one supporting 1 USB OTG & 100M ethernet with a cortex-M7@500Mhz
> up to the latter with i.MXRT1170 with cortex-M7@1Ghz and
> cortex-M4@400Mhz, 2MB of internal SRAM, 2D GPU, 2x 1Gb and
> 1x 100Mb ENET. The i.MXRT family is NXP's answer to STM32F7xx, as it
> uses only simple SDRAM, it gives the chance of a 4 or less layer PCBs.
> Seeing that these chips are comparable to the STM32F7xxs which have
> buildroot ported to them it seems reasonable to add support for them.
> 
> https://www.nxp.com/design/development-boards/i-mx-evaluation-and-development-boards/i-mx-rt1050-evaluation-kit:MIMXRT1050-EVK
> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>

Thanks a lot, it looks mostly good, but I have a few comments/questions.

>  N:	Giulio Benetti <giulio.benetti@benettiengineering.com>
> +F:	board/freescale/imxrt1050evk/*
>  F:	board/olimex/a*
>  F:	configs/amarula_vyasa_rk3288_defconfig
>  F:	configs/asus_tinker_rk3288_defconfig
> +F:	configs/imxrt1050-evk_defconfig
>  F:	configs/olimex_a*
>  F:	package/at/
>  F:	package/binutils/

Any reason to add Giulio here?


> diff --git a/board/freescale/imxrt1050evk/post-build.sh b/board/freescale/imxrt1050evk/post-build.sh
> new file mode 100755
> index 0000000000..476958a691
> --- /dev/null
> +++ b/board/freescale/imxrt1050evk/post-build.sh
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +MKIMAGE=$HOST_DIR/bin/mkimage
> +
> +if [ -e $BINARIES_DIR/Image ]; then

Why is this test needed? The Image file will be produced by the kernel
build, so it should be there. If it's not, we should really error out
hard. So I think doing without the test is better.

> +	$MKIMAGE -A arm -O linux -T kernel -C none -a 0x80008000 -e 0x80008000 -n "Linux kernel" -d $BINARIES_DIR/Image $BINARIES_DIR/uImage
> +fi
> diff --git a/board/freescale/imxrt1050evk/readme.txt b/board/freescale/imxrt1050evk/readme.txt
> new file mode 100644
> index 0000000000..bbbcd5307f
> --- /dev/null
> +++ b/board/freescale/imxrt1050evk/readme.txt
> @@ -0,0 +1,24 @@
> +NXP i.MXRT1050 EVK board
> +---------------------

Add the few missing '-'

Also, as requested by Giulio, a short intro about the board + a link
would be useful here.


> +# Kernel
> +BR2_LINUX_KERNEL=y
> +BR2_LINUX_KERNEL_CUSTOM_VERSION=y
> +BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="5.19-rc5"

5.19 is out now, so you can use a non-rc version :)

> +BR2_LINUX_KERNEL_DEFCONFIG="imxrt"
> +BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM=y
> +BR2_LINUX_KERNEL_IMAGE_TARGET_NAME="Image"
> +BR2_LINUX_KERNEL_IMAGE_NAME="Image"
> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="imxrt1050-evk"
> +
> +# Filesystem
> +BR2_TARGET_ROOTFS_EXT2=y
> +BR2_TARGET_ROOTFS_EXT2_4=y
> +BR2_TARGET_ROOTFS_EXT2_LABEL="root"
> +BR2_TARGET_ROOTFS_EXT2_SIZE="3M"

Why limit to 3 MB ? Is there a strong reason? Also, leave the label
undefined.

> +# Bootloader
> +BR2_TARGET_UBOOT=y

Please used a version of U-Boot explicitly defined in the defconfig.

> +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="imxrt1050-evk"
> +BR2_TARGET_UBOOT_CONFIG_FRAGMENT_FILES="board/freescale/imxrt1050evk/uboot.fragment"
> +BR2_TARGET_UBOOT_FORMAT_IMG=y
> +BR2_TARGET_UBOOT_SPL=y
> +BR2_TARGET_UBOOT_SPL_NAME="SPL"
> +BR2_TARGET_UBOOT_NEEDS_OPENSSL=y
> +
> +# Required tools to create the SD card image
> +BR2_PACKAGE_HOST_GENIMAGE=y
> +BR2_PACKAGE_HOST_MKPASSWD=y
> +BR2_PACKAGE_HOST_UBOOT_TOOLS=y
> +
> +# Misc
> +BR2_BINUTILS_VERSION_2_36_X=y
> +# thumb2 ADR bug introduced in 2.37 fix is not yet in u-boot

Do you have some detail on this bug? Like a link to the bug report, or
something like this? Note that binutils 2.39 is soon going to arrive,
so we will get rid of binutils 2.36... which means we will very soon
have a problem with this defconfig.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot