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 <ansh...@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
signature.asc
Description: PGP signature
