| Message ID | 1453124997-8927-1-git-send-email-sr@denx.de |
|---|---|
| State | Accepted |
| Commit | f7c3638c9f6a9beb812adcfb7c61e4c65ac1888d |
| Delegated to: | Bin Meng |
| Headers | show |
2016-01-18 21:49 GMT+08:00 Stefan Roese <sr@denx.de>: > 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@denx.de> > Cc: Miao Yan <yanmiaobest@gmail.com> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > --- > include/configs/x86-common.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/configs/x86-common.h b/include/configs/x86-common.h > index 4182a3b..6e73656 100644 > --- a/include/configs/x86-common.h > +++ b/include/configs/x86-common.h > @@ -235,4 +235,6 @@ > "tftpboot $loadaddr $bootfile;" \ > "zboot $loadaddr" > > +#define CONFIG_BOOTDELAY 2 > + > #endif /* __CONFIG_H */ > -- > 2.6.5 > Tested-by: Miao Yan <yanmiaobest@gmail.com>
On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> > Cc: Miao Yan <yanmiaobest@gmail.com> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > --- > include/configs/x86-common.h | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Bin Meng <bmeng.cn@gmail.com>
On Mon, Jan 18, 2016 at 02:49:56PM +0100, Stefan Roese 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@denx.de> > Cc: Miao Yan <yanmiaobest@gmail.com> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> I think the right direction (and I haven't had time myself) is to make x86 use the generic distro framework. Dennis Gilmore pointed out a while ago Fedora at least can be easily made to generate the extlinux file on x86 even so it'll just work there :)
Hi Tom, On 19.01.2016 18:25, Tom Rini wrote: > On Mon, Jan 18, 2016 at 02:49:56PM +0100, Stefan Roese 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@denx.de> >> Cc: Miao Yan <yanmiaobest@gmail.com> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> > > I think the right direction (and I haven't had time myself) is to make > x86 use the generic distro framework. Dennis Gilmore pointed out a > while ago Fedora at least can be easily made to generate the extlinux > file on x86 even so it'll just work there :) Yes, this is definitely the "right direction". Unfortunately I'm a bit limited with the time I could spend on this. And frankly, I have not much experience with the generic distro framework. So I would like to postpone this distro rework for a bit now. And since x86 does not support auto-booting at all without CONFIG_BOOTDELAY, I think it makes sense to apply this patch now. And add the distro stuff a bit later. Thanks, Stefan
Hi Stefan, On Tue, Jan 19, 2016 at 2:19 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> >> Cc: Miao Yan <yanmiaobest@gmail.com> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> include/configs/x86-common.h | 2 ++ >> 1 file changed, 2 insertions(+) >> > > Acked-by: Bin Meng <bmeng.cn@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. Regards, Bin
Hi Bin, On 26.01.2016 07:48, Bin Meng wrote: > On Tue, Jan 19, 2016 at 2:19 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >> On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> >>> Cc: Miao Yan <yanmiaobest@gmail.com> >>> Cc: Bin Meng <bmeng.cn@gmail.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- >>> include/configs/x86-common.h | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >> >> Acked-by: Bin Meng <bmeng.cn@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. No need to be sorry for you. I should have spotted this problem before submitting. I'll fix this soon and will resubmit. Thanks, Stefan
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@gmail.com> wrote: >> On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> >>> Cc: Miao Yan <yanmiaobest@gmail.com> >>> Cc: Bin Meng <bmeng.cn@gmail.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- >>> include/configs/x86-common.h | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >> >> Acked-by: Bin Meng <bmeng.cn@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? Thanks, Stefan
Hi Stefan, On 28 January 2016 at 09:13, Stefan Roese <sr@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@gmail.com> wrote: >>> >>> On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> >>>> Cc: Miao Yan <yanmiaobest@gmail.com> >>>> Cc: Bin Meng <bmeng.cn@gmail.com> >>>> Cc: Simon Glass <sjg@chromium.org> >>>> --- >>>> include/configs/x86-common.h | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>> >>> Acked-by: Bin Meng <bmeng.cn@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
Hi Simon, On 28.01.2016 17:16, Simon Glass wrote: > Hi Stefan, > > On 28 January 2016 at 09:13, Stefan Roese <sr@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@gmail.com> wrote: >>>> >>>> On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> >>>>> Cc: Miao Yan <yanmiaobest@gmail.com> >>>>> Cc: Bin Meng <bmeng.cn@gmail.com> >>>>> Cc: Simon Glass <sjg@chromium.org> >>>>> --- >>>>> include/configs/x86-common.h | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>> >>>> Acked-by: Bin Meng <bmeng.cn@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. Okay. I'll "solve" it this way. Thanks, Stefan
On Tue, Jan 19, 2016 at 2:19 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > On Mon, Jan 18, 2016 at 9:49 PM, Stefan Roese <sr@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@denx.de> >> Cc: Miao Yan <yanmiaobest@gmail.com> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> include/configs/x86-common.h | 2 ++ >> 1 file changed, 2 insertions(+) >> > > Acked-by: Bin Meng <bmeng.cn@gmail.com> applied to u-boot-x86/master, thanks!
diff --git a/include/configs/x86-common.h b/include/configs/x86-common.h index 4182a3b..6e73656 100644 --- a/include/configs/x86-common.h +++ b/include/configs/x86-common.h @@ -235,4 +235,6 @@ "tftpboot $loadaddr $bootfile;" \ "zboot $loadaddr" +#define CONFIG_BOOTDELAY 2 + #endif /* __CONFIG_H */
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@denx.de> Cc: Miao Yan <yanmiaobest@gmail.com> Cc: Bin Meng <bmeng.cn@gmail.com> Cc: Simon Glass <sjg@chromium.org> --- include/configs/x86-common.h | 2 ++ 1 file changed, 2 insertions(+)