Re: [PATCH v2 01/10] commands: devlookup: honour global.bootm.root_param for -k

[Marco Felsch <m.felsch@pengutronix.de>]

Hi Ahmad,

On 26-10-15 11:18, Ahmad Fatoum wrote:
> From: Ahmad Fatoum <a.fatoum@barebox.org>
> 
> devlookup -k prints the root= option that bootm would add for
> global.bootm.root_dev, but it always calls it root=, even though
> global.bootm.root_param now allows bootm to use e.g. verity= instead, so
> only the initramfs and not the kernel picks up the root file system.
> 
> Let's use the same option name as bootm, so scripts can assemble the
> option bootm would append for images it doesn't append it to, like
> unified kernel images.
> 
> Assisted-by: Claude:opus-5.5
> Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
> ---
>  commands/devlookup.c | 5 +++--
>  common/block.c       | 5 +++--
>  include/block.h      | 6 ++++--
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/commands/devlookup.c b/commands/devlookup.c
> index 8f3468735267..164c005236de 100644
> --- a/commands/devlookup.c
> +++ b/commands/devlookup.c
> @@ -85,7 +85,8 @@ static int do_devlookup(int argc, char *argv[])
>  	}
>  
>  	if (kernelopt)
> -		val = buf = cdev_get_linux_rootarg(cdev);
> +		val = buf = cdev_get_linux_rootarg(cdev,
> +			getenv_nonempty("global.bootm.root_param") ?: "root");
				^
Wouldn't it make more sense if cdev_get_linux_rootarg() returns the
correct <key>= so it's fixed for all possible cdev_get_linux_rootarg()
users?
In other words put this logic into cdev_get_linux_rootarg() and keep the
API.

Regards,
  Marco

>  	else if (paramname)
>  		val = dev_get_param(cdev->dev, paramname);
>  	else
> @@ -109,7 +110,7 @@ BAREBOX_CMD_HELP_TEXT("parameter with that name is looked up.")
>  BAREBOX_CMD_HELP_TEXT("")
>  BAREBOX_CMD_HELP_TEXT("Options:")
>  BAREBOX_CMD_HELP_OPT ("-v",  "write output to VARIABLE instead of printing it")
> -BAREBOX_CMD_HELP_OPT ("-k",  "output kernel rootarg line")
> +BAREBOX_CMD_HELP_OPT ("-k",  "output kernel rootarg line (see global.bootm.root_param)")
>  BAREBOX_CMD_HELP_OPT ("-a",  "output resolution of cdev alias")
>  BAREBOX_CMD_HELP_END
>  
> diff --git a/common/block.c b/common/block.c
> index 5441c7ca1848..16075bb146ec 100644
> --- a/common/block.c
> +++ b/common/block.c
> @@ -665,7 +665,8 @@ int cdev_get_linux_root_and_opts(const struct cdev *partcdev, const char **root,
>  	return 0;
>  }
>  
> -char *cdev_get_linux_rootarg(const struct cdev *partcdev)
> +char *cdev_get_linux_rootarg(const struct cdev *partcdev,
> +			     const char *root_param)
>  {
>  	int err;
>  	const char *root = NULL;
> @@ -675,5 +676,5 @@ char *cdev_get_linux_rootarg(const struct cdev *partcdev)
>  	if (err)
>  		return NULL;
>  
> -	return format_root_bootarg("root", root, rootopts);
> +	return format_root_bootarg(root_param, root, rootopts);
>  }
> diff --git a/include/block.h b/include/block.h
> index 065ae81358b4..91ab91f9109b 100644
> --- a/include/block.h
> +++ b/include/block.h
> @@ -109,7 +109,8 @@ static inline int block_flush(struct block_device *blk)
>  
>  #ifdef CONFIG_BLOCK
>  unsigned file_list_add_blockdevs(struct file_list *files);
> -char *cdev_get_linux_rootarg(const struct cdev *partcdev);
> +char *cdev_get_linux_rootarg(const struct cdev *partcdev,
> +			     const char *root_param);
>  int cdev_get_linux_root_and_opts(const struct cdev *partcdev, const char **root,
>  				 const char **rootopts);
>  #else
> @@ -117,7 +118,8 @@ static inline unsigned file_list_add_blockdevs(struct file_list *files)
>  {
>  	return 0;
>  }
> -static inline char *cdev_get_linux_rootarg(const struct cdev *partcdev)
> +static inline char *cdev_get_linux_rootarg(const struct cdev *partcdev,
> +					   const char *root_param)
>  {
>  	return NULL;
>  }
> -- 
> 2.47.3
> 
>