Hi Guillaume,

Thank you for the patch.

On Mon, Jan 12, 2026 at 11:55, "Guillaume La Roque (TI.com)" 
<[email protected]> wrote:

> For android vendor boot image version 4 bootconfig is mandatory.[1]
>
> In the android_image_get_ramdisk function, after copying both vendor and
> boot ramdisks, we extract all androidboot.* entries from the kernel
> command line. These entries are added to the bootconfig section.
> We then update the sizes of the ramdisk and bootconfig.
> Finally, all androidboot.* entries are removed from the kernel command
> line.
>
> [1] 
> https://source.android.com/docs/core/architecture/partitions/vendor-boot-partitions#bootloader-support
>
> Reviewed-by: Simon Glass <[email protected]>
> Signed-off-by: Guillaume La Roque (TI.com) <[email protected]>
> ---
>  boot/image-android.c | 174 
> +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 163 insertions(+), 11 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index b9e0b3f68b0..c4a0cb138eb 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -505,6 +505,166 @@ ulong android_image_get_kcomp(const void *hdr,
>               return image_decomp_type(p, sizeof(u32));
>  }
>  
> +/**
> + * android_boot_append_bootconfig() - Append bootconfig parameters to ramdisk
> + * @img_data: Pointer to Android image data
> + * @params: Pointer to boot config parameters to append
> + * @params_len: Length of boot config parameters
> + * @ramdisk_dest: Destination address for the merged ramdisk
> + *
> + * This function copies the vendor ramdisk, boot ramdisk, and bootconfig to
> + * the destination. It then appends the provided bootconfig parameters.
> + *
> + * Return: Bytes added to the bootconfig on success, negative on error.
> + */
> +static long android_boot_append_bootconfig(const struct andr_image_data 
> *img_data,
> +                                        char *params, long params_len,
> +                                        void *ramdisk_dest)
> +{
> +     void *vendor_ramdisk_src;
> +     void *boot_ramdisk_src;
> +     void *bootconfig_src;
> +     long bytes_added = 0;
> +
> +     /* Map sources */
> +     vendor_ramdisk_src = map_sysmem(img_data->vendor_ramdisk_ptr,
> +                                     img_data->vendor_ramdisk_size);
> +     boot_ramdisk_src = map_sysmem(img_data->ramdisk_ptr,
> +                                   img_data->boot_ramdisk_size);
> +
> +     /* Copy Vendor Ramdisk */
> +     memcpy(ramdisk_dest, vendor_ramdisk_src, img_data->vendor_ramdisk_size);
> +
> +     /* Copy Boot Ramdisk */
> +     memcpy((char *)ramdisk_dest + img_data->vendor_ramdisk_size,
> +            boot_ramdisk_src, img_data->boot_ramdisk_size);
> +
> +     /* Copy Bootconfig and Append Params */
> +     if (img_data->bootconfig_size) {
> +             bootconfig_src = map_sysmem(img_data->bootconfig_addr,
> +                                         img_data->bootconfig_size);
> +             memcpy((char *)ramdisk_dest + img_data->vendor_ramdisk_size +
> +                    img_data->boot_ramdisk_size,
> +                    bootconfig_src, img_data->bootconfig_size);
> +             unmap_sysmem(bootconfig_src);
> +
> +             if (params && params_len > 1) {
> +                     void *bootconfig_ptr = (char *)ramdisk_dest +
> +                                            img_data->vendor_ramdisk_size +
> +                                            img_data->boot_ramdisk_size;
> +                     bytes_added = add_bootconfig_parameters(params, 
> params_len,
> +                                                             
> (ulong)bootconfig_ptr,
> +                                                             
> img_data->bootconfig_size);
> +             }
> +     }
> +
> +     unmap_sysmem(boot_ramdisk_src);
> +     unmap_sysmem(vendor_ramdisk_src);
> +
> +     if (bytes_added < 0)
> +             return bytes_added;
> +
> +     return bytes_added;
> +}
> +
> +/**
> + * android_image_set_bootconfig() - Extract androidboot.* args and append to 
> bootconfig
> + * @hdr: Pointer to boot image header
> + * @vendor_boot_img: Pointer to vendor boot image header
> + * @ramdisk_addr: Destination address for the merged ramdisk
> + *
> + * Return: Size of the bootconfig section (including new params) on success, 
> negative on error.
> + */
> +static long android_image_set_bootconfig(const void *hdr,
> +                                      const void *vendor_boot_img,
> +                                      ulong ramdisk_addr)
> +{
> +     const char *bootargs = env_get("bootargs");
> +     char *params = NULL;
> +     char *new_bootargs = NULL;
> +     long params_len = 0; /* Renamed from androidboot_params_len */

What is this comment for? Please drop it. It makes no sense to have a
comment about a variable name that was in a previous patch version.

> +     struct andr_image_data img_data;
> +     long ret;
> +     size_t len;
> +     const char *src;
> +     char *bc_dst;
> +     char *args_dst;
> +     ulong total_size;
> +     void *ramdisk_dest;
> +
> +     if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
> +             return -EINVAL; /* Use errno */

same? Use errno? What does it mean?

If you agree that these comments are not useful, I can fix up while applying.

> +
> +     /* Extract androidboot.* parameters from bootargs */

Reply via email to