[v2] configs/stm32mp135f-dk: new defconfig

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>
---
This commit series is a first attempt to improve the organization and
functionality of the STM32 platform in Buildroot. It comes with two
patches:

1. Reorder common ST folder:
   - Rename the common ST folder from stm32mp157 to stm32mp1xx.
   - Update references and paths in related configuration files.
   - Prepare to receive new stm32mp13 board.

2. Add new defconfig for STM32MP13:
   - Introduce a new defconfig file (stm32mp13_dk).
   - Boot following the TF-A/OP-TEE/U-Boot/Linux chain.
   v1:
   - Include generic "multi_v7" defconfig for Linux configuration.
   v2:
   - Include a minimalistic defconfig for Linux configuration.

Since this is a RFC, please feel free to discuss how those changes can
be improved.
---
Changes in v2:
- Removed merged patch:
https://gitlab.com/buildroot.org/buildroot/-/commit/f21db2bd83bf3073a50b8bcd79af6e7aed6fb000
- Include a minimalistic defconfig for Linux configuration
- Removed BR2_PACKAGE_HOST_BMAP_TOOLS option.
- Move BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL to the according section.
- Hardcode OP-TEE version using tarball link.
- Use BR2_GLOBAL_PATCH_DIR and add OP-TEE hash to common folder.
- Link to v1: https://lore.kernel.org/r/20240814-master-v1-0-7834f3deddcb@gmail.com
---
 .../stm32mp1xx/patches/optee-os/optee-os.hash      |   2 +
 .../stmicroelectronics/stm32mp135f-dk/linux.config | 190 +++++++++++++++++++++
 .../overlay/boot/extlinux/extlinux.conf            |   4 +
 board/stmicroelectronics/stm32mp135f-dk/readme.txt |  38 +++++
 configs/stm32mp135f_dk_defconfig                   |  64 +++++++
 5 files changed, 298 insertions(+)


---
base-commit: e3f464778945d3e7ed31c50d07de02fb2cd85464
change-id: 20240729-master-3a7c9cb9c96b

Best regards,

Le 17/10/2024 à 23:28, Raphael Gallais-Pou a écrit :
> 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>
> ---
> This commit series is a first attempt to improve the organization and
> functionality of the STM32 platform in Buildroot. It comes with two
> patches:
> 
> 1. Reorder common ST folder:
>     - Rename the common ST folder from stm32mp157 to stm32mp1xx.
>     - Update references and paths in related configuration files.
>     - Prepare to receive new stm32mp13 board.
> 
> 2. Add new defconfig for STM32MP13:
>     - Introduce a new defconfig file (stm32mp13_dk).
>     - Boot following the TF-A/OP-TEE/U-Boot/Linux chain.
>     v1:
>     - Include generic "multi_v7" defconfig for Linux configuration.
>     v2:
>     - Include a minimalistic defconfig for Linux configuration.
> 
> Since this is a RFC, please feel free to discuss how those changes can
> be improved.
> ---
> Changes in v2:
> - Removed merged patch:
> https://gitlab.com/buildroot.org/buildroot/-/commit/f21db2bd83bf3073a50b8bcd79af6e7aed6fb000
> - Include a minimalistic defconfig for Linux configuration
> - Removed BR2_PACKAGE_HOST_BMAP_TOOLS option.
> - Move BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL to the according section.
> - Hardcode OP-TEE version using tarball link.
> - Use BR2_GLOBAL_PATCH_DIR and add OP-TEE hash to common folder.
> - Link to v1: https://lore.kernel.org/r/20240814-master-v1-0-7834f3deddcb@gmail.com

Hi,

Just a gentle ping on this v2. :-)

Thanks for your time,

> 
> Best regards,

Raphaël

Hi Raphael,

  Although this is good enough to merge, I have a few comments that you still 
may want to include. If you tell me you have no time it can be merged as is.

On 17/10/2024 23:28, Raphael Gallais-Pou wrote:
[snip]
> diff --git a/board/stmicroelectronics/stm32mp135f-dk/linux.config b/board/stmicroelectronics/stm32mp135f-dk/linux.config
> new file mode 100644
> index 0000000000000000000000000000000000000000..533f22345b0c6fe69edf7fc901996809377f6162
> --- /dev/null
> +++ b/board/stmicroelectronics/stm32mp135f-dk/linux.config
> @@ -0,0 +1,190 @@
> +CONFIG_SYSVIPC=y
> +CONFIG_POSIX_MQUEUE=y
> +CONFIG_USELIB=y
> +CONFIG_NO_HZ=y
> +CONFIG_HIGH_RES_TIMERS=y
> +CONFIG_PREEMPT=y
> +CONFIG_IKCONFIG=y
> +CONFIG_IKCONFIG_PROC=y
> +CONFIG_LOG_BUF_SHIFT=16
> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_ARCH_STM32=y
> +CONFIG_ARM_THUMBEE=y
> +# CONFIG_CACHE_L2X0 is not set
> +CONFIG_ARM_ERRATA_430973=y
> +CONFIG_ARM_ERRATA_720789=y
> +CONFIG_ARM_ERRATA_754322=y
> +CONFIG_ARM_ERRATA_754327=y
> +CONFIG_ARM_ERRATA_764369=y
> +CONFIG_ARM_ERRATA_775420=y
> +CONFIG_ARM_ERRATA_798181=y
> +CONFIG_SMP=y
> +CONFIG_MCPM=y
> +CONFIG_HIGHMEM=y
> +CONFIG_FORCE_MAX_ZONEORDER=12
> +CONFIG_SECCOMP=y
> +# CONFIG_ATAGS is not set
> +CONFIG_ZBOOT_ROM_TEXT=0x0
> +CONFIG_ZBOOT_ROM_BSS=0x0
> +CONFIG_ARM_APPENDED_DTB=y
> +CONFIG_ARM_ATAG_DTB_COMPAT=y
> +CONFIG_VFP=y
> +CONFIG_NEON=y
> +CONFIG_KERNEL_MODE_NEON=y
> +CONFIG_MODULES=y
> +CONFIG_MODULE_UNLOAD=y
> +CONFIG_PARTITION_ADVANCED=y
> +CONFIG_CMDLINE_PARTITION=y
> +CONFIG_CMA=y
> +CONFIG_NET=y
> +CONFIG_PACKET=y
> +CONFIG_UNIX=y
> +CONFIG_INET=y
> +CONFIG_CAN=y
> +CONFIG_CAN_M_CAN=y
> +CONFIG_DEVTMPFS=y
> +CONFIG_DEVTMPFS_MOUNT=y
> +CONFIG_DMA_CMA=y
> +CONFIG_CMA_SIZE_MBYTES=128
> +CONFIG_SIMPLE_PM_BUS=y
> +CONFIG_ARM_SCMI_PROTOCOL=y
> +CONFIG_MTD=y
> +CONFIG_MTD_CMDLINE_PARTS=y
> +CONFIG_MTD_BLOCK=y
> +CONFIG_MTD_M25P80=y
> +CONFIG_MTD_NAND=y
> +CONFIG_MTD_NAND_STM32_FMC2=y
> +CONFIG_MTD_SPI_NOR=y
> +# CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
> +CONFIG_MTD_UBI=y
> +CONFIG_BLK_DEV_LOOP=y
> +CONFIG_BLK_DEV_RAM=y
> +CONFIG_BLK_DEV_RAM_SIZE=65536
> +CONFIG_SRAM=y
> +CONFIG_EEPROM_AT24=y
> +CONFIG_NETDEVICES=y
> +CONFIG_VIRTIO_NET=y
> +CONFIG_KS8851=y
> +CONFIG_SMSC911X=y
> +CONFIG_STMMAC_ETH=y
> +CONFIG_DWMAC_DWC_QOS_ETH=y
> +CONFIG_MDIO_BITBANG=y
> +CONFIG_INPUT_JOYDEV=y
> +CONFIG_INPUT_EVDEV=y
> +CONFIG_INPUT_TOUCHSCREEN=y
> +CONFIG_TOUCHSCREEN_EDT_FT5X06=y
> +CONFIG_INPUT_MISC=y
> +CONFIG_INPUT_STPMIC1_ONKEY=y
> +CONFIG_SERIAL_STM32=y
> +CONFIG_SERIAL_STM32_CONSOLE=y
> +CONFIG_SERIAL_DEV_BUS=y
> +CONFIG_HW_RANDOM=y
> +CONFIG_I2C_CHARDEV=y
> +CONFIG_I2C_MUX=y
> +CONFIG_I2C_STM32F7=y
> +CONFIG_SPI=y
> +CONFIG_SPI_STM32=y
> +CONFIG_SPI_STM32_QSPI=y
> +CONFIG_PINCTRL_MCP23S08=m

  IIUC this chip exists on the board, so there's no reason to make it a module, 
right?

> +CONFIG_PINCTRL_SINGLE=y
> +CONFIG_PINCTRL_STMFX=y
> +CONFIG_POWER_RESET=y
> +CONFIG_POWER_RESET_GPIO=y
> +CONFIG_POWER_RESET_GPIO_RESTART=y
> +CONFIG_POWER_RESET_SYSCON=y
> +CONFIG_POWER_RESET_SYSCON_POWEROFF=y
> +CONFIG_SYSCON_REBOOT_MODE=y
> +CONFIG_WATCHDOG=y
> +CONFIG_STPMIC1_WATCHDOG=y
> +CONFIG_MFD_STM32_LPTIMER=y
> +CONFIG_MFD_STPMIC1=y
> +CONFIG_REGULATOR=y
> +CONFIG_REGULATOR_FIXED_VOLTAGE=y
> +CONFIG_REGULATOR_ARM_SCMI=y
> +CONFIG_REGULATOR_STM32_BOOSTER=y
> +CONFIG_REGULATOR_STM32_VREFBUF=y
> +CONFIG_REGULATOR_STM32_PWR=y
> +CONFIG_REGULATOR_STPMIC1=y
> +CONFIG_DRM=y
> +CONFIG_DRM_STM=y
> +CONFIG_DRM_STM_DSI=y
> +CONFIG_DRM_PANEL_ORISETECH_OTM8009A=y
> +CONFIG_DRM_SII902X=y
> +CONFIG_BACKLIGHT_LCD_SUPPORT=y
> +# CONFIG_LCD_CLASS_DEVICE is not set
> +CONFIG_BACKLIGHT_CLASS_DEVICE=y
> +# CONFIG_BACKLIGHT_GENERIC is not set
> +CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
> +CONFIG_SOUND=y
> +CONFIG_SND=y
> +CONFIG_SND_DYNAMIC_MINORS=y
> +CONFIG_USB=y
> +CONFIG_USB_OTG=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_PLATFORM=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_EHCI_HCD_PLATFORM=y
> +CONFIG_USB_OHCI_HCD=y
> +CONFIG_USB_OHCI_HCD_PLATFORM=y
> +CONFIG_USB_DWC2=y
> +CONFIG_USB_CHIPIDEA=y
> +CONFIG_USB_CHIPIDEA_HOST=y
> +CONFIG_USB_ISP1760=y
> +CONFIG_USB_HSIC_USB3503=y
> +CONFIG_USB_GPIO_VBUS=y
> +CONFIG_USB_ISP1301=y
> +CONFIG_USB_ULPI=y

  If there are pluggable ports, it makes sense to enable all possible USB device 
drivers as modules.

  Same for the gadgets BTW.

> +CONFIG_TYPEC=y
> +CONFIG_TYPEC_STUSB=y
> +CONFIG_MMC=y
> +CONFIG_MMC_BLOCK_MINORS=16
> +CONFIG_MMC_ARMMMCI=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_PLTFM=y
> +CONFIG_MMC_DW=y
> +CONFIG_NEW_LEDS=y
> +CONFIG_LEDS_CLASS=y
> +CONFIG_LEDS_GPIO=y
> +CONFIG_LEDS_PWM=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_STM32=y
> +CONFIG_DMADEVICES=y
> +CONFIG_STM32_DMA=y
> +CONFIG_STM32_DMAMUX=y
> +CONFIG_STM32_MDMA=y
> +CONFIG_COMMON_CLK_SCMI=y
> +CONFIG_HWSPINLOCK=y
> +CONFIG_HWSPINLOCK_STM32=y
> +CONFIG_STM32_IPCC=y
> +CONFIG_REMOTEPROC=y
> +CONFIG_STM32_RPROC=y
> +CONFIG_RPMSG_VIRTIO=y
> +CONFIG_RPMSG_TTY=y
> +CONFIG_IIO=y
> +CONFIG_IIO_SW_TRIGGER=y
> +CONFIG_SD_ADC_MODULATOR=y
> +CONFIG_STM32_ADC_CORE=y
> +CONFIG_STM32_ADC=y
> +CONFIG_STM32_ADC_TEMP=y
> +CONFIG_STM32_DFSDM_ADC=y
> +CONFIG_STM32_LPTIMER_CNT=y
> +CONFIG_STM32_DAC=y
> +CONFIG_IIO_HRTIMER_TRIGGER=y
> +CONFIG_IIO_STM32_LPTIMER_TRIGGER=y
> +CONFIG_PWM=y
> +CONFIG_PWM_STM32=y
> +CONFIG_PWM_STM32_LP=y
> +CONFIG_RESET_SCMI=y
> +CONFIG_PHY_STM32_USBPHYC=y
> +CONFIG_NVMEM_STM32_ROMEM=y
> +CONFIG_TEE=y
> +CONFIG_OPTEE=y
> +CONFIG_COUNTER=m
> +CONFIG_STM32_LPTIMER_CNT=m
> +CONFIG_STM32_TIMER_CNT=m

  Again, no reason for these to be modules.

> +CONFIG_EXT4_FS=y
> +CONFIG_MSDOS_FS=y
> +CONFIG_VFAT_FS=y
> +CONFIG_TMPFS=y
> +CONFIG_TMPFS_POSIX_ACL=y
> +CONFIG_PRINTK_TIME=y

[snip]
> diff --git a/configs/stm32mp135f_dk_defconfig b/configs/stm32mp135f_dk_defconfig
> new file mode 100644
> index 0000000000000000000000000000000000000000..d41821bdaf6e94c255e38f468752dcd28abb6bbb
> --- /dev/null
> +++ b/configs/stm32mp135f_dk_defconfig
> @@ -0,0 +1,64 @@
> +# Architecture

  In the mean time we decided to save all defconfigs as a defconfig, without 
comments etc.

> +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_GLOBAL_PATCH_DIR="board/stmicroelectronics/common/stm32mp1xx/patches"
> +BR2_DOWNLOAD_FORCE_CHECK_HASHES=y
> +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"

  Not agreed on yet in the community, but I would like to use LTS versions if 
possible (so 6.6.x). If that is too old, then a 6.11 would be better of course.


> +BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
> +BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/stmicroelectronics/stm32mp135f-dk/linux.config"
> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="st/stm32mp135f-dk"
> +BR2_LINUX_KERNEL_INSTALL_TARGET=y
> +BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
> +
> +# Filesystem
> +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"

  Again, no agreement has been reached, but I'd prefer 
BR2_TARGET_ARM_TRUSTED_FIRMWARE_LATEST_LTS_2_10_VERSION.


> +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"

  Is the E=0 actually needed?

> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGES="fip.bin *.stm32"
> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_DTC=y
> +BR2_TARGET_OPTEE_OS=y
> +BR2_TARGET_OPTEE_OS_CUSTOM_TARBALL=y
> +BR2_TARGET_OPTEE_OS_CUSTOM_TARBALL_LOCATION="https://github.com/OP-TEE/optee_os/archive/refs/tags/4.3.0.tar.gz"

  We also decided to avoid github tarballs and instead use the git download 
method, because then the hash is actually guaranteed to be stable.


  Regards,
  Arnout

> +BR2_TARGET_OPTEE_OS_NEEDS_PYTHON_CRYPTOGRAPHY=y
> +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_GENIMAGE=y
> 
> ---
> base-commit: e3f464778945d3e7ed31c50d07de02fb2cd85464
> change-id: 20240729-master-3a7c9cb9c96b
> 
> Best regards,

Le 06/11/2024 à 23:24, Arnout Vandecappelle a écrit :
>   Hi Raphael,
Hi Arnout,

Thank you for taking the time to do this review.
> 
>   Although this is good enough to merge, I have a few comments that you 
> still may want to include. If you tell me you have no time it can be 
> merged as is.

I would prefer to do a v3 of this patch, and take into account your 
review for another version.
> 
> On 17/10/2024 23:28, Raphael Gallais-Pou wrote:
> [snip]
>> diff --git a/board/stmicroelectronics/stm32mp135f-dk/linux.config b/ 
>> board/stmicroelectronics/stm32mp135f-dk/linux.config
>> new file mode 100644
>> index 
>> 0000000000000000000000000000000000000000..533f22345b0c6fe69edf7fc901996809377f6162
>> --- /dev/null
>> +++ b/board/stmicroelectronics/stm32mp135f-dk/linux.config
>> @@ -0,0 +1,190 @@
>> +CONFIG_SYSVIPC=y
>> +CONFIG_POSIX_MQUEUE=y
>> +CONFIG_USELIB=y
>> +CONFIG_NO_HZ=y
>> +CONFIG_HIGH_RES_TIMERS=y
>> +CONFIG_PREEMPT=y
>> +CONFIG_IKCONFIG=y
>> +CONFIG_IKCONFIG_PROC=y
>> +CONFIG_LOG_BUF_SHIFT=16
>> +CONFIG_BLK_DEV_INITRD=y
>> +CONFIG_ARCH_STM32=y
>> +CONFIG_ARM_THUMBEE=y
>> +# CONFIG_CACHE_L2X0 is not set
>> +CONFIG_ARM_ERRATA_430973=y
>> +CONFIG_ARM_ERRATA_720789=y
>> +CONFIG_ARM_ERRATA_754322=y
>> +CONFIG_ARM_ERRATA_754327=y
>> +CONFIG_ARM_ERRATA_764369=y
>> +CONFIG_ARM_ERRATA_775420=y
>> +CONFIG_ARM_ERRATA_798181=y
>> +CONFIG_SMP=y
>> +CONFIG_MCPM=y
>> +CONFIG_HIGHMEM=y
>> +CONFIG_FORCE_MAX_ZONEORDER=12
>> +CONFIG_SECCOMP=y
>> +# CONFIG_ATAGS is not set
>> +CONFIG_ZBOOT_ROM_TEXT=0x0
>> +CONFIG_ZBOOT_ROM_BSS=0x0
>> +CONFIG_ARM_APPENDED_DTB=y
>> +CONFIG_ARM_ATAG_DTB_COMPAT=y
>> +CONFIG_VFP=y
>> +CONFIG_NEON=y
>> +CONFIG_KERNEL_MODE_NEON=y
>> +CONFIG_MODULES=y
>> +CONFIG_MODULE_UNLOAD=y
>> +CONFIG_PARTITION_ADVANCED=y
>> +CONFIG_CMDLINE_PARTITION=y
>> +CONFIG_CMA=y
>> +CONFIG_NET=y
>> +CONFIG_PACKET=y
>> +CONFIG_UNIX=y
>> +CONFIG_INET=y
>> +CONFIG_CAN=y
>> +CONFIG_CAN_M_CAN=y
>> +CONFIG_DEVTMPFS=y
>> +CONFIG_DEVTMPFS_MOUNT=y
>> +CONFIG_DMA_CMA=y
>> +CONFIG_CMA_SIZE_MBYTES=128
>> +CONFIG_SIMPLE_PM_BUS=y
>> +CONFIG_ARM_SCMI_PROTOCOL=y
>> +CONFIG_MTD=y
>> +CONFIG_MTD_CMDLINE_PARTS=y
>> +CONFIG_MTD_BLOCK=y
>> +CONFIG_MTD_M25P80=y
>> +CONFIG_MTD_NAND=y
>> +CONFIG_MTD_NAND_STM32_FMC2=y
>> +CONFIG_MTD_SPI_NOR=y
>> +# CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
>> +CONFIG_MTD_UBI=y
>> +CONFIG_BLK_DEV_LOOP=y
>> +CONFIG_BLK_DEV_RAM=y
>> +CONFIG_BLK_DEV_RAM_SIZE=65536
>> +CONFIG_SRAM=y
>> +CONFIG_EEPROM_AT24=y
>> +CONFIG_NETDEVICES=y
>> +CONFIG_VIRTIO_NET=y
>> +CONFIG_KS8851=y
>> +CONFIG_SMSC911X=y
>> +CONFIG_STMMAC_ETH=y
>> +CONFIG_DWMAC_DWC_QOS_ETH=y
>> +CONFIG_MDIO_BITBANG=y
>> +CONFIG_INPUT_JOYDEV=y
>> +CONFIG_INPUT_EVDEV=y
>> +CONFIG_INPUT_TOUCHSCREEN=y
>> +CONFIG_TOUCHSCREEN_EDT_FT5X06=y
>> +CONFIG_INPUT_MISC=y
>> +CONFIG_INPUT_STPMIC1_ONKEY=y
>> +CONFIG_SERIAL_STM32=y
>> +CONFIG_SERIAL_STM32_CONSOLE=y
>> +CONFIG_SERIAL_DEV_BUS=y
>> +CONFIG_HW_RANDOM=y
>> +CONFIG_I2C_CHARDEV=y
>> +CONFIG_I2C_MUX=y
>> +CONFIG_I2C_STM32F7=y
>> +CONFIG_SPI=y
>> +CONFIG_SPI_STM32=y
>> +CONFIG_SPI_STM32_QSPI=y
>> +CONFIG_PINCTRL_MCP23S08=m
> 
>   IIUC this chip exists on the board, so there's no reason to make it a 
> module, right?

Yes, looking at ST's Linux fork, CONFIG_PINCTRL_MCP23S08=y in the addons 
fragment. I will do so in the v3.

cf.
https://github.com/STMicroelectronics/linux/blob/v6.1-stm32mp/arch/arm/configs/fragment-02-multiv7_addons.config#L58
> 
>> +CONFIG_PINCTRL_SINGLE=y
>> +CONFIG_PINCTRL_STMFX=y
>> +CONFIG_POWER_RESET=y
>> +CONFIG_POWER_RESET_GPIO=y
>> +CONFIG_POWER_RESET_GPIO_RESTART=y
>> +CONFIG_POWER_RESET_SYSCON=y
>> +CONFIG_POWER_RESET_SYSCON_POWEROFF=y
>> +CONFIG_SYSCON_REBOOT_MODE=y
>> +CONFIG_WATCHDOG=y
>> +CONFIG_STPMIC1_WATCHDOG=y
>> +CONFIG_MFD_STM32_LPTIMER=y
>> +CONFIG_MFD_STPMIC1=y
>> +CONFIG_REGULATOR=y
>> +CONFIG_REGULATOR_FIXED_VOLTAGE=y
>> +CONFIG_REGULATOR_ARM_SCMI=y
>> +CONFIG_REGULATOR_STM32_BOOSTER=y
>> +CONFIG_REGULATOR_STM32_VREFBUF=y
>> +CONFIG_REGULATOR_STM32_PWR=y
>> +CONFIG_REGULATOR_STPMIC1=y
>> +CONFIG_DRM=y
>> +CONFIG_DRM_STM=y
>> +CONFIG_DRM_STM_DSI=y
>> +CONFIG_DRM_PANEL_ORISETECH_OTM8009A=y
>> +CONFIG_DRM_SII902X=y
>> +CONFIG_BACKLIGHT_LCD_SUPPORT=y
>> +# CONFIG_LCD_CLASS_DEVICE is not set
>> +CONFIG_BACKLIGHT_CLASS_DEVICE=y
>> +# CONFIG_BACKLIGHT_GENERIC is not set
>> +CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
>> +CONFIG_SOUND=y
>> +CONFIG_SND=y
>> +CONFIG_SND_DYNAMIC_MINORS=y
>> +CONFIG_USB=y
>> +CONFIG_USB_OTG=y
>> +CONFIG_USB_XHCI_HCD=y
>> +CONFIG_USB_XHCI_PLATFORM=y
>> +CONFIG_USB_EHCI_HCD=y
>> +CONFIG_USB_EHCI_HCD_PLATFORM=y
>> +CONFIG_USB_OHCI_HCD=y
>> +CONFIG_USB_OHCI_HCD_PLATFORM=y
>> +CONFIG_USB_DWC2=y
>> +CONFIG_USB_CHIPIDEA=y
>> +CONFIG_USB_CHIPIDEA_HOST=y
>> +CONFIG_USB_ISP1760=y
>> +CONFIG_USB_HSIC_USB3503=y
>> +CONFIG_USB_GPIO_VBUS=y
>> +CONFIG_USB_ISP1301=y
>> +CONFIG_USB_ULPI=y
> 
>   If there are pluggable ports, it makes sense to enable all possible 
> USB device drivers as modules.
> 
>   Same for the gadgets BTW.

Ack, those will set as modules in the next version.
> 
>> +CONFIG_TYPEC=y
>> +CONFIG_TYPEC_STUSB=y
>> +CONFIG_MMC=y
>> +CONFIG_MMC_BLOCK_MINORS=16
>> +CONFIG_MMC_ARMMMCI=y
>> +CONFIG_MMC_SDHCI=y
>> +CONFIG_MMC_SDHCI_PLTFM=y
>> +CONFIG_MMC_DW=y
>> +CONFIG_NEW_LEDS=y
>> +CONFIG_LEDS_CLASS=y
>> +CONFIG_LEDS_GPIO=y
>> +CONFIG_LEDS_PWM=y
>> +CONFIG_RTC_CLASS=y
>> +CONFIG_RTC_DRV_STM32=y
>> +CONFIG_DMADEVICES=y
>> +CONFIG_STM32_DMA=y
>> +CONFIG_STM32_DMAMUX=y
>> +CONFIG_STM32_MDMA=y
>> +CONFIG_COMMON_CLK_SCMI=y
>> +CONFIG_HWSPINLOCK=y
>> +CONFIG_HWSPINLOCK_STM32=y
>> +CONFIG_STM32_IPCC=y
>> +CONFIG_REMOTEPROC=y
>> +CONFIG_STM32_RPROC=y
>> +CONFIG_RPMSG_VIRTIO=y
>> +CONFIG_RPMSG_TTY=y
>> +CONFIG_IIO=y
>> +CONFIG_IIO_SW_TRIGGER=y
>> +CONFIG_SD_ADC_MODULATOR=y
>> +CONFIG_STM32_ADC_CORE=y
>> +CONFIG_STM32_ADC=y
>> +CONFIG_STM32_ADC_TEMP=y
>> +CONFIG_STM32_DFSDM_ADC=y
>> +CONFIG_STM32_LPTIMER_CNT=y
>> +CONFIG_STM32_DAC=y
>> +CONFIG_IIO_HRTIMER_TRIGGER=y
>> +CONFIG_IIO_STM32_LPTIMER_TRIGGER=y
>> +CONFIG_PWM=y
>> +CONFIG_PWM_STM32=y
>> +CONFIG_PWM_STM32_LP=y
>> +CONFIG_RESET_SCMI=y
>> +CONFIG_PHY_STM32_USBPHYC=y
>> +CONFIG_NVMEM_STM32_ROMEM=y
>> +CONFIG_TEE=y
>> +CONFIG_OPTEE=y
>> +CONFIG_COUNTER=m
>> +CONFIG_STM32_LPTIMER_CNT=m
>> +CONFIG_STM32_TIMER_CNT=m
> 
>   Again, no reason for these to be modules.

CONFIG_STM32_LPTIMER_CNT and CONFIG_STM32_TIMER_CNT are set as modules 
in the generic multi_v7 defconfig. I have no will against making those 
drivers built-in, but would it be better to have aligned options with 
the upstream defconfig ? We would get a smaller kernel, in a certain way 
(I'm not sure two built-in drivers will make a difference in the kernel 
size). If it is not relevant I will gladly test and set drivers with 
built-in configuration.
> 
>> +CONFIG_EXT4_FS=y
>> +CONFIG_MSDOS_FS=y
>> +CONFIG_VFAT_FS=y
>> +CONFIG_TMPFS=y
>> +CONFIG_TMPFS_POSIX_ACL=y
>> +CONFIG_PRINTK_TIME=y
> 
> [snip]
>> diff --git a/configs/stm32mp135f_dk_defconfig b/configs/ 
>> stm32mp135f_dk_defconfig
>> new file mode 100644
>> index 
>> 0000000000000000000000000000000000000000..d41821bdaf6e94c255e38f468752dcd28abb6bbb
>> --- /dev/null
>> +++ b/configs/stm32mp135f_dk_defconfig
>> @@ -0,0 +1,64 @@
>> +# Architecture
> 
>   In the mean time we decided to save all defconfigs as a defconfig, 
> without comments etc.

Ack. I'll change that.
> 
>> +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_GLOBAL_PATCH_DIR="board/stmicroelectronics/common/stm32mp1xx/ 
>> patches"
>> +BR2_DOWNLOAD_FORCE_CHECK_HASHES=y
>> +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"
> 
>   Not agreed on yet in the community, but I would like to use LTS 
> versions if possible (so 6.6.x). If that is too old, then a 6.11 would 
> be better of course.

Agreed. It makes more sense to use a LTS version, that allows a more 
stable software and costs less to the maintainers. Although 6.6 is 
rather old for the boards, I will use and test with the 6.11.6 kernel 
version.
> 
> 
>> +BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
>> +BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/stmicroelectronics/ 
>> stm32mp135f-dk/linux.config"
>> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
>> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="st/stm32mp135f-dk"
>> +BR2_LINUX_KERNEL_INSTALL_TARGET=y
>> +BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
>> +
>> +# Filesystem
>> +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"
> 
>   Again, no agreement has been reached, but I'd prefer 
> BR2_TARGET_ARM_TRUSTED_FIRMWARE_LATEST_LTS_2_10_VERSION.

Ack. I will set this.
> 
> 
>> +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"
> 
>   Is the E=0 actually needed?

TBH I do not remember why I have set this option. In the main TF-A 
makefile, this option is set by default. So the E=0 is not really needed 
in the command line.

cf.
https://github.com/ARM-software/arm-trusted-firmware/blob/master/Makefile#L239
https://github.com/ARM-software/arm-trusted-firmware/blob/master/Makefile#L273
> 
>> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGES="fip.bin *.stm32"
>> +BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_DTC=y
>> +BR2_TARGET_OPTEE_OS=y
>> +BR2_TARGET_OPTEE_OS_CUSTOM_TARBALL=y
>> +BR2_TARGET_OPTEE_OS_CUSTOM_TARBALL_LOCATION="https://github.com/OP- 
>> TEE/optee_os/archive/refs/tags/4.3.0.tar.gz"
> 
>   We also decided to avoid github tarballs and instead use the git 
> download method, because then the hash is actually guaranteed to be stable.

Ack, I will provide OP-TEE through the git download method. And 
regenerate the hashes accordingly.

Thanks,
Best regards,
Raphaël
> 
> 
>   Regards,
>   Arnout
> 
>> +BR2_TARGET_OPTEE_OS_NEEDS_PYTHON_CRYPTOGRAPHY=y
>> +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_GENIMAGE=y
>>
>> ---
>> base-commit: e3f464778945d3e7ed31c50d07de02fb2cd85464
>> change-id: 20240729-master-3a7c9cb9c96b
>>
>> Best regards,
>

On 14/11/2024 11:32, Raphaël Gallais-Pou wrote:
> 
> 
> Le 06/11/2024 à 23:24, Arnout Vandecappelle a écrit :
>>   Hi Raphael,
> Hi Arnout,
> 
> Thank you for taking the time to do this review.
>>
>>   Although this is good enough to merge, I have a few comments that you still 
>> may want to include. If you tell me you have no time it can be merged as is.
> 
> I would prefer to do a v3 of this patch, and take into account your review for 
> another version.
>>
>> On 17/10/2024 23:28, Raphael Gallais-Pou wrote:

[snip]
>>> +CONFIG_COUNTER=m
>>> +CONFIG_STM32_LPTIMER_CNT=m
>>> +CONFIG_STM32_TIMER_CNT=m
>>
>>   Again, no reason for these to be modules.
> 
> CONFIG_STM32_LPTIMER_CNT and CONFIG_STM32_TIMER_CNT are set as modules in the 
> generic multi_v7 defconfig. I have no will against making those drivers built- 
> in, but would it be better to have aligned options with the upstream defconfig ? 
> We would get a smaller kernel, in a certain way (I'm not sure two built-in 
> drivers will make a difference in the kernel size). If it is not relevant I will 
> gladly test and set drivers with built-in configuration.

  The idea is that we have two options for the kernel config in Buildroot:

- Use an upstream defconfig from the kernel itself. That defconfig should be as 
specific as possible, but nowadays the only upstream option is multi_v7 for 
32-bit and arm64 for 64-bit.
- Use a specific defconfig in Buildroot, to remove the "fat" and therefore size 
and build time. In this case, we want everything that is always there built in 
("always there" means that it's not hot-pluggable, and enabled in device tree), 
because that still reduces total size (a module is bigger than if it's linked in 
the kernel).

  If these two peripherals are enabled in device tree, then their modules will 
anyway get loaded (if module autoloading is enabled), so putting them in modules 
doesn't decrease the size in any meaningful way.

  Regards,
  Arnout

[snip]

Le 14/11/2024 à 11:49, Arnout Vandecappelle a écrit :
> 
>   The idea is that we have two options for the kernel config in Buildroot:
> 
> - Use an upstream defconfig from the kernel itself. That defconfig 
> should be as specific as possible, but nowadays the only upstream option 
> is multi_v7 for 32-bit and arm64 for 64-bit.
> - Use a specific defconfig in Buildroot, to remove the "fat" and 
> therefore size and build time. In this case, we want everything that is 
> always there built in ("always there" means that it's not hot-pluggable, 
> and enabled in device tree), because that still reduces total size (a 
> module is bigger than if it's linked in the kernel).
> 
>   If these two peripherals are enabled in device tree, then their 
> modules will anyway get loaded (if module autoloading is enabled), so 
> putting them in modules doesn't decrease the size in any meaningful way.
> 

Thanks for the clarification. This is clearer :)

So I will sent a v3 promptly with the STM32_{LP}TIMER_CNT set as builtin 
drivers.

Regards,
Raphaël
>   Regards,
>   Arnout