Hi Simon,
On Mon, Feb 25, 2013 at 11:50 PM, Simon Glass <[email protected]> wrote:
> Hi Joe,
>
> On Sun, Feb 24, 2013 at 1:33 PM, Joe Hershberger
> <[email protected]> wrote:
>> Hi Simon,
>>
>> On Sun, Feb 24, 2013 at 11:26 AM, Simon Glass <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
