Re: [Buildroot] [PATCH RESEND 2/2] configs/stm32mp135f-dk: new defconfig

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

Hello Raphael,

On Wed, 14 Aug 2024 20:37:03 +0200
Raphael Gallais-Pou <rgallaispou@gmail.com> wrote:

> Add new defconfig for STMicroelectronics board STM32MP135F-DK.
> 
> STM32MP135F-DK features can be found here:
>   https://www.st.com/en/evaluation-tools/stm32mp135f-dk.html
> 
> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>

Thanks for the resend, but there was no need to resend: your previous
patch was still in our queue of patches to review/merge. I have a
number of comments below.

> ---
>  .../overlay/boot/extlinux/extlinux.conf            |  4 ++
>  board/stmicroelectronics/stm32mp135f-dk/readme.txt | 38 ++++++++++++++
>  configs/stm32mp135f_dk_defconfig                   | 59 ++++++++++++++++++++++
>  3 files changed, 101 insertions(+)

First of all, you need to enable BR2_DOWNLOAD_FORCE_CHECK_HASHES=y,
which requires adding:

BR2_GLOBAL_PATCH_DIR="board/stmicroelectronics/stm32mp135f-dk/patches/"

and then run ./utils/add-custom-hashes, which will automatically
populate this folder.


> diff --git a/configs/stm32mp135f_dk_defconfig b/configs/stm32mp135f_dk_defconfig
> new file mode 100644
> index 0000000000..cc01a2bd40
> --- /dev/null
> +++ b/configs/stm32mp135f_dk_defconfig
> @@ -0,0 +1,59 @@
> +# Architecture
> +BR2_arm=y
> +BR2_cortex_a7=y
> +
> +# Linux headers same as kernel, a 6.9 series
> +BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_6_9=y
> +
> +# System configuration
> +BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV=y
> +BR2_ROOTFS_OVERLAY="board/stmicroelectronics/stm32mp135f-dk/overlay"
> +BR2_ROOTFS_POST_IMAGE_SCRIPT="board/stmicroelectronics/common/stm32mp1xx/post-image.sh"
> +
> +# Kernel
> +BR2_LINUX_KERNEL=y
> +BR2_LINUX_KERNEL_CUSTOM_VERSION=y
> +BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="6.9.8"
> +BR2_LINUX_KERNEL_DEFCONFIG="multi_v7"

I see on STM32MP157, we're using some custom kernel configuration
files, with presumably a more "optimized" configuration than multi_v7.
Should we do the same here?

Or maybe we should even have a single common kernel config file for all
STM32MP1 platforms?

> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="st/stm32mp135f-dk"
> +BR2_LINUX_KERNEL_INSTALL_TARGET=y
> +
> +# Filesystem
> +BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y

Please group this with the Linux kernel options above, this is not
"filesystem related".

> +BR2_PACKAGE_OPTEE_CLIENT=y
> +BR2_TARGET_ROOTFS_EXT2=y
> +BR2_TARGET_ROOTFS_EXT2_4=y
> +BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
> +# BR2_TARGET_ROOTFS_TAR is not set
> +
> +# Bootloaders
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE=y
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_VERSION=y
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_VERSION_VALUE="v2.9"
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="stm32mp1"
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP=y
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31=y
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL32_OPTEE=y
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33=y
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_BL33_IMAGE="u-boot-nodtb.bin"
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES="STM32MP_SDMMC=1 DTB_FILE_NAME=stm32mp135f-dk.dtb E=0 BL33_CFG=$(BINARIES_DIR)/u-boot.dtb"
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGES="fip.bin *.stm32"
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_DTC=y
> +BR2_TARGET_OPTEE_OS=y

Please use the "custom" version mechanism to specify the exact version
of OP-TEE to be used, like you did for TF-A, Linux and U-Boot.

> +BR2_TARGET_OPTEE_OS_PLATFORM="stm32mp1"
> +BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR="135F_DK"
> +BR2_TARGET_UBOOT=y
> +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
> +BR2_TARGET_UBOOT_CUSTOM_VERSION=y
> +BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2024.07"
> +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="stm32mp13"
> +BR2_TARGET_UBOOT_NEEDS_PYLIBFDT=y
> +# BR2_TARGET_UBOOT_FORMAT_BIN is not set
> +BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
> +BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-nodtb.bin u-boot.dtb"
> +BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="DEVICE_TREE=stm32mp135f-dk"
> +
> +# Additional tools
> +BR2_PACKAGE_HOST_BMAP_TOOLS=y

Please drop, we don't enable bmap-tools in our defconfigs, we leave
that choice up to the user.

Thanks!

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