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

[Heiko Thiery]

Hi,

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.

My suggestion is to remove the BR2_PACKAGE_TI_K3_SOC_AMxx and
BR2_PACKAGE_TI_K3_SECTYP_xx stuff, let the user define a binary file
name in BR2_TARGET_TI_K3_R5_LOADER_TIBOOT3_BIN or do a default to
tiboot3.bin

> We should also do something similar for BR2_TARGET_TI_K3_R5_LOADER_SWSFW_ITB

This sounds similar to the thing above, but on the port I used for the
new upcoming board this was not used and therefore not considered in
detail.

-- 
Heiko

> Best regards,
> Romain
>
>
> >
> >>
> >> -michael
> >
> >
> > --
> > Heiko
>