Re: [PATCH 3/4] bootm: use new api to get kernel command line params

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 11/26/25 7:42 AM, Fabian Pflug wrote:
> Getting root device adn root kernel options seperatily any combining

device and root kernel options seperatly and combining

> them in this function instead of getting the full string from the helper
> functions. This is in preperation for replacing the root= string in the

preparation

> kernel commandline with something defined during runtime.
> 
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
>  common/bootm.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index 17792b2a1d..5d7dc00e3e 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -4,6 +4,7 @@
>  #include <bootm.h>
>  #include <bootm-overrides.h>
>  #include <fs.h>
> +#include <fcntl.h>
>  #include <malloc.h>
>  #include <memory.h>
>  #include <block.h>
> @@ -841,33 +842,38 @@ int bootm_boot(struct bootm_data *bootm_data)
>  	}
>  
>  	if (bootm_data->appendroot) {
> -		char *rootarg;
> +		char *root;
> +		char *rootopts;
>  
>  		if (bootm_data->root_dev) {
>  			const char *root_dev_name = devpath_to_name(bootm_data->root_dev);
>  			struct cdev *root_cdev = cdev_open_by_name(root_dev_name, O_RDONLY);
>  
> -			rootarg = cdev_get_linux_rootarg(root_cdev);
> -			if (!rootarg) {
> -				rootarg = ERR_PTR(-EINVAL);
> -
> -				if (!root_cdev)
> -					pr_err("no cdev found for %s, cannot set root= option\n",
> -						root_dev_name);
> -				else if (!root_cdev->partuuid[0])
> -					pr_err("%s doesn't have a PARTUUID, cannot set root= option\n",
> -						root_dev_name);

This warning is useful, because it's very confusing otherwise when a MBR
disk image is created without a NT signature.

> -			}
> +			cdev_get_linux_root_and_opts(root_cdev, &root, &rootopts);
>  
>  			if (root_cdev)
>  				cdev_close(root_cdev);
> +			else
> +				pr_err("no cdev found for %s, cannot set root= option\n", root_dev_name);
>  		} else {
> -			rootarg = path_get_linux_rootarg(data->os_file);

path_get_linux_rootarg() is stale now right? Can you squash its removal
into the patch that gets rid of linux.bootargs?

> +			struct fs_device *fsdev = get_fsdevice_by_path(AT_FDCWD, data->os_file);
> +			if(fsdev)
> +				fsdev_get_linux_root_options(fsdev, &root, &rootopts);
> +			else
> +				pr_err("no fsdevice under path: %s\n", data->os_file);
>  		}
>  
> -		if (IS_ERR(rootarg)) {
> +		if (!root) {
>  			pr_err("Failed to append kernel cmdline parameter 'root='\n");
>  		} else {
> +			char *rootarg;
> +			if(rootopts) {
> +				rootarg = xasprintf("root=%s %s", root, rootopts);
> +				free(rootopts);
> +			} else {
> +				rootarg = xasprintf("root=%s", root);
> +			}

I wonder if we should add a helper into a header that does this, given
how many times this is repeated...

char *format_root_bootarg(char *root, char *rootopts);

which consumes (and frees) two pointers and returns a new buffer..

Cheers,
Ahmad

> +			free(root);
>  			pr_info("Adding \"%s\" to Kernel commandline\n", rootarg);
>  			globalvar_add_simple("linux.bootargs.bootm.appendroot",
>  					     rootarg);

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |