On Fri, Aug 15, 2014 at 11:04 AM, Stephen Warren swarren@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.