Re: [PATCH v2 1/5] image-android: Prepend/postpend default bootargs value with given bootcmd

[Mattijs Korpershoek <mkorpershoek@kernel.org>]

Hi George,

Thank you for the patch.

I have a couple of small remarks on wording/spelling but the code
change looks good already.

On Sat, Jun 07, 2025 at 13:24, George Chan via B4 Relay <devnull+gchan9527.gmail.com@kernel.org> wrote:

> From: George Chan <gchan9527@gmail.com>
>
> Control how default bootargs is prepended or postpended to boot param found
> from androidboot img.

Let's rephrase to be slightly more accurate, and add a problem statement
first:

By default, the boot.img's cmdline are appended to the bootargs
environment.
If we take a cmdline example of:
* androidboot.hardware=warm (in U-Boot environment)
* androidboot.hardware=chilly (in boot.img's cmdline)

The resulting commandline will be:
androidboot.hardware=warm [...] androidboot.hardware=chilly.

Because of this, the U-Boot environment always take priority on the
boot.img.

If we want to have a single U-Boot binary that support multiple
board variants, we can't override androidboot.hardware via the boot.img.

Add a new Kconfig option, ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS that
reverse the logic.

with ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS=y, the resulting
commandline would be:
androidboot.hardware=chilly [...] androidboot.hardware=warm.

(feel free to rephrase some parts to your liking)

>
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>  boot/Kconfig         |  7 +++++++
>  boot/image-android.c | 10 ++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 30eb5b328d7..7b71a340620 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -11,6 +11,13 @@ config ANDROID_BOOT_IMAGE
>  	  This enables support for booting images which use the Android
>  	  image format header.
>  
> +config ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS
> +	bool "Android Boot Image boot cmd param will prepend to env bootargs"
> +	help
> +	  This control how androidboot img with bootcmd param integrate with u-boot

Please, no abbreviations and some minor spelling tweaks:

This controls how Android boot image embedded cmdline integrates
with U-Boot bootargs environment.

By enabling this, the boot.img's cmdline is prepended to the bootargs
environment. By default, when disabled, the cmdline is appended.

(Note: We use cmdline here since that's the terminology used by
Android's mkbootimg tool [1])

[1] https://android.googlesource.com/platform/system/tools/mkbootimg/+/refs/heads/main/mkbootimg.py#511

With the wording fixed, please add

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

Also, Casey, feel free to pick of the reworded version through your tree !

> +	  env bootargs. By enable this, androidboot boot param will prepend to
> +	  head of bootargs env.
> +
>  config TIMESTAMP
>  	bool "Show image date and time when displaying image information"
>  	default y if CMD_DATE
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 1746b018900..fbcd2682a5e 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -347,14 +347,14 @@ int android_image_get_kernel(const void *hdr,
>  		len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */
>  	}
>  
> -	char *newbootargs = malloc(len + 1); /* +1 for the '\0' */
> +	char *newbootargs = malloc(len + 2); /* +2 for 2x '\0' */
>  	if (!newbootargs) {
>  		puts("Error: malloc in android_image_get_kernel failed!\n");
>  		return -ENOMEM;
>  	}
>  	*newbootargs = '\0'; /* set to Null in case no components below are present */
>  
> -	if (bootargs)
> +	if (bootargs && !IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>  		strcpy(newbootargs, bootargs);
>  
>  	if (img_data.kcmdline && *img_data.kcmdline) {
> @@ -369,6 +369,12 @@ int android_image_get_kernel(const void *hdr,
>  		strcat(newbootargs, img_data.kcmdline_extra);
>  	}
>  
> +	if (bootargs && IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS)) {
> +		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
> +			strcat(newbootargs, " ");
> +		strcat(newbootargs, bootargs);
> +	}
> +
>  	env_set("bootargs", newbootargs);
>  	free(newbootargs);
>  
>
> -- 
> 2.43.0