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

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

Hello Raphael,

On Sat, 17 Aug 2024 12:55:47 +0200
Raphaël Gallais-Pou <rgallaispou@gmail.com> wrote:

> > 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.  
> 
> Sorry for the inconvenience. I had no news of the patchset which led me 
> to resend it. Thanks for taking the time to review this one and merge 
> the preview.

Patches are never forgotten: they are tracked under patchwork as soon
as they are posted on the mailing list. If you have no news, you can
ping on the patch series if you want, but resending without any changes
isn't very useful.

> > 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.  
> 
> For this one I think that it will be better to set the directory as the 
> same of the common directory, eg. 
> "board/stmicroelectronics/common/stm32mp1xx/patches/". So that the 
> versions between TF-A/U-Boot, the Linux kernel as well as OP-TEE, if it 
> is activated in the future for the other platforms, will be synced 
> between all stm32mp1* platforms.

But then it means that all the stm32mp1 defconfigs must be updated in
sync whenever Linux, U-Boot, TF-A or OP-TEE are bumped.


> > 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?  
> 
> Effectively this reduces the overall kernel size. My thought on this 
> would be to set the general multi_v7 kernel defconfig on the initial 
> stm32mp13 support, and fine tune after. Because there is many hardware 
> differences between stm32mp15 and stm32mp13 platforms (for example there 
> is no audio jack in my knowledge), I feel like this would take quite 
> some time for me to sort every drivers.
> 
> What do you think about this process ?

Why not, but since we'll have a single common kernel config for all,
this story of "not having audio jack on STM32MP13" is not very
relevant. I.e, what not just use the existing kernel configuration used
for MP15 also for MP13, and just add what's missing in this common
configuration? We're also not trying to make the super super optimized
kernel configuration with only strictly what's needed. But multi_v7 is
massively bloated.

> > 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.  
> 
> Ack, forgot about that one. I saw that there was no "custom version" 
> option, like there is for U-Boot and the Kernel. And greping through the 
> other defconfig there is no hardcoded versions. Maybe this can be a idea 
> for the future. For now I will set a custom tarball pointing to the 
> official 4.3.0 tarball. Would this be okay ?
> 
> cf. https://github.com/OP-TEE/optee_os/archive/refs/tags/4.3.0.tar.gz

Well, there is definitely a custom version for OP-TEE, like there is
for Linux and U-Boot:

choice
        prompt "OP-TEE OS version"
        default BR2_TARGET_OPTEE_OS_LATEST if BR2_PACKAGE_HOST_RUSTC_ARCH_SUPPORTS
        help
          Select the version of OP-TEE OS you want to use

config BR2_TARGET_OPTEE_OS_LATEST
        bool "4.3.0"
        depends on BR2_PACKAGE_HOST_RUSTC_ARCH_SUPPORTS
        select BR2_TARGET_OPTEE_OS_NEEDS_PYTHON_CRYPTOGRAPHY
        help
          Use the latest release tag from the OP-TEE OS official Git
          repository.

config BR2_TARGET_OPTEE_OS_CUSTOM_TARBALL
        bool "Custom tarball"
        help
          This option allows to specify a URL pointing to an OP-TEE OS
          source tarball. This URL can use any protocol recognized by
          Buildroot, like http://, ftp://, file:// or scp://.

          When pointing to a local tarball using file://, you may want
          to use a make variable like $(TOPDIR) to reference the root of
          the Buildroot tree.

config BR2_TARGET_OPTEE_OS_CUSTOM_GIT
        bool "Custom Git repository"
        help
          Use a custom version fetched from a Git repository.

endchoice

So do the same as what you do for Linux and U-Boot :-)

> > Please drop, we don't enable bmap-tools in our defconfigs, we leave
> > that choice up to the user.  
> 
> Ack. Mind that I will leave the BR2_PACKAGE_HOST_GENIMAGE=y option since 
> it is mandatory for building the image.

Absolutely!

Thanks a lot!

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