
On Saturday, April 12, 2014 at 11:54:10 PM, Rob Herring wrote:
On Fri, Apr 11, 2014 at 4:44 PM, Marek Vasut marex@denx.de wrote:
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 ?
The android header has 3 images: kernel, ramdisk and "second". I think this is for secondary bootloader, but I'll have to investigate.
Viva hardcoded b/s :'-(
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?
This is to address Wolfgang's prior comments about moving setting of images.ep into the switch statement above. Do you like the previous version better?
I think I might be missing that part of the discussion. My point is just that you might rather have a separate variable to detect error instead of having a special value like this.
Does my rant make sense to you please ?
[...]
@@ -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 ? [...]
Of course it compiles as the same typo is everywhere for this function... :) I've compiled and run this (although I have not tested with a ramdisk).
Okay, thanks for clearing this up (and fixing this in v4)!