From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Fri, 11 Apr 2014 23:44:37 +0200
Subject: [U-Boot] [PATCH v3 3/5] image: add support for Android's boot
	image format
In-Reply-To: <1397157488-8695-4-git-send-email-robherring2@gmail.com>
References: <1397157488-8695-1-git-send-email-robherring2@gmail.com>
	<1397157488-8695-4-git-send-email-robherring2@gmail.com>
Message-ID: <201404112344.37141.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Thursday, April 10, 2014 at 09:18:05 PM, Rob Herring wrote:
> From: Sebastian Siewior <bigeasy@linutronix.de>
> 
> This patch adds support for the Android boot-image format. The header
> file is from the Android project and got slightly alterted so the struct +
> its defines are not generic but have something like a namespace. The
> header file is from bootloader/legacy/include/boot/bootimg.h. The header
> parsing has been written from scratch and I looked at
> bootloader/legacy/usbloader/usbloader.c for some details.
> The image contains the physical address (load address) of the kernel and
> ramdisk. This address is considered only for the kernel image.
> The "second image" is currently ignored. I haven't found anything that
> is creating this.

Can you please elaborate on those last two sentences ?

> v3 (Rob Herring):
> This is based on http://patchwork.ozlabs.org/patch/126797/ with the
> following changes:
> - Rebased to current mainline
> - Moved android image handling to separate functions in
>   common/image-android.c
> - s/u8/char/ in header to fix string function warnings
> - Use SPDX identifiers for licenses
> - Cleaned-up file source information:
>   android_image.h is from file include/boot/bootimg.h in repository:
>   https://android.googlesource.com/platform/bootable/bootloader/legacy
>   The git commit hash is 4205b865141ff2e255fe1d3bd16de18e217ef06a
>   usbloader.c would be from the same commit, but it does not appear
>   to have been used for any actual code.

[...]

> @@ -293,7 +306,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag,
> int argc, return 1;
>  		}
>  #endif
> -	} else {
> +	} else if (images.ep == ~0UL) {


I don't find this really portable. While it's unlikely the kernel will have the 
EP here, don't we have a better solution than using special value?

[...]

> @@ -949,7 +951,15 @@ int boot_get_ramdisk(int argc, char * const argv[],
> bootm_headers_t *images, (ulong)images->legacy_hdr_os);
> 
>  		image_multi_getimg(images->legacy_hdr_os, 1, &rd_data, &rd_len);
> -	} else {
> +	}
> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
> +	else if ((genimg_get_format(images) == IMAGE_FORMAT_ANDROID) &&
> +		 (!andriod_image_get_ramdisk((void *)images->os.start,

andriod_image_get_ramdisk() ? There is a typo in the function name, did you 
actually ever even compile this patchset please ?
[...]