[PATCH v3 22/22] bootm: Create a new boot_run() function to handle booting

[Tom Rini]

On Fri, Dec 15, 2023 at 08:14:26PM -0700, Simon Glass wrote:
> Create a common function used by the three existing bootz/i/m_run()
> functions, to reduce duplicated code.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Suggested-by: Tom Rini <trini at konsulko.com>
> ---
> 
> Changes in v3:
> - Add new boot_run() function
> 
>  boot/bootm.c    | 40 ++++++++++++++--------------------------
>  include/bootm.h | 18 ++++++++++++++++++
>  2 files changed, 32 insertions(+), 26 deletions(-)
> 
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 53236136f489..6a4cebcf7a08 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -1123,47 +1123,35 @@ err:
>  	return ret;
>  }
>  
> -int bootm_run(struct bootm_info *bmi)
> +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states)
>  {
>  	int states;
>  
> -	bmi->cmd_name = "bootm";
> -	states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD |
> -		BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS |
> -		BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
> -		BOOTM_STATE_OS_GO | BOOTM_STATE_MEASURE;
> +	bmi->cmd_name = cmd;
> +	states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP |
> +		BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO;
>  	if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
>  		states |= BOOTM_STATE_RAMDISK;
> -	if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS))
> -		states |= BOOTM_STATE_OS_CMDLINE;
> +	states |= extra_states;
>  
>  	return bootm_run_states(bmi, states);
>  }
>  
> -int bootz_run(struct bootm_info *bmi)
> +int bootm_run(struct bootm_info *bmi)
>  {
> -	int states;
> -
> -	bmi->cmd_name = "bootz";
> -	states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP |
> -		BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO;
> -	if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
> -		states |= BOOTM_STATE_RAMDISK;
> +	return boot_run(bmi, "bootm", BOOTM_STATE_START | BOOTM_STATE_FINDOS |
> +			BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER |
> +			BOOTM_STATE_LOADOS);
> +}
>  
> -	return bootm_run_states(bmi, states);
> +int bootz_run(struct bootm_info *bmi)
> +{
> +	return boot_run(bmi, "bootz", 0);
>  }
>  
>  int booti_run(struct bootm_info *bmi)
>  {
> -	int states;
> -
> -	bmi->cmd_name = "booti";
> -	states = BOOTM_STATE_MEASURE | BOOTM_STATE_OS_PREP |
> -		BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO;
> -	if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
> -		states |= BOOTM_STATE_RAMDISK;
> -
> -	return bootm_run_states(bmi, states);
> +	return boot_run(bmi, "booti", 0);
>  }
>  
>  int bootm_boot_start(ulong addr, const char *cmdline)
> diff --git a/include/bootm.h b/include/bootm.h
> index eba35b33b4e5..9e0f8d60de0a 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -150,6 +150,24 @@ int bootm_measure(struct bootm_headers *images);
>   */
>  int bootm_run_states(struct bootm_info *bmi, int states);
>  
> +/**
> + * boot_run() - Run the entire bootm/booti/bootz process
> + *
> + * This runs through the boot process from start to finish, with a base set of
> + * states, along with the extra ones supplied.
> + *
> + * This uses bootm_run_states().
> + *
> + * Note that it is normally easier to use bootm_run(), etc. since they handle
> + * the extra states correctly.
> + *
> + * @bmi: bootm information
> + * @cmd: command being run, NULL if none
> + * @extra_states: Mask of extra states to use for the boot
> + * Return: 0 if ok, something else on error
> + */
> +int boot_run(struct bootm_info *bmi, const char *cmd, int extra_states);
> +
>  /**
>   * bootm_run() - Run the entire bootm process
>   *

Overall good, thanks. My question is, should we mark the others as
static, or is this new helper static and not to be called externally?

-- 
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/20231216/f6c3dd47/attachment.sig>