[U-Boot] [PATCH v2] fastboot: Add support for flashing zImage

[Sam Protsenko]

On 18 May 2017 at 14:20, Tom Rini <trini at konsulko.com> wrote:
> On Thu, May 18, 2017 at 11:50:23AM +0300, Sam Protsenko wrote:
>> On 18 May 2017 at 01:20, Tom Rini <trini at konsulko.com> wrote:
>> > On Wed, May 17, 2017 at 08:20:47PM +0300, Sam Protsenko wrote:
>> >
>> >> This patch adds support for flashing zImage to the Android boot
>> >> partition on eMMC.
>> > [snip]
>> >> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
>> >> +     if (strcmp(cmd, "zImage") == 0 || strcmp(cmd, "zimage") == 0) {
>> >
>> > strncasecmp(cmd, "zimage", 6) ?
>> >
>>
>> Good catch, didn't know this routine exists. Will fix it in v3.
>>
>> >> diff --git a/include/android_image.h b/include/android_image.h
>> >> index dfd4d9d72c..c058b1d388 100644
>> >> --- a/include/android_image.h
>> >> +++ b/include/android_image.h
>> >> @@ -12,6 +12,8 @@
>> >>  #ifndef _ANDROID_IMAGE_H_
>> >>  #define _ANDROID_IMAGE_H_
>> >>
>> >> +#include <linux/types.h>
>> >> +
>> >>  typedef struct andr_img_hdr andr_img_hdr;
>> >>
>> >>  #define ANDR_BOOT_MAGIC "ANDROID!"
>> >
>> > Unneeded.
>> >
>>
>> If you remove that inclusion, build will fail with errors like this:
>>
>>     include/android_image.h:28:2: error: unknown type name ‘u32’
>>
>> So it should be included either in android_image.h, or I should
>> include it in common/fb_mmc.c *before* including android_image.h.
>> Which is (I presume) a bad style. So I incline to do that the way I
>> did it. If you still don't agree -- I'll rework it.
>
> In this case, it's a header we're mirroring from upstream Android, so
> I'd rather not modify it.  That is indeed not the normal way to address
> this issue.
>

Done. Please review patch v3.

>> >> diff --git a/include/linux/compat.h b/include/linux/compat.h
>> >> index a43e4d6698..b0ff6b91f0 100644
>> >> --- a/include/linux/compat.h
>> >> +++ b/include/linux/compat.h
>> >> @@ -111,6 +111,9 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep)
>> >>
>> >>  #define PAGE_SIZE    4096
>> >>
>> >> +/* to align the pointer to the (next) page boundary */
>> >> +#define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE)
>> >
>> > Just use ALIGN() in the code you're writing?  This is for things we're
>> > pulling directly from Linux really.
>> >
>>
>> Thought it would be more clear that way. Btw, I pulled PAGE_ALIGN()
>> from kernel (from include/linux/mm.h). But it's not important to me,
>> so if you want me to use ALIGN() -- I will change it in v3.
>
> Thanks!
>
> --
> Tom