diff mbox

[U-Boot] rockchip: spl: make boot0 hook TPL safe

Message ID 1499799885-35391-1-git-send-email-philipp.tomsich@theobroma-systems.com
State Accepted
Commit 700f3108d3970a0f8ea8b05e1a6c1907ea75dfc8
Delegated to: Philipp Tomsich
Headers show

Commit Message

Philipp Tomsich July 11, 2017, 7:04 p.m. UTC
When building for a TPL/SPL setup (e.g. on the RK3368), we need the
TPL stage to have the extra space for for the 'Rockchip SPL name'
(i.e. 'RK33' word).  Yet, the SPL will start execution at its first
word (i.e. the first word in the SPL binary needs to be a valid
instruction).  To make things a bit more involved, CONFIG_SPL_BUILD
is defined both for the SPL and the TPL stage.

To avoid having to explicitly test for the first stage (TPL, if and
only if TPL and SPL are built, SPL otherwise), this commit modifies
the sequence to repeat the 'b reset' (instead of reserving 4 bytes
of undefined space) at the start of the boot0 hook: if overwritten
(and execution starts at the second word), the first instruction is
still a 'b reset'... if not overwritten, we start on a 'b reset' as
well.

This solution wouldn't even require the check whether we are in the
SPL/TPL build (i.e. CONFIG_SPL_BUILD), but we leave this check in for
documentation purposes.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 arch/arm/include/asm/arch-rockchip/boot0.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Simon Glass July 12, 2017, 12:32 a.m. UTC | #1
On 11 July 2017 at 13:04, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> When building for a TPL/SPL setup (e.g. on the RK3368), we need the
> TPL stage to have the extra space for for the 'Rockchip SPL name'
> (i.e. 'RK33' word).  Yet, the SPL will start execution at its first
> word (i.e. the first word in the SPL binary needs to be a valid
> instruction).  To make things a bit more involved, CONFIG_SPL_BUILD
> is defined both for the SPL and the TPL stage.
>
> To avoid having to explicitly test for the first stage (TPL, if and
> only if TPL and SPL are built, SPL otherwise), this commit modifies
> the sequence to repeat the 'b reset' (instead of reserving 4 bytes
> of undefined space) at the start of the boot0 hook: if overwritten
> (and execution starts at the second word), the first instruction is
> still a 'b reset'... if not overwritten, we start on a 'b reset' as
> well.
>
> This solution wouldn't even require the check whether we are in the
> SPL/TPL build (i.e. CONFIG_SPL_BUILD), but we leave this check in for
> documentation purposes.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  arch/arm/include/asm/arch-rockchip/boot0.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Philipp Tomsich July 12, 2017, 10:36 a.m. UTC | #2
> When building for a TPL/SPL setup (e.g. on the RK3368), we need the
> TPL stage to have the extra space for for the 'Rockchip SPL name'
> (i.e. 'RK33' word).  Yet, the SPL will start execution at its first
> word (i.e. the first word in the SPL binary needs to be a valid
> instruction).  To make things a bit more involved, CONFIG_SPL_BUILD
> is defined both for the SPL and the TPL stage.
> 
> To avoid having to explicitly test for the first stage (TPL, if and
> only if TPL and SPL are built, SPL otherwise), this commit modifies
> the sequence to repeat the 'b reset' (instead of reserving 4 bytes
> of undefined space) at the start of the boot0 hook: if overwritten
> (and execution starts at the second word), the first instruction is
> still a 'b reset'... if not overwritten, we start on a 'b reset' as
> well.
> 
> This solution wouldn't even require the check whether we are in the
> SPL/TPL build (i.e. CONFIG_SPL_BUILD), but we leave this check in for
> documentation purposes.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  arch/arm/include/asm/arch-rockchip/boot0.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 

Applied to u-boot-rockchip, thanks!
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h
index 7346876..72d264b 100644
--- a/arch/arm/include/asm/arch-rockchip/boot0.h
+++ b/arch/arm/include/asm/arch-rockchip/boot0.h
@@ -1,3 +1,4 @@ 
+
 /*
  * Copyright 2017 Theobroma Systems Design und Consulting GmbH
  *
@@ -13,7 +14,17 @@ 
  */
 
 #ifdef CONFIG_SPL_BUILD
-	.space 0x4         /* space for the 'RK33' */
+	/*
+	 * We need to add 4 bytes of space for the 'RK33' at the
+	 * beginning of the executable.	 However, as we want to keep
+	 * this generic and make it applicable to builds that are like
+	 * the RK3368 (TPL needs this, SPL doesn't) or the RK3399 (no
+	 * TPL, but extra space needed in the SPL), we simply repeat
+	 * the 'b reset' with the expectation that the first one will
+	 * be overwritten, if this is the first stage contained in the
+	 * final image created with mkimage)...
+	 */
+	b reset	 /* may be overwritten --- should be 'nop' or a 'b reset' */
 #endif
 	b reset