On Sun, Sep 25, 2022 at 09:02:10AM -0600, Simon Glass wrote:

> The expression in boot_from_devices() is fairly long. Move it into a
> function with a suitable comment.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/spl.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index edf24b05162..e7f16a18228 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -659,6 +659,18 @@ static int spl_load_image(struct spl_image_info *spl_image,
>  	return ret;
>  }
>  
> +/**
> + * spl_show_output() - Selects whether to show info output in SPL
> + *
> + * Returns: true if info output should be shown, false if not
> + */
> +static inline bool spl_show_output(void)
> +{
> +	return CONFIG_IS_ENABLED(SERIAL) &&
> +	    CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) &&
> +	    !IS_ENABLED(CONFIG_SILENT_CONSOLE);
> +}
> +
>  /**
>   * boot_from_devices() - Try loading a booting U-Boot from a list of devices
>   *
> @@ -682,9 +694,7 @@ static int boot_from_devices(struct spl_image_info *spl_image,
>  		if (CONFIG_IS_ENABLED(SHOW_ERRORS))
>  			ret = -ENXIO;
>  		loader = spl_ll_find_loader(bootdev);
> -		if (CONFIG_IS_ENABLED(SERIAL) &&
> -		    CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) &&
> -		    !IS_ENABLED(CONFIG_SILENT_CONSOLE)) {
> +		if (spl_show_output()) {
>  			if (loader)
>  				printf("Trying to boot from %s\n",
>  				       spl_loader_name(loader));

I don't think this really helps readability, even down in "spl: Allow
multiple loaders of the same type" where it would be indented another
level. I _think_ this is a bit of an artifact from before we could
easily call printf(...) and have the call be nop'd out? So it should
just be a check for CONFIG_SILENT_CONSOLE.

-- 
Tom