[U-Boot] [RFC PATCH v2 12/15] main: Use autoconf in main_loop()

Simon Glass sjg at chromium.org
Tue Feb 26 06:50:59 CET 2013


Hi Joe,

On Sun, Feb 24, 2013 at 1:33 PM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> Hi Simon,
>
> On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass <sjg at 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 at 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.

>
>> +               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 actually have a patch on patchwork to remove the next line since I
think it is a bad idea. But for now it stays.

>
>> +               set_working_fdt_addr((void *)gd->fdt_blob);
>>
>>         /*
>>          * Main Loop for Monitor Command Processing
>> @@ -472,14 +464,13 @@ void main_loop(void)
>>                 }
>>
>>                 if (len == -1)
>> -                       puts ("<INTERRUPT>\n");
>> +                       puts("<INTERRUPT>\n");
>>                 else
>>                         rc = run_command(lastcommand, flag);
>>
>> -               if (rc <= 0) {
>> -                       /* invalid command or not repeatable, forget it */
>> +               /* If an invalid command or not repeatable, forget it */
>> +               if (rc <= 0)
>>                         lastcommand[0] = 0;
>> -               }
>>         }
>>  }
>>
>> diff --git a/include/common.h b/include/common.h
>> index 1457349..e5eb882 100644
>> --- a/include/common.h
>> +++ b/include/common.h
>> @@ -310,6 +310,7 @@ extern ulong monitor_flash_len;
>>  int mac_read_from_eeprom(void);
>>  extern u8 _binary_dt_dtb_start[];      /* embedded device tree blob */
>>  int set_cpu_clk_info(void);
>> +extern int mdm_init(void); /* defined in board.c */
>>
>>  /**
>>   * Show the DRAM size in a board-specific way
>> diff --git a/include/fdt_support.h b/include/fdt_support.h
>> index 2cccc35..cf8f5e0 100644
>> --- a/include/fdt_support.h
>> +++ b/include/fdt_support.h
>> @@ -24,7 +24,7 @@
>>  #ifndef __FDT_SUPPORT_H
>>  #define __FDT_SUPPORT_H
>>
>> -#ifdef CONFIG_OF_LIBFDT
>> +#ifndef USE_HOSTCC
>>
>>  #include <libfdt.h>
>>
>> @@ -132,5 +132,5 @@ static inline int fdt_status_disabled_by_alias(void *fdt, const char* alias)
>>         return fdt_set_status_by_alias(fdt, alias, FDT_STATUS_DISABLED, 0);
>>  }
>>
>> -#endif /* ifdef CONFIG_OF_LIBFDT */
>> +#endif /* ifdef USE_HOSTCC */
>>  #endif /* ifndef __FDT_SUPPORT_H */
>> --
>> 1.8.1.3
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
> Reviewed-by: Joe Hershberger <joe.hershberger at ni.com>

Regards,
Simon


More information about the U-Boot mailing list