Hi Siarhei, On 1 February 2015 at 11:48, Siarhei Siamashka <siarhei.siamas...@gmail.com> wrote: > On Sun, 1 Feb 2015 09:28:36 -0700 > Simon Glass <s...@chromium.org> wrote: > >> Hi, >> >> On 30 January 2015 at 04:58, Siarhei Siamashka >> <siarhei.siamas...@gmail.com> wrote: >> > The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95 >> > 'sunxi: Move SPL s_init() code to board_init_f()' >> > broke the FEL boot mode. >> > >> > This patch moves the DRAM initialization back to s_init() and >> > introduces an assembly entry point for FEL in order to provide >> > guaranteed initialization of the gdata pointer (r9). The assembly >> > entry point is also needed to ensure that the SPL code starts >> > executing in ARM mode. >> > >> > Because the sunxi board_init_f() does not contain anything that >> > is not already done by the default board_init_f(), it is removed >> > too. >> > >> > Signed-off-by: Siarhei Siamashka <siarhei.siamas...@gmail.com> >> > --- >> > arch/arm/cpu/armv7/sunxi/Makefile | 1 + >> > arch/arm/cpu/armv7/sunxi/board.c | 26 ++++++++++---------------- >> > arch/arm/cpu/armv7/sunxi/start_fel.S | 16 ++++++++++++++++ >> > arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 3 ++- >> > 4 files changed, 29 insertions(+), 17 deletions(-) >> > create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S >> > >> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile >> > index 48db744..e0d413d 100644 >> > --- a/arch/arm/cpu/armv7/sunxi/Makefile >> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile >> > @@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I) += dram_sun4i.o >> > obj-$(CONFIG_MACH_SUN8I) += dram_sun8i.o >> > ifdef CONFIG_SPL_FEL >> > obj-y += start.o >> > +extra-y += start_fel.o >> > endif >> > endif >> > diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c >> > index 6e28bcd..ea6cb60 100644 >> > --- a/arch/arm/cpu/armv7/sunxi/board.c >> > +++ b/arch/arm/cpu/armv7/sunxi/board.c >> > @@ -85,6 +85,16 @@ void s_init(void) >> > timer_init(); >> > gpio_init(); >> > i2c_init_board(); >> > + >> > +#ifdef CONFIG_SPL_BUILD >> > + preloader_console_init(); >> >> s_init() is called before we have global_data, so you can't use a console. > > Oh, but somehow it just works for me.
I should have said that there *should* be no global_data at this point, i.e. we need to drop the hacks that add this. In fact global_data should be set up once in crt0.S and not before. > >> > + >> > +#ifdef CONFIG_SPL_I2C_SUPPORT >> > + /* Needed early by sunxi_board_init if PMU is enabled */ >> > + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); >> > +#endif >> > + sunxi_board_init(); >> > +#endif >> >> Why do you need this code here? > > The i2c_init() call is needed to initialize the PMIC as the comment > says. The PMIC is needed to set correct voltages, necessary for > the DRAM controller. > > And the sunxi_board_init() initializes DRAM. > >> Can it not go in board_init_f()? > > Then we probably would need a special stripped down FEL variant of > board_init_f(), which makes the code a bit more messy. Yes I think it is well worth figuring out the really differences are between an SPL that boots from board storage and one that boots from FEL. For Exynos is it just a single switch() to select the boot source. For Tegra it essentially nothing. Exynos has a flag that tells SPL when it is booting from USB A-A. Does sunxi? > >> > } >> > >> > #ifdef CONFIG_SPL_BUILD >> > @@ -103,22 +113,6 @@ u32 spl_boot_mode(void) >> > { >> > return MMCSD_MODE_RAW; >> > } >> > - >> > -void board_init_f(ulong dummy) >> > -{ >> > - preloader_console_init(); >> > - >> > -#ifdef CONFIG_SPL_I2C_SUPPORT >> > - /* Needed early by sunxi_board_init if PMU is enabled */ >> > - i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); >> > -#endif >> > - sunxi_board_init(); >> > - >> > - /* Clear the BSS. */ >> > - memset(__bss_start, 0, __bss_end - __bss_start); >> > - >> > - board_init_r(NULL, 0); >> > -} >> > #endif >> > >> > void reset_cpu(ulong addr) >> > diff --git a/arch/arm/cpu/armv7/sunxi/start_fel.S b/arch/arm/cpu/armv7/sunxi/start_fel.S >> > new file mode 100644 >> > index 0000000..e1c7cd4 >> > --- /dev/null >> > +++ b/arch/arm/cpu/armv7/sunxi/start_fel.S >> > @@ -0,0 +1,16 @@ >> > +/* >> > + * Entry point of the FEL mode SPL. >> > + * >> > + * Copyright (c) 2015 Siarhei Siamashka <siarhei.siamas...@gmail.com> >> > + * >> > + * SPDX-License-Identifier: GPL-2.0+ >> > + */ >> > + >> > +#include <asm-offsets.h> >> > +#include <config.h> >> > +#include <linux/linkage.h> >> > + >> > +ENTRY(_start_fel) >> > + ldr r9, =gdata >> > + b s_init >> >> No we don't want global data here, and need to get rid of gdata so we >> can use driver model, etc. > > Appears that I need to educate myself on the global data vs. gdata > differences. > > Using driver model in the sunxi SPL is a bit challenging because > we don't have abundant amounts of SRAM there: > http://linux-sunxi.org/SRAM_Controller > Without relying on SoC-variant specific SRAM sections, we have 32 KiB > for normal SPL, and only ~15 KiB for FEL SPL (the rest is used by the > BROM FEL code). And SRAM is very much needed for the other important > features. > > So far I can see that the pointer to gdata is stored in the r9 register. > And gdata resides in the ".data" section, which means that it is > initialized to 0 automatically. And this works now, unless I'm > misunderstanding something. > > I would be more than happy to fix it in a future proof way. However it > is very much desired to have a properly functioning FEL boot mode in > u-boot 2015.04 and IMHO a quick hack relying on the legacy gdata feature > might be not the worst possible option today. > >> > +ENDPROC(_start_fel) >> > diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds >> > index 928b7c1..beb8900 100644 >> > --- a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds >> > +++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds >> > @@ -6,7 +6,7 @@ >> > */ >> > OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") >> > OUTPUT_ARCH(arm) >> > -ENTRY(s_init) >> > +ENTRY(_start_fel) >> > SECTIONS >> > { >> > . = 0x00002000; >> > @@ -14,6 +14,7 @@ SECTIONS >> > . = ALIGN(4); >> > .text : >> > { >> > + arch/arm/cpu/armv7/sunxi/start_fel.o (.text) >> > *(.text.s_init) >> >> Why does this have to jump to a special s_init? Can it not just start >> SPL normally as it does on Tegra, Exynos, etc? >> >> > *(.text*) >> > } >> >> There has to be a better way of making this work. Also do you have >> instructions on how I can try this out on a pcduino3 or other low-cost >> board? >> >> I understand that we need to fix this, but other archs deal with this >> within the existing framework, so I'd really like to get sunxi into >> the same state. > > For these parts, see my replies in: > http://lists.denx.de/pipermail/u-boot/2015-February/203439.html > http://lists.denx.de/pipermail/u-boot/2015-February/203485.html > > I'm afraid that we can't always fit the BROM code into the existing > frameworks. But maybe some good solution exists for this particular > case. > > -- > Best regards, > Siarhei Siamashka _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot