Re: [PATCH 07/32] spl: Avoid #ifdef with CONFIG_SPL_SYS_ARGS_ADDR

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]

On Wed, Aug 30, 2023 at 12:04:38PM -0600, Simon Glass wrote:
> Use IF_ENABLED_INT() to avoid needing to use the preprocessor. Give the
> Kconfig option a default since we try to avoid hex values without
> defaults.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/Kconfig |  1 +
>  common/spl/spl.c   | 18 ++++++++++++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index c23a1f7750b..e863aac2b34 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -1067,6 +1067,7 @@ config SPL_SYS_ARGS_ADDR
>  	hex "Address in memory to load 'args' file for Falcon Mode to"
>  	depends on SPL_OS_BOOT
>  	default 0x88000000 if ARCH_OMAP2PLUS
> +	default 0

No, we don't do this.  We do not put fake address defaults on questions
that must be answered.  Further, default 0 on a hex is wrong.

> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 77fe4cdb053..2da5bc0c4f5 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -814,9 +814,11 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  	}
>  
>  	memset(&spl_image, '\0', sizeof(spl_image));
> -#ifdef CONFIG_SPL_SYS_ARGS_ADDR
> -	spl_image.arg = (void *)CONFIG_SPL_SYS_ARGS_ADDR;
> -#endif
> +	if (IS_ENABLED(CONFIG_SPL_SYS_ARGS_ADDR)) {
> +		spl_image.arg =
> +			map_sysmem(IF_ENABLED_INT(CONFIG_SPL_OS_BOOT,
> +						  CONFIG_SPL_SYS_ARGS_ADDR), 0);
> +	}
>  	spl_image.boot_device = BOOT_DEVICE_NONE;
>  	board_boot_order(spl_boot_list);
>  
> @@ -873,8 +875,16 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  #if CONFIG_IS_ENABLED(OS_BOOT)
>  	case IH_OS_LINUX:
>  		debug("Jumping to Linux\n");
> +		if (IS_ENABLED(CONFIG_SPL_SYS_ARGS_ADDR)) {
> +			ulong addr;
> +
> +			addr = IF_ENABLED_INT(CONFIG_SPL_OS_BOOT,
> +					      CONFIG_SPL_SYS_ARGS_ADDR);
> +			spl_fixup_fdt(map_sysmem(addr, 0));
> +		}
> +	}
> +
>  #if defined(CONFIG_SPL_SYS_ARGS_ADDR)
> -		spl_fixup_fdt((void *)CONFIG_SPL_SYS_ARGS_ADDR);
>  #endif
>  		spl_board_prepare_for_linux();
>  		jump_to_image_linux(&spl_image);

This is also not more readable.  The CONFIG option is oddly named, and
this doesn't help.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]