[PATCH v7 07/10] board: ti: add default dtb for am62 in falcon mode

[Tom Rini]

On Tue, Jun 10, 2025 at 09:37:54PM +0530, Anshul Dalal wrote:
> On Tue Jun 10, 2025 at 8:16 PM IST, Tom Rini wrote:
> > On Tue, Jun 10, 2025 at 03:41:21PM +0530, Anshul Dalal wrote:
> >> On Mon Jun 9, 2025 at 8:27 PM IST, Tom Rini wrote:
> >> > On Mon, Jun 09, 2025 at 01:05:36PM +0530, Anshul Dalal wrote:
> >> >> On Sat Jun 7, 2025 at 12:39 AM IST, Tom Rini wrote:
> >> >> > On Tue, Jun 03, 2025 at 07:54:47PM +0530, Anshul Dalal wrote:
> >> >> >
> >> >> >> The SPL_FS_LOAD_ARGS_NAME config is used for the arguments to the kernel
> >> >> >> (dtb in our case) in falcon boot.
> >> >> >> 
> >> >> >> Setting it in board specific Kconfig allows us to reuse the same config
> >> >> >> fragment 'am62x_r5_falcon.config' for all 3 platforms for enabling
> >> >> >> falcon boot.
> >> >> >> 
> >> >> >> Signed-off-by: Anshul Dalal <anshuld at ti.com>
> >> >> >> ---
> >> >> >>  board/ti/am62ax/Kconfig | 3 +++
> >> >> >>  board/ti/am62px/Kconfig | 3 +++
> >> >> >>  board/ti/am62x/Kconfig  | 3 +++
> >> >> >>  3 files changed, 9 insertions(+)
> >> >> >
> >> >> > Why can't whatever is using the fragment also set this? Is it because
> >> >> > you're only using this as part of say:
> >> >> > make am62ax_evm_r5_defconfig am62x_falcon.config
> >> >> > ?
> >> >> 
> >> >> Correct, having three different config fragments with only changes to the
> >> >> dtb path doesn't seem very useful imo.
> >> >
> >> > Then these belong back in the main defconfig itself I think.
> >> 
> >> That's not an option since SPL_FS_LOAD_ARGS_NAME depends on
> >> SPL_OS_BOOT[1] and due to the way config fragments are applied. Since we
> >> only set OS_BOOT to y inside of the fragment, we can not define any
> >> configs that depend on it before that in the defconfig like
> >> SPL_FS_LOAD_ARGS_NAME, it would just default to "args" instead.
> >> 
> >> [1]: common/spl/Kconfig:794
> >> config SPL_FS_LOAD_ARGS_NAME
> >> 	string "File to load for the OS kernel argument parameters from the filesystem"
> >> 	depends on (SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS) && SPL_OS_BOOT
> >> 	default "args"
> >
> > Config fragments can be nested, so maybe the falcon variant will need to
> > include the main defconfig, the falcon fragment and then set the args to
> > the dtb.
> 
> So we add 3 new defconfigs for each platforms that include their
> respective defconfigs and the fragment with only the dtb changes?
> 
> That sounds good to me though why not modify the Kconfig's directly like
> I did in the patch originally?

The problem is that just because we can put values in a Kconfig file,
and are much more relaxed about it than the linux kernel is, does not
mean it's the right place for values used on a single platform. We do
have a few anti-pattern examples of this, but they should be fixed and
not expanded upon.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250610/67d7d0c7/attachment.sig>