[U-Boot] [PATCH v2 3/3] sunxi: Normalise FEL support
Siarhei Siamashka
siarhei.siamashka at gmail.com
Wed Feb 11 04:05:31 CET 2015
On Mon, 9 Feb 2015 15:23:17 -0700
Simon Glass <sjg at chromium.org> wrote:
> Hi Siarhei,
>
> On 7 February 2015 at 20:48, Siarhei Siamashka
> <siarhei.siamashka at gmail.com> wrote:
> > On Sat, 7 Feb 2015 10:47:30 -0700
> > Simon Glass <sjg at chromium.org> wrote:
> >
> >> Make sunxi's FEL code fit with the normal U-Boot boot sequence instead of
> >> creating its own. There are some #ifdefs required in start.S. Future work
> >> will hopefully remove these.
> >>
> >> This series is available at u-boot-dm, branch sunxi-working.
> >>
> >> Signed-off-by: Simon Glass <sjg at chromium.org>
> >> ---
> >>
> >> Changes in v2:
> >> - Adjust for new save_boot_params() API
> >> - Drop patch to change r0 to r2 in start.S
> >> - Add #ifdefs to start.S to deal with FEL
> >> - Use 'Fast Early Loader' as the full name for FEL
> >
> > Thanks for working on these patches. It looks like we are finally
> > getting really close to resolving the sunxi FEL boot problem.
> >
> > Some comments are below.
> >
> >> arch/arm/cpu/armv7/start.S | 5 +-
> >> arch/arm/cpu/armv7/sunxi/Makefile | 4 +-
> >> arch/arm/cpu/armv7/sunxi/board.c | 21 ++++++++
> >> arch/arm/cpu/armv7/sunxi/config.mk | 2 -
> >> arch/arm/cpu/armv7/sunxi/fel_utils.S | 25 +++++++++
> >> arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 82 -----------------------------
> >> arch/arm/include/asm/arch-sunxi/sys_proto.h | 10 ++++
> >> board/sunxi/Kconfig | 10 ++++
> >> include/configs/sunxi-common.h | 6 +--
> >> scripts/Makefile.spl | 2 -
> >> 10 files changed, 73 insertions(+), 94 deletions(-)
> >> create mode 100644 arch/arm/cpu/armv7/sunxi/fel_utils.S
> >> delete mode 100644 arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> >>
> >> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> >> index 9b49ece..098a83a 100644
> >> --- a/arch/arm/cpu/armv7/start.S
> >> +++ b/arch/arm/cpu/armv7/start.S
> >> @@ -54,7 +54,8 @@ save_boot_params_ret:
> >> * (OMAP4 spl TEXT_BASE is not 32 byte aligned.
> >> * Continue to use ROM code vector only in OMAP4 spl)
> >> */
> >> -#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
> >> +#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD)) && \
> >> + !defined(CONFIG_SPL_FEL)
> >
> > Maybe we can just update the 'save_boot_params' function to save the
> > important configuration registers and then undo this configuration
> > in 'return_to_fel'? This allows us to avoid the sunxi specific ifdefs
> > in the core ARM code.
> >
> > In this particular case, restoring VBAR and CPSR is important because
> > the BROM code uses an IRQ handler for FEL (presumably USB related).
> > And we want to ensure that this IRQ handler works properly again
> > after resuming the FEL code.
>
> We could indeed.
>
> It would avoid the #ifdef but my understanding is that you might be
> able to avoid having to 'return' to the BROM through another means. In
> any case we could perhaps leave that change until the next release?
I just previously thought that the FEL tool could store the return
address somewhere in beginning of SRAM in the eGON header extension.
In this case, restoring the 'lr' and 'sp' registers would be unnecessary
and the use of the 'save_boot_params' function could be dropped.
But if we still need to save/restore VBAR and the other control
registers, then this idea about not saving 'lr' and 'sp' does not
really improve anything.
[...]
> >> u32 spl_boot_device(void)
> >> {
> >> + /*
> >> + * Have we been asked to return to the FEL portion of the boot ROM?
> >> + * TODO: We need a more robust test here, or bracket this with
> >> + * #ifdef CONFIG_SPL_FEL.
> >> + */
> >> + if (fel_stash.lr >= 0xffff0000 && fel_stash.lr < 0xffff4000)
> >> + return BOOT_DEVICE_BOARD;
> >> return BOOT_DEVICE_MMC1;
> >
> > It is probably better to do it this way:
> >
> > #ifdef CONFIG_SPL_FEL
> > return BOOT_DEVICE_BOARD;
> > #else
> > if (memcmp((void *)4, "eGON.BT0", 8) == 0)
> > return BOOT_DEVICE_MMC1;
> > else
> > return BOOT_DEVICE_BOARD;
> > #endif
> >
> > The memcmp (or equivalent code) ensures that it is compatible with
> > https://github.com/ssvb/sunxi-tools/commit/aa7e1880986e5c9a825b08aed9dc5621b821805f
> >
> > Then the new 'fel spl u-boot-sunxi-with-spl.bin' command for loading
> > and executing the SPL works fine without CONFIG_SPL_FEL defined
> > (because the SD card specific "eGON.BT0" signature is replaced with
> > the "eGON.FEL" signature by the 'fel' tool in the device memory
> > before transferring control to the SPL code). Needless to say that
> > the SPL built this way still works when written to the SD card.
> >
> > And if CONFIG_SPL_FEL is defined, then the FEL support still works in a
> > legacy way (via 'fel write 0x2000 u-boot-spl.bin; fel exe 0x2000').
> >
> > We would only need to drop the legacy way in u-boot v2015.07 if the
> > new one proves to be problem free by that time :-)
>
> So you mean that we can drop CONFIG_SPL_FEL?
Yes, we can already do this. But in order to play safe, the
CONFIG_SPL_FEL option could be still kept for a while to provide
the users with an alternative method just in case.
> [snip]
>
> >
> > ENTRY(save_boot_params)
> > ldr r0, =fel_stash
> > str sp, [r0, #0]
> > str lr, [r0, #4]
> > mrs lr, cpsr @ Read CPSR
> > str lr, [r0, #8]
> > mrc p15, 0, lr, c1, c0, 0 @ Read CP15 SCTLR
> > str lr, [r0, #12]
> > mrc p15, 0, lr, c12, c0, 0 @ Read VBAR
> > str lr, [r0, #16]
> > mrc p15, 0, lr, c1, c0, 0 @ Read CP15 Control
> > str lr, [r0, #20]
> > b save_boot_params_ret
> > ENDPROC(save_boot_params)
> >
> > ENTRY(return_to_fel)
> > ldr r0, =fel_stash
> > ldr lr, [r0, #20]
> > mcr p15, 0, lr, c1, c0, 0 @ Write CP15 Control
> > ldr lr, [r0, #16]
> > mcr p15, 0, lr, c12, c0, 0 @ Write VBAR
> > ldr lr, [r0, #12]
> > mcr p15, 0, lr, c1, c0, 0 @ Write CP15 SCTLR
> > ldr lr, [r0, #8]
> > msr cpsr, lr @ Write CPSR
> > ldr sp, [r0, #0]
> > ldr lr, [r0, #4]
> > bx lr
> > ENDPROC(return_to_fel)
> >
> >
> > This seems to work for me. However we probably might try to skip
> > restoring some of these registers. Also somebody might want to
> > review the order in which the registers should be restored.
>
> I can certainly incorporate this into the patch.
>
> Is that what you would prefer? It seems more complicated, and also a
> bit odd to save a whole load of data that we then corrupt and restore,
> but it is fine with me.
This is a way not to touch the Albert's code in 'start.S', but instead
deal with the problem in the sunxi code.
If Albert has objections against the introduction of special flags to
skip setup of these control registers, then I think that saving them
early and restoring back to the original values when returning back
to the BROM as the only possible alternative.
The BROM will not be able to work correctly if the VBAR register is
clobbered by the SPL.
Yes, this all is a bit odd. But as Hans explained earlier, the main
u-boot binary has to be pretty much standalone and can't rely on the
stuff being configured by the SPL:
http://lists.denx.de/pipermail/u-boot/2015-January/202204.html
[...]
> > One more theoretical concern is the placement of the stack in the legacy
> > FEL mode after your changes. Now it seems to be moved to 0x8000, which
> > might be fine though, according to the memory map from
> >
> > https://github.com/hno/Allwinner-Info/blob/master/FEL-usb/USB-protocol.txt
> >
> > If we want to preserve the old behaviour, then we would need to place
> > it around 0x5d00.
>
> I'm wondering if it might be better for you to take over and respin
> this patch? My role has perhaps been to provide a proof-of-concept
> implementation, but I don't need to do the final bit.
>
> Let me know what you think.
I'm just applying these changes on top of your patches (just all the
same as is discussed here):
https://github.com/ssvb/u-boot-sunxi/commit/6385323f7704b3802f594a05fc8c373a4617ebbc
And after this, the CONFIG_SPL_FEL define becomes unnecessary. Even
though this old method still works (relying on the CONFIG_SPL_FEL
define and uploading the SPL to 0x2000).
You could squash these changes in your third patch if nobody has
objections.
Regarding the stack location. The stack at 0x8000 might be also
perfectly fine. We have not done a detailed analysis of the BROM
disassembly, so can't be absolutely sure about anything. But
based on just treating the BROM as a black box and running some
experimental tests, it looks OK so far.
Maybe Hans has an opinion about how we should deal with it?
--
Best regards,
Siarhei Siamashka
More information about the U-Boot
mailing list