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

Mon Feb 2 19:45:57 CET 2015

Hi Siarhei,

On 1 February 2015 at 16:59, Siarhei Siamashka
<siarhei.siamashka at gmail.com> wrote:
>
> 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.

How about we save sp and lr in other registers before they get changed.

Then sunxi lowlevel_init can stash them in SRAM, or if we are clever,
board_init_f() can do it.

Then we can just restore the original stack and jump to lr when SPL is finished.

I see no problem with using our own stack in SPL. In fact I'd prefer it.

>
> 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}

No, let's just declare a locate data section variable for sunxi. It
does not need to go in global_data. That just confuses things.

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

Can we not return in board_init_r() like SPL normally does? Even in
FEL mode we might want to do something else.

>
> 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.

I believe that even FEL has enough SRAM for driver model, but I agree
there will be cases where it is impossible (e.g. Atmel's 4KB seems
really hard).

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

Oh dear that is awful! Perhaps the build should create two SPLs, one
for FEL and one for normal? That avoids all the fiddling, although
presumably FEL mode is mostly for U-Boot development?

Why does FEL mode need so much SRAM? The model is wrong. It should not
be calling into SPL - maybe someone should take this up with Allwinner
for future chips.

Regards,
Simon