
Hi Simon,
On Mon, Feb 25, 2013 at 11:50 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On Sun, Feb 24, 2013 at 1:33 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Simon,
On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass sjg@chromium.org wrote:
Convert main_loop() over to use autoconf, and add a required prototype to common.h.
The do_mdm_init variable is now always defined, but this seems like an acceptable compromise.
In fdt_support.h the #ifdef used is CONFIG_OF_LIBFDT. However, even if this is not defined we want to make the functions available for our conditional-compilation scheme. The only place where we really don't have access to these support functions is when USE_HOSTCC is defined. So change the #ifdef to that.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2: None
common/main.c | 77 +++++++++++++++++++++++---------------------------- include/common.h | 1 + include/fdt_support.h | 4 +-- 3 files changed, 37 insertions(+), 45 deletions(-)
diff --git a/common/main.c b/common/main.c index 3966321..40a79b7 100644 --- a/common/main.c +++ b/common/main.c @@ -63,10 +63,7 @@ static int retry_time = -1; /* -1 so can call readline before main_loop */
#define endtick(seconds) (get_ticks() + (uint64_t)(seconds) * get_tbclk())
-#ifdef CONFIG_MODEM_SUPPORT int do_mdm_init = 0; -extern void mdm_init(void); /* defined in board.c */ -#endif
/***************************************************************************
- Watch for 'delay' seconds for autoboot stop or autoboot delay string.
@@ -383,51 +380,47 @@ void main_loop(void) int len; int rc = 1; int flag; -#ifdef CONFIG_PREBOOT
char *p;
-#endif
bootstage_mark_name(BOOTSTAGE_ID_MAIN_LOOP, "main_loop");
-#ifdef CONFIG_MODEM_SUPPORT
debug("DEBUG: main_loop: do_mdm_init=%d\n", do_mdm_init);
if (do_mdm_init) {
char *str = strdup(getenv("mdm_cmd"));
setenv("preboot", str); /* set or delete definition */
if (str != NULL)
free(str);
mdm_init(); /* wait for modem connection */
if (autoconf_modem_support()) {
Why not just remove do_mdm_init and use gd->do_mdm_init here?
Would that be valid? There is board code to set that - I am not sure what the intent is but it seems beyond the scope of this patch to change it.
From the looks of the source, it's perfectly valid. I can certainly
see your position about it being beyond the scope of this patch. It would be great if someone who cares about this modem code would clean up the mess.
debug("DEBUG: main_loop: do_mdm_init=%d\n", do_mdm_init);
if (do_mdm_init) {
char *str = strdup(getenv("mdm_cmd"));
setenv("preboot", str); /* set or delete definition */
if (str != NULL)
free(str);
mdm_init(); /* wait for modem connection */
} }
-#endif /* CONFIG_MODEM_SUPPORT */
-#ifdef CONFIG_VERSION_VARIABLE
{
if (autoconf_version_variable()) setenv("ver", version_string); /* set version variable */
}
-#endif /* CONFIG_VERSION_VARIABLE */
-#ifdef CONFIG_SYS_HUSH_PARSER
u_boot_hush_start();
-#endif
if (autoconf_sys_hush_parser())
u_boot_hush_start();
-#if defined(CONFIG_HUSH_INIT_VAR)
hush_init_var();
-#endif
if (autoconf_hush_init_var())
hush_init_var();
if (autoconf_preboot()) {
char *p = getenv("preboot");
if (p) {
int prev;
-#ifdef CONFIG_PREBOOT
p = getenv("preboot");
if (p) {
-# ifdef CONFIG_AUTOBOOT_KEYED
int prev = disable_ctrlc(1); /* disable Control C checking */
-# endif
/* disable Control C checking */
if (autoconf_autoboot_keyed())
prev = disable_ctrlc(1);
run_command_list(p, -1, 0);
run_command_list(p, -1, 0);
-# ifdef CONFIG_AUTOBOOT_KEYED
disable_ctrlc(prev); /* restore Control C checking */
-# endif
/* restore Control C checking */
if (autoconf_autoboot_keyed())
disable_ctrlc(prev);
} }
-#endif /* CONFIG_PREBOOT */
if (autoconf_update_tftp()) update_tftp(0UL);
@@ -435,9 +428,8 @@ void main_loop(void) if (autoconf_has_bootdelay() && autoconf_bootdelay() >= 0) process_boot_delay();
-#if defined CONFIG_OF_CONTROL
set_working_fdt_addr((void *)gd->fdt_blob);
-#endif /* CONFIG_OF_CONTROL */
if (autoconf_of_control() && autoconf_of_libfdt())
Why are you adding an additional condition on autoconf_of_libfdt()?
I think I had a build warning somewhere - I will take another look, and then add a comment if it is still needed.
I guess you determined this to be superfluous.
<snip>
Cheers, -Joe