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

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

Hello Marco,

On 10/15/26 11:46 AM, Marco Felsch wrote:
> On 26-10-15 11:18, Ahmad Fatoum wrote:
>> From: Ahmad Fatoum <a.fatoum@barebox.org>
>>  	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.

There are no other users of this API. I can imagine other users (e.g. a
board's custom boot entry provider) wanting to tightly control whether
global.bootm.root_param is used or not.

Cheers,
Ahmad

> 
> 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
>>
>>
> 

-- 
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 |