[Buildroot] [PATCH v11 00/17] Add support for AM62x-SK HS-FS devices

[Heiko Thiery]

Hi Romain,

Am Do., 4. Apr. 2024 um 14:14 Uhr schrieb Romain Naour <romain.naour at smile.fr>:
>
> Hello Heiko, Michael, All,
>
> Le 04/04/2024 à 13:46, Heiko Thiery a écrit :
> > Hi,
> >
> > Am Do., 4. Apr. 2024 um 13:04 Uhr schrieb Michael Walle <michael at walle.cc>:
> >>
> >> Hi Romain,
> >>
> >> thanks for working on the series!
> >>
> >>>> I know there are some discussions ongoing about the handling of the
> >>>> BR2_PACKAGE_TI_K3_SECTYPE_X and BR2_PACKAGE_TI_K3_SOC_X. I see that
> >>>> these variables are only used for selecting the right value for
> >>>> BR2_TARGET_TI_K3_R5_LOADER_TIBOOT3_BIN.  Since we can set this value
> >>>> directly for each defconfig I think we can get rid of all this stuff.
> >>>> Nevertheless there is a problem with the
> >>>> BR2_TARGET_TI_K3_R5_LOADER_TIBOOT3_BIN since it is not possible to
> >>>> select a value in menuconfig because of the missing help text.
> >>>
> >>> It seems easier to select the SoC variant and the Sectype using two boolean than
> >>> letting the user to provide the right file name in
> >>> BR2_TARGET_TI_K3_R5_LOADER_TIBOOT3_BIN from the defconfig.
> >>
> >> Doesn't seem easier to me to update the package each time a new SoC
> >> is introduced. Besides..
> >>
> >>> Is a use case where the we must use a custom tiboot3*.bin name ?
> >>
> >> ..you are literally encoding the board name "-evm" of the TI EVM
> >> into that name. So it is unusable for any other board. E.g.  there
> >> is already this board upstream:
> >> https://elixir.bootlin.com/u-boot/v2024.04/source/arch/arm/dts/k3-am625-phycore-som-binman.dtsi#L14
> >>
> >> As I said, that filename is just a free form type of filename and
> >> the dtsi author is free to choose its naming.
> >>
> >> IMHO with the removal of the image gen package, there is no need
> >> for the SOC and SECTYPE choice anymore, as it is just used to
> >> assemble the board specific filename which will end up being the
> >> tiboot3.bin. But u-boot is already choosing the default tiboot3.bin
> >> for you:
> >> https://elixir.bootlin.com/u-boot/v2024.04/source/arch/arm/dts/k3-am625-phycore-som-binman.dtsi#L60
> >>
> >> Sorry for being so persistent, I really don't think this is the
> >> correct approach. Just defaulting the binary name to tiboot3.bin
> >> will probably handle most cases and for the oddballs, just set the
> >> filename in the defconfig.
> >
> > Michael is right at this point. This option only can select the TI evm
> > specific tiboot3 files defined in the U-Boot binman dtsi files. I just
> > checked and in U-Boot are 3 other boards that use different names
> > than the one used by TI. And at least one other will come in the near future.
>
> Indeed, you're right.
>
> Last time I've checked -evm was used on most (I thought on all) boards.
>
> See the file k3-j721e-binman.dtsi shared between the real TI EVM and the
> sk-tda4VM boards.
>
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721e-binman.dtsi
>
> Even the beapleplay uses the -evm suffix:
>
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-am625-r5-beagleplay.dts#L81
>
> I missed the verdin examples:
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi#L12
>
> In other hand, we should not rely on the tiboot3.bin symlink since it links the
> -gp, -hs-fs, -hs tiboot3 binary according to where "symlink = "tiboot3.bin";"
> line is located in k3-*binman.dtsi.
>
> So, instead of BR2_TARGET_TI_K3_R5_LOADER_TIBOOT3_BIN you suggest to let the
> user to define a custom tiboot3 binary name or if empty use the default one.
>
> We should also do something similar for BR2_TARGET_TI_K3_R5_LOADER_SWSFW_ITB

If you want I can provide the patches to you to remove all that stuff.
What do you think? Or in case the series will be merged before I can
do that afterwards.

I think with removing that we also could remove the "package/ti-k3"
package. The only thing that then still needs some info from here is:

274 ifeq ($(BR2_TARGET_UBOOT_USE_BINMAN)$(BR2_PACKAGE_TI_K3),yy)
275 UBOOT_DEPENDENCIES += ti-k3-boot-firmware
276 endif

And this could be changed by adding BR2_TARGET_UBOOT_NEEDS_TI_K3_BOOT_FIRMWARE.

What do you think?


--
Heiko