[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Siarhei Siamashka]

On Sun, 1 Feb 2015 13:59:41 -0700
Simon Glass <sjg at chromium.org> wrote:

> Hi Siarhei,
> 
> On 1 February 2015 at 11:48, Siarhei Siamashka <siarhei.siamashka at gmail.com>
> wrote:
> > On Sun, 1 Feb 2015 09:28:36 -0700
> > Simon Glass <sjg at chromium.org> wrote:
> >
> >> Hi,
> >>
> >> On 30 January 2015 at 04:58, Siarhei Siamashka
> >> <siarhei.siamashka at 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.siamashka at 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.

OK, I understand.

However crt0.S does not seem to be particularly compatible with FEL.
It tries to override the stack pointer (in the FEL mode we need to
use the original stack pointer provided to us by the BROM). And
implies that 'board_init_f' needs to return control back to the
BROM, however doing return from multiple nested levels of function
calls is a tricky exercise, especially considering that the stack
has been already moved elsewhere.

On the other hand, I see that crt0.S just allocates global data on
stack, clears it and then sets all the same r9 register. So what
is the real difference compared to having global data defined in
the ".data" section?

I would say that right now an easy hack would be to remove gdata
globally in u-boot (that's an admirable goal), but keep it with a
different name under the CONFIG_SPL_FEL define just for sunxi.
We might just rework this patch by providing the following FEL
entry point:

    push    {r9, lr}
    ldr     r9, =sunxi_fel_gdata
    bl      s_init
    bl      board_init_f
    pop     {r9, pc}

Then hide the unneeded parts of board_init_f() under
!defined(CONFIG_SPL_FEL) check, so that it just returns
right after initializing DRAM.

I see that the rationale for gdata removal is to allow having an
early malloc pool for the driver model in SPL:
    http://lists.denx.de/pipermail/u-boot/2014-December/199528.html

I think that you can keep experimenting with the driver model
on sunxi with the regular SPL build, but just leave the FEL mode
build alone for now. It is not like u-boot is going to ever switch
to the driver model in the SPL on every platform (there are platforms
where the SRAM size is extremely small, even smaller than on sunxi),
so the sunxi FEL mode is not going to be alone doing this.

> >> > +
> >> > +#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?

Currently sunxi has the CONFIG_SPL_FEL define.

Even though I'm generally in favour of runtime detection, we can't do
it in this case. This is only because the amount of available SRAM
space is much smaller in the FEL mode and the normal SPL simply does
not fit. If we could afford it, then surely having a single SPL
binary (both bootable in the USB FEL mode and from the SD card) would
be much preferable without separate '*_felconfig' and '*_defconfig'
configs.

-- 
Best regards,
Siarhei Siamashka