Patchwork [U-Boot,1/2] image: add support for Android's boot image format

login
register
mail settings
Submitter Sebastian Siewior
Date Nov. 21, 2011, 2:09 p.m.
Message ID <1321884575-2993-2-git-send-email-bigeasy@linutronix.de>
Download mbox | patch
Permalink /patch/126797/
State Changes Requested
Delegated to: Wolfgang Denk
Headers show

Comments

Sebastian Siewior - Nov. 21, 2011, 2:09 p.m.
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.

Cc: Wolfgang Denk <wd@denx.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 common/cmd_bootm.c      |   87 +++++++++++++++++++++++++++++++++++++++++++++-
 common/image.c          |   17 +++++++++
 include/android_image.h |   89 +++++++++++++++++++++++++++++++++++++++++++++++
 include/image.h         |    4 ++
 4 files changed, 196 insertions(+), 1 deletions(-)
 create mode 100644 include/android_image.h
Wolfgang Denk - Nov. 21, 2011, 8:19 p.m.
Dear Sebastian Andrzej Siewior,

In message <1321884575-2993-2-git-send-email-bigeasy@linutronix.de> you wrote:
> 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.

Please provide _exact_ reference where this code has been copied from,
see bullet 4 etc. at
http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign

Also please provide exact information about the applicable licenses
for this copied code.

...
>  	/* get image parameters */
> -	switch (genimg_get_format(os_hdr)) {
> +	img_type = genimg_get_format(os_hdr);
> +	switch (img_type) {
...
> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
> +	} else if (img_type == IMAGE_FORMAT_ANDROID) {
> +		images.ep = images.os.load;
> +#endif

Why don't you handle the Andoid image case inside the switch() ?

> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
> +static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
> +static int android_image_get_kernel(struct andr_img_hdr *hdr, int verify)
> +{
> +	/*
> +	 * Not all Android tools use the id field for signing the image with
> +	 * sha1 (or anything) so we don't check it. It is not obvious that the
> +	 * string is null terminated so we take care of this.
> +	 */
> +	strncpy(andr_tmp_str, hdr->name, ANDR_BOOT_NAME_SIZE);
> +	andr_tmp_str[ANDR_BOOT_NAME_SIZE] = '\0';
> +	if (strlen(andr_tmp_str))
> +		printf("Android's image name: %s\n", andr_tmp_str);
> +
> +	printf("Kernel load addr 0x%08x size %u KiB\n",
> +			hdr->kernel_addr, DIV_ROUND_UP(hdr->kernel_size, 1024));
> +	strncpy(andr_tmp_str, hdr->cmdline, ANDR_BOOT_ARGS_SIZE);
> +	andr_tmp_str[ANDR_BOOT_ARGS_SIZE] = '\0';
> +	if (strlen(andr_tmp_str)) {
> +		printf("Kernel command line: %s\n", andr_tmp_str);
> +		setenv("bootargs", andr_tmp_str);
> +	}
> +	if (hdr->ramdisk_size)
> +		printf("RAM disk load addr 0x%08x size %u KiB\n",
> +				hdr->ramdisk_addr,
> +				DIV_ROUND_UP(hdr->ramdisk_size, 1024));
> +	return 0;
> +}
> +#endif

This and similar Android image related code shoudl eventually go into
a separate file, exposing only a few functions.


> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
> +	if (format == IMAGE_FORMAT_INVALID) {
> +		const struct andr_img_hdr *ahdr = img_addr;
>  
> +		if (!memcmp(ANDR_BOOT_MAGIC, ahdr->magic, ANDR_BOOT_MAGIC_SIZE))
> +			format = IMAGE_FORMAT_ANDROID;
> +	}
> +#endif

This is all we have for testing for a valid image?

> index 0000000..b231b66
> --- /dev/null
> +++ b/include/android_image.h
> @@ -0,0 +1,89 @@
> +/*
> + * This is from the Android Project,
> + * bootloader/legacy/include/boot/bootimg.h
> + *
> + * Copyright (C) 2008 The Android Open Source Project
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *  * Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + *  * Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in
> + *    the documentation and/or other materials provided with the
> + *    distribution.

Sorry, but this is not GPL compatible.



Best regards,

Wolfgang Denk
Sebastian Siewior - Nov. 22, 2011, 12:30 p.m.
* Wolfgang Denk | 2011-11-21 21:19:07 [+0100]:

>Dear Sebastian Andrzej Siewior,
Hi Wolfgang,

>Please provide _exact_ reference where this code has been copied from,
>see bullet 4 etc. at
>http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign
>
>Also please provide exact information about the applicable licenses
>for this copied code.

I added a detailed description to the repository and the commit head
where I took it from.

>...
>>  	/* get image parameters */
>> -	switch (genimg_get_format(os_hdr)) {
>> +	img_type = genimg_get_format(os_hdr);
>> +	switch (img_type) {
>...
>> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
>> +	} else if (img_type == IMAGE_FORMAT_ANDROID) {
>> +		images.ep = images.os.load;
>> +#endif
>
>Why don't you handle the Andoid image case inside the switch() ?

Because the code flow looks like:
|         if (images.legacy_hdr_valid) {
|                 images.ep = image_get_ep(&images.legacy_hdr_os_copy);
| #if defined(CONFIG_FIT)
|         } else if (images.fit_uname_os) {
...
|                 }
| #endif
| #ifdef CONFIG_ANDROID_BOOT_IMAGE
...
| #endif
|         } else {
|                 puts("Could not find kernel entry point!\n");
|                 return 1;
|         }
|

So if I don't add an extra android block here, I end up in this else
part complaining about the entrypoint.
I guess that this part could be merged into the previous switch
statement if you want me to.

>> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
>> +static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
>> +static int android_image_get_kernel(struct andr_img_hdr *hdr, int verify)
>> +{
>> +	/*
>> +	 * Not all Android tools use the id field for signing the image with
>> +	 * sha1 (or anything) so we don't check it. It is not obvious that the
>> +	 * string is null terminated so we take care of this.
>> +	 */
>> +	strncpy(andr_tmp_str, hdr->name, ANDR_BOOT_NAME_SIZE);
>> +	andr_tmp_str[ANDR_BOOT_NAME_SIZE] = '\0';
>> +	if (strlen(andr_tmp_str))
>> +		printf("Android's image name: %s\n", andr_tmp_str);
>> +
>> +	printf("Kernel load addr 0x%08x size %u KiB\n",
>> +			hdr->kernel_addr, DIV_ROUND_UP(hdr->kernel_size, 1024));
>> +	strncpy(andr_tmp_str, hdr->cmdline, ANDR_BOOT_ARGS_SIZE);
>> +	andr_tmp_str[ANDR_BOOT_ARGS_SIZE] = '\0';
>> +	if (strlen(andr_tmp_str)) {
>> +		printf("Kernel command line: %s\n", andr_tmp_str);
>> +		setenv("bootargs", andr_tmp_str);
>> +	}
>> +	if (hdr->ramdisk_size)
>> +		printf("RAM disk load addr 0x%08x size %u KiB\n",
>> +				hdr->ramdisk_addr,
>> +				DIV_ROUND_UP(hdr->ramdisk_size, 1024));
>> +	return 0;
>> +}
>> +#endif
>
>This and similar Android image related code shoudl eventually go into
>a separate file, exposing only a few functions.
Okay.

>> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
>> +	if (format == IMAGE_FORMAT_INVALID) {
>> +		const struct andr_img_hdr *ahdr = img_addr;
>>  
>> +		if (!memcmp(ANDR_BOOT_MAGIC, ahdr->magic, ANDR_BOOT_MAGIC_SIZE))
>> +			format = IMAGE_FORMAT_ANDROID;
>> +	}
>> +#endif
>
>This is all we have for testing for a valid image?

More or less, yes. The fastboot client [0] does nothing else if you
provide it a kernel (and it creates the ANDROID image out of it).
The comment field is used by mkbootimg/mkbootimg.c tool to stash a sha1
checksum. The format how to create the sha1 checksum (i.e. pad, don't
pad, include header or not, ...) isn't documented (atleast I did not
find anything) so the only source of documentation is
the source code wich is under Apache2 license which is compatible with
GPLv3 but not with v2 so I can't look at it.

[0] http://android-dls.com/wiki/index.php?title=Fastboot

>> index 0000000..b231b66
>> --- /dev/null
>> +++ b/include/android_image.h
>> @@ -0,0 +1,89 @@
>> +/*
>> + * This is from the Android Project,
>> + * bootloader/legacy/include/boot/bootimg.h
>> + *
>> + * Copyright (C) 2008 The Android Open Source Project
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *  * Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + *  * Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer in
>> + *    the documentation and/or other materials provided with the
>> + *    distribution.
>
>Sorry, but this is not GPL compatible.

Ehm. Is this the All rights reserved issue? If so then I assumed that I
cleared up things in
  http://lists.denx.de/pipermail/u-boot/2011-September/101793.html

>
>Best regards,
>
>Wolfgang Denk

Sebastian
Wolfgang Denk - Nov. 22, 2011, 7:04 p.m.
Dear Sebastian Andrzej Siewior,

In message <20111122123007.GA5755@linutronix.de> you wrote:
> 
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + *  * Redistributions of source code must retain the above copyright
> >> + *    notice, this list of conditions and the following disclaimer.
> >> + *  * Redistributions in binary form must reproduce the above copyright
> >> + *    notice, this list of conditions and the following disclaimer in
> >> + *    the documentation and/or other materials provided with the
> >> + *    distribution.
> >
> >Sorry, but this is not GPL compatible.
> 
> Ehm. Is this the All rights reserved issue? If so then I assumed that I
> cleared up things in

No, it's the "Redistributions in binary form must reproduce..."
clause.

Best regards,

Wolfgang Denk
Sebastian Siewior - Nov. 23, 2011, 10:03 a.m.
* Wolfgang Denk | 2011-11-22 20:04:47 [+0100]:

>Dear Sebastian Andrzej Siewior,
>
>In message <20111122123007.GA5755@linutronix.de> you wrote:
>> 
>> >> + * Redistribution and use in source and binary forms, with or without
>> >> + * modification, are permitted provided that the following conditions
>> >> + * are met:
>> >> + *  * Redistributions of source code must retain the above copyright
>> >> + *    notice, this list of conditions and the following disclaimer.
>> >> + *  * Redistributions in binary form must reproduce the above copyright
>> >> + *    notice, this list of conditions and the following disclaimer in
>> >> + *    the documentation and/or other materials provided with the
>> >> + *    distribution.
>> >
>> >Sorry, but this is not GPL compatible.
>> 
>> Ehm. Is this the All rights reserved issue? If so then I assumed that I
>> cleared up things in
>
>No, it's the "Redistributions in binary form must reproduce..."
>clause.

How so? If you distribute it as source nothing changes. I don't see much
difference in binary form either: section 1 of the GPL says 

|.. keep intact all the notices that refer to this License and to the
|absence of any warranty; and give any other recipients of the Program a
|copy of this License along with the Program.

and this is no different. It does not mention whether the software has
to be passed in source or binary form. The BSD part does not push any
restrictions on the GPL, it "wants" the same thing. Section 6 of the GPL
says that by redistributing the receiptient should receive a copy of
this license. The section you mentioed is no different. If you
distribute GPL in binary code you have let the receiptient know, that he
is using GPL code. A note in the documentation is enough as far as I
know [if remeber correctly Harald went after a few companies which were
using Linux and were not letting the customers know about it].

If you look at the fresh released Quake3 source [0] you see that there
is a readme file which points out that it is GPL code and enumerates
various other licenses.

So right now, I don't see why those two should not be compatible. Plus
the FSF claims that they are [1].

[0] https://github.com/TTimo/doom3.gpl
[1] http://www.gnu.org/licenses/license-list.html#FreeBSD

>Best regards,
>
>Wolfgang Denk

Sebastian
Aneesh V - Jan. 17, 2012, 9:16 a.m.
Dear Wolfgang,

On Wednesday 23 November 2011 03:33 PM, Sebastian Andrzej Siewior wrote:
> * Wolfgang Denk | 2011-11-22 20:04:47 [+0100]:
>
>> Dear Sebastian Andrzej Siewior,
>>
>> In message<20111122123007.GA5755@linutronix.de>  you wrote:
>>>
>>>>> + * Redistribution and use in source and binary forms, with or without
>>>>> + * modification, are permitted provided that the following conditions
>>>>> + * are met:
>>>>> + *  * Redistributions of source code must retain the above copyright
>>>>> + *    notice, this list of conditions and the following disclaimer.
>>>>> + *  * Redistributions in binary form must reproduce the above copyright
>>>>> + *    notice, this list of conditions and the following disclaimer in
>>>>> + *    the documentation and/or other materials provided with the
>>>>> + *    distribution.
>>>>
>>>> Sorry, but this is not GPL compatible.
>>>
>>> Ehm. Is this the All rights reserved issue? If so then I assumed that I
>>> cleared up things in
>>
>> No, it's the "Redistributions in binary form must reproduce..."
>> clause.
>
> How so? If you distribute it as source nothing changes. I don't see much
> difference in binary form either: section 1 of the GPL says
>
> |.. keep intact all the notices that refer to this License and to the
> |absence of any warranty; and give any other recipients of the Program a
> |copy of this License along with the Program.
>
> and this is no different. It does not mention whether the software has
> to be passed in source or binary form. The BSD part does not push any
> restrictions on the GPL, it "wants" the same thing. Section 6 of the GPL
> says that by redistributing the receiptient should receive a copy of
> this license. The section you mentioed is no different. If you
> distribute GPL in binary code you have let the receiptient know, that he
> is using GPL code. A note in the documentation is enough as far as I
> know [if remeber correctly Harald went after a few companies which were
> using Linux and were not letting the customers know about it].
>
> If you look at the fresh released Quake3 source [0] you see that there
> is a readme file which points out that it is GPL code and enumerates
> various other licenses.
>
> So right now, I don't see why those two should not be compatible. Plus
> the FSF claims that they are [1].
>
> [0] https://github.com/TTimo/doom3.gpl
> [1] http://www.gnu.org/licenses/license-list.html#FreeBSD

What is your final call on this? The above arguments sound convincing
to me, but I have to admit that I am no legal expert. Either way, it
will be great to have a closure on this. Lack of fastboot support was
the greatest impediment to adoption of mainline U-Boot in our previous
platforms. It will be really unfortunate if the same happens to OMAP5
that has just arrived.

best regards,
Aneesh
Aneesh V - Feb. 2, 2012, 9:28 a.m.
Dear Wolfgang,

On Tuesday 17 January 2012 02:46 PM, Aneesh V wrote:
> Dear Wolfgang,
>
> On Wednesday 23 November 2011 03:33 PM, Sebastian Andrzej Siewior wrote:
>> * Wolfgang Denk | 2011-11-22 20:04:47 [+0100]:
>>
>>> Dear Sebastian Andrzej Siewior,
>>>
>>> In message<20111122123007.GA5755@linutronix.de> you wrote:
>>>>
>>>>>> + * Redistribution and use in source and binary forms, with or
>>>>>> without
>>>>>> + * modification, are permitted provided that the following
>>>>>> conditions
>>>>>> + * are met:
>>>>>> + * * Redistributions of source code must retain the above copyright
>>>>>> + * notice, this list of conditions and the following disclaimer.
>>>>>> + * * Redistributions in binary form must reproduce the above
>>>>>> copyright
>>>>>> + * notice, this list of conditions and the following disclaimer in
>>>>>> + * the documentation and/or other materials provided with the
>>>>>> + * distribution.
>>>>>
>>>>> Sorry, but this is not GPL compatible.
>>>>
>>>> Ehm. Is this the All rights reserved issue? If so then I assumed that I
>>>> cleared up things in
>>>
>>> No, it's the "Redistributions in binary form must reproduce..."
>>> clause.
>>
>> How so? If you distribute it as source nothing changes. I don't see much
>> difference in binary form either: section 1 of the GPL says
>>
>> |.. keep intact all the notices that refer to this License and to the
>> |absence of any warranty; and give any other recipients of the Program a
>> |copy of this License along with the Program.
>>
>> and this is no different. It does not mention whether the software has
>> to be passed in source or binary form. The BSD part does not push any
>> restrictions on the GPL, it "wants" the same thing. Section 6 of the GPL
>> says that by redistributing the receiptient should receive a copy of
>> this license. The section you mentioed is no different. If you
>> distribute GPL in binary code you have let the receiptient know, that he
>> is using GPL code. A note in the documentation is enough as far as I
>> know [if remeber correctly Harald went after a few companies which were
>> using Linux and were not letting the customers know about it].
>>
>> If you look at the fresh released Quake3 source [0] you see that there
>> is a readme file which points out that it is GPL code and enumerates
>> various other licenses.
>>
>> So right now, I don't see why those two should not be compatible. Plus
>> the FSF claims that they are [1].
>>
>> [0] https://github.com/TTimo/doom3.gpl
>> [1] http://www.gnu.org/licenses/license-list.html#FreeBSD
>
> What is your final call on this? The above arguments sound convincing
> to me, but I have to admit that I am no legal expert. Either way, it
> will be great to have a closure on this. Lack of fastboot support was
> the greatest impediment to adoption of mainline U-Boot in our previous
> platforms. It will be really unfortunate if the same happens to OMAP5
> that has just arrived.

Ping.

br,
Aneesh
Tom Rini - Feb. 8, 2012, 5:36 p.m.
On Thu, Feb 2, 2012 at 2:28 AM, Aneesh V <aneesh@ti.com> wrote:
> Dear Wolfgang,
>
>
> On Tuesday 17 January 2012 02:46 PM, Aneesh V wrote:
>>
>> Dear Wolfgang,
>>
>> On Wednesday 23 November 2011 03:33 PM, Sebastian Andrzej Siewior wrote:
>>>
>>> * Wolfgang Denk | 2011-11-22 20:04:47 [+0100]:
>>>
>>>> Dear Sebastian Andrzej Siewior,
>>>>
>>>> In message<20111122123007.GA5755@linutronix.de> you wrote:
>>>>>
>>>>>
>>>>>>> + * Redistribution and use in source and binary forms, with or
>>>>>>> without
>>>>>>> + * modification, are permitted provided that the following
>>>>>>> conditions
>>>>>>> + * are met:
>>>>>>> + * * Redistributions of source code must retain the above copyright
>>>>>>> + * notice, this list of conditions and the following disclaimer.
>>>>>>> + * * Redistributions in binary form must reproduce the above
>>>>>>> copyright
>>>>>>> + * notice, this list of conditions and the following disclaimer in
>>>>>>> + * the documentation and/or other materials provided with the
>>>>>>> + * distribution.
>>>>>>
>>>>>>
>>>>>> Sorry, but this is not GPL compatible.
>>>>>
>>>>>
>>>>> Ehm. Is this the All rights reserved issue? If so then I assumed that I
>>>>> cleared up things in
>>>>
>>>>
>>>> No, it's the "Redistributions in binary form must reproduce..."
>>>> clause.
>>>
>>>
>>> How so? If you distribute it as source nothing changes. I don't see much
>>> difference in binary form either: section 1 of the GPL says
>>>
>>> |.. keep intact all the notices that refer to this License and to the
>>> |absence of any warranty; and give any other recipients of the Program a
>>> |copy of this License along with the Program.
>>>
>>> and this is no different. It does not mention whether the software has
>>> to be passed in source or binary form. The BSD part does not push any
>>> restrictions on the GPL, it "wants" the same thing. Section 6 of the GPL
>>> says that by redistributing the receiptient should receive a copy of
>>> this license. The section you mentioed is no different. If you
>>> distribute GPL in binary code you have let the receiptient know, that he
>>> is using GPL code. A note in the documentation is enough as far as I
>>> know [if remeber correctly Harald went after a few companies which were
>>> using Linux and were not letting the customers know about it].
>>>
>>> If you look at the fresh released Quake3 source [0] you see that there
>>> is a readme file which points out that it is GPL code and enumerates
>>> various other licenses.
>>>
>>> So right now, I don't see why those two should not be compatible. Plus
>>> the FSF claims that they are [1].
>>>
>>> [0] https://github.com/TTimo/doom3.gpl
>>> [1] http://www.gnu.org/licenses/license-list.html#FreeBSD
>>
>>
>> What is your final call on this? The above arguments sound convincing
>> to me, but I have to admit that I am no legal expert. Either way, it
>> will be great to have a closure on this. Lack of fastboot support was
>> the greatest impediment to adoption of mainline U-Boot in our previous
>> platforms. It will be really unfortunate if the same happens to OMAP5
>> that has just arrived.
>
>
> Ping.

Part of the feedback (see http://patchwork.ozlabs.org/patch/126797/)
was not addressed, namely the complete reference (including hash)
where the code came from.
Wolfgang Denk - March 17, 2012, 10:04 p.m.
Dear Sebastian,

In message <20111123100316.GA6654@linutronix.de> you wrote:
> 
> >No, it's the "Redistributions in binary form must reproduce..."
> >clause.
...
> So right now, I don't see why those two should not be compatible. Plus
> the FSF claims that they are [1].

Sorry, my fault.  I confused the "reproduce the above copyright
notice" with the "must display the following acknowledgement" phrase
of the 4-clause original BSD license.

So please ignore this part of my remarks.

However,  I still ask you to change the commit message to include
_exact_ information where the code was derived from, as explained in 
bullet 4 etc. at
http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign

Mind the part "provide terse but precise information which exact
version or even commit ID was used."

Thanks.

Best regards,

Wolfgang Denk
Wolfgang Denk - March 17, 2012, 10:05 p.m.
Dear Aneesh V,

In message <4F153C83.20703@ti.com> you wrote:
> 
> What is your final call on this? The above arguments sound convincing
> to me, but I have to admit that I am no legal expert. Either way, it
> will be great to have a closure on this. Lack of fastboot support was
> the greatest impediment to adoption of mainline U-Boot in our previous
> platforms. It will be really unfortunate if the same happens to OMAP5
> that has just arrived.

The license issue was a mistake on my side.

However, there is still cleanup needed for the commit message.

Best regards,

Wolfgang Denk
Aneesh V - April 12, 2012, 2:54 p.m.
On 03/17/2012 03:05 PM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<4F153C83.20703@ti.com>  you wrote:
>>
>> What is your final call on this? The above arguments sound convincing
>> to me, but I have to admit that I am no legal expert. Either way, it
>> will be great to have a closure on this. Lack of fastboot support was
>> the greatest impediment to adoption of mainline U-Boot in our previous
>> platforms. It will be really unfortunate if the same happens to OMAP5
>> that has just arrived.
> > The license issue was a mistake on my side.
>
> However, there is still cleanup needed for the commit message.

Thanks. I will see if somebody can take it forward.

br,
Aneesh

Patch

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d301332..3a4efe4 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -36,6 +36,7 @@ 
 #include <lmb.h>
 #include <linux/ctype.h>
 #include <asm/byteorder.h>
+#include <android_image.h>
 
 #if defined(CONFIG_CMD_USB)
 #include <usb.h>
@@ -188,10 +189,33 @@  static void bootm_start_lmb(void)
 #endif
 }
 
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+static u32 android_img_get_end(struct andr_img_hdr *hdr)
+{
+	u32 size = 0;
+	/*
+	 * The header takes a full page, the remaining components are aligned
+	 * on page boundary
+	 */
+	size += hdr->page_size;
+	size += ALIGN(hdr->kernel_size, hdr->page_size);
+	size += ALIGN(hdr->ramdisk_size, hdr->page_size);
+	size += ALIGN(hdr->second_size, hdr->page_size);
+
+	return size;
+}
+
+static u32 android_img_get_kload(struct andr_img_hdr *hdr)
+{
+	return hdr->kernel_addr;
+}
+#endif
+
 static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	void		*os_hdr;
 	int		ret;
+	int		img_type;
 
 	memset((void *)&images, 0, sizeof(images));
 	images.verify = getenv_yesno("verify");
@@ -207,7 +231,8 @@  static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	}
 
 	/* get image parameters */
-	switch (genimg_get_format(os_hdr)) {
+	img_type = genimg_get_format(os_hdr);
+	switch (img_type) {
 	case IMAGE_FORMAT_LEGACY:
 		images.os.type = image_get_type(os_hdr);
 		images.os.comp = image_get_comp(os_hdr);
@@ -249,6 +274,16 @@  static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 		}
 		break;
 #endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	case IMAGE_FORMAT_ANDROID:
+		images.os.type = IH_TYPE_KERNEL;
+		images.os.comp = IH_COMP_NONE;
+		images.os.os = IH_OS_LINUX;
+
+		images.os.end = android_img_get_end(os_hdr);
+		images.os.load = android_img_get_kload(os_hdr);
+		break;
+#endif
 	default:
 		puts("ERROR: unknown image format type!\n");
 		return 1;
@@ -266,6 +301,10 @@  static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 			return 1;
 		}
 #endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	} else if (img_type == IMAGE_FORMAT_ANDROID) {
+		images.ep = images.os.load;
+#endif
 	} else {
 		puts("Could not find kernel entry point!\n");
 		return 1;
@@ -806,6 +845,36 @@  static int fit_check_kernel(const void *fit, int os_noffset, int verify)
 }
 #endif /* CONFIG_FIT */
 
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
+static int android_image_get_kernel(struct andr_img_hdr *hdr, int verify)
+{
+	/*
+	 * Not all Android tools use the id field for signing the image with
+	 * sha1 (or anything) so we don't check it. It is not obvious that the
+	 * string is null terminated so we take care of this.
+	 */
+	strncpy(andr_tmp_str, hdr->name, ANDR_BOOT_NAME_SIZE);
+	andr_tmp_str[ANDR_BOOT_NAME_SIZE] = '\0';
+	if (strlen(andr_tmp_str))
+		printf("Android's image name: %s\n", andr_tmp_str);
+
+	printf("Kernel load addr 0x%08x size %u KiB\n",
+			hdr->kernel_addr, DIV_ROUND_UP(hdr->kernel_size, 1024));
+	strncpy(andr_tmp_str, hdr->cmdline, ANDR_BOOT_ARGS_SIZE);
+	andr_tmp_str[ANDR_BOOT_ARGS_SIZE] = '\0';
+	if (strlen(andr_tmp_str)) {
+		printf("Kernel command line: %s\n", andr_tmp_str);
+		setenv("bootargs", andr_tmp_str);
+	}
+	if (hdr->ramdisk_size)
+		printf("RAM disk load addr 0x%08x size %u KiB\n",
+				hdr->ramdisk_addr,
+				DIV_ROUND_UP(hdr->ramdisk_size, 1024));
+	return 0;
+}
+#endif
+
 /**
  * boot_get_kernel - find kernel image
  * @os_data: pointer to a ulong variable, will hold os data start address
@@ -833,6 +902,9 @@  static void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 	int		cfg_noffset;
 	int		os_noffset;
 #endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	struct andr_img_hdr *ahdr;
+#endif
 
 	/* find out kernel image address */
 	if (argc < 2) {
@@ -976,6 +1048,19 @@  static void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 		images->fit_noffset_os = os_noffset;
 		break;
 #endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	case IMAGE_FORMAT_ANDROID:
+		printf("## Booting Android Image at 0x%08lx ...\n", img_addr);
+		ahdr = (void *)img_addr;
+		if (android_image_get_kernel(ahdr, images->verify))
+			return NULL;
+
+		*os_data = img_addr;
+		*os_data += ahdr->page_size;
+		*os_len = ahdr->kernel_size;
+		images->ahdr = ahdr;
+		break;
+#endif
 	default:
 		printf("Wrong Image Format for %s command\n", cmdtp->name);
 		show_boot_progress(-108);
diff --git a/common/image.c b/common/image.c
index 555d9d9..0eba19a 100644
--- a/common/image.c
+++ b/common/image.c
@@ -26,6 +26,7 @@ 
 #ifndef USE_HOSTCC
 #include <common.h>
 #include <watchdog.h>
+#include <android_image.h>
 
 #ifdef CONFIG_SHOW_BOOT_PROGRESS
 #include <status_led.h>
@@ -672,7 +673,14 @@  int genimg_get_format(void *img_addr)
 			format = IMAGE_FORMAT_FIT;
 	}
 #endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	if (format == IMAGE_FORMAT_INVALID) {
+		const struct andr_img_hdr *ahdr = img_addr;
 
+		if (!memcmp(ANDR_BOOT_MAGIC, ahdr->magic, ANDR_BOOT_MAGIC_SIZE))
+			format = IMAGE_FORMAT_ANDROID;
+	}
+#endif
 	return format;
 }
 
@@ -1006,6 +1014,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);
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+		if (images->ahdr && images->ahdr->ramdisk_size) {
+			rd_data = (unsigned long) images->ahdr;
+			rd_data += images->ahdr->page_size;
+			rd_data += ALIGN(images->ahdr->kernel_size,
+					images->ahdr->page_size);
+			rd_len = images->ahdr->ramdisk_size;
+		}
+#endif
 	} else {
 		/*
 		 * no initrd image
diff --git a/include/android_image.h b/include/android_image.h
new file mode 100644
index 0000000..b231b66
--- /dev/null
+++ b/include/android_image.h
@@ -0,0 +1,89 @@ 
+/*
+ * This is from the Android Project,
+ * bootloader/legacy/include/boot/bootimg.h
+ *
+ * Copyright (C) 2008 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef _ANDROID_IMAGE_H_
+#define _ANDROID_IMAGE_H_
+
+#define ANDR_BOOT_MAGIC "ANDROID!"
+#define ANDR_BOOT_MAGIC_SIZE 8
+#define ANDR_BOOT_NAME_SIZE 16
+#define ANDR_BOOT_ARGS_SIZE 512
+
+struct andr_img_hdr {
+	u8 magic[ANDR_BOOT_MAGIC_SIZE];
+
+	u32 kernel_size;	/* size in bytes */
+	u32 kernel_addr;	/* physical load addr */
+
+	u32 ramdisk_size;	/* size in bytes */
+	u32 ramdisk_addr;	/* physical load addr */
+
+	u32 second_size;	/* size in bytes */
+	u32 second_addr;	/* physical load addr */
+
+	u32 tags_addr;		/* physical addr for kernel tags */
+	u32 page_size;		/* flash page size we assume */
+	u32 unused[2];		/* future expansion: should be 0 */
+
+	u8 name[ANDR_BOOT_NAME_SIZE]; /* asciiz product name */
+
+	u8 cmdline[ANDR_BOOT_ARGS_SIZE];
+
+	u32 id[8]; /* timestamp / checksum / sha1 / etc */
+};
+
+/*
+ * +-----------------+
+ * | boot header     | 1 page
+ * +-----------------+
+ * | kernel          | n pages
+ * +-----------------+
+ * | ramdisk         | m pages
+ * +-----------------+
+ * | second stage    | o pages
+ * +-----------------+
+ *
+ * n = (kernel_size + page_size - 1) / page_size
+ * m = (ramdisk_size + page_size - 1) / page_size
+ * o = (second_size + page_size - 1) / page_size
+ *
+ * 0. all entities are page_size aligned in flash
+ * 1. kernel and ramdisk are required (size != 0)
+ * 2. second is optional (second_size == 0 -> no second)
+ * 3. load each element (kernel, ramdisk, second) at
+ *    the specified physical address (kernel_addr, etc)
+ * 4. prepare tags at tag_addr.  kernel_args[] is
+ *    appended to the kernel commandline in the tags.
+ * 5. r0 = 0, r1 = MACHINE_TYPE, r2 = tags_addr
+ * 6. if second_size != 0: jump to second_addr
+ *    else: jump to kernel_addr
+ */
+#endif
diff --git a/include/image.h b/include/image.h
index c56a18d..b2b580c 100644
--- a/include/image.h
+++ b/include/image.h
@@ -264,6 +264,9 @@  typedef struct bootm_headers {
 #ifdef CONFIG_LMB
 	struct lmb	lmb;		/* for memory mgmt */
 #endif
+#ifdef CONFIG_ANDROID_BOOT_IMAGE
+	struct andr_img_hdr	*ahdr;
+#endif
 } bootm_headers_t;
 
 /*
@@ -329,6 +332,7 @@  void genimg_print_size(uint32_t size);
 #define IMAGE_FORMAT_INVALID	0x00
 #define IMAGE_FORMAT_LEGACY	0x01	/* legacy image_header based format */
 #define IMAGE_FORMAT_FIT	0x02	/* new, libfdt based format */
+#define IMAGE_FORMAT_ANDROID	0x03	/* Android boot image */
 
 int genimg_get_format(void *img_addr);
 int genimg_has_config(bootm_headers_t *images);