[U-Boot] [PATCH] image: fix bootm failure for FIT image

[Bryan Wu]

On Fri, Aug 15, 2014 at 11:04 AM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 08/15/2014 11:49 AM, Bryan Wu wrote:
>>
>> Commit b3dd64f5d537b41fc52a03e697271774891f4a70 introduced a bug for
>
>
> Can we spell out the commit description too, so it's easier to know what it
> refers too, and is useful if someone cherry-picks the commit so the commit
> ID changes:
>
> Commit b3dd64f5d537 "bootm: use genimg_get_kernel_addr()" introduced...
>
>
>> booting FIT image. It's because calling fit_parse_config() twice will
>> give us wrong value in img_addr.
>>
>> Add a new version for CONFIG_FIT of genimg_get_kernel_addr() and
>> return fit_uname_config and fit_uname_kernel for CONFIG_FIT.
>
>
>> diff --git a/common/image.c b/common/image.c
>
>
>> -ulong genimg_get_kernel_addr(char * const img_addr)
>> -{
>>   #if defined(CONFIG_FIT)
>> -       const char      *fit_uname_config = NULL;
>> -       const char      *fit_uname_kernel = NULL;
>> +ulong genimg_get_kernel_addr(char * const img_addr,
>> +                            const char **fit_uname_config,
>> +                            const char **fit_uname_kernel)
>> +#else
>> +ulong genimg_get_kernel_addr(char * const img_addr)
>>   #endif
>
>
> Indentation looks wrong on that #endif.
>
> Wouldn't it be better to avoid repeating the common parts, so this instead:
>
> ulong genimg_get_kernel_addr(char * const img_addr,
> #if defined(CONFIG_FIT)
>                              const char **fit_uname_config,
>                              const char **fit_uname_kernel)
> #endif
> )
> {
>

I got compiling error like this when I try this method then I moved to
2 current one:

---
ulong genimg_get_kernel_addr(char * const img_addr,
#if defined(CONFIG_FIT)
                             const char **fit_uname_config,
                             const char **fit_uname_kernel
#endif
);
---
include/image.h:432:1: error: expected declaration specifiers or ‘...’
before ‘)’ token
 );
 ^

But if I use this:

----
ulong genimg_get_kernel_addr(char * const img_addr
#if defined(CONFIG_FIT)
                             , const char **fit_uname_config,
                             const char **fit_uname_kernel
#endif
);
----
Then compiler is happy, but the style looks weird to me.

This is for declaration but definition has the same problem.

-Bryan


>> diff --git a/include/configs/jetson-tk1.h b/include/configs/jetson-tk1.h
>
>
>> +#define CONFIG_FIT
>
>
> I think that crept in by mistake.
>
>
>> diff --git a/include/image.h b/include/image.h
>> index ca2fe86..a47c146 100644
>
>
>> +#if defined(CONFIG_FIT)
>> +ulong genimg_get_kernel_addr(char * const img_addr,
>> +                            const char **fit_uname_config,
>> +                            const char **fit_uname_kernel);
>> +#else
>>   ulong genimg_get_kernel_addr(char * const img_addr);
>> +#endif
>
>
> Same comment about #ifdef placement here.