* 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