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
