[U-Boot] [PATCH 1/2] x86: x86-common.h: Add CONFIG_BOOTDELAY

[Simon Glass]

Hi Stefan,

On 28 January 2016 at 09:13, Stefan Roese <sr at denx.de> wrote:
>
> Hi Bin,
>
> (added Simon to Cc)
>
> On 26.01.2016 07:48, Bin Meng wrote:
>>
>> On Tue, Jan 19, 2016 at 2:19 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>
>>> On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr at denx.de> wrote:
>>>>
>>>> Without this CONFIG_BOOTDELAY, autobooting does not work at all. As
>>>> autoboot_command() from common/* will not get called. So lets define
>>>> CONFIG_BOOTDELAY, so that auto-booting works on x86.
>>>>
>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>> Cc: Miao Yan <yanmiaobest at gmail.com>
>>>> Cc: Bin Meng <bmeng.cn at gmail.com>
>>>> Cc: Simon Glass <sjg at chromium.org>
>>>> ---
>>>>   include/configs/x86-common.h | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>
>>> Acked-by: Bin Meng <bmeng.cn at gmail.com>
>>
>>
>> Sorry, this patch does not build for efi-x86.
>>
>>         x86:  +   efi-x86
>> +../common/autoboot.c: In function 'process_fdt_options':
>> +../common/autoboot.c:296:36: error: 'CONFIG_SYS_TEXT_BASE' undeclared
>> (first use in this function)
>> +   setenv_addr("kernaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr));
>> +                                    ^
>> +../common/autoboot.c:296:36: note: each undeclared identifier is
>> reported only once for each function it appears in
>> +make[2]: *** [common/autoboot.o] Error 1
>> +make[1]: *** [common] Error 2
>> +make: *** [sub-make] Error 2
>>
>> Could you please fix this? Sorry I did not run buildman earlier.
>
>
> I'm a bit hesitant on how to fix this. As I don't really know this
> "efi-x86" target in detail. Is this code in process_fdt_options()
> really needed for this target? To configure the env variables
> "kernaddr" and "rootaddr" dynamically from the DT properties
> "kernel-offset" and "rootdisk-offset". I can't find any references
> to these DT properties anywhere?
>
> Simon, you introduced this env variable handling with the patch
> [fdt: Set kernaddr if fdt indicates a kernel is present] (git ID
> fcabc24f) in October 2012.
>
> Perhaps its best to assign TEXT_BASE to 0 if its not defined at
> all? Or is this in general the correct value for the "efi-x86"
> target and should be set specifically for it?

It is a funny target since we actually produce a relocatable ELF. I'd
suggest putting #ifdef CONFIG_SYS_TEXT_BASE around it.

Regards,
Simon