On Fri, Aug 15, 2014 at 11:04 AM, Stephen Warren <swar...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
