[PATCH v4 0/6] A series of patch for enable sc7180 android boot

[PATCH v4 0/6] A series of patch for enable sc7180 android boot

[George Chan]

Since attempt[1] to embed android specific boot param into fdt bootargs,
a new idea is formed as to make use of env file to contain default param
value. Current code logic is already working fine, unless the priority
of param at begining is higher than at tail. From env file, bootargs is
treated important, and prepend at begining. So there is a need to
reverse the logic, and make sure default bootargs value at the end, and
let androidboot img bootargs value sit at begining.

So a new kconfig item is introduced. Once enabled, the bootargs strcat
param will get reversed.

A similar logic is needed for fastboot->bootm glue function, so split 
function of patch #1 into 2 functions as #2, fixed some issues #3, and 
apply #4 with same trick to glue function too.

Some one-liners patch is included to enable device driver for sc7180
soc. It is about iommu for usb, usb gadget vendor/product id.

Reduce dependency on abootimg when androidboot v3 or greater is not
needed. Since Tom suggested to reduce __maybe_unused directive and 
use IS_ENABLE() instead; two __weak func proto need to add into image.h

[1]https://lists.denx.de/pipermail/u-boot/2025-May/588828.html
[2]https://lists.denx.de/pipermail/u-boot/2025-May/589926.html

Signed-off-by: George Chan <gchan9527@gmail.com>
---
Changes in v4:
- Drop patch #3.
- Refine #2 to remove intermediate variables and code style.
- Link to v3: https://lore.kernel.org/r/20250628-sc7180-android-boot-v3-0-105098a19165@gmail.com

Changes in v3:
- Collect acks-by and review-by.
- Split function and replace old #2 into #2-#4, code reuse in #4
- Modify changelog and Kconfig description of #1, suggested by Mattijs
- Slightly modify #5, add back line break.
- Rebase to latest next
- Link to v2: https://lore.kernel.org/r/20250607-sc7180-android-boot-v2-0-2df5d7f61124@gmail.com

Changes in v2:
- Add new patch #2 that is same trick as #1 to fastboot->bootm glue layer.
- Remove default n to patch #1, suggested by Tom.
- Use IS_ENABLE() instead, suggested by Tom.
- Collect review-by from Casey and Neil.
- Rebase to u-boot/next branch.
- CCing myself too.
- Link to v1: https://lore.kernel.org/r/20250520-sc7180-android-boot-v1-0-3075a84ea094@gmail.com

---
George Chan (6):
      image-android: Prepend/postpend default bootargs value with given bootcmd
      boot/image-android.c: Split android_image_get_kernel into two functions
      bootm: Append bootargs value when bootmeth_android provide cmdline
      boot: bootmeth_android: Conditionally dependent on abootimg
      iommu: qcom-smmu: Introduce sc7180 compatible string
      usb: gadget: Introduce usb gadget vendor/product default id for ARCH_QCOM

 boot/Kconfig                  |  9 ++++
 boot/bootm.c                  |  2 +-
 boot/bootmeth_android.c       |  2 +-
 boot/image-android.c          | 97 ++++++++++++++++++++++++++++---------------
 drivers/iommu/qcom-hyp-smmu.c |  1 +
 drivers/usb/gadget/Kconfig    |  2 +
 include/image.h               | 14 +++++++
 7 files changed, 92 insertions(+), 35 deletions(-)
---
base-commit: 490ae8ceae2d5999c9de863e014e463f5c1495a6
change-id: 20250520-sc7180-android-boot-a845ecf48a48

Best regards,
-- 
George Chan <gchan9527@gmail.com>

[PATCH v4 0/6] A series of patch for enable sc7180 android boot

[George Chan via B4 Relay]

Since attempt[1] to embed android specific boot param into fdt bootargs,
a new idea is formed as to make use of env file to contain default param
value. Current code logic is already working fine, unless the priority
of param at begining is higher than at tail. From env file, bootargs is
treated important, and prepend at begining. So there is a need to
reverse the logic, and make sure default bootargs value at the end, and
let androidboot img bootargs value sit at begining.

So a new kconfig item is introduced. Once enabled, the bootargs strcat
param will get reversed.

A similar logic is needed for fastboot->bootm glue function, so split 
function of patch #1 into 2 functions as #2, fixed some issues #3, and 
apply #4 with same trick to glue function too.

Some one-liners patch is included to enable device driver for sc7180
soc. It is about iommu for usb, usb gadget vendor/product id.

Reduce dependency on abootimg when androidboot v3 or greater is not
needed. Since Tom suggested to reduce __maybe_unused directive and 
use IS_ENABLE() instead; two __weak func proto need to add into image.h

[1]https://lists.denx.de/pipermail/u-boot/2025-May/588828.html
[2]https://lists.denx.de/pipermail/u-boot/2025-May/589926.html

Signed-off-by: George Chan <gchan9527@gmail.com>
---
Changes in v4:
- Drop patch #3.
- Refine #2 to remove intermediate variables and code style.
- Link to v3: https://lore.kernel.org/r/20250628-sc7180-android-boot-v3-0-105098a19165@gmail.com

Changes in v3:
- Collect acks-by and review-by.
- Split function and replace old #2 into #2-#4, code reuse in #4
- Modify changelog and Kconfig description of #1, suggested by Mattijs
- Slightly modify #5, add back line break.
- Rebase to latest next
- Link to v2: https://lore.kernel.org/r/20250607-sc7180-android-boot-v2-0-2df5d7f61124@gmail.com

Changes in v2:
- Add new patch #2 that is same trick as #1 to fastboot->bootm glue layer.
- Remove default n to patch #1, suggested by Tom.
- Use IS_ENABLE() instead, suggested by Tom.
- Collect review-by from Casey and Neil.
- Rebase to u-boot/next branch.
- CCing myself too.
- Link to v1: https://lore.kernel.org/r/20250520-sc7180-android-boot-v1-0-3075a84ea094@gmail.com

---
George Chan (6):
      image-android: Prepend/postpend default bootargs value with given bootcmd
      boot/image-android.c: Split android_image_get_kernel into two functions
      bootm: Append bootargs value when bootmeth_android provide cmdline
      boot: bootmeth_android: Conditionally dependent on abootimg
      iommu: qcom-smmu: Introduce sc7180 compatible string
      usb: gadget: Introduce usb gadget vendor/product default id for ARCH_QCOM

 boot/Kconfig                  |  9 ++++
 boot/bootm.c                  |  2 +-
 boot/bootmeth_android.c       |  2 +-
 boot/image-android.c          | 97 ++++++++++++++++++++++++++++---------------
 drivers/iommu/qcom-hyp-smmu.c |  1 +
 drivers/usb/gadget/Kconfig    |  2 +
 include/image.h               | 14 +++++++
 7 files changed, 92 insertions(+), 35 deletions(-)
---
base-commit: 490ae8ceae2d5999c9de863e014e463f5c1495a6
change-id: 20250520-sc7180-android-boot-a845ecf48a48

Best regards,
-- 
George Chan <gchan9527@gmail.com>

[PATCH v4 1/6] image-android: Prepend/postpend default bootargs value with given bootcmd

[George Chan]

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.

Above detail is suggested from Mattijs Korpershoek <mkorpershoek@kernel.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/Kconfig         |  9 +++++++++
 boot/image-android.c | 10 ++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index a671d78e570..bcc5f886a01 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -22,6 +22,15 @@ config ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR
 	  addr by repacking the boot.img (mainly due to AVB signature mismatch),
 	  we need a way to use kernel_addr_r and ramdisk_addr_r.
 
+config ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS
+	bool "Android Boot Image boot cmd param will prepend to env bootargs"
+	help
+	  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.
+
 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 14cf611cee5..d78e8a4148a 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -348,14 +348,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) {
@@ -370,6 +370,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

[PATCH v4 1/6] image-android: Prepend/postpend default bootargs value with given bootcmd

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

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.

Above detail is suggested from Mattijs Korpershoek <mkorpershoek@kernel.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/Kconfig         |  9 +++++++++
 boot/image-android.c | 10 ++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index a671d78e570..bcc5f886a01 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -22,6 +22,15 @@ config ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR
 	  addr by repacking the boot.img (mainly due to AVB signature mismatch),
 	  we need a way to use kernel_addr_r and ramdisk_addr_r.
 
+config ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS
+	bool "Android Boot Image boot cmd param will prepend to env bootargs"
+	help
+	  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.
+
 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 14cf611cee5..d78e8a4148a 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -348,14 +348,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) {
@@ -370,6 +370,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

[PATCH v4 2/6] boot/image-android.c: Split android_image_get_kernel into two functions

[George Chan]

Since fastboot->bootm glue layer also need to cater with bootargs
environment, so move common logic from android_image_get_kernel into
a new function to avoid some degree of code duplication.

This new function is android_image_modify_bootargs_env to specially
cater bootargs env value with boot image provided cmdline.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/image-android.c | 103 ++++++++++++++++++++++++++++++++-------------------
 include/image.h      |  12 ++++++
 2 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index d78e8a4148a..5e16564a714 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -285,6 +285,65 @@ static ulong android_image_get_kernel_addr(struct andr_image_data *img_data,
 	return img_data->kernel_addr;
 }
 
+/**
+ * android_image_modify_bootargs_env() - helper to set new bootargs
+ *
+ * set boot based on provided cmdline and u-boot pre-set value
+ *
+ * @cmd: usually contain kernel boot command line string
+ * @cmd_extra: usually contain kernel boot command line string from vendor img
+ * return: 0, success; otherwise fail in various problem.
+*/
+int android_image_modify_bootargs_env(const char *cmd, const char *cmd_extra) {
+	char *bootargs = env_get("bootargs");
+	char *newbootargs;
+	int len = 0;
+
+	if (bootargs)
+		len += strlen(bootargs);
+
+	if (cmd && *cmd)
+		len += strlen(cmd) + (len ? 1 : 0); /* +1 for extra space */
+
+	if (cmd_extra && *cmd_extra)
+		len += strlen(cmd_extra) + (len ? 1 : 0); /* +1 for extra space */
+
+	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 && !IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
+		strcpy(newbootargs, bootargs);
+
+	if (cmd && *cmd) {
+		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
+			strcat(newbootargs, " ");
+		strcat(newbootargs, cmd);
+	}
+
+	if (cmd_extra && *cmd_extra) {
+		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
+			strcat(newbootargs, " ");
+		strcat(newbootargs, cmd_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);
+
+	return 0;
+}
+
 /**
  * android_image_get_kernel() - processes kernel part of Android boot images
  * @hdr:	Pointer to boot image header, which is at the start
@@ -310,6 +369,7 @@ int android_image_get_kernel(const void *hdr,
 	ulong kernel_addr;
 	const struct legacy_img_hdr *ihdr;
 	ulong comp;
+	int ret;
 
 	if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
 		return -EINVAL;
@@ -330,54 +390,19 @@ int android_image_get_kernel(const void *hdr,
 		printf("Android's image name: %s\n", andr_tmp_str);
 
 	printf("Kernel load addr 0x%08lx size %u KiB\n",
-	       kernel_addr, DIV_ROUND_UP(img_data.kernel_size, 1024));
-
-	int len = 0;
-	char *bootargs = env_get("bootargs");
-
-	if (bootargs)
-		len += strlen(bootargs);
+		kernel_addr, DIV_ROUND_UP(img_data.kernel_size, 1024));
 
 	if (img_data.kcmdline && *img_data.kcmdline) {
 		printf("Kernel command line: %s\n", img_data.kcmdline);
-		len += strlen(img_data.kcmdline) + (len ? 1 : 0); /* +1 for extra space */
 	}
 
 	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
 		printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
-		len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */
 	}
 
-	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 && !IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
-		strcpy(newbootargs, bootargs);
-
-	if (img_data.kcmdline && *img_data.kcmdline) {
-		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
-			strcat(newbootargs, " ");
-		strcat(newbootargs, img_data.kcmdline);
-	}
-
-	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
-		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
-			strcat(newbootargs, " ");
-		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);
+	ret = android_image_modify_bootargs_env(img_data.kcmdline, img_data.kcmdline_extra);
+	if (ret)
+		return ret;
 
 	if (os_data) {
 		if (image_get_magic(ihdr) == IH_MAGIC) {
diff --git a/include/image.h b/include/image.h
index b695cc39447..11d33ceeb39 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1890,6 +1890,18 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr,
 
 struct andr_boot_img_hdr_v0;
 
+/**
+ * android_image_modify_bootargs_env() - helper to set new bootargs
+ *
+ * set boot based on provided cmdline and u-boot pre-set value
+ *
+ * @cmd: usually contain kernel boot command line string
+ * @cmd_extra: usually contain kernel boot command line string from vendor img
+ * return: 0, success; otherwise fail in various problem.
+*/
+int android_image_modify_bootargs_env(const char *cmd,
+					 const char *cmd_extra);
+
 /**
  * android_image_get_kernel() - Processes kernel part of Android boot images
  *

-- 
2.43.0

[PATCH v4 2/6] boot/image-android.c: Split android_image_get_kernel into two functions

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Since fastboot->bootm glue layer also need to cater with bootargs
environment, so move common logic from android_image_get_kernel into
a new function to avoid some degree of code duplication.

This new function is android_image_modify_bootargs_env to specially
cater bootargs env value with boot image provided cmdline.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/image-android.c | 103 ++++++++++++++++++++++++++++++++-------------------
 include/image.h      |  12 ++++++
 2 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index d78e8a4148a..5e16564a714 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -285,6 +285,65 @@ static ulong android_image_get_kernel_addr(struct andr_image_data *img_data,
 	return img_data->kernel_addr;
 }
 
+/**
+ * android_image_modify_bootargs_env() - helper to set new bootargs
+ *
+ * set boot based on provided cmdline and u-boot pre-set value
+ *
+ * @cmd: usually contain kernel boot command line string
+ * @cmd_extra: usually contain kernel boot command line string from vendor img
+ * return: 0, success; otherwise fail in various problem.
+*/
+int android_image_modify_bootargs_env(const char *cmd, const char *cmd_extra) {
+	char *bootargs = env_get("bootargs");
+	char *newbootargs;
+	int len = 0;
+
+	if (bootargs)
+		len += strlen(bootargs);
+
+	if (cmd && *cmd)
+		len += strlen(cmd) + (len ? 1 : 0); /* +1 for extra space */
+
+	if (cmd_extra && *cmd_extra)
+		len += strlen(cmd_extra) + (len ? 1 : 0); /* +1 for extra space */
+
+	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 && !IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
+		strcpy(newbootargs, bootargs);
+
+	if (cmd && *cmd) {
+		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
+			strcat(newbootargs, " ");
+		strcat(newbootargs, cmd);
+	}
+
+	if (cmd_extra && *cmd_extra) {
+		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
+			strcat(newbootargs, " ");
+		strcat(newbootargs, cmd_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);
+
+	return 0;
+}
+
 /**
  * android_image_get_kernel() - processes kernel part of Android boot images
  * @hdr:	Pointer to boot image header, which is at the start
@@ -310,6 +369,7 @@ int android_image_get_kernel(const void *hdr,
 	ulong kernel_addr;
 	const struct legacy_img_hdr *ihdr;
 	ulong comp;
+	int ret;
 
 	if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
 		return -EINVAL;
@@ -330,54 +390,19 @@ int android_image_get_kernel(const void *hdr,
 		printf("Android's image name: %s\n", andr_tmp_str);
 
 	printf("Kernel load addr 0x%08lx size %u KiB\n",
-	       kernel_addr, DIV_ROUND_UP(img_data.kernel_size, 1024));
-
-	int len = 0;
-	char *bootargs = env_get("bootargs");
-
-	if (bootargs)
-		len += strlen(bootargs);
+		kernel_addr, DIV_ROUND_UP(img_data.kernel_size, 1024));
 
 	if (img_data.kcmdline && *img_data.kcmdline) {
 		printf("Kernel command line: %s\n", img_data.kcmdline);
-		len += strlen(img_data.kcmdline) + (len ? 1 : 0); /* +1 for extra space */
 	}
 
 	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
 		printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
-		len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */
 	}
 
-	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 && !IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
-		strcpy(newbootargs, bootargs);
-
-	if (img_data.kcmdline && *img_data.kcmdline) {
-		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
-			strcat(newbootargs, " ");
-		strcat(newbootargs, img_data.kcmdline);
-	}
-
-	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
-		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
-			strcat(newbootargs, " ");
-		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);
+	ret = android_image_modify_bootargs_env(img_data.kcmdline, img_data.kcmdline_extra);
+	if (ret)
+		return ret;
 
 	if (os_data) {
 		if (image_get_magic(ihdr) == IH_MAGIC) {
diff --git a/include/image.h b/include/image.h
index b695cc39447..11d33ceeb39 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1890,6 +1890,18 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr,
 
 struct andr_boot_img_hdr_v0;
 
+/**
+ * android_image_modify_bootargs_env() - helper to set new bootargs
+ *
+ * set boot based on provided cmdline and u-boot pre-set value
+ *
+ * @cmd: usually contain kernel boot command line string
+ * @cmd_extra: usually contain kernel boot command line string from vendor img
+ * return: 0, success; otherwise fail in various problem.
+*/
+int android_image_modify_bootargs_env(const char *cmd,
+					 const char *cmd_extra);
+
 /**
  * android_image_get_kernel() - Processes kernel part of Android boot images
  *

-- 
2.43.0

[PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[George Chan]

Old logic wipe bootargs env with cmdline, new logic cater the value
by prepending cmdline value to bootargs.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/bootm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 4bdca22ea8c..d8e0c3527d0 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -1165,7 +1165,7 @@ int bootm_boot_start(ulong addr, const char *cmdline)
 
 	snprintf(addr_str, sizeof(addr_str), "%lx", addr);
 
-	ret = env_set("bootargs", cmdline);
+	ret = android_image_modify_bootargs_env(cmdline, NULL);
 	if (ret) {
 		printf("Failed to set cmdline\n");
 		return ret;

-- 
2.43.0

[PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Old logic wipe bootargs env with cmdline, new logic cater the value
by prepending cmdline value to bootargs.

Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/bootm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 4bdca22ea8c..d8e0c3527d0 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -1165,7 +1165,7 @@ int bootm_boot_start(ulong addr, const char *cmdline)
 
 	snprintf(addr_str, sizeof(addr_str), "%lx", addr);
 
-	ret = env_set("bootargs", cmdline);
+	ret = android_image_modify_bootargs_env(cmdline, NULL);
 	if (ret) {
 		printf("Failed to set cmdline\n");
 		return ret;

-- 
2.43.0

[PATCH v4 4/6] boot: bootmeth_android: Conditionally dependent on abootimg

[George Chan]

If target u-boot img do not support androidboot v3 or greater,
abootimg might not be necessary.

aarch64-linux-gnu-ld.bfd: boot/bootmeth_android.o: in function `boot_android_normal':
/home/user/sources/u-boot-next/boot/bootmeth_android.c:541:(.text.boot_android_normal+0xd0): undefined reference to `set_avendor_bootimg_addr'
aarch64-linux-gnu-ld.bfd: /home/user/sources/u-boot-next/boot/bootmeth_android.c:543:(.text.boot_android_normal+0xd8): undefined reference to `set_abootimg_addr'
Segmentation fault (core dumped)

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Acked-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/bootmeth_android.c | 2 +-
 include/image.h         | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
index 8c2bde10e17..d7740b86d67 100644
--- a/boot/bootmeth_android.c
+++ b/boot/bootmeth_android.c
@@ -534,7 +534,7 @@ static int boot_android_normal(struct bootflow *bflow)
 	if (ret < 0)
 		return log_msg_ret("read boot", ret);
 
-	if (priv->header_version >= 3) {
+	if (IS_ENABLED(CONFIG_CMD_ABOOTIMG) && priv->header_version >= 3) {
 		ret = read_slotted_partition(desc, "vendor_boot", priv->slot,
 					     priv->vendor_boot_img_size, vloadaddr);
 		if (ret < 0)
diff --git a/include/image.h b/include/image.h
index 11d33ceeb39..82a861ba8bf 100644
--- a/include/image.h
+++ b/include/image.h
@@ -2050,6 +2050,7 @@ ulong get_abootimg_addr(void);
  * Return: no returned results
  */
 void set_abootimg_addr(ulong addr);
+void __weak set_abootimg_addr(ulong addr) {}
 
 /**
  * get_ainit_bootimg_addr() - Get Android init boot image address
@@ -2071,6 +2072,7 @@ ulong get_avendor_bootimg_addr(void);
  * Return: no returned results
  */
 void set_avendor_bootimg_addr(ulong addr);
+void __weak set_avendor_bootimg_addr(ulong addr) {}
 
 /**
  * board_fit_config_name_match() - Check for a matching board name

-- 
2.43.0

[PATCH v4 4/6] boot: bootmeth_android: Conditionally dependent on abootimg

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

If target u-boot img do not support androidboot v3 or greater,
abootimg might not be necessary.

aarch64-linux-gnu-ld.bfd: boot/bootmeth_android.o: in function `boot_android_normal':
/home/user/sources/u-boot-next/boot/bootmeth_android.c:541:(.text.boot_android_normal+0xd0): undefined reference to `set_avendor_bootimg_addr'
aarch64-linux-gnu-ld.bfd: /home/user/sources/u-boot-next/boot/bootmeth_android.c:543:(.text.boot_android_normal+0xd8): undefined reference to `set_abootimg_addr'
Segmentation fault (core dumped)

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Acked-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Signed-off-by: George Chan <gchan9527@gmail.com>
---
 boot/bootmeth_android.c | 2 +-
 include/image.h         | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
index 8c2bde10e17..d7740b86d67 100644
--- a/boot/bootmeth_android.c
+++ b/boot/bootmeth_android.c
@@ -534,7 +534,7 @@ static int boot_android_normal(struct bootflow *bflow)
 	if (ret < 0)
 		return log_msg_ret("read boot", ret);
 
-	if (priv->header_version >= 3) {
+	if (IS_ENABLED(CONFIG_CMD_ABOOTIMG) && priv->header_version >= 3) {
 		ret = read_slotted_partition(desc, "vendor_boot", priv->slot,
 					     priv->vendor_boot_img_size, vloadaddr);
 		if (ret < 0)
diff --git a/include/image.h b/include/image.h
index 11d33ceeb39..82a861ba8bf 100644
--- a/include/image.h
+++ b/include/image.h
@@ -2050,6 +2050,7 @@ ulong get_abootimg_addr(void);
  * Return: no returned results
  */
 void set_abootimg_addr(ulong addr);
+void __weak set_abootimg_addr(ulong addr) {}
 
 /**
  * get_ainit_bootimg_addr() - Get Android init boot image address
@@ -2071,6 +2072,7 @@ ulong get_avendor_bootimg_addr(void);
  * Return: no returned results
  */
 void set_avendor_bootimg_addr(ulong addr);
+void __weak set_avendor_bootimg_addr(ulong addr) {}
 
 /**
  * board_fit_config_name_match() - Check for a matching board name

-- 
2.43.0

[PATCH v4 5/6] iommu: qcom-smmu: Introduce sc7180 compatible string

[George Chan]

Add basic compatible string for sc7180 family soc.

Signed-off-by: Vitalii Skorkin <nikroks@mainlining.org>
Co-developed-by: George Chan <gchan9527@gmail.com>
Signed-off-by: George Chan <gchan9527@gmail.com>
Reviewed-by: Casey Connolly <casey.connolly@linaro.org>
---
 drivers/iommu/qcom-hyp-smmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/qcom-hyp-smmu.c b/drivers/iommu/qcom-hyp-smmu.c
index 2e51ce4f242..519e7f4e7b2 100644
--- a/drivers/iommu/qcom-hyp-smmu.c
+++ b/drivers/iommu/qcom-hyp-smmu.c
@@ -389,6 +389,7 @@ static struct iommu_ops qcom_smmu_ops = {
 
 static const struct udevice_id qcom_smmu500_ids[] = {
 	{ .compatible = "qcom,sdm845-smmu-500" },
+	{ .compatible = "qcom,sc7180-smmu-500" },
 	{ .compatible = "qcom,sc7280-smmu-500" },
 	{ .compatible = "qcom,smmu-500", },
 	{ /* sentinel */ }

-- 
2.43.0

[PATCH v4 5/6] iommu: qcom-smmu: Introduce sc7180 compatible string

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Add basic compatible string for sc7180 family soc.

Signed-off-by: Vitalii Skorkin <nikroks@mainlining.org>
Co-developed-by: George Chan <gchan9527@gmail.com>
Signed-off-by: George Chan <gchan9527@gmail.com>
Reviewed-by: Casey Connolly <casey.connolly@linaro.org>
---
 drivers/iommu/qcom-hyp-smmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/qcom-hyp-smmu.c b/drivers/iommu/qcom-hyp-smmu.c
index 2e51ce4f242..519e7f4e7b2 100644
--- a/drivers/iommu/qcom-hyp-smmu.c
+++ b/drivers/iommu/qcom-hyp-smmu.c
@@ -389,6 +389,7 @@ static struct iommu_ops qcom_smmu_ops = {
 
 static const struct udevice_id qcom_smmu500_ids[] = {
 	{ .compatible = "qcom,sdm845-smmu-500" },
+	{ .compatible = "qcom,sc7180-smmu-500" },
 	{ .compatible = "qcom,sc7280-smmu-500" },
 	{ .compatible = "qcom,smmu-500", },
 	{ /* sentinel */ }

-- 
2.43.0

[PATCH v4 6/6] usb: gadget: Introduce usb gadget vendor/product default id for ARCH_QCOM

[George Chan]

Currently vendor/product id are both 0, and that might not as we want.
Set to some arbitary known value that we can make it work more smoothly.

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Acked-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: George Chan <gchan9527@gmail.com>
---
 drivers/usb/gadget/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 46a83141481..2a56e360b5f 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -60,6 +60,7 @@ config USB_GADGET_VENDOR_NUM
 	default 0x0955 if ARCH_TEGRA
 	default 0x1f3a if ARCH_SUNXI
 	default 0x2207 if ARCH_ROCKCHIP
+	default 0x18d1 if ARCH_QCOM
 	default 0x0
 	help
 	  Vendor ID of the USB device emulated, reported to the host device.
@@ -86,6 +87,7 @@ config USB_GADGET_PRODUCT_NUM
 	default 0x350a if ROCKCHIP_RK3568
 	default 0x350b if ROCKCHIP_RK3588
 	default 0x350c if ROCKCHIP_RK3528
+	default 0x4ee0 if ARCH_QCOM
 	default 0x0
 	help
 	  Product ID of the USB device emulated, reported to the host device.

-- 
2.43.0

[PATCH v4 6/6] usb: gadget: Introduce usb gadget vendor/product default id for ARCH_QCOM

[George Chan via B4 Relay]

From: George Chan <gchan9527@gmail.com>

Currently vendor/product id are both 0, and that might not as we want.
Set to some arbitary known value that we can make it work more smoothly.

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Acked-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: George Chan <gchan9527@gmail.com>
---
 drivers/usb/gadget/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 46a83141481..2a56e360b5f 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -60,6 +60,7 @@ config USB_GADGET_VENDOR_NUM
 	default 0x0955 if ARCH_TEGRA
 	default 0x1f3a if ARCH_SUNXI
 	default 0x2207 if ARCH_ROCKCHIP
+	default 0x18d1 if ARCH_QCOM
 	default 0x0
 	help
 	  Vendor ID of the USB device emulated, reported to the host device.
@@ -86,6 +87,7 @@ config USB_GADGET_PRODUCT_NUM
 	default 0x350a if ROCKCHIP_RK3568
 	default 0x350b if ROCKCHIP_RK3588
 	default 0x350c if ROCKCHIP_RK3528
+	default 0x4ee0 if ARCH_QCOM
 	default 0x0
 	help
 	  Product ID of the USB device emulated, reported to the host device.

-- 
2.43.0

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Mattijs Korpershoek]

Hi George,

Thank you for the patch.

On Mon, Jun 30, 2025 at 15:56, George Chan via B4 Relay <devnull+gchan9527.gmail.com@kernel.org> wrote:

> From: George Chan <gchan9527@gmail.com>
>
> Old logic wipe bootargs env with cmdline, new logic cater the value
> by prepending cmdline value to bootargs.
>
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>  boot/bootm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 4bdca22ea8c..d8e0c3527d0 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -1165,7 +1165,7 @@ int bootm_boot_start(ulong addr, const char *cmdline)
>  
>  	snprintf(addr_str, sizeof(addr_str), "%lx", addr);
>  
> -	ret = env_set("bootargs", cmdline);
> +	ret = android_image_modify_bootargs_env(cmdline, NULL);

I don't think it can be done this way. bootm_boot_start() is used in the
ChromeOS bootmethod as well (boot/bootmeth_cros.c)

Changing this would potentially break ChromeOS boot behaviour so I'd
prefer to find another solution.

I know that TI has a downstream patch that changes bootmeth_android.c
instead:

https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63

Would that work for you?


>  	if (ret) {
>  		printf("Failed to set cmdline\n");
>  		return ret;
>
> -- 
> 2.43.0

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

Hi Mattijs,

Thx for reply.

在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：

> Hi George,
>
> Thank you for the patch.
>
...

>
> > -     ret = env_set("bootargs", cmdline);
> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>
> I don't think it can be done this way. bootm_boot_start() is used in the
> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>

I got your point. I have 3 ideas come out.

(1) The logic purposely empty the env bootargs, either developer don't use
env bootargs or those use cases touched bootargs in some early step. And
then wanna disregard previous u-boot bootmeth operation, and empty bootargs
for that ongoing bootmeth stage...well that's not congruent behavior; I
have a gut feeling this is a bug or band-aid anyway, it closed the door for
people (potentially abootimg) inject needed boot param between bootmeth
stage. Perhaps, either clean up bootargs before bootmeth load stage, or add
kconfig to check and enable this logic, a better representation.

(2) One more thing, the vendor_boot cmdline is not handle neither in
original logic nor in url from you provided. So I can say the url one is
not capable for extended with Android boot img version > 2 since
vendor_boot cmdline not handled.

(3) I don't think it is very much different between your mentioned url with
my patch. There is nothing advanced, but just existing code logic reuse and
that logic handled vendor_cmdline in other path. I prefer code logic reuse.

And I believe above are not the important thing....

>
> Changing this would potentially break ChromeOS boot behaviour so I'd
> prefer to find another solution.
>
> I know that TI has a downstream patch that changes bootmeth_android.c
> instead:
>
>
> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63


...


> Would that work for you?
>
>
Well, the one from url also fine with me.

The important thing is here. So as to ease the change with "minimal impact"
gets in. I can add one extra check to maintain original behavior in case
people have concern of breakage. Code example as below:

+ If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
+   ret = android_image_modify_bootargs_env(cmdline, NULL);
+ else
      ret = env_set("bootargs", cmdline);

This logic eliminated the concern, but it also limited those potential use
cases for Android boot header version > 2, unless the newly introduced
CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.

Or this one with good extendible.
+ /* Clean up bootargs purposely */
+ if (IS_ENABLED(SOME_FLAG))
+    env_set("bootargs", "");
+ ret = android_image_modify_bootargs_env(cmdline, NULL);

Either way. I prefer first one and can blindly apply without afford. I
leave the idea above to people more concern with it. Please let me know
your decision, I can provide one more revision later.

Regards,
George

>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Mattijs Korpershoek]

Hi George,

On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:

> Hi Mattijs,
>
> Thx for reply.
>
> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>
>> Hi George,
>>
>> Thank you for the patch.
>>
> ...
>
>>
>> > -     ret = env_set("bootargs", cmdline);
>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>>
>> I don't think it can be done this way. bootm_boot_start() is used in the
>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>>
>
> I got your point. I have 3 ideas come out.
>
> (1) The logic purposely empty the env bootargs, either developer don't use
> env bootargs or those use cases touched bootargs in some early step. And
> then wanna disregard previous u-boot bootmeth operation, and empty bootargs
> for that ongoing bootmeth stage...well that's not congruent behavior; I
> have a gut feeling this is a bug or band-aid anyway, it closed the door for

Simon, is it *intentional* that override the bootargs variable in
commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?

Shouldn't we get the bootargs from the environment, and append cmdline
to the existing bootargs instead ?


> people (potentially abootimg) inject needed boot param between bootmeth
> stage. Perhaps, either clean up bootargs before bootmeth load stage, or add
> kconfig to check and enable this logic, a better representation.
>
> (2) One more thing, the vendor_boot cmdline is not handle neither in
> original logic nor in url from you provided. So I can say the url one is
> not capable for extended with Android boot img version > 2 since
> vendor_boot cmdline not handled.

What do you mean by the vendor_boot cmdline is not handled?

When parsing the vendor_boot image, we go through
android_vendor_boot_image_v3_v4_parse_hdr()

That function copies the vendor_boot cmdline with:

	data->kcmdline_extra = hdr->cmdline;

(to the struct andr_image_data).

Later on, in android_image_get_kernel(), this gets copied to the
bootargs:

	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
			strcat(newbootargs, " ");
		strcat(newbootargs, img_data.kcmdline_extra);
	}

	env_set("bootargs", newbootargs);

>
> (3) I don't think it is very much different between your mentioned url with
> my patch. There is nothing advanced, but just existing code logic reuse and
> that logic handled vendor_cmdline in other path. I prefer code logic reuse.

The difference is that in the patch I've linked is that we only change
Android specific boot behaviour. So less risk to impact ChromeOS.

>
> And I believe above are not the important thing....
>
>>
>> Changing this would potentially break ChromeOS boot behaviour so I'd
>> prefer to find another solution.
>>
>> I know that TI has a downstream patch that changes bootmeth_android.c
>> instead:
>>
>>
>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>
>
> ...
>
>
>> Would that work for you?
>>
>>
> Well, the one from url also fine with me.
>
> The important thing is here. So as to ease the change with "minimal impact"
> gets in. I can add one extra check to maintain original behavior in case
> people have concern of breakage. Code example as below:
>
> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
> + else
>       ret = env_set("bootargs", cmdline);
>
> This logic eliminated the concern, but it also limited those potential use
> cases for Android boot header version > 2, unless the newly introduced
> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.

I'm not sure about why this is better, as from what I understand we
handle vendor_boot cmdline properly already.

Can you provide me with a test case or some example commands that show
that vendor_boot cmdline is not handled properly?

>
> Or this one with good extendible.
> + /* Clean up bootargs purposely */
> + if (IS_ENABLED(SOME_FLAG))
> +    env_set("bootargs", "");
> + ret = android_image_modify_bootargs_env(cmdline, NULL);
>
> Either way. I prefer first one and can blindly apply without afford. I
> leave the idea above to people more concern with it. Please let me know
> your decision, I can provide one more revision later.

I'm sorry there is so much back and forth on this series. I hope we can
come to a common agreement and get something merged.

Thanks
Mattijs

>
> Regards,
> George
>
>>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

Hi Mattijs,

Congrats to be a new dad.

在 2025年7月18日週五 20:03，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：

> Hi George,
>
> On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:
>
> > Hi Mattijs,
> >
> > Thx for reply.
> >
> > 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
> >
> >> Hi George,
> >>
> >> Thank you for the patch.
> >>
> > ...
> >
> >>
> >> > -     ret = env_set("bootargs", cmdline);
> >> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
> >>
> >> I don't think it can be done this way. bootm_boot_start() is used in the
> >> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
> >>
> >
> > I got your point. I have 3 ideas come out.
> >
> > (1) The logic purposely empty the env bootargs, either developer don't
> use
> > env bootargs or those use cases touched bootargs in some early step. And
> > then wanna disregard previous u-boot bootmeth operation, and empty
> bootargs
> > for that ongoing bootmeth stage...well that's not congruent behavior; I
> > have a gut feeling this is a bug or band-aid anyway, it closed the door
> for
>
> Simon, is it *intentional* that override the bootargs variable in
> commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>
> Shouldn't we get the bootargs from the environment, and append cmdline
> to the existing bootargs instead ?
>
>
> > people (potentially abootimg) inject needed boot param between bootmeth
> > stage. Perhaps, either clean up bootargs before bootmeth load stage, or
> add
> > kconfig to check and enable this logic, a better representation.
> >
> > (2) One more thing, the vendor_boot cmdline is not handle neither in
> > original logic nor in url from you provided. So I can say the url one is
> > not capable for extended with Android boot img version > 2 since
> > vendor_boot cmdline not handled.
>
> What do you mean by the vendor_boot cmdline is not handled?
>

Yes I mean it. If origin/master logic (for fastboot, my test case) had gone
through android_image_get_kernel then this patch is not necessary.


> When parsing the vendor_boot image, we go through
> android_vendor_boot_image_v3_v4_parse_hdr()
>
> That function copies the vendor_boot cmdline with:
>
>         data->kcmdline_extra = hdr->cmdline;
>

Yes and that it is. Since many routes now through bootm_boot_start() with
single cmdline so obviously vndboot_cmd is yet to finalized unless in
previous stage logic combine boot/vendor cmdline by purpose. So here a
better choice is to extend with 3rd param for vndrboot_cmdline. And let the
new bootm_boot_start_ex() for example to combine/modify.

for example:
Int bootm_boot_start_ex(ulong addr, const char *cmdline, const char
*vendor_cmdline) {
...
ret = android_image_modify_bootargs_env(cmdline, vendor_cmdline);
...
}

I did not do this way because it is a bit  clumsy. But in code maintain
point it might be good.


> (to the struct andr_image_data).
>
> Later on, in android_image_get_kernel(), this gets copied to the
> bootargs:
>
>         if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>                 if (*newbootargs) /* If there is something in newbootargs,
> a space is needed */
>                         strcat(newbootargs, " ");
>                 strcat(newbootargs, img_data.kcmdline_extra);
>         }
>
>         env_set("bootargs", newbootargs);
>
> >
> > (3) I don't think it is very much different between your mentioned url
> with
> > my patch. There is nothing advanced, but just existing code logic reuse
> and
> > that logic handled vendor_cmdline in other path. I prefer code logic
> reuse.
>
> The difference is that in the patch I've linked is that we only change
> Android specific boot behaviour. So less risk to impact ChromeOS.
>

Yes. As above suggested _ex() function to get around the limitation. Just
leave those old caller to stick with the old bootm_boot_start().


> >
> > And I believe above are not the important thing....
> >
> >>
> >> Changing this would potentially break ChromeOS boot behaviour so I'd
> >> prefer to find another solution.
> >>
> >> I know that TI has a downstream patch that changes bootmeth_android.c
> >> instead:
> >>
> >>
> >>
> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
> >
> >
> > ...
> >
> >
> >> Would that work for you?
> >>
> >>
> > Well, the one from url also fine with me.
> >
> > The important thing is here. So as to ease the change with "minimal
> impact"
> > gets in. I can add one extra check to maintain original behavior in case
> > people have concern of breakage. Code example as below:
> >
> > + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
> > +   ret = android_image_modify_bootargs_env(cmdline, NULL);
> > + else
> >       ret = env_set("bootargs", cmdline);
> >
> > This logic eliminated the concern, but it also limited those potential
> use
> > cases for Android boot header version > 2, unless the newly introduced
> > CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>
> I'm not sure about why this is better, as from what I understand we
> handle vendor_boot cmdline properly already.
>

This is my previous patch aimed to do....I moved all bootargs handling from
android_image_get_kernel into a new function and reuse it in
bootm_boot_start. I am glad you have same assumption on how bootargs should
be handled.


> Can you provide me with a test case or some example commands that show
> that vendor_boot cmdline is not handled properly?
>

May be above explain is enough?


> >
> > Or this one with good extendible.
> > + /* Clean up bootargs purposely */
> > + if (IS_ENABLED(SOME_FLAG))
> > +    env_set("bootargs", "");
> > + ret = android_image_modify_bootargs_env(cmdline, NULL);
> >
> > Either way. I prefer first one and can blindly apply without afford. I
> > leave the idea above to people more concern with it. Please let me know
> > your decision, I can provide one more revision later.
>
> I'm sorry there is so much back and forth on this series. I hope we can
> come to a common agreement and get something merged.
>

Yes I am struggle with this as well. If there are still some uncertain,
please consider merge patches with review-by and leave theose questioning
patch alone?

Thx again for reviewing.

Regards,
George


> Thanks
> Mattijs
>
> >
> > Regards,
> > George
> >
> >>
>

Re: [PATCH v4 0/6] A series of patch for enable sc7180 android boot

[Casey Connolly]

Hi George,

Sorry for not getting around to this sooner, I got it mixed up with your
other patch series!

I think this is ready to go but there's a few checkpatch errors, can you
run "b4 prep --check", fix the issues and then resend this and I'll pick
it up for next.

Thanks and regards,

On 30/06/2025 09:56, George Chan via B4 Relay wrote:
> Since attempt[1] to embed android specific boot param into fdt bootargs,
> a new idea is formed as to make use of env file to contain default param
> value. Current code logic is already working fine, unless the priority
> of param at begining is higher than at tail. From env file, bootargs is
> treated important, and prepend at begining. So there is a need to
> reverse the logic, and make sure default bootargs value at the end, and
> let androidboot img bootargs value sit at begining.
> 
> So a new kconfig item is introduced. Once enabled, the bootargs strcat
> param will get reversed.
> 
> A similar logic is needed for fastboot->bootm glue function, so split 
> function of patch #1 into 2 functions as #2, fixed some issues #3, and 
> apply #4 with same trick to glue function too.
> 
> Some one-liners patch is included to enable device driver for sc7180
> soc. It is about iommu for usb, usb gadget vendor/product id.
> 
> Reduce dependency on abootimg when androidboot v3 or greater is not
> needed. Since Tom suggested to reduce __maybe_unused directive and 
> use IS_ENABLE() instead; two __weak func proto need to add into image.h
> 
> [1]https://lists.denx.de/pipermail/u-boot/2025-May/588828.html
> [2]https://lists.denx.de/pipermail/u-boot/2025-May/589926.html
> 
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
> Changes in v4:
> - Drop patch #3.
> - Refine #2 to remove intermediate variables and code style.
> - Link to v3: https://lore.kernel.org/r/20250628-sc7180-android-boot-v3-0-105098a19165@gmail.com
> 
> Changes in v3:
> - Collect acks-by and review-by.
> - Split function and replace old #2 into #2-#4, code reuse in #4
> - Modify changelog and Kconfig description of #1, suggested by Mattijs
> - Slightly modify #5, add back line break.
> - Rebase to latest next
> - Link to v2: https://lore.kernel.org/r/20250607-sc7180-android-boot-v2-0-2df5d7f61124@gmail.com
> 
> Changes in v2:
> - Add new patch #2 that is same trick as #1 to fastboot->bootm glue layer.
> - Remove default n to patch #1, suggested by Tom.
> - Use IS_ENABLE() instead, suggested by Tom.
> - Collect review-by from Casey and Neil.
> - Rebase to u-boot/next branch.
> - CCing myself too.
> - Link to v1: https://lore.kernel.org/r/20250520-sc7180-android-boot-v1-0-3075a84ea094@gmail.com
> 
> ---
> George Chan (6):
>       image-android: Prepend/postpend default bootargs value with given bootcmd
>       boot/image-android.c: Split android_image_get_kernel into two functions
>       bootm: Append bootargs value when bootmeth_android provide cmdline
>       boot: bootmeth_android: Conditionally dependent on abootimg
>       iommu: qcom-smmu: Introduce sc7180 compatible string
>       usb: gadget: Introduce usb gadget vendor/product default id for ARCH_QCOM
> 
>  boot/Kconfig                  |  9 ++++
>  boot/bootm.c                  |  2 +-
>  boot/bootmeth_android.c       |  2 +-
>  boot/image-android.c          | 97 ++++++++++++++++++++++++++++---------------
>  drivers/iommu/qcom-hyp-smmu.c |  1 +
>  drivers/usb/gadget/Kconfig    |  2 +
>  include/image.h               | 14 +++++++
>  7 files changed, 92 insertions(+), 35 deletions(-)
> ---
> base-commit: 490ae8ceae2d5999c9de863e014e463f5c1495a6
> change-id: 20250520-sc7180-android-boot-a845ecf48a48
> 
> Best regards,

-- 
// Casey (she/her)

Re: [PATCH v4 0/6] A series of patch for enable sc7180 android boot

[george chan]

[resend with full list, sorry for duplicated email]

Hi Casey,

Thanks for getting back with the the patch series.

在 2025年8月13日週三 21:39，Casey Connolly <casey.connolly@linaro.org> 寫道：

> Hi George,
>
> Sorry for not getting around to this sooner, I got it mixed up with your
> other patch series!
>
> I think this is ready to go but there's a few checkpatch errors, can you
> run "b4 prep --check", fix the issues and then resend this and I'll pick
> it up for next.
>

i am afraid it is still have some sort of concern about the logic change.
So even through with respin....I suggested[1] to get those patches with
review-by to get in this round, and leave the yet-to-discuss as independent
patch later.

If you agreed with this arrangement then please git cherry-pick from this
series, or once found difficulty then let me know I can start a fresh new
series for you.

Thanks,
George

[1]https://lists.denx.de/pipermail/u-boot/2025-July/594610.html


> Thanks and regards,
>
> On 30/06/2025 09:56, George Chan via B4 Relay wrote:
> > Since attempt[1] to embed android specific boot param into fdt bootargs,
> > a new idea is formed as to make use of env file to contain default param
> > value. Current code logic is already working fine, unless the priority
> > of param at begining is higher than at tail. From env file, bootargs is
> > treated important, and prepend at begining. So there is a need to
> > reverse the logic, and make sure default bootargs value at the end, and
> > let androidboot img bootargs value sit at begining.
> >
> > So a new kconfig item is introduced. Once enabled, the bootargs strcat
> > param will get reversed.
> >
> > A similar logic is needed for fastboot->bootm glue function, so split
> > function of patch #1 into 2 functions as #2, fixed some issues #3, and
> > apply #4 with same trick to glue function too.
> >
> > Some one-liners patch is included to enable device driver for sc7180
> > soc. It is about iommu for usb, usb gadget vendor/product id.
> >
> > Reduce dependency on abootimg when androidboot v3 or greater is not
> > needed. Since Tom suggested to reduce __maybe_unused directive and
> > use IS_ENABLE() instead; two __weak func proto need to add into image.h
> >
> > [1]https://lists.denx.de/pipermail/u-boot/2025-May/588828.html
> > [2]https://lists.denx.de/pipermail/u-boot/2025-May/589926.html
> >
> > Signed-off-by: George Chan <gchan9527@gmail.com>
> > ---
> > Changes in v4:
> > - Drop patch #3.
> > - Refine #2 to remove intermediate variables and code style.
> > - Link to v3:
> https://lore.kernel.org/r/20250628-sc7180-android-boot-v3-0-105098a19165@gmail.com
> >
> > Changes in v3:
> > - Collect acks-by and review-by.
> > - Split function and replace old #2 into #2-#4, code reuse in #4
> > - Modify changelog and Kconfig description of #1, suggested by Mattijs
> > - Slightly modify #5, add back line break.
> > - Rebase to latest next
> > - Link to v2:
> https://lore.kernel.org/r/20250607-sc7180-android-boot-v2-0-2df5d7f61124@gmail.com
> >
> > Changes in v2:
> > - Add new patch #2 that is same trick as #1 to fastboot->bootm glue
> layer.
> > - Remove default n to patch #1, suggested by Tom.
> > - Use IS_ENABLE() instead, suggested by Tom.
> > - Collect review-by from Casey and Neil.
> > - Rebase to u-boot/next branch.
> > - CCing myself too.
> > - Link to v1:
> https://lore.kernel.org/r/20250520-sc7180-android-boot-v1-0-3075a84ea094@gmail.com
> >
> > ---
> > George Chan (6):
> >       image-android: Prepend/postpend default bootargs value with given
> bootcmd
> >       boot/image-android.c: Split android_image_get_kernel into two
> functions
> >       bootm: Append bootargs value when bootmeth_android provide cmdline
> >       boot: bootmeth_android: Conditionally dependent on abootimg
> >       iommu: qcom-smmu: Introduce sc7180 compatible string
> >       usb: gadget: Introduce usb gadget vendor/product default id for
> ARCH_QCOM
> >
> >  boot/Kconfig                  |  9 ++++
> >  boot/bootm.c                  |  2 +-
> >  boot/bootmeth_android.c       |  2 +-
> >  boot/image-android.c          | 97
> ++++++++++++++++++++++++++++---------------
> >  drivers/iommu/qcom-hyp-smmu.c |  1 +
> >  drivers/usb/gadget/Kconfig    |  2 +
> >  include/image.h               | 14 +++++++
> >  7 files changed, 92 insertions(+), 35 deletions(-)
> > ---
> > base-commit: 490ae8ceae2d5999c9de863e014e463f5c1495a6
> > change-id: 20250520-sc7180-android-boot-a845ecf48a48
> >
> > Best regards,
>
> --
> // Casey (she/her)
>
>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Mattijs Korpershoek]

Hi Simon,

Now that you are back, could you please have a look at ...

On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:

> Hi George,
>
> On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:
>
>> Hi Mattijs,
>>
>> Thx for reply.
>>
>> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>>
>>> Hi George,
>>>
>>> Thank you for the patch.
>>>
>> ...
>>
>>>
>>> > -     ret = env_set("bootargs", cmdline);
>>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>>>
>>> I don't think it can be done this way. bootm_boot_start() is used in the
>>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>>>
>>
>> I got your point. I have 3 ideas come out.
>>
>> (1) The logic purposely empty the env bootargs, either developer don't use
>> env bootargs or those use cases touched bootargs in some early step. And
>> then wanna disregard previous u-boot bootmeth operation, and empty bootargs
>> for that ongoing bootmeth stage...well that's not congruent behavior; I
>> have a gut feeling this is a bug or band-aid anyway, it closed the door for
>
> Simon, is it *intentional* that override the bootargs variable in
> commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>
> Shouldn't we get the bootargs from the environment, and append cmdline
> to the existing bootargs instead ?

... this question ?

We are wondering why bootm_boot_start() overrides the bootargs variable
instead of appending cmdline to the existing bootargs instead.

>
>
>> people (potentially abootimg) inject needed boot param between bootmeth
>> stage. Perhaps, either clean up bootargs before bootmeth load stage, or add
>> kconfig to check and enable this logic, a better representation.
>>
>> (2) One more thing, the vendor_boot cmdline is not handle neither in
>> original logic nor in url from you provided. So I can say the url one is
>> not capable for extended with Android boot img version > 2 since
>> vendor_boot cmdline not handled.
>
> What do you mean by the vendor_boot cmdline is not handled?
>
> When parsing the vendor_boot image, we go through
> android_vendor_boot_image_v3_v4_parse_hdr()
>
> That function copies the vendor_boot cmdline with:
>
> 	data->kcmdline_extra = hdr->cmdline;
>
> (to the struct andr_image_data).
>
> Later on, in android_image_get_kernel(), this gets copied to the
> bootargs:
>
> 	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> 		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
> 			strcat(newbootargs, " ");
> 		strcat(newbootargs, img_data.kcmdline_extra);
> 	}
>
> 	env_set("bootargs", newbootargs);
>
>>
>> (3) I don't think it is very much different between your mentioned url with
>> my patch. There is nothing advanced, but just existing code logic reuse and
>> that logic handled vendor_cmdline in other path. I prefer code logic reuse.
>
> The difference is that in the patch I've linked is that we only change
> Android specific boot behaviour. So less risk to impact ChromeOS.
>
>>
>> And I believe above are not the important thing....
>>
>>>
>>> Changing this would potentially break ChromeOS boot behaviour so I'd
>>> prefer to find another solution.
>>>
>>> I know that TI has a downstream patch that changes bootmeth_android.c
>>> instead:
>>>
>>>
>>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>>
>>
>> ...
>>
>>
>>> Would that work for you?
>>>
>>>
>> Well, the one from url also fine with me.
>>
>> The important thing is here. So as to ease the change with "minimal impact"
>> gets in. I can add one extra check to maintain original behavior in case
>> people have concern of breakage. Code example as below:
>>
>> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
>> + else
>>       ret = env_set("bootargs", cmdline);
>>
>> This logic eliminated the concern, but it also limited those potential use
>> cases for Android boot header version > 2, unless the newly introduced
>> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>
> I'm not sure about why this is better, as from what I understand we
> handle vendor_boot cmdline properly already.
>
> Can you provide me with a test case or some example commands that show
> that vendor_boot cmdline is not handled properly?
>
>>
>> Or this one with good extendible.
>> + /* Clean up bootargs purposely */
>> + if (IS_ENABLED(SOME_FLAG))
>> +    env_set("bootargs", "");
>> + ret = android_image_modify_bootargs_env(cmdline, NULL);
>>
>> Either way. I prefer first one and can blindly apply without afford. I
>> leave the idea above to people more concern with it. Please let me know
>> your decision, I can provide one more revision later.
>
> I'm sorry there is so much back and forth on this series. I hope we can
> come to a common agreement and get something merged.
>
> Thanks
> Mattijs
>
>>
>> Regards,
>> George
>>
>>>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Mattijs Korpershoek]

Hi George,

On Sat, Jul 19, 2025 at 01:17, george chan <gchan9527@gmail.com> wrote:

> Hi Mattijs,
>
> Congrats to be a new dad.

Thank you! Sorry for all the delays. I'm just back from paternity leave.
I hope we can work together to get your patches merged.

>
> 在 2025年7月18日週五 20:03，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>
>> Hi George,
>>
>> On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:
>>
>> > Hi Mattijs,
>> >
>> > Thx for reply.
>> >
>> > 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>> >
>> >> Hi George,
>> >>
>> >> Thank you for the patch.
>> >>
>> > ...
>> >
>> >>
>> >> > -     ret = env_set("bootargs", cmdline);
>> >> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>
>> >> I don't think it can be done this way. bootm_boot_start() is used in the
>> >> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>> >>
>> >
>> > I got your point. I have 3 ideas come out.
>> >
>> > (1) The logic purposely empty the env bootargs, either developer don't
>> use
>> > env bootargs or those use cases touched bootargs in some early step. And
>> > then wanna disregard previous u-boot bootmeth operation, and empty
>> bootargs
>> > for that ongoing bootmeth stage...well that's not congruent behavior; I
>> > have a gut feeling this is a bug or band-aid anyway, it closed the door
>> for
>>
>> Simon, is it *intentional* that override the bootargs variable in
>> commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>>
>> Shouldn't we get the bootargs from the environment, and append cmdline
>> to the existing bootargs instead ?
>>
>>
>> > people (potentially abootimg) inject needed boot param between bootmeth
>> > stage. Perhaps, either clean up bootargs before bootmeth load stage, or
>> add
>> > kconfig to check and enable this logic, a better representation.

If this is indeed a bug, maybe fixing it would be the easiest. Now that
Simon is back, pinged him about this.

>> >
>> > (2) One more thing, the vendor_boot cmdline is not handle neither in
>> > original logic nor in url from you provided. So I can say the url one is
>> > not capable for extended with Android boot img version > 2 since
>> > vendor_boot cmdline not handled.
>>
>> What do you mean by the vendor_boot cmdline is not handled?
>>
>
> Yes I mean it. If origin/master logic (for fastboot, my test case) had gone
> through android_image_get_kernel then this patch is not necessary.
>
>
>> When parsing the vendor_boot image, we go through
>> android_vendor_boot_image_v3_v4_parse_hdr()
>>
>> That function copies the vendor_boot cmdline with:
>>
>>         data->kcmdline_extra = hdr->cmdline;
>>
>
> Yes and that it is. Since many routes now through bootm_boot_start() with
> single cmdline so obviously vndboot_cmd is yet to finalized unless in
> previous stage logic combine boot/vendor cmdline by purpose. So here a
> better choice is to extend with 3rd param for vndrboot_cmdline. And let the
> new bootm_boot_start_ex() for example to combine/modify.
>
> for example:
> Int bootm_boot_start_ex(ulong addr, const char *cmdline, const char
> *vendor_cmdline) {
> ...
> ret = android_image_modify_bootargs_env(cmdline, vendor_cmdline);
> ...
> }
>
> I did not do this way because it is a bit  clumsy. But in code maintain
> point it might be good.
>
>
>> (to the struct andr_image_data).
>>
>> Later on, in android_image_get_kernel(), this gets copied to the
>> bootargs:
>>
>>         if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>>                 if (*newbootargs) /* If there is something in newbootargs,
>> a space is needed */
>>                         strcat(newbootargs, " ");
>>                 strcat(newbootargs, img_data.kcmdline_extra);
>>         }
>>
>>         env_set("bootargs", newbootargs);
>>
>> >
>> > (3) I don't think it is very much different between your mentioned url
>> with
>> > my patch. There is nothing advanced, but just existing code logic reuse
>> and
>> > that logic handled vendor_cmdline in other path. I prefer code logic
>> reuse.
>>
>> The difference is that in the patch I've linked is that we only change
>> Android specific boot behaviour. So less risk to impact ChromeOS.
>>
>
> Yes. As above suggested _ex() function to get around the limitation. Just
> leave those old caller to stick with the old bootm_boot_start().
>
>
>> >
>> > And I believe above are not the important thing....
>> >
>> >>
>> >> Changing this would potentially break ChromeOS boot behaviour so I'd
>> >> prefer to find another solution.
>> >>
>> >> I know that TI has a downstream patch that changes bootmeth_android.c
>> >> instead:
>> >>
>> >>
>> >>
>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>> >
>> >
>> > ...
>> >
>> >
>> >> Would that work for you?
>> >>
>> >>
>> > Well, the one from url also fine with me.
>> >
>> > The important thing is here. So as to ease the change with "minimal
>> impact"
>> > gets in. I can add one extra check to maintain original behavior in case
>> > people have concern of breakage. Code example as below:
>> >
>> > + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>> > +   ret = android_image_modify_bootargs_env(cmdline, NULL);
>> > + else
>> >       ret = env_set("bootargs", cmdline);
>> >
>> > This logic eliminated the concern, but it also limited those potential
>> use
>> > cases for Android boot header version > 2, unless the newly introduced
>> > CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>>
>> I'm not sure about why this is better, as from what I understand we
>> handle vendor_boot cmdline properly already.
>>
>
> This is my previous patch aimed to do....I moved all bootargs handling from
> android_image_get_kernel into a new function and reuse it in
> bootm_boot_start. I am glad you have same assumption on how bootargs should
> be handled.
>
>
>> Can you provide me with a test case or some example commands that show
>> that vendor_boot cmdline is not handled properly?
>>
>
> May be above explain is enough?
>
>
>> >
>> > Or this one with good extendible.
>> > + /* Clean up bootargs purposely */
>> > + if (IS_ENABLED(SOME_FLAG))
>> > +    env_set("bootargs", "");
>> > + ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >
>> > Either way. I prefer first one and can blindly apply without afford. I
>> > leave the idea above to people more concern with it. Please let me know
>> > your decision, I can provide one more revision later.
>>
>> I'm sorry there is so much back and forth on this series. I hope we can
>> come to a common agreement and get something merged.
>>
>
> Yes I am struggle with this as well. If there are still some uncertain,
> please consider merge patches with review-by and leave theose questioning
> patch alone?
>
> Thx again for reviewing.
>
> Regards,
> George
>
>
>> Thanks
>> Mattijs
>>
>> >
>> > Regards,
>> > George
>> >
>> >>
>>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Simon Glass]

Hi Mattijs, George,

On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
<mkorpershoek@kernel.org> wrote:
>
> Hi Simon,
>
> Now that you are back, could you please have a look at ...
>
> On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>
> > Hi George,
> >
> > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:
> >
> >> Hi Mattijs,
> >>
> >> Thx for reply.
> >>
> >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
> >>
> >>> Hi George,
> >>>
> >>> Thank you for the patch.
> >>>
> >> ...
> >>
> >>>
> >>> > -     ret = env_set("bootargs", cmdline);
> >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
> >>>
> >>> I don't think it can be done this way. bootm_boot_start() is used in the
> >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
> >>>
> >>
> >> I got your point. I have 3 ideas come out.
> >>
> >> (1) The logic purposely empty the env bootargs, either developer don't use
> >> env bootargs or those use cases touched bootargs in some early step. And
> >> then wanna disregard previous u-boot bootmeth operation, and empty bootargs
> >> for that ongoing bootmeth stage...well that's not congruent behavior; I
> >> have a gut feeling this is a bug or band-aid anyway, it closed the door for
> >
> > Simon, is it *intentional* that override the bootargs variable in
> > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
> >
> > Shouldn't we get the bootargs from the environment, and append cmdline
> > to the existing bootargs instead ?

The way U-Boot operates today, bootargs defines the entire cmdline,
not just part of it. So if you use 'env set bootargs ..' you are
changing the entire cmdline passed to Linux, etc.

With ChromeOS, after 'bootflow read', the cmdline can be edited using
'bootflow cmdline set' etc., which also changes bootargs. But in
general ChromeOS provides the whole Linux cmdline to U-Boot.

If you are wanting to append, I suggest writing a new function, or
perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
other code is useful for Android.

>
> ... this question ?
>
> We are wondering why bootm_boot_start() overrides the bootargs variable
> instead of appending cmdline to the existing bootargs instead.
>
> >
> >
> >> people (potentially abootimg) inject needed boot param between bootmeth
> >> stage. Perhaps, either clean up bootargs before bootmeth load stage, or add
> >> kconfig to check and enable this logic, a better representation.
> >>
> >> (2) One more thing, the vendor_boot cmdline is not handle neither in
> >> original logic nor in url from you provided. So I can say the url one is
> >> not capable for extended with Android boot img version > 2 since
> >> vendor_boot cmdline not handled.
> >
> > What do you mean by the vendor_boot cmdline is not handled?
> >
> > When parsing the vendor_boot image, we go through
> > android_vendor_boot_image_v3_v4_parse_hdr()
> >
> > That function copies the vendor_boot cmdline with:
> >
> >       data->kcmdline_extra = hdr->cmdline;
> >
> > (to the struct andr_image_data).
> >
> > Later on, in android_image_get_kernel(), this gets copied to the
> > bootargs:
> >
> >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> >               if (*newbootargs) /* If there is something in newbootargs, a space is needed */
> >                       strcat(newbootargs, " ");
> >               strcat(newbootargs, img_data.kcmdline_extra);
> >       }
> >
> >       env_set("bootargs", newbootargs);
> >
> >>
> >> (3) I don't think it is very much different between your mentioned url with
> >> my patch. There is nothing advanced, but just existing code logic reuse and
> >> that logic handled vendor_cmdline in other path. I prefer code logic reuse.
> >
> > The difference is that in the patch I've linked is that we only change
> > Android specific boot behaviour. So less risk to impact ChromeOS.
> >
> >>
> >> And I believe above are not the important thing....
> >>
> >>>
> >>> Changing this would potentially break ChromeOS boot behaviour so I'd
> >>> prefer to find another solution.
> >>>
> >>> I know that TI has a downstream patch that changes bootmeth_android.c
> >>> instead:
> >>>
> >>>
> >>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
> >>
> >>
> >> ...
> >>
> >>
> >>> Would that work for you?
> >>>
> >>>
> >> Well, the one from url also fine with me.
> >>
> >> The important thing is here. So as to ease the change with "minimal impact"
> >> gets in. I can add one extra check to maintain original behavior in case
> >> people have concern of breakage. Code example as below:
> >>
> >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
> >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
> >> + else
> >>       ret = env_set("bootargs", cmdline);
> >>
> >> This logic eliminated the concern, but it also limited those potential use
> >> cases for Android boot header version > 2, unless the newly introduced
> >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
> >
> > I'm not sure about why this is better, as from what I understand we
> > handle vendor_boot cmdline properly already.
> >
> > Can you provide me with a test case or some example commands that show
> > that vendor_boot cmdline is not handled properly?
> >
> >>
> >> Or this one with good extendible.
> >> + /* Clean up bootargs purposely */
> >> + if (IS_ENABLED(SOME_FLAG))
> >> +    env_set("bootargs", "");
> >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
> >>
> >> Either way. I prefer first one and can blindly apply without afford. I
> >> leave the idea above to people more concern with it. Please let me know
> >> your decision, I can provide one more revision later.
> >
> > I'm sorry there is so much back and forth on this series. I hope we can
> > come to a common agreement and get something merged.

Regards,
Simon

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

Hi Simon and Mattijs,

Thx for feedback. Just in case the discussion drift to other direction, do
let me have a chance to put some background info here.

there are ways for pre-set env variable and its value. Firstly external env
file and loaded by uboot, like, runtime handling; and another type is build
time embed env file into uboot body.

maybe in chromeos case first stage boot loader will pass along bootinfo
struct to uboot and treat as preset defaults.

so in either case uboot can have right to choose the preset from (by
disable external env file, secured/trusted boot?), and then either follow
the bootinfo struct preset value and/or concat and/or overrite with uboot’s
own embedded env value at boot time, and then runtime modify.

This behavior maybe enable or disable by kconfig. but in secured boot
context point of view, env file kind of thing, might have potential design
wise conflict. And this is not within my scope.

in some other use case like simulation of vendor bootloader behavior to
have a build time preset default value, this is within my scope. and thats
why i decided to put those preset into env file and embeded.

let me reply below inline.

在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：

> Hi Mattijs, George,
>
> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
> <mkorpershoek@kernel.org> wrote:
> >
> > Hi Simon,
> >
> > Now that you are back, could you please have a look at ...
> >
> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <
> mkorpershoek@kernel.org> wrote:
> >
> > > Hi George,
> > >
> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com>
> wrote:
> > >
> > >> Hi Mattijs,
> > >>
> > >> Thx for reply.
> > >>
> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org>
> 寫道：
> > >>
> > >>> Hi George,
> > >>>
> > >>> Thank you for the patch.
> > >>>
> > >> ...
> > >>
> > >>>
> > >>> > -     ret = env_set("bootargs", cmdline);
> > >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
> > >>>
> > >>> I don't think it can be done this way. bootm_boot_start() is used in
> the
> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
> > >>>
> > >>
> > >> I got your point. I have 3 ideas come out.
> > >>
> > >> (1) The logic purposely empty the env bootargs, either developer
> don't use
> > >> env bootargs or those use cases touched bootargs in some early step.
> And
> > >> then wanna disregard previous u-boot bootmeth operation, and empty
> bootargs
> > >> for that ongoing bootmeth stage...well that's not congruent behavior;
> I
> > >> have a gut feeling this is a bug or band-aid anyway, it closed the
> door for
> > >
> > > Simon, is it *intentional* that override the bootargs variable in
> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
> > >
> > > Shouldn't we get the bootargs from the environment, and append cmdline
> > > to the existing bootargs instead ?
>
> The way U-Boot operates today, bootargs defines the entire cmdline,
> not just part of it. So if you use 'env set bootargs ..' you are
> changing the entire cmdline passed to Linux, etc.
>

This is runtime change. My test case in fastboot path, it get bootargs
wiped and filling in with something from boot img. That might share same
procedure with other boot method like chromeos do. Wipe env default each
time in fastboot (when fail) might be a secure-wise handling.

And I feel it is a critical entry point logic like a central hub, and
looking good to decide uboot behavior here. By kconfig whatever? Again, out
of my scope.


> With ChromeOS, after 'bootflow read', the cmdline can be edited using
> 'bootflow cmdline set' etc., which also changes bootargs. But in
> general ChromeOS provides the whole Linux cmdline to U-Boot.
>

this is also runtime change. And as a side note, if by doing bootflow
select, should env bootargs gets overrided or appended? This is a bigger
arch-wise question worth at least a public paper. and I might not go into
this depth.


> If you are wanting to append, I suggest writing a new function, or
>

yeah agreed. I do more appreciate and feel it really lower project
management/test cost.

perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
> other code is useful for Android.
>

A more simpler method is to create bootm_boot_start_ex(kernel_cmd_line,
vendor_cmd_line) as
 I proposed to complement with
 bootm_boot_start(cmdline) or even add one layer of abstract, called from
bootm_boot_start(), make vendor_cmd_line param as null. As can preserve the
old logic and behavior.

So build time env preset value, and runtime value override decision can
live in this new function, or elsewhere; but is not within my scope.

So I leave it as a open-end question here. Once you had decided then feel
free to let me know and I can do one extra round patch. Thank again for
feedback.

George


> >
> > ... this question ?
> >
> > We are wondering why bootm_boot_start() overrides the bootargs variable
> > instead of appending cmdline to the existing bootargs instead.
> >
> > >
> > >
> > >> people (potentially abootimg) inject needed boot param between
> bootmeth
> > >> stage. Perhaps, either clean up bootargs before bootmeth load stage,
> or add
> > >> kconfig to check and enable this logic, a better representation.
> > >>
> > >> (2) One more thing, the vendor_boot cmdline is not handle neither in
> > >> original logic nor in url from you provided. So I can say the url one
> is
> > >> not capable for extended with Android boot img version > 2 since
> > >> vendor_boot cmdline not handled.
> > >
> > > What do you mean by the vendor_boot cmdline is not handled?
> > >
> > > When parsing the vendor_boot image, we go through
> > > android_vendor_boot_image_v3_v4_parse_hdr()
> > >
> > > That function copies the vendor_boot cmdline with:
> > >
> > >       data->kcmdline_extra = hdr->cmdline;
> > >
> > > (to the struct andr_image_data).
> > >
> > > Later on, in android_image_get_kernel(), this gets copied to the
> > > bootargs:
> > >
> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> > >               if (*newbootargs) /* If there is something in
> newbootargs, a space is needed */
> > >                       strcat(newbootargs, " ");
> > >               strcat(newbootargs, img_data.kcmdline_extra);
> > >       }
> > >
> > >       env_set("bootargs", newbootargs);
> > >
> > >>
> > >> (3) I don't think it is very much different between your mentioned
> url with
> > >> my patch. There is nothing advanced, but just existing code logic
> reuse and
> > >> that logic handled vendor_cmdline in other path. I prefer code logic
> reuse.
> > >
> > > The difference is that in the patch I've linked is that we only change
> > > Android specific boot behaviour. So less risk to impact ChromeOS.
> > >
> > >>
> > >> And I believe above are not the important thing....
> > >>
> > >>>
> > >>> Changing this would potentially break ChromeOS boot behaviour so I'd
> > >>> prefer to find another solution.
> > >>>
> > >>> I know that TI has a downstream patch that changes bootmeth_android.c
> > >>> instead:
> > >>>
> > >>>
> > >>>
> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
> > >>
> > >>
> > >> ...
> > >>
> > >>
> > >>> Would that work for you?
> > >>>
> > >>>
> > >> Well, the one from url also fine with me.
> > >>
> > >> The important thing is here. So as to ease the change with "minimal
> impact"
> > >> gets in. I can add one extra check to maintain original behavior in
> case
> > >> people have concern of breakage. Code example as below:
> > >>
> > >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
> > >> + else
> > >>       ret = env_set("bootargs", cmdline);
> > >>
> > >> This logic eliminated the concern, but it also limited those
> potential use
> > >> cases for Android boot header version > 2, unless the newly introduced
> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
> > >
> > > I'm not sure about why this is better, as from what I understand we
> > > handle vendor_boot cmdline properly already.
> > >
> > > Can you provide me with a test case or some example commands that show
> > > that vendor_boot cmdline is not handled properly?
> > >
> > >>
> > >> Or this one with good extendible.
> > >> + /* Clean up bootargs purposely */
> > >> + if (IS_ENABLED(SOME_FLAG))
> > >> +    env_set("bootargs", "");
> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
> > >>
> > >> Either way. I prefer first one and can blindly apply without afford. I
> > >> leave the idea above to people more concern with it. Please let me
> know
> > >> your decision, I can provide one more revision later.
> > >
> > > I'm sorry there is so much back and forth on this series. I hope we can
> > > come to a common agreement and get something merged.
>
> Regards,
> Simon
>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

在 2025年10月5日週日 00:35，george chan <gchan9527@gmail.com> 寫道：

> Hi Simon and Mattijs,
>
> Thx for feedback. Just in case the discussion drift to other direction, do
> let me have a chance to put some background info here.
>
> there are ways for pre-set env variable and its value. Firstly external
> env file and loaded by uboot, like, runtime handling; and another type is
> build time embed env file into uboot body.
>
> maybe in chromeos case first stage boot loader will pass along bootinfo
> struct to uboot and treat as preset defaults.
>
> so in either case uboot can have right to choose the preset from (by
> disable external env file, secured/trusted boot?), and then either follow
> the bootinfo struct preset value and/or concat and/or overrite with uboot’s
> own embedded env value at boot time, and then runtime modify.
>
> This behavior maybe enable or disable by kconfig. but in secured boot
> context point of view, env file kind of thing, might have potential design
> wise conflict. And this is not within my scope.
>
> in some other use case like simulation of vendor bootloader behavior to
> have a build time preset default value, this is within my scope. and thats
> why i decided to put those preset into env file and embeded.
>
> let me reply below inline.
>
> 在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：
>
>> Hi Mattijs, George,
>>
>> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
>> <mkorpershoek@kernel.org> wrote:
>> >
>> > Hi Simon,
>> >
>> > Now that you are back, could you please have a look at ...
>> >
>> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <
>> mkorpershoek@kernel.org> wrote:
>> >
>> > > Hi George,
>> > >
>> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com>
>> wrote:
>> > >
>> > >> Hi Mattijs,
>> > >>
>> > >> Thx for reply.
>> > >>
>> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org>
>> 寫道：
>> > >>
>> > >>> Hi George,
>> > >>>
>> > >>> Thank you for the patch.
>> > >>>
>> > >> ...
>> > >>
>> > >>>
>> > >>> > -     ret = env_set("bootargs", cmdline);
>> > >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>> > >>>
>> > >>> I don't think it can be done this way. bootm_boot_start() is used
>> in the
>> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>> > >>>
>> > >>
>> > >> I got your point. I have 3 ideas come out.
>> > >>
>> > >> (1) The logic purposely empty the env bootargs, either developer
>> don't use
>> > >> env bootargs or those use cases touched bootargs in some early step.
>> And
>> > >> then wanna disregard previous u-boot bootmeth operation, and empty
>> bootargs
>> > >> for that ongoing bootmeth stage...well that's not congruent
>> behavior; I
>> > >> have a gut feeling this is a bug or band-aid anyway, it closed the
>> door for
>> > >
>> > > Simon, is it *intentional* that override the bootargs variable in
>> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>> > >
>> > > Shouldn't we get the bootargs from the environment, and append cmdline
>> > > to the existing bootargs instead ?
>>
>> The way U-Boot operates today, bootargs defines the entire cmdline,
>> not just part of it. So if you use 'env set bootargs ..' you are
>> changing the entire cmdline passed to Linux, etc.
>>
>
> This is runtime change. My test case in fastboot path, it get bootargs
> wiped and filling in with something from boot img. That might share same
> procedure with other boot method like chromeos do. Wipe env default each
> time in fastboot (when fail) might be a secure-wise handling.
>
> And I feel it is a critical entry point logic like a central hub, and
> looking good to decide uboot behavior here. By kconfig whatever? Again, out
> of my scope.
>
>
>> With ChromeOS, after 'bootflow read', the cmdline can be edited using
>> 'bootflow cmdline set' etc., which also changes bootargs. But in
>> general ChromeOS provides the whole Linux cmdline to U-Boot.
>>
>
> this is also runtime change. And as a side note, if by doing bootflow
> select, should env bootargs gets overrided or appended? This is a bigger
> arch-wise question worth at least a public paper. and I might not go into
> this depth.
>
>
>> If you are wanting to append, I suggest writing a new function, or
>>
>
> yeah agreed. I do more appreciate and feel it really lower project
> management/test cost.
>
> perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
>> other code is useful for Android.
>>
>
> A more simpler method is to create bootm_boot_start_ex(kernel_cmd_line,
> vendor_cmd_line) as
>  I proposed to complement with
>  bootm_boot_start(cmdline) or even add one layer of abstract, called from
> bootm_boot_start(), make vendor_cmd_line param as null. As can preserve the
> old logic and behavior.
>
> So build time env preset value, and runtime value override decision can
> live in this new function, or elsewhere; but is not within my scope.
>

Tr/Dr I wrote a proof of concept code here:

https://github.com/99degree/u-boot/compare/before_tag...reboot-mode?expand=1



> So I leave it as a open-end question here. Once you had decided then feel
> free to let me know and I can do one extra round patch. Thank again for
> feedback.
>
> George
>
>
>> >
>> > ... this question ?
>> >
>> > We are wondering why bootm_boot_start() overrides the bootargs variable
>> > instead of appending cmdline to the existing bootargs instead.
>> >
>> > >
>> > >
>> > >> people (potentially abootimg) inject needed boot param between
>> bootmeth
>> > >> stage. Perhaps, either clean up bootargs before bootmeth load stage,
>> or add
>> > >> kconfig to check and enable this logic, a better representation.
>> > >>
>> > >> (2) One more thing, the vendor_boot cmdline is not handle neither in
>> > >> original logic nor in url from you provided. So I can say the url
>> one is
>> > >> not capable for extended with Android boot img version > 2 since
>> > >> vendor_boot cmdline not handled.
>> > >
>> > > What do you mean by the vendor_boot cmdline is not handled?
>> > >
>> > > When parsing the vendor_boot image, we go through
>> > > android_vendor_boot_image_v3_v4_parse_hdr()
>> > >
>> > > That function copies the vendor_boot cmdline with:
>> > >
>> > >       data->kcmdline_extra = hdr->cmdline;
>> > >
>> > > (to the struct andr_image_data).
>> > >
>> > > Later on, in android_image_get_kernel(), this gets copied to the
>> > > bootargs:
>> > >
>> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>> > >               if (*newbootargs) /* If there is something in
>> newbootargs, a space is needed */
>> > >                       strcat(newbootargs, " ");
>> > >               strcat(newbootargs, img_data.kcmdline_extra);
>> > >       }
>> > >
>> > >       env_set("bootargs", newbootargs);
>> > >
>> > >>
>> > >> (3) I don't think it is very much different between your mentioned
>> url with
>> > >> my patch. There is nothing advanced, but just existing code logic
>> reuse and
>> > >> that logic handled vendor_cmdline in other path. I prefer code logic
>> reuse.
>> > >
>> > > The difference is that in the patch I've linked is that we only change
>> > > Android specific boot behaviour. So less risk to impact ChromeOS.
>> > >
>> > >>
>> > >> And I believe above are not the important thing....
>> > >>
>> > >>>
>> > >>> Changing this would potentially break ChromeOS boot behaviour so I'd
>> > >>> prefer to find another solution.
>> > >>>
>> > >>> I know that TI has a downstream patch that changes
>> bootmeth_android.c
>> > >>> instead:
>> > >>>
>> > >>>
>> > >>>
>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>> > >>
>> > >>
>> > >> ...
>> > >>
>> > >>
>> > >>> Would that work for you?
>> > >>>
>> > >>>
>> > >> Well, the one from url also fine with me.
>> > >>
>> > >> The important thing is here. So as to ease the change with "minimal
>> impact"
>> > >> gets in. I can add one extra check to maintain original behavior in
>> case
>> > >> people have concern of breakage. Code example as below:
>> > >>
>> > >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
>> > >> + else
>> > >>       ret = env_set("bootargs", cmdline);
>> > >>
>> > >> This logic eliminated the concern, but it also limited those
>> potential use
>> > >> cases for Android boot header version > 2, unless the newly
>> introduced
>> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>> > >
>> > > I'm not sure about why this is better, as from what I understand we
>> > > handle vendor_boot cmdline properly already.
>> > >
>> > > Can you provide me with a test case or some example commands that show
>> > > that vendor_boot cmdline is not handled properly?
>> > >
>> > >>
>> > >> Or this one with good extendible.
>> > >> + /* Clean up bootargs purposely */
>> > >> + if (IS_ENABLED(SOME_FLAG))
>> > >> +    env_set("bootargs", "");
>> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
>> > >>
>> > >> Either way. I prefer first one and can blindly apply without afford.
>> I
>> > >> leave the idea above to people more concern with it. Please let me
>> know
>> > >> your decision, I can provide one more revision later.
>> > >
>> > > I'm sorry there is so much back and forth on this series. I hope we
>> can
>> > > come to a common agreement and get something merged.
>>
>> Regards,
>> Simon
>>
>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Simon Glass]

Hi George,

On Sun, 5 Oct 2025 at 09:41, george chan <gchan9527@gmail.com> wrote:
>
>
>
> 在 2025年10月5日週日 00:35，george chan <gchan9527@gmail.com> 寫道：
>>
>> Hi Simon and Mattijs,
>>
>> Thx for feedback. Just in case the discussion drift to other direction, do let me have a chance to put some background info here.
>>
>> there are ways for pre-set env variable and its value. Firstly external env file and loaded by uboot, like, runtime handling; and another type is build time embed env file into uboot body.
>>
>> maybe in chromeos case first stage boot loader will pass along bootinfo struct to uboot and treat as preset defaults.
>>
>> so in either case uboot can have right to choose the preset from (by disable external env file, secured/trusted boot?), and then either follow the bootinfo struct preset value and/or concat and/or overrite with uboot’s own embedded env value at boot time, and then runtime modify.
>>
>> This behavior maybe enable or disable by kconfig. but in secured boot context point of view, env file kind of thing, might have potential design wise conflict. And this is not within my scope.
>>
>> in some other use case like simulation of vendor bootloader behavior to have a build time preset default value, this is within my scope. and thats why i decided to put those preset into env file and embeded.
>>
>> let me reply below inline.
>>
>> 在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：
>>>
>>> Hi Mattijs, George,
>>>
>>> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
>>> <mkorpershoek@kernel.org> wrote:
>>> >
>>> > Hi Simon,
>>> >
>>> > Now that you are back, could you please have a look at ...
>>> >
>>> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>>> >
>>> > > Hi George,
>>> > >
>>> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:
>>> > >
>>> > >> Hi Mattijs,
>>> > >>
>>> > >> Thx for reply.
>>> > >>
>>> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>>> > >>
>>> > >>> Hi George,
>>> > >>>
>>> > >>> Thank you for the patch.
>>> > >>>
>>> > >> ...
>>> > >>
>>> > >>>
>>> > >>> > -     ret = env_set("bootargs", cmdline);
>>> > >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>>> > >>>
>>> > >>> I don't think it can be done this way. bootm_boot_start() is used in the
>>> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>>> > >>>
>>> > >>
>>> > >> I got your point. I have 3 ideas come out.
>>> > >>
>>> > >> (1) The logic purposely empty the env bootargs, either developer don't use
>>> > >> env bootargs or those use cases touched bootargs in some early step. And
>>> > >> then wanna disregard previous u-boot bootmeth operation, and empty bootargs
>>> > >> for that ongoing bootmeth stage...well that's not congruent behavior; I
>>> > >> have a gut feeling this is a bug or band-aid anyway, it closed the door for
>>> > >
>>> > > Simon, is it *intentional* that override the bootargs variable in
>>> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>>> > >
>>> > > Shouldn't we get the bootargs from the environment, and append cmdline
>>> > > to the existing bootargs instead ?
>>>
>>> The way U-Boot operates today, bootargs defines the entire cmdline,
>>> not just part of it. So if you use 'env set bootargs ..' you are
>>> changing the entire cmdline passed to Linux, etc.
>>
>>
>> This is runtime change. My test case in fastboot path, it get bootargs wiped and filling in with something from boot img. That might share same procedure with other boot method like chromeos do. Wipe env default each time in fastboot (when fail) might be a secure-wise handling.
>>
>> And I feel it is a critical entry point logic like a central hub, and looking good to decide uboot behavior here. By kconfig whatever? Again, out of my scope.
>>
>>>
>>> With ChromeOS, after 'bootflow read', the cmdline can be edited using
>>> 'bootflow cmdline set' etc., which also changes bootargs. But in
>>> general ChromeOS provides the whole Linux cmdline to U-Boot.
>>
>>
>> this is also runtime change. And as a side note, if by doing bootflow select, should env bootargs gets overrided or appended? This is a bigger arch-wise question worth at least a public paper. and I might not go into this depth.
>>
>>>
>>> If you are wanting to append, I suggest writing a new function, or
>>
>>
>> yeah agreed. I do more appreciate and feel it really lower project management/test cost.
>>
>>> perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
>>> other code is useful for Android.
>>
>>
>> A more simpler method is to create bootm_boot_start_ex(kernel_cmd_line, vendor_cmd_line) as
>>  I proposed to complement with
>>  bootm_boot_start(cmdline) or even add one layer of abstract, called from bootm_boot_start(), make vendor_cmd_line param as null. As can preserve the old logic and behavior.
>>
>> So build time env preset value, and runtime value override decision can live in this new function, or elsewhere; but is not within my scope.
>
>
> Tr/Dr I wrote a proof of concept code here:
>
> https://github.com/99degree/u-boot/compare/before_tag...reboot-mode?expand=1

This thread is a bit confusing now, but it seems you have written a
new function and that is fine with me, as it won't affect ChromeOS.


>
>
>>
>> So I leave it as a open-end question here. Once you had decided then feel free to let me know and I can do one extra round patch. Thank again for feedback.
>>
>> George
>>
>>>
>>> >
>>> > ... this question ?
>>> >
>>> > We are wondering why bootm_boot_start() overrides the bootargs variable
>>> > instead of appending cmdline to the existing bootargs instead.
>>> >
>>> > >
>>> > >
>>> > >> people (potentially abootimg) inject needed boot param between bootmeth
>>> > >> stage. Perhaps, either clean up bootargs before bootmeth load stage, or add
>>> > >> kconfig to check and enable this logic, a better representation.
>>> > >>
>>> > >> (2) One more thing, the vendor_boot cmdline is not handle neither in
>>> > >> original logic nor in url from you provided. So I can say the url one is
>>> > >> not capable for extended with Android boot img version > 2 since
>>> > >> vendor_boot cmdline not handled.
>>> > >
>>> > > What do you mean by the vendor_boot cmdline is not handled?
>>> > >
>>> > > When parsing the vendor_boot image, we go through
>>> > > android_vendor_boot_image_v3_v4_parse_hdr()
>>> > >
>>> > > That function copies the vendor_boot cmdline with:
>>> > >
>>> > >       data->kcmdline_extra = hdr->cmdline;
>>> > >
>>> > > (to the struct andr_image_data).
>>> > >
>>> > > Later on, in android_image_get_kernel(), this gets copied to the
>>> > > bootargs:
>>> > >
>>> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>>> > >               if (*newbootargs) /* If there is something in newbootargs, a space is needed */
>>> > >                       strcat(newbootargs, " ");
>>> > >               strcat(newbootargs, img_data.kcmdline_extra);
>>> > >       }
>>> > >
>>> > >       env_set("bootargs", newbootargs);
>>> > >
>>> > >>
>>> > >> (3) I don't think it is very much different between your mentioned url with
>>> > >> my patch. There is nothing advanced, but just existing code logic reuse and
>>> > >> that logic handled vendor_cmdline in other path. I prefer code logic reuse.
>>> > >
>>> > > The difference is that in the patch I've linked is that we only change
>>> > > Android specific boot behaviour. So less risk to impact ChromeOS.
>>> > >
>>> > >>
>>> > >> And I believe above are not the important thing....
>>> > >>
>>> > >>>
>>> > >>> Changing this would potentially break ChromeOS boot behaviour so I'd
>>> > >>> prefer to find another solution.
>>> > >>>
>>> > >>> I know that TI has a downstream patch that changes bootmeth_android.c
>>> > >>> instead:
>>> > >>>
>>> > >>>
>>> > >>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>>> > >>
>>> > >>
>>> > >> ...
>>> > >>
>>> > >>
>>> > >>> Would that work for you?
>>> > >>>
>>> > >>>
>>> > >> Well, the one from url also fine with me.
>>> > >>
>>> > >> The important thing is here. So as to ease the change with "minimal impact"
>>> > >> gets in. I can add one extra check to maintain original behavior in case
>>> > >> people have concern of breakage. Code example as below:
>>> > >>
>>> > >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>>> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
>>> > >> + else
>>> > >>       ret = env_set("bootargs", cmdline);
>>> > >>
>>> > >> This logic eliminated the concern, but it also limited those potential use
>>> > >> cases for Android boot header version > 2, unless the newly introduced
>>> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>>> > >
>>> > > I'm not sure about why this is better, as from what I understand we
>>> > > handle vendor_boot cmdline properly already.
>>> > >
>>> > > Can you provide me with a test case or some example commands that show
>>> > > that vendor_boot cmdline is not handled properly?
>>> > >
>>> > >>
>>> > >> Or this one with good extendible.
>>> > >> + /* Clean up bootargs purposely */
>>> > >> + if (IS_ENABLED(SOME_FLAG))
>>> > >> +    env_set("bootargs", "");
>>> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
>>> > >>
>>> > >> Either way. I prefer first one and can blindly apply without afford. I
>>> > >> leave the idea above to people more concern with it. Please let me know
>>> > >> your decision, I can provide one more revision later.
>>> > >
>>> > > I'm sorry there is so much back and forth on this series. I hope we can
>>> > > come to a common agreement and get something merged.

Regards,
Simon

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

Hi Simon,

Thx for feedback.


在 2025年10月7日週二 04:38，Simon Glass <sjg@chromium.org> 寫道：

> Hi George,
>
> On Sun, 5 Oct 2025 at 09:41, george chan <gchan9527@gmail.com> wrote:
> >
> >
> >
> > 在 2025年10月5日週日 00:35，george chan <gchan9527@gmail.com> 寫道：
> >>
> >> Hi Simon and Mattijs,
> >>
> >> Thx for feedback. Just in case the discussion drift to other direction,
> do let me have a chance to put some background info here.
> >>
> >> there are ways for pre-set env variable and its value. Firstly external
> env file and loaded by uboot, like, runtime handling; and another type is
> build time embed env file into uboot body.
> >>
> >> maybe in chromeos case first stage boot loader will pass along bootinfo
> struct to uboot and treat as preset defaults.
> >>
> >> so in either case uboot can have right to choose the preset from (by
> disable external env file, secured/trusted boot?), and then either follow
> the bootinfo struct preset value and/or concat and/or overrite with uboot’s
> own embedded env value at boot time, and then runtime modify.
> >>
> >> This behavior maybe enable or disable by kconfig. but in secured boot
> context point of view, env file kind of thing, might have potential design
> wise conflict. And this is not within my scope.
> >>
> >> in some other use case like simulation of vendor bootloader behavior to
> have a build time preset default value, this is within my scope. and thats
> why i decided to put those preset into env file and embeded.
> >>
> >> let me reply below inline.
> >>
> >> 在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：
> >>>
> >>> Hi Mattijs, George,
> >>>
> >>> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
> >>> <mkorpershoek@kernel.org> wrote:
> >>> >
> >>> > Hi Simon,
> >>> >
> >>> > Now that you are back, could you please have a look at ...
> >>> >
> >>> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <
> mkorpershoek@kernel.org> wrote:
> >>> >
> >>> > > Hi George,
> >>> > >
> >>> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com>
> wrote:
> >>> > >
> >>> > >> Hi Mattijs,
> >>> > >>
> >>> > >> Thx for reply.
> >>> > >>
> >>> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org>
> 寫道：
> >>> > >>
> >>> > >>> Hi George,
> >>> > >>>
> >>> > >>> Thank you for the patch.
> >>> > >>>
> >>> > >> ...
> >>> > >>
> >>> > >>>
> >>> > >>> > -     ret = env_set("bootargs", cmdline);
> >>> > >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
> >>> > >>>
> >>> > >>> I don't think it can be done this way. bootm_boot_start() is
> used in the
> >>> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
> >>> > >>>
> >>> > >>
> >>> > >> I got your point. I have 3 ideas come out.
> >>> > >>
> >>> > >> (1) The logic purposely empty the env bootargs, either developer
> don't use
> >>> > >> env bootargs or those use cases touched bootargs in some early
> step. And
> >>> > >> then wanna disregard previous u-boot bootmeth operation, and
> empty bootargs
> >>> > >> for that ongoing bootmeth stage...well that's not congruent
> behavior; I
> >>> > >> have a gut feeling this is a bug or band-aid anyway, it closed
> the door for
> >>> > >
> >>> > > Simon, is it *intentional* that override the bootargs variable in
> >>> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
> >>> > >
> >>> > > Shouldn't we get the bootargs from the environment, and append
> cmdline
> >>> > > to the existing bootargs instead ?
> >>>
> >>> The way U-Boot operates today, bootargs defines the entire cmdline,
> >>> not just part of it. So if you use 'env set bootargs ..' you are
> >>> changing the entire cmdline passed to Linux, etc.
> >>
> >>
> >> This is runtime change. My test case in fastboot path, it get bootargs
> wiped and filling in with something from boot img. That might share same
> procedure with other boot method like chromeos do. Wipe env default each
> time in fastboot (when fail) might be a secure-wise handling.
> >>
> >> And I feel it is a critical entry point logic like a central hub, and
> looking good to decide uboot behavior here. By kconfig whatever? Again, out
> of my scope.
> >>
> >>>
> >>> With ChromeOS, after 'bootflow read', the cmdline can be edited using
> >>> 'bootflow cmdline set' etc., which also changes bootargs. But in
> >>> general ChromeOS provides the whole Linux cmdline to U-Boot.
> >>
> >>
> >> this is also runtime change. And as a side note, if by doing bootflow
> select, should env bootargs gets overrided or appended? This is a bigger
> arch-wise question worth at least a public paper. and I might not go into
> this depth.
> >>
> >>>
> >>> If you are wanting to append, I suggest writing a new function, or
> >>
> >>
> >> yeah agreed. I do more appreciate and feel it really lower project
> management/test cost.
> >>
> >>> perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
> >>> other code is useful for Android.
> >>
> >>
> >> A more simpler method is to create bootm_boot_start_ex(kernel_cmd_line,
> vendor_cmd_line) as
> >>  I proposed to complement with
> >>  bootm_boot_start(cmdline) or even add one layer of abstract, called
> from bootm_boot_start(), make vendor_cmd_line param as null. As can
> preserve the old logic and behavior.
> >>
> >> So build time env preset value, and runtime value override decision can
> live in this new function, or elsewhere; but is not within my scope.
> >
> >
> > Tr/Dr I wrote a proof of concept code here:
> >
> >
> https://github.com/99degree/u-boot/compare/before_tag...reboot-mode?expand=1
>
> This thread is a bit confusing now, but it seems you have written a
>

I am sorry but I had to prove my patch is necessary and correctnessly and
also limit the scope of the patch. Thanks for your understand.

new function and that is fine with me, as it won't affect ChromeOS.
>

There is no new function at all. I am sorry to put more info here again to
let related code to be clear for every audience here.

As you may notice function bootm_modify_bootargs_env() is a renamed and
move to bootm.c which is split/extracted from existing function
android_image_get_kernel()

And bootm_boot_start_ex() is split from
bootm_boot_start() with your suggested enable param.

I am afraid if you or other reviewer might wonder why move to bootm.c and
here is the reason.

Supposed with below 2 code paths:

Boot->bootflow->bootmeth_android->bootm
Boot->fastboot->do_bootm->bootm

I believe android_image_get_kernel is living around within bootmeth_android
stage so first two patch has taken care of. But fastboot path is not. that
might be the reason above two path need separate special handling for.

So as to code reuse with fastboot path then those code had to move to bootm
layer.

And my understanding each stage is isolated, and depends on env as a kind
of api to link up each stage. Then I believe the patch is now a clean
solution. Anyway welcome to jot a mail if there are suggestions, if any
doubt, please jot a mail too.

Thanks,
George


>
> >
> >
> >>
> >> So I leave it as a open-end question here. Once you had decided then
> feel free to let me know and I can do one extra round patch. Thank again
> for feedback.
> >>
> >> George
> >>
> >>>
> >>> >
> >>> > ... this question ?
> >>> >
> >>> > We are wondering why bootm_boot_start() overrides the bootargs
> variable
> >>> > instead of appending cmdline to the existing bootargs instead.
> >>> >
> >>> > >
> >>> > >
> >>> > >> people (potentially abootimg) inject needed boot param between
> bootmeth
> >>> > >> stage. Perhaps, either clean up bootargs before bootmeth load
> stage, or add
> >>> > >> kconfig to check and enable this logic, a better representation.
> >>> > >>
> >>> > >> (2) One more thing, the vendor_boot cmdline is not handle neither
> in
> >>> > >> original logic nor in url from you provided. So I can say the url
> one is
> >>> > >> not capable for extended with Android boot img version > 2 since
> >>> > >> vendor_boot cmdline not handled.
> >>> > >
> >>> > > What do you mean by the vendor_boot cmdline is not handled?
> >>> > >
> >>> > > When parsing the vendor_boot image, we go through
> >>> > > android_vendor_boot_image_v3_v4_parse_hdr()
> >>> > >
> >>> > > That function copies the vendor_boot cmdline with:
> >>> > >
> >>> > >       data->kcmdline_extra = hdr->cmdline;
> >>> > >
> >>> > > (to the struct andr_image_data).
> >>> > >
> >>> > > Later on, in android_image_get_kernel(), this gets copied to the
> >>> > > bootargs:
> >>> > >
> >>> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> >>> > >               if (*newbootargs) /* If there is something in
> newbootargs, a space is needed */
> >>> > >                       strcat(newbootargs, " ");
> >>> > >               strcat(newbootargs, img_data.kcmdline_extra);
> >>> > >       }
> >>> > >
> >>> > >       env_set("bootargs", newbootargs);
> >>> > >
> >>> > >>
> >>> > >> (3) I don't think it is very much different between your
> mentioned url with
> >>> > >> my patch. There is nothing advanced, but just existing code logic
> reuse and
> >>> > >> that logic handled vendor_cmdline in other path. I prefer code
> logic reuse.
> >>> > >
> >>> > > The difference is that in the patch I've linked is that we only
> change
> >>> > > Android specific boot behaviour. So less risk to impact ChromeOS.
> >>> > >
> >>> > >>
> >>> > >> And I believe above are not the important thing....
> >>> > >>
> >>> > >>>
> >>> > >>> Changing this would potentially break ChromeOS boot behaviour so
> I'd
> >>> > >>> prefer to find another solution.
> >>> > >>>
> >>> > >>> I know that TI has a downstream patch that changes
> bootmeth_android.c
> >>> > >>> instead:
> >>> > >>>
> >>> > >>>
> >>> > >>>
> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
> >>> > >>
> >>> > >>
> >>> > >> ...
> >>> > >>
> >>> > >>
> >>> > >>> Would that work for you?
> >>> > >>>
> >>> > >>>
> >>> > >> Well, the one from url also fine with me.
> >>> > >>
> >>> > >> The important thing is here. So as to ease the change with
> "minimal impact"
> >>> > >> gets in. I can add one extra check to maintain original behavior
> in case
> >>> > >> people have concern of breakage. Code example as below:
> >>> > >>
> >>> > >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
> >>> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
> >>> > >> + else
> >>> > >>       ret = env_set("bootargs", cmdline);
> >>> > >>
> >>> > >> This logic eliminated the concern, but it also limited those
> potential use
> >>> > >> cases for Android boot header version > 2, unless the newly
> introduced
> >>> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
> >>> > >
> >>> > > I'm not sure about why this is better, as from what I understand we
> >>> > > handle vendor_boot cmdline properly already.
> >>> > >
> >>> > > Can you provide me with a test case or some example commands that
> show
> >>> > > that vendor_boot cmdline is not handled properly?
> >>> > >
> >>> > >>
> >>> > >> Or this one with good extendible.
> >>> > >> + /* Clean up bootargs purposely */
> >>> > >> + if (IS_ENABLED(SOME_FLAG))
> >>> > >> +    env_set("bootargs", "");
> >>> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
> >>> > >>
> >>> > >> Either way. I prefer first one and can blindly apply without
> afford. I
> >>> > >> leave the idea above to people more concern with it. Please let
> me know
> >>> > >> your decision, I can provide one more revision later.
> >>> > >
> >>> > > I'm sorry there is so much back and forth on this series. I hope
> we can
> >>> > > come to a common agreement and get something merged.
>
> Regards,
> Simon
>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Simon Glass]

Hi George,

On Tue, 7 Oct 2025 at 10:03, george chan <gchan9527@gmail.com> wrote:
>
> Hi Simon,
>
> Thx for feedback.
>
>
> 在 2025年10月7日週二 04:38，Simon Glass <sjg@chromium.org> 寫道：
>>
>> Hi George,
>>
>> On Sun, 5 Oct 2025 at 09:41, george chan <gchan9527@gmail.com> wrote:
>> >
>> >
>> >
>> > 在 2025年10月5日週日 00:35，george chan <gchan9527@gmail.com> 寫道：
>> >>
>> >> Hi Simon and Mattijs,
>> >>
>> >> Thx for feedback. Just in case the discussion drift to other direction, do let me have a chance to put some background info here.
>> >>
>> >> there are ways for pre-set env variable and its value. Firstly external env file and loaded by uboot, like, runtime handling; and another type is build time embed env file into uboot body.
>> >>
>> >> maybe in chromeos case first stage boot loader will pass along bootinfo struct to uboot and treat as preset defaults.
>> >>
>> >> so in either case uboot can have right to choose the preset from (by disable external env file, secured/trusted boot?), and then either follow the bootinfo struct preset value and/or concat and/or overrite with uboot’s own embedded env value at boot time, and then runtime modify.
>> >>
>> >> This behavior maybe enable or disable by kconfig. but in secured boot context point of view, env file kind of thing, might have potential design wise conflict. And this is not within my scope.
>> >>
>> >> in some other use case like simulation of vendor bootloader behavior to have a build time preset default value, this is within my scope. and thats why i decided to put those preset into env file and embeded.
>> >>
>> >> let me reply below inline.
>> >>
>> >> 在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：
>> >>>
>> >>> Hi Mattijs, George,
>> >>>
>> >>> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
>> >>> <mkorpershoek@kernel.org> wrote:
>> >>> >
>> >>> > Hi Simon,
>> >>> >
>> >>> > Now that you are back, could you please have a look at ...
>> >>> >
>> >>> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>> >>> >
>> >>> > > Hi George,
>> >>> > >
>> >>> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com> wrote:
>> >>> > >
>> >>> > >> Hi Mattijs,
>> >>> > >>
>> >>> > >> Thx for reply.
>> >>> > >>
>> >>> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>> >>> > >>
>> >>> > >>> Hi George,
>> >>> > >>>
>> >>> > >>> Thank you for the patch.
>> >>> > >>>
>> >>> > >> ...
>> >>> > >>
>> >>> > >>>
>> >>> > >>> > -     ret = env_set("bootargs", cmdline);
>> >>> > >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>> > >>>
>> >>> > >>> I don't think it can be done this way. bootm_boot_start() is used in the
>> >>> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>> >>> > >>>
>> >>> > >>
>> >>> > >> I got your point. I have 3 ideas come out.
>> >>> > >>
>> >>> > >> (1) The logic purposely empty the env bootargs, either developer don't use
>> >>> > >> env bootargs or those use cases touched bootargs in some early step. And
>> >>> > >> then wanna disregard previous u-boot bootmeth operation, and empty bootargs
>> >>> > >> for that ongoing bootmeth stage...well that's not congruent behavior; I
>> >>> > >> have a gut feeling this is a bug or band-aid anyway, it closed the door for
>> >>> > >
>> >>> > > Simon, is it *intentional* that override the bootargs variable in
>> >>> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>> >>> > >
>> >>> > > Shouldn't we get the bootargs from the environment, and append cmdline
>> >>> > > to the existing bootargs instead ?
>> >>>
>> >>> The way U-Boot operates today, bootargs defines the entire cmdline,
>> >>> not just part of it. So if you use 'env set bootargs ..' you are
>> >>> changing the entire cmdline passed to Linux, etc.
>> >>
>> >>
>> >> This is runtime change. My test case in fastboot path, it get bootargs wiped and filling in with something from boot img. That might share same procedure with other boot method like chromeos do. Wipe env default each time in fastboot (when fail) might be a secure-wise handling.
>> >>
>> >> And I feel it is a critical entry point logic like a central hub, and looking good to decide uboot behavior here. By kconfig whatever? Again, out of my scope.
>> >>
>> >>>
>> >>> With ChromeOS, after 'bootflow read', the cmdline can be edited using
>> >>> 'bootflow cmdline set' etc., which also changes bootargs. But in
>> >>> general ChromeOS provides the whole Linux cmdline to U-Boot.
>> >>
>> >>
>> >> this is also runtime change. And as a side note, if by doing bootflow select, should env bootargs gets overrided or appended? This is a bigger arch-wise question worth at least a public paper. and I might not go into this depth.
>> >>
>> >>>
>> >>> If you are wanting to append, I suggest writing a new function, or
>> >>
>> >>
>> >> yeah agreed. I do more appreciate and feel it really lower project management/test cost.
>> >>
>> >>> perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
>> >>> other code is useful for Android.
>> >>
>> >>
>> >> A more simpler method is to create bootm_boot_start_ex(kernel_cmd_line, vendor_cmd_line) as
>> >>  I proposed to complement with
>> >>  bootm_boot_start(cmdline) or even add one layer of abstract, called from bootm_boot_start(), make vendor_cmd_line param as null. As can preserve the old logic and behavior.
>> >>
>> >> So build time env preset value, and runtime value override decision can live in this new function, or elsewhere; but is not within my scope.
>> >
>> >
>> > Tr/Dr I wrote a proof of concept code here:
>> >
>> > https://github.com/99degree/u-boot/compare/before_tag...reboot-mode?expand=1
>>
>> This thread is a bit confusing now, but it seems you have written a
>
>
> I am sorry but I had to prove my patch is necessary and correctnessly and also limit the scope of the patch. Thanks for your understand.
>
>> new function and that is fine with me, as it won't affect ChromeOS.
>
>
> There is no new function at all. I am sorry to put more info here again to let related code to be clear for every audience here.
>
> As you may notice function bootm_modify_bootargs_env() is a renamed and move to bootm.c which is split/extracted from existing function android_image_get_kernel()
>
> And bootm_boot_start_ex() is split from
> bootm_boot_start() with your suggested enable param.
>
> I am afraid if you or other reviewer might wonder why move to bootm.c and here is the reason.
>
> Supposed with below 2 code paths:
>
> Boot->bootflow->bootmeth_android->bootm
> Boot->fastboot->do_bootm->bootm
>
> I believe android_image_get_kernel is living around within bootmeth_android stage so first two patch has taken care of. But fastboot path is not. that might be the reason above two path need separate special handling for.
>
> So as to code reuse with fastboot path then those code had to move to bootm layer.
>
> And my understanding each stage is isolated, and depends on env as a kind of api to link up each stage. Then I believe the patch is now a clean solution. Anyway welcome to jot a mail if there are suggestions, if any doubt, please jot a mail too.

It seems fine to me, but the thread is becoming confusing. Could you
send a patch?



>
> Thanks,
> George
>
>>
>>
>> >
>> >
>> >>
>> >> So I leave it as a open-end question here. Once you had decided then feel free to let me know and I can do one extra round patch. Thank again for feedback.
>> >>
>> >> George
>> >>
>> >>>
>> >>> >
>> >>> > ... this question ?
>> >>> >
>> >>> > We are wondering why bootm_boot_start() overrides the bootargs variable
>> >>> > instead of appending cmdline to the existing bootargs instead.
>> >>> >
>> >>> > >
>> >>> > >
>> >>> > >> people (potentially abootimg) inject needed boot param between bootmeth
>> >>> > >> stage. Perhaps, either clean up bootargs before bootmeth load stage, or add
>> >>> > >> kconfig to check and enable this logic, a better representation.
>> >>> > >>
>> >>> > >> (2) One more thing, the vendor_boot cmdline is not handle neither in
>> >>> > >> original logic nor in url from you provided. So I can say the url one is
>> >>> > >> not capable for extended with Android boot img version > 2 since
>> >>> > >> vendor_boot cmdline not handled.
>> >>> > >
>> >>> > > What do you mean by the vendor_boot cmdline is not handled?
>> >>> > >
>> >>> > > When parsing the vendor_boot image, we go through
>> >>> > > android_vendor_boot_image_v3_v4_parse_hdr()
>> >>> > >
>> >>> > > That function copies the vendor_boot cmdline with:
>> >>> > >
>> >>> > >       data->kcmdline_extra = hdr->cmdline;
>> >>> > >
>> >>> > > (to the struct andr_image_data).
>> >>> > >
>> >>> > > Later on, in android_image_get_kernel(), this gets copied to the
>> >>> > > bootargs:
>> >>> > >
>> >>> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>> >>> > >               if (*newbootargs) /* If there is something in newbootargs, a space is needed */
>> >>> > >                       strcat(newbootargs, " ");
>> >>> > >               strcat(newbootargs, img_data.kcmdline_extra);
>> >>> > >       }
>> >>> > >
>> >>> > >       env_set("bootargs", newbootargs);
>> >>> > >
>> >>> > >>
>> >>> > >> (3) I don't think it is very much different between your mentioned url with
>> >>> > >> my patch. There is nothing advanced, but just existing code logic reuse and
>> >>> > >> that logic handled vendor_cmdline in other path. I prefer code logic reuse.
>> >>> > >
>> >>> > > The difference is that in the patch I've linked is that we only change
>> >>> > > Android specific boot behaviour. So less risk to impact ChromeOS.
>> >>> > >
>> >>> > >>
>> >>> > >> And I believe above are not the important thing....
>> >>> > >>
>> >>> > >>>
>> >>> > >>> Changing this would potentially break ChromeOS boot behaviour so I'd
>> >>> > >>> prefer to find another solution.
>> >>> > >>>
>> >>> > >>> I know that TI has a downstream patch that changes bootmeth_android.c
>> >>> > >>> instead:
>> >>> > >>>
>> >>> > >>>
>> >>> > >>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>> >>> > >>
>> >>> > >>
>> >>> > >> ...
>> >>> > >>
>> >>> > >>
>> >>> > >>> Would that work for you?
>> >>> > >>>
>> >>> > >>>
>> >>> > >> Well, the one from url also fine with me.
>> >>> > >>
>> >>> > >> The important thing is here. So as to ease the change with "minimal impact"
>> >>> > >> gets in. I can add one extra check to maintain original behavior in case
>> >>> > >> people have concern of breakage. Code example as below:
>> >>> > >>
>> >>> > >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>> >>> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>> > >> + else
>> >>> > >>       ret = env_set("bootargs", cmdline);
>> >>> > >>
>> >>> > >> This logic eliminated the concern, but it also limited those potential use
>> >>> > >> cases for Android boot header version > 2, unless the newly introduced
>> >>> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>> >>> > >
>> >>> > > I'm not sure about why this is better, as from what I understand we
>> >>> > > handle vendor_boot cmdline properly already.
>> >>> > >
>> >>> > > Can you provide me with a test case or some example commands that show
>> >>> > > that vendor_boot cmdline is not handled properly?
>> >>> > >
>> >>> > >>
>> >>> > >> Or this one with good extendible.
>> >>> > >> + /* Clean up bootargs purposely */
>> >>> > >> + if (IS_ENABLED(SOME_FLAG))
>> >>> > >> +    env_set("bootargs", "");
>> >>> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>> > >>
>> >>> > >> Either way. I prefer first one and can blindly apply without afford. I
>> >>> > >> leave the idea above to people more concern with it. Please let me know
>> >>> > >> your decision, I can provide one more revision later.
>> >>> > >
>> >>> > > I'm sorry there is so much back and forth on this series. I hope we can
>> >>> > > come to a common agreement and get something merged.
Regards,
Simon

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[Mattijs Korpershoek]

Hi George,

On Wed, Oct 08, 2025 at 00:03, george chan <gchan9527@gmail.com> wrote:

> Hi Simon,
>
> Thx for feedback.
>
>
> 在 2025年10月7日週二 04:38，Simon Glass <sjg@chromium.org> 寫道：
>
>> Hi George,
>>
>> On Sun, 5 Oct 2025 at 09:41, george chan <gchan9527@gmail.com> wrote:
>> >
>> >
>> >
>> > 在 2025年10月5日週日 00:35，george chan <gchan9527@gmail.com> 寫道：
>> >>
>> >> Hi Simon and Mattijs,
>> >>
>> >> Thx for feedback. Just in case the discussion drift to other direction,
>> do let me have a chance to put some background info here.
>> >>
>> >> there are ways for pre-set env variable and its value. Firstly external
>> env file and loaded by uboot, like, runtime handling; and another type is
>> build time embed env file into uboot body.
>> >>
>> >> maybe in chromeos case first stage boot loader will pass along bootinfo
>> struct to uboot and treat as preset defaults.
>> >>
>> >> so in either case uboot can have right to choose the preset from (by
>> disable external env file, secured/trusted boot?), and then either follow
>> the bootinfo struct preset value and/or concat and/or overrite with uboot’s
>> own embedded env value at boot time, and then runtime modify.
>> >>
>> >> This behavior maybe enable or disable by kconfig. but in secured boot
>> context point of view, env file kind of thing, might have potential design
>> wise conflict. And this is not within my scope.
>> >>
>> >> in some other use case like simulation of vendor bootloader behavior to
>> have a build time preset default value, this is within my scope. and thats
>> why i decided to put those preset into env file and embeded.
>> >>
>> >> let me reply below inline.
>> >>
>> >> 在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：
>> >>>
>> >>> Hi Mattijs, George,
>> >>>
>> >>> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
>> >>> <mkorpershoek@kernel.org> wrote:
>> >>> >
>> >>> > Hi Simon,
>> >>> >
>> >>> > Now that you are back, could you please have a look at ...
>> >>> >
>> >>> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <
>> mkorpershoek@kernel.org> wrote:
>> >>> >
>> >>> > > Hi George,
>> >>> > >
>> >>> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com>
>> wrote:
>> >>> > >
>> >>> > >> Hi Mattijs,
>> >>> > >>
>> >>> > >> Thx for reply.
>> >>> > >>
>> >>> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <mkorpershoek@kernel.org>
>> 寫道：
>> >>> > >>
>> >>> > >>> Hi George,
>> >>> > >>>
>> >>> > >>> Thank you for the patch.
>> >>> > >>>
>> >>> > >> ...
>> >>> > >>
>> >>> > >>>
>> >>> > >>> > -     ret = env_set("bootargs", cmdline);
>> >>> > >>> > +     ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>> > >>>
>> >>> > >>> I don't think it can be done this way. bootm_boot_start() is
>> used in the
>> >>> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
>> >>> > >>>
>> >>> > >>
>> >>> > >> I got your point. I have 3 ideas come out.
>> >>> > >>
>> >>> > >> (1) The logic purposely empty the env bootargs, either developer
>> don't use
>> >>> > >> env bootargs or those use cases touched bootargs in some early
>> step. And
>> >>> > >> then wanna disregard previous u-boot bootmeth operation, and
>> empty bootargs
>> >>> > >> for that ongoing bootmeth stage...well that's not congruent
>> behavior; I
>> >>> > >> have a gut feeling this is a bug or band-aid anyway, it closed
>> the door for
>> >>> > >
>> >>> > > Simon, is it *intentional* that override the bootargs variable in
>> >>> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
>> >>> > >
>> >>> > > Shouldn't we get the bootargs from the environment, and append
>> cmdline
>> >>> > > to the existing bootargs instead ?
>> >>>
>> >>> The way U-Boot operates today, bootargs defines the entire cmdline,
>> >>> not just part of it. So if you use 'env set bootargs ..' you are
>> >>> changing the entire cmdline passed to Linux, etc.
>> >>
>> >>
>> >> This is runtime change. My test case in fastboot path, it get bootargs
>> wiped and filling in with something from boot img. That might share same
>> procedure with other boot method like chromeos do. Wipe env default each
>> time in fastboot (when fail) might be a secure-wise handling.
>> >>
>> >> And I feel it is a critical entry point logic like a central hub, and
>> looking good to decide uboot behavior here. By kconfig whatever? Again, out
>> of my scope.
>> >>
>> >>>
>> >>> With ChromeOS, after 'bootflow read', the cmdline can be edited using
>> >>> 'bootflow cmdline set' etc., which also changes bootargs. But in
>> >>> general ChromeOS provides the whole Linux cmdline to U-Boot.
>> >>
>> >>
>> >> this is also runtime change. And as a side note, if by doing bootflow
>> select, should env bootargs gets overrided or appended? This is a bigger
>> arch-wise question worth at least a public paper. and I might not go into
>> this depth.
>> >>
>> >>>
>> >>> If you are wanting to append, I suggest writing a new function, or
>> >>
>> >>
>> >> yeah agreed. I do more appreciate and feel it really lower project
>> management/test cost.
>> >>
>> >>> perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
>> >>> other code is useful for Android.
>> >>
>> >>
>> >> A more simpler method is to create bootm_boot_start_ex(kernel_cmd_line,
>> vendor_cmd_line) as
>> >>  I proposed to complement with
>> >>  bootm_boot_start(cmdline) or even add one layer of abstract, called
>> from bootm_boot_start(), make vendor_cmd_line param as null. As can
>> preserve the old logic and behavior.
>> >>
>> >> So build time env preset value, and runtime value override decision can
>> live in this new function, or elsewhere; but is not within my scope.
>> >
>> >
>> > Tr/Dr I wrote a proof of concept code here:
>> >
>> >
>> https://github.com/99degree/u-boot/compare/before_tag...reboot-mode?expand=1
>>
>> This thread is a bit confusing now, but it seems you have written a
>>
>
> I am sorry but I had to prove my patch is necessary and correctnessly and
> also limit the scope of the patch. Thanks for your understand.
>
> new function and that is fine with me, as it won't affect ChromeOS.
>>
>
> There is no new function at all. I am sorry to put more info here again to
> let related code to be clear for every audience here.
>
> As you may notice function bootm_modify_bootargs_env() is a renamed and
> move to bootm.c which is split/extracted from existing function
> android_image_get_kernel()
>
> And bootm_boot_start_ex() is split from
> bootm_boot_start() with your suggested enable param.
>
> I am afraid if you or other reviewer might wonder why move to bootm.c and
> here is the reason.
>
> Supposed with below 2 code paths:
>
> Boot->bootflow->bootmeth_android->bootm
> Boot->fastboot->do_bootm->bootm
>
> I believe android_image_get_kernel is living around within bootmeth_android
> stage so first two patch has taken care of. But fastboot path is not. that
> might be the reason above two path need separate special handling for.
>
> So as to code reuse with fastboot path then those code had to move to bootm
> layer.

Initially, I did not understand that you were wanting to support both
"fastboot boot" and booting via bootmeth_android.

Things make more sense now. Please send a new series so that Simon and I
can review. Thanks again for your patience!

>
> And my understanding each stage is isolated, and depends on env as a kind
> of api to link up each stage. Then I believe the patch is now a clean
> solution. Anyway welcome to jot a mail if there are suggestions, if any
> doubt, please jot a mail too.
>
> Thanks,
> George
>
>
>>
>> >
>> >
>> >>
>> >> So I leave it as a open-end question here. Once you had decided then
>> feel free to let me know and I can do one extra round patch. Thank again
>> for feedback.
>> >>
>> >> George
>> >>
>> >>>
>> >>> >
>> >>> > ... this question ?
>> >>> >
>> >>> > We are wondering why bootm_boot_start() overrides the bootargs
>> variable
>> >>> > instead of appending cmdline to the existing bootargs instead.
>> >>> >
>> >>> > >
>> >>> > >
>> >>> > >> people (potentially abootimg) inject needed boot param between
>> bootmeth
>> >>> > >> stage. Perhaps, either clean up bootargs before bootmeth load
>> stage, or add
>> >>> > >> kconfig to check and enable this logic, a better representation.
>> >>> > >>
>> >>> > >> (2) One more thing, the vendor_boot cmdline is not handle neither
>> in
>> >>> > >> original logic nor in url from you provided. So I can say the url
>> one is
>> >>> > >> not capable for extended with Android boot img version > 2 since
>> >>> > >> vendor_boot cmdline not handled.
>> >>> > >
>> >>> > > What do you mean by the vendor_boot cmdline is not handled?
>> >>> > >
>> >>> > > When parsing the vendor_boot image, we go through
>> >>> > > android_vendor_boot_image_v3_v4_parse_hdr()
>> >>> > >
>> >>> > > That function copies the vendor_boot cmdline with:
>> >>> > >
>> >>> > >       data->kcmdline_extra = hdr->cmdline;
>> >>> > >
>> >>> > > (to the struct andr_image_data).
>> >>> > >
>> >>> > > Later on, in android_image_get_kernel(), this gets copied to the
>> >>> > > bootargs:
>> >>> > >
>> >>> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>> >>> > >               if (*newbootargs) /* If there is something in
>> newbootargs, a space is needed */
>> >>> > >                       strcat(newbootargs, " ");
>> >>> > >               strcat(newbootargs, img_data.kcmdline_extra);
>> >>> > >       }
>> >>> > >
>> >>> > >       env_set("bootargs", newbootargs);
>> >>> > >
>> >>> > >>
>> >>> > >> (3) I don't think it is very much different between your
>> mentioned url with
>> >>> > >> my patch. There is nothing advanced, but just existing code logic
>> reuse and
>> >>> > >> that logic handled vendor_cmdline in other path. I prefer code
>> logic reuse.
>> >>> > >
>> >>> > > The difference is that in the patch I've linked is that we only
>> change
>> >>> > > Android specific boot behaviour. So less risk to impact ChromeOS.
>> >>> > >
>> >>> > >>
>> >>> > >> And I believe above are not the important thing....
>> >>> > >>
>> >>> > >>>
>> >>> > >>> Changing this would potentially break ChromeOS boot behaviour so
>> I'd
>> >>> > >>> prefer to find another solution.
>> >>> > >>>
>> >>> > >>> I know that TI has a downstream patch that changes
>> bootmeth_android.c
>> >>> > >>> instead:
>> >>> > >>>
>> >>> > >>>
>> >>> > >>>
>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
>> >>> > >>
>> >>> > >>
>> >>> > >> ...
>> >>> > >>
>> >>> > >>
>> >>> > >>> Would that work for you?
>> >>> > >>>
>> >>> > >>>
>> >>> > >> Well, the one from url also fine with me.
>> >>> > >>
>> >>> > >> The important thing is here. So as to ease the change with
>> "minimal impact"
>> >>> > >> gets in. I can add one extra check to maintain original behavior
>> in case
>> >>> > >> people have concern of breakage. Code example as below:
>> >>> > >>
>> >>> > >> + If (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
>> >>> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>> > >> + else
>> >>> > >>       ret = env_set("bootargs", cmdline);
>> >>> > >>
>> >>> > >> This logic eliminated the concern, but it also limited those
>> potential use
>> >>> > >> cases for Android boot header version > 2, unless the newly
>> introduced
>> >>> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
>> >>> > >
>> >>> > > I'm not sure about why this is better, as from what I understand we
>> >>> > > handle vendor_boot cmdline properly already.
>> >>> > >
>> >>> > > Can you provide me with a test case or some example commands that
>> show
>> >>> > > that vendor_boot cmdline is not handled properly?
>> >>> > >
>> >>> > >>
>> >>> > >> Or this one with good extendible.
>> >>> > >> + /* Clean up bootargs purposely */
>> >>> > >> + if (IS_ENABLED(SOME_FLAG))
>> >>> > >> +    env_set("bootargs", "");
>> >>> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
>> >>> > >>
>> >>> > >> Either way. I prefer first one and can blindly apply without
>> afford. I
>> >>> > >> leave the idea above to people more concern with it. Please let
>> me know
>> >>> > >> your decision, I can provide one more revision later.
>> >>> > >
>> >>> > > I'm sorry there is so much back and forth on this series. I hope
>> we can
>> >>> > > come to a common agreement and get something merged.
>>
>> Regards,
>> Simon
>>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

Hi Simon and Mattijs,




在 2025年10月9日週四 20:46，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：

> Hi George,
>
> On Wed, Oct 08, 2025 at 00:03, george chan <gchan9527@gmail.com> wrote:
>
> > Hi Simon,
> >
> > Thx for feedback.
> >
> >
> > 在 2025年10月7日週二 04:38，Simon Glass <sjg@chromium.org> 寫道：
> >
> >> Hi George,
> >>
> >> On Sun, 5 Oct 2025 at 09:41, george chan <gchan9527@gmail.com> wrote:
> >> >
> >> >
> >> >
> >> > 在 2025年10月5日週日 00:35，george chan <gchan9527@gmail.com> 寫道：
> >> >>
> >> >> Hi Simon and Mattijs,
> >> >>
> >> >> Thx for feedback. Just in case the discussion drift to other
> direction,
> >> do let me have a chance to put some background info here.
> >> >>
> >> >> there are ways for pre-set env variable and its value. Firstly
> external
> >> env file and loaded by uboot, like, runtime handling; and another type
> is
> >> build time embed env file into uboot body.
> >> >>
> >> >> maybe in chromeos case first stage boot loader will pass along
> bootinfo
> >> struct to uboot and treat as preset defaults.
> >> >>
> >> >> so in either case uboot can have right to choose the preset from (by
> >> disable external env file, secured/trusted boot?), and then either
> follow
> >> the bootinfo struct preset value and/or concat and/or overrite with
> uboot’s
> >> own embedded env value at boot time, and then runtime modify.
> >> >>
> >> >> This behavior maybe enable or disable by kconfig. but in secured boot
> >> context point of view, env file kind of thing, might have potential
> design
> >> wise conflict. And this is not within my scope.
> >> >>
> >> >> in some other use case like simulation of vendor bootloader behavior
> to
> >> have a build time preset default value, this is within my scope. and
> thats
> >> why i decided to put those preset into env file and embeded.
> >> >>
> >> >> let me reply below inline.
> >> >>
> >> >> 在 2025年10月2日週四 00:22，Simon Glass <sjg@chromium.org> 寫道：
> >> >>>
> >> >>> Hi Mattijs, George,
> >> >>>
> >> >>> On Tue, 30 Sept 2025 at 01:29, Mattijs Korpershoek
> >> >>> <mkorpershoek@kernel.org> wrote:
> >> >>> >
> >> >>> > Hi Simon,
> >> >>> >
> >> >>> > Now that you are back, could you please have a look at ...
> >> >>> >
> >> >>> > On Fri, Jul 18, 2025 at 14:03, Mattijs Korpershoek <
> >> mkorpershoek@kernel.org> wrote:
> >> >>> >
> >> >>> > > Hi George,
> >> >>> > >
> >> >>> > > On Wed, Jul 16, 2025 at 19:55, george chan <gchan9527@gmail.com
> >
> >> wrote:
> >> >>> > >
> >> >>> > >> Hi Mattijs,
> >> >>> > >>
> >> >>> > >> Thx for reply.
> >> >>> > >>
> >> >>> > >> 在 2025年7月16日週三 17:05，Mattijs Korpershoek <
> mkorpershoek@kernel.org>
> >> 寫道：
> >> >>> > >>
> >> >>> > >>> Hi George,
> >> >>> > >>>
> >> >>> > >>> Thank you for the patch.
> >> >>> > >>>
> >> >>> > >> ...
> >> >>> > >>
> >> >>> > >>>
> >> >>> > >>> > -     ret = env_set("bootargs", cmdline);
> >> >>> > >>> > +     ret = android_image_modify_bootargs_env(cmdline,
> NULL);
> >> >>> > >>>
> >> >>> > >>> I don't think it can be done this way. bootm_boot_start() is
> >> used in the
> >> >>> > >>> ChromeOS bootmethod as well (boot/bootmeth_cros.c)
> >> >>> > >>>
> >> >>> > >>
> >> >>> > >> I got your point. I have 3 ideas come out.
> >> >>> > >>
> >> >>> > >> (1) The logic purposely empty the env bootargs, either
> developer
> >> don't use
> >> >>> > >> env bootargs or those use cases touched bootargs in some early
> >> step. And
> >> >>> > >> then wanna disregard previous u-boot bootmeth operation, and
> >> empty bootargs
> >> >>> > >> for that ongoing bootmeth stage...well that's not congruent
> >> behavior; I
> >> >>> > >> have a gut feeling this is a bug or band-aid anyway, it closed
> >> the door for
> >> >>> > >
> >> >>> > > Simon, is it *intentional* that override the bootargs variable
> in
> >> >>> > > commit daffb0be2c83 ("bootstd: cros: Add ARM support") ?
> >> >>> > >
> >> >>> > > Shouldn't we get the bootargs from the environment, and append
> >> cmdline
> >> >>> > > to the existing bootargs instead ?
> >> >>>
> >> >>> The way U-Boot operates today, bootargs defines the entire cmdline,
> >> >>> not just part of it. So if you use 'env set bootargs ..' you are
> >> >>> changing the entire cmdline passed to Linux, etc.
> >> >>
> >> >>
> >> >> This is runtime change. My test case in fastboot path, it get
> bootargs
> >> wiped and filling in with something from boot img. That might share same
> >> procedure with other boot method like chromeos do. Wipe env default each
> >> time in fastboot (when fail) might be a secure-wise handling.
> >> >>
> >> >> And I feel it is a critical entry point logic like a central hub, and
> >> looking good to decide uboot behavior here. By kconfig whatever? Again,
> out
> >> of my scope.
> >> >>
> >> >>>
> >> >>> With ChromeOS, after 'bootflow read', the cmdline can be edited
> using
> >> >>> 'bootflow cmdline set' etc., which also changes bootargs. But in
> >> >>> general ChromeOS provides the whole Linux cmdline to U-Boot.
> >> >>
> >> >>
> >> >> this is also runtime change. And as a side note, if by doing bootflow
> >> select, should env bootargs gets overrided or appended? This is a bigger
> >> arch-wise question worth at least a public paper. and I might not go
> into
> >> this depth.
> >> >>
> >> >>>
> >> >>> If you are wanting to append, I suggest writing a new function, or
> >> >>
> >> >>
> >> >> yeah agreed. I do more appreciate and feel it really lower project
> >> management/test cost.
> >> >>
> >> >>> perhaps adding a 'bool append' arg to bootm_boot_start(), if all the
> >> >>> other code is useful for Android.
> >> >>
> >> >>
> >> >> A more simpler method is to create
> bootm_boot_start_ex(kernel_cmd_line,
> >> vendor_cmd_line) as
> >> >>  I proposed to complement with
> >> >>  bootm_boot_start(cmdline) or even add one layer of abstract, called
> >> from bootm_boot_start(), make vendor_cmd_line param as null. As can
> >> preserve the old logic and behavior.
> >> >>
> >> >> So build time env preset value, and runtime value override decision
> can
> >> live in this new function, or elsewhere; but is not within my scope.
> >> >
> >> >
> >> > Tr/Dr I wrote a proof of concept code here:
> >> >
> >> >
> >>
> https://github.com/99degree/u-boot/compare/before_tag...reboot-mode?expand=1
> >>
> >> This thread is a bit confusing now, but it seems you have written a
> >>
> >
> > I am sorry but I had to prove my patch is necessary and correctnessly and
> > also limit the scope of the patch. Thanks for your understand.
> >
> > new function and that is fine with me, as it won't affect ChromeOS.
> >>
> >
> > There is no new function at all. I am sorry to put more info here again
> to
> > let related code to be clear for every audience here.
> >
> > As you may notice function bootm_modify_bootargs_env() is a renamed and
> > move to bootm.c which is split/extracted from existing function
> > android_image_get_kernel()
> >
> > And bootm_boot_start_ex() is split from
> > bootm_boot_start() with your suggested enable param.
> >
> > I am afraid if you or other reviewer might wonder why move to bootm.c and
> > here is the reason.
> >
> > Supposed with below 2 code paths:
> >
> > Boot->bootflow->bootmeth_android->bootm
> > Boot->fastboot->do_bootm->bootm
> >
> > I believe android_image_get_kernel is living around within
> bootmeth_android
> > stage so first two patch has taken care of. But fastboot path is not.
> that
> > might be the reason above two path need separate special handling for.
> >
> > So as to code reuse with fastboot path then those code had to move to
> bootm
> > layer.
>
> Initially, I did not understand that you were wanting to support both
> "fastboot boot" and booting via bootmeth_android.
>
> Things make more sense now. Please send a new series so that Simon and I
> can review. Thanks again for your patience!
>

Thanks for feedback. if you have other detail wanted to share or have in
next round, please let me know. I planed to have next submission mid/end
next week. If you feel comfortable with GitHub code diff, please take a
closer look and I can make modification accordingly.

Regards,
George

>
>
> >
> > And my understanding each stage is isolated, and depends on env as a kind
> > of api to link up each stage. Then I believe the patch is now a clean
> > solution. Anyway welcome to jot a mail if there are suggestions, if any
> > doubt, please jot a mail too.
> >
> > Thanks,
> > George
> >
> >
> >>
> >> >
> >> >
> >> >>
> >> >> So I leave it as a open-end question here. Once you had decided then
> >> feel free to let me know and I can do one extra round patch. Thank again
> >> for feedback.
> >> >>
> >> >> George
> >> >>
> >> >>>
> >> >>> >
> >> >>> > ... this question ?
> >> >>> >
> >> >>> > We are wondering why bootm_boot_start() overrides the bootargs
> >> variable
> >> >>> > instead of appending cmdline to the existing bootargs instead.
> >> >>> >
> >> >>> > >
> >> >>> > >
> >> >>> > >> people (potentially abootimg) inject needed boot param between
> >> bootmeth
> >> >>> > >> stage. Perhaps, either clean up bootargs before bootmeth load
> >> stage, or add
> >> >>> > >> kconfig to check and enable this logic, a better
> representation.
> >> >>> > >>
> >> >>> > >> (2) One more thing, the vendor_boot cmdline is not handle
> neither
> >> in
> >> >>> > >> original logic nor in url from you provided. So I can say the
> url
> >> one is
> >> >>> > >> not capable for extended with Android boot img version > 2
> since
> >> >>> > >> vendor_boot cmdline not handled.
> >> >>> > >
> >> >>> > > What do you mean by the vendor_boot cmdline is not handled?
> >> >>> > >
> >> >>> > > When parsing the vendor_boot image, we go through
> >> >>> > > android_vendor_boot_image_v3_v4_parse_hdr()
> >> >>> > >
> >> >>> > > That function copies the vendor_boot cmdline with:
> >> >>> > >
> >> >>> > >       data->kcmdline_extra = hdr->cmdline;
> >> >>> > >
> >> >>> > > (to the struct andr_image_data).
> >> >>> > >
> >> >>> > > Later on, in android_image_get_kernel(), this gets copied to the
> >> >>> > > bootargs:
> >> >>> > >
> >> >>> > >       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> >> >>> > >               if (*newbootargs) /* If there is something in
> >> newbootargs, a space is needed */
> >> >>> > >                       strcat(newbootargs, " ");
> >> >>> > >               strcat(newbootargs, img_data.kcmdline_extra);
> >> >>> > >       }
> >> >>> > >
> >> >>> > >       env_set("bootargs", newbootargs);
> >> >>> > >
> >> >>> > >>
> >> >>> > >> (3) I don't think it is very much different between your
> >> mentioned url with
> >> >>> > >> my patch. There is nothing advanced, but just existing code
> logic
> >> reuse and
> >> >>> > >> that logic handled vendor_cmdline in other path. I prefer code
> >> logic reuse.
> >> >>> > >
> >> >>> > > The difference is that in the patch I've linked is that we only
> >> change
> >> >>> > > Android specific boot behaviour. So less risk to impact
> ChromeOS.
> >> >>> > >
> >> >>> > >>
> >> >>> > >> And I believe above are not the important thing....
> >> >>> > >>
> >> >>> > >>>
> >> >>> > >>> Changing this would potentially break ChromeOS boot behaviour
> so
> >> I'd
> >> >>> > >>> prefer to find another solution.
> >> >>> > >>>
> >> >>> > >>> I know that TI has a downstream patch that changes
> >> bootmeth_android.c
> >> >>> > >>> instead:
> >> >>> > >>>
> >> >>> > >>>
> >> >>> > >>>
> >>
> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2024.04-next&id=9d802d798ac143e06ffe545606aa657a6c32cc63
> >> >>> > >>
> >> >>> > >>
> >> >>> > >> ...
> >> >>> > >>
> >> >>> > >>
> >> >>> > >>> Would that work for you?
> >> >>> > >>>
> >> >>> > >>>
> >> >>> > >> Well, the one from url also fine with me.
> >> >>> > >>
> >> >>> > >> The important thing is here. So as to ease the change with
> >> "minimal impact"
> >> >>> > >> gets in. I can add one extra check to maintain original
> behavior
> >> in case
> >> >>> > >> people have concern of breakage. Code example as below:
> >> >>> > >>
> >> >>> > >> + If
> (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS))
> >> >>> > >> +   ret = android_image_modify_bootargs_env(cmdline, NULL);
> >> >>> > >> + else
> >> >>> > >>       ret = env_set("bootargs", cmdline);
> >> >>> > >>
> >> >>> > >> This logic eliminated the concern, but it also limited those
> >> potential use
> >> >>> > >> cases for Android boot header version > 2, unless the newly
> >> introduced
> >> >>> > >> CONFIG_ANDROID_BOOT_IMAGE_PREPEND_ENV_BOOTARGS is defined.
> >> >>> > >
> >> >>> > > I'm not sure about why this is better, as from what I
> understand we
> >> >>> > > handle vendor_boot cmdline properly already.
> >> >>> > >
> >> >>> > > Can you provide me with a test case or some example commands
> that
> >> show
> >> >>> > > that vendor_boot cmdline is not handled properly?
> >> >>> > >
> >> >>> > >>
> >> >>> > >> Or this one with good extendible.
> >> >>> > >> + /* Clean up bootargs purposely */
> >> >>> > >> + if (IS_ENABLED(SOME_FLAG))
> >> >>> > >> +    env_set("bootargs", "");
> >> >>> > >> + ret = android_image_modify_bootargs_env(cmdline, NULL);
> >> >>> > >>
> >> >>> > >> Either way. I prefer first one and can blindly apply without
> >> afford. I
> >> >>> > >> leave the idea above to people more concern with it. Please let
> >> me know
> >> >>> > >> your decision, I can provide one more revision later.
> >> >>> > >
> >> >>> > > I'm sorry there is so much back and forth on this series. I hope
> >> we can
> >> >>> > > come to a common agreement and get something merged.
> >>
> >> Regards,
> >> Simon
> >>
>

Re: [PATCH v4 3/6] bootm: Append bootargs value when bootmeth_android provide cmdline

[george chan]

Dear all,

Since as the known situation, I am letting go this patch serise.

l will split those soc related and abootimg related patch into individual
series as followup.

and there is nothing i can do for fastboot case any more.

So thanks again for your time.

Regards,
George

Re: [PATCH v4 4/6] boot: bootmeth_android: Conditionally dependent on abootimg

[Casey Connolly]

Hi George,

This patch doesn't seem to build properly when cmd/abootimg.c is enabled:

../cmd/abootimg.c:26:6: error: redefinition of 'set_abootimg_addr'
   26 | void set_abootimg_addr(ulong addr)
      |      ^~~~~~~~~~~~~~~~~
In file included from ../cmd/abootimg.c:10:
../include/image.h:2044:13: note: previous definition of
'set_abootimg_addr' with type 'void(ulong)' {aka 'void(long unsigned int)'}
 2044 | void __weak set_abootimg_addr(ulong addr) {}
      |             ^~~~~~~~~~~~~~~~~
../cmd/abootimg.c:41:6: error: redefinition of 'set_avendor_bootimg_addr'
   41 | void set_avendor_bootimg_addr(ulong addr)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
../include/image.h:2065:13: note: previous definition of
'set_avendor_bootimg_addr' with type 'void(ulong)' {aka 'void(long
unsigned int)'}
 2065 | void __weak set_avendor_bootimg_addr(ulong addr) {}
      |             ^~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [../scripts/Makefile.build:271: cmd/abootimg.o] Error 1

On 30/06/2025 09:56, George Chan via B4 Relay wrote:
> From: George Chan <gchan9527@gmail.com>
> 
> If target u-boot img do not support androidboot v3 or greater,
> abootimg might not be necessary.
> 
> aarch64-linux-gnu-ld.bfd: boot/bootmeth_android.o: in function `boot_android_normal':
> /home/user/sources/u-boot-next/boot/bootmeth_android.c:541:(.text.boot_android_normal+0xd0): undefined reference to `set_avendor_bootimg_addr'
> aarch64-linux-gnu-ld.bfd: /home/user/sources/u-boot-next/boot/bootmeth_android.c:543:(.text.boot_android_normal+0xd8): undefined reference to `set_abootimg_addr'
> Segmentation fault (core dumped)
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> Acked-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> Signed-off-by: George Chan <gchan9527@gmail.com>
> ---
>  boot/bootmeth_android.c | 2 +-
>  include/image.h         | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
> index 8c2bde10e17..d7740b86d67 100644
> --- a/boot/bootmeth_android.c
> +++ b/boot/bootmeth_android.c
> @@ -534,7 +534,7 @@ static int boot_android_normal(struct bootflow *bflow)
>  	if (ret < 0)
>  		return log_msg_ret("read boot", ret);
>  
> -	if (priv->header_version >= 3) {
> +	if (IS_ENABLED(CONFIG_CMD_ABOOTIMG) && priv->header_version >= 3) {
>  		ret = read_slotted_partition(desc, "vendor_boot", priv->slot,
>  					     priv->vendor_boot_img_size, vloadaddr);
>  		if (ret < 0)
> diff --git a/include/image.h b/include/image.h
> index 11d33ceeb39..82a861ba8bf 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -2050,6 +2050,7 @@ ulong get_abootimg_addr(void);
>   * Return: no returned results
>   */
>  void set_abootimg_addr(ulong addr);
> +void __weak set_abootimg_addr(ulong addr) {}
>  
>  /**
>   * get_ainit_bootimg_addr() - Get Android init boot image address
> @@ -2071,6 +2072,7 @@ ulong get_avendor_bootimg_addr(void);
>   * Return: no returned results
>   */
>  void set_avendor_bootimg_addr(ulong addr);
> +void __weak set_avendor_bootimg_addr(ulong addr) {}
>  
>  /**
>   * board_fit_config_name_match() - Check for a matching board name
> 

-- 
// Casey (she/her)

Re: [PATCH v4 4/6] boot: bootmeth_android: Conditionally dependent on abootimg

[george chan]

Hi Casey,

Thx for report. I am busy with other stuff recently. Sorry for that. Would
you or somebody willing to follow that?

The first failing report is at:

https://lists.denx.de/pipermail/u-boot/2025-November/602397.html

Thanks,
George