* 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
