[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Kever Yang]

I think the boot0 hook is suppose to add some data in the very beginning
of the SPL image, am I right?

Rockchip SoCs bootrom design is like this:
- First 2KB or 4KB internal memory is for bootrom stack and heap;
- Then the first 4-byte suppose to be a TAG like 'RK33';
- The the following memory address end with '0004' is the first
  instruction load and running by bootrom;

Example for RK3288:
Before this patch set, the SPL_TEXT_BASE is ff704004, and image write to
media device after mkimage like this:

ff704000: 32334b52 00000000 00000000 00000000    RK32............
ff704010: 00000000 00000000 00000000 00000000    ................
ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................

Where the first instruction from bootrom is '00000000', which is a
undefined instruction.
The '_start' and 'reset' have to align to 0x20 for the requirement of
VBAR, the memory offset '004'~'01c' are filled with '00000000'.

We can use the boot0 hook to fix this issue, after this patch set,
the SPL_TEXT_BASE is ff704000 and image write to media device after
mkimage like this:

ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................

The first instruction from bootrom is a 'b reset', and memory of
'008'~'01c' are filled with 'nop' instruction.

This patch set does not provide patch for socfpga, bcm and sunxi SoCs which also
enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
compatible with those three platforms.



Kever Yang (5):
  armv7: move boot hook before '_start'
  rockchip: boot0: align to 0x20 for armv7 '_start'
  rockchip: enable BOOT0_HOOK for SoCs
  rockchip: configs: use aligned address for SPL_TEXT_BASE
  rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

 arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
 arch/arm/lib/vectors.S                     | 19 ++++++++++---------
 arch/arm/mach-rockchip/Kconfig             |  3 +++
 include/configs/rk3036_common.h            |  2 +-
 include/configs/rk3288_common.h            |  2 +-
 tools/rkcommon.c                           |  8 ++++----
 6 files changed, 27 insertions(+), 16 deletions(-)

-- 
1.9.1

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Kever Yang]

The boot0 hook suppose to add some data before the SPL data,
let's move it at very begining and before '_start'.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 arch/arm/lib/vectors.S | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index f53b1e9..b4cd825 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -35,6 +35,16 @@
 
 	.section ".vectors", "ax"
 
+#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
+/*
+ * Various SoCs need something special and SoC-specific up front in
+ * order to boot, allow them to set that in their boot0.h file and then
+ * use it here.
+ */
+#include <asm/arch/boot0.h>
+
+#endif
+
 /*
  *************************************************************************
  *
@@ -60,15 +70,6 @@ _start:
 	ldr	pc, _irq
 	ldr	pc, _fiq
 
-#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
-/*
- * Various SoCs need something special and SoC-specific up front in
- * order to boot, allow them to set that in their boot0.h file and then
- * use it here.
- */
-#include <asm/arch/boot0.h>
-#endif
-
 /*
  *************************************************************************
  *
-- 
1.9.1

[U-Boot] [RFC PATCH 2/5] rockchip: boot0: align to 0x20 for armv7 '_start'

[Kever Yang]

The '_start' is using as vector table base address, and will write
to VBAR register, need to align to 0x20 for armv7.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

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

diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h
index 7346876..88f0ff6 100644
--- a/arch/arm/include/asm/arch-rockchip/boot0.h
+++ b/arch/arm/include/asm/arch-rockchip/boot0.h
@@ -13,10 +13,17 @@
  */
 
 #ifdef CONFIG_SPL_BUILD
-	.space 0x4         /* space for the 'RK33' */
+	.space 0x4         /* space for Rockchip SoC tag like 'RK33' */
 #endif
 	b reset
 
+/* For armv7, the addr '_start' will used as vector start address
+ * and write to VBAR register, which need to aligned to 0x20.
+ */
+#ifdef CONFIG_CPU_V7
+	.align(5)
+#endif
+
 #if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD)
 	.space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM	/* space for the ATF data */
 #endif
-- 
1.9.1

[U-Boot] [RFC PATCH 3/5] rockchip: enable BOOT0_HOOK for SoCs

[Kever Yang]

Rockchip SoCs bootrom design is like this:
- First 2KB or 4KB internal memory is for bootrom stack and heap;
- Then the first 4-byte suppose to be a TAG like 'RK33';
- The the following memory address end with '0004' is the first
  instruction load and running by bootrom;

Let's use the boot0 hook to reserve the first 4-byte tag for all
the Rockchip SoCs.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 arch/arm/mach-rockchip/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 740dbdf..82aa2d2 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -5,6 +5,7 @@ config ROCKCHIP_RK3036
 	select CPU_V7
 	select SUPPORT_SPL
 	select SPL
+	select ENABLE_ARM_SOC_BOOT0_HOOK
 	help
 	  The Rockchip RK3036 is a ARM-based SoC with a dual-core Cortex-A7
 	  including NEON and GPU, Mali-400 graphics, several DDR3 options
@@ -20,6 +21,7 @@ config ROCKCHIP_RK3188
 	select TPL
 	select BOARD_LATE_INIT
 	select ROCKCHIP_BROM_HELPER
+	select ENABLE_ARM_SOC_BOOT0_HOOK
 	help
 	  The Rockchip RK3188 is a ARM-based SoC with a quad-core Cortex-A9
 	  including NEON and GPU, 512KB L2 cache, Mali-400 graphics, two
@@ -32,6 +34,7 @@ config ROCKCHIP_RK3288
 	select CPU_V7
 	select SUPPORT_SPL
 	select SPL
+	select ENABLE_ARM_SOC_BOOT0_HOOK
 	help
 	  The Rockchip RK3288 is a ARM-based SoC with a quad-core Cortex-A17
 	  including NEON and GPU, 1MB L2 cache, Mali-T7 graphics, two
-- 
1.9.1

[U-Boot] [RFC PATCH 4/5] rockchip: configs: use aligned address for SPL_TEXT_BASE

[Kever Yang]

After we use boot0 hook, we can use offset '000' instead of '004' as
SPL_TEXT_BASE.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 include/configs/rk3036_common.h | 2 +-
 include/configs/rk3288_common.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h
index 2893f80..6e15fc5 100644
--- a/include/configs/rk3036_common.h
+++ b/include/configs/rk3036_common.h
@@ -28,7 +28,7 @@
 #define CONFIG_SYS_INIT_SP_ADDR		0x60100000
 #define CONFIG_SYS_LOAD_ADDR		0x60800800
 #define CONFIG_SPL_STACK		0x10081fff
-#define CONFIG_SPL_TEXT_BASE		0x10081004
+#define CONFIG_SPL_TEXT_BASE		0x10081000
 
 #define CONFIG_ROCKCHIP_MAX_INIT_SIZE	(4 << 10)
 #define CONFIG_ROCKCHIP_CHIP_TAG	"RK30"
diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
index e7a8f72..752404c 100644
--- a/include/configs/rk3288_common.h
+++ b/include/configs/rk3288_common.h
@@ -34,7 +34,7 @@
 #define CONFIG_SYS_INIT_SP_ADDR		0x00100000
 #define CONFIG_SYS_LOAD_ADDR		0x00800800
 #define CONFIG_SPL_STACK		0xff718000
-#define CONFIG_SPL_TEXT_BASE		0xff704004
+#define CONFIG_SPL_TEXT_BASE		0xff704000
 
 /* MMC/SD IP block */
 #define CONFIG_BOUNCE_BUFFER
-- 
1.9.1

[U-Boot] [RFC PATCH 5/5] rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

[Kever Yang]

Enable the spl_boot0 in SPL and use the pre-padding TAG memory,
the mkimage do not need to pad it but only need to replace the value
with correct TAG value.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 tools/rkcommon.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/rkcommon.c b/tools/rkcommon.c
index 836a5a5..b58dfae 100644
--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -68,10 +68,10 @@ struct spl_info {
 };
 
 static struct spl_info spl_infos[] = {
-	{ "rk3036", "RK30", 0x1000, false, false },
-	{ "rk3188", "RK31", 0x8000 - 0x800, true, false },
-	{ "rk3288", "RK32", 0x8000, false, false },
-	{ "rk3328", "RK32", 0x8000 - 0x1000, false, false },
+	{ "rk3036", "RK30", 0x1000, false, true },
+	{ "rk3188", "RK31", 0x8000 - 0x800, true, true },
+	{ "rk3288", "RK32", 0x8000, false, true },
+	{ "rk3328", "RK32", 0x8000 - 0x1000, false, true },
 	{ "rk3399", "RK33", 0x20000, false, true },
 };
 
-- 
1.9.1

[U-Boot] [RFC PATCH 5/5] rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

[Dr. Philipp Tomsich]

Now that there are no remaining cases where (spl_boot0 == false), we can
drop the spl_boot0 field from struct spl_info and also remove the if-check
(leaving only the code-path for true in place).

There should be no need to keep the legacy code-path around, as GIT will
always remember them anyway.

Regards,
Philipp.

> On 31 May 2017, at 12:50, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Enable the spl_boot0 in SPL and use the pre-padding TAG memory,
> the mkimage do not need to pad it but only need to replace the value
> with correct TAG value.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> tools/rkcommon.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/rkcommon.c b/tools/rkcommon.c
> index 836a5a5..b58dfae 100644
> --- a/tools/rkcommon.c
> +++ b/tools/rkcommon.c
> @@ -68,10 +68,10 @@ struct spl_info {
> };
> 
> static struct spl_info spl_infos[] = {
> -	{ "rk3036", "RK30", 0x1000, false, false },
> -	{ "rk3188", "RK31", 0x8000 - 0x800, true, false },
> -	{ "rk3288", "RK32", 0x8000, false, false },
> -	{ "rk3328", "RK32", 0x8000 - 0x1000, false, false },
> +	{ "rk3036", "RK30", 0x1000, false, true },
> +	{ "rk3188", "RK31", 0x8000 - 0x800, true, true },
> +	{ "rk3288", "RK32", 0x8000, false, true },
> +	{ "rk3328", "RK32", 0x8000 - 0x1000, false, true },
> 	{ "rk3399", "RK33", 0x20000, false, true },
> };
> 
> -- 
> 1.9.1
> 

[U-Boot] [RFC PATCH 5/5] rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

[Andy Yan]

Hi Philipp:

2017-05-31 18:58 GMT+08:00 Dr. Philipp Tomsich <
philipp.tomsich@theobroma-systems.com>:

> Now that there are no remaining cases where (spl_boot0 == false), we can
> drop the spl_boot0 field from struct spl_info and also remove the if-check
> (leaving only the code-path for true in place).
>
>

  I think we still have one case: use mkimage to pack ddr.bin  from
rockchip as spl.bin.



> There should be no need to keep the legacy code-path around, as GIT will
> always remember them anyway.
>
> Regards,
> Philipp.
>
> > On 31 May 2017, at 12:50, Kever Yang <kever.yang@rock-chips.com> wrote:
> >
> > Enable the spl_boot0 in SPL and use the pre-padding TAG memory,
> > the mkimage do not need to pad it but only need to replace the value
> > with correct TAG value.
> >
> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> > ---
> >
> > tools/rkcommon.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/rkcommon.c b/tools/rkcommon.c
> > index 836a5a5..b58dfae 100644
> > --- a/tools/rkcommon.c
> > +++ b/tools/rkcommon.c
> > @@ -68,10 +68,10 @@ struct spl_info {
> > };
> >
> > static struct spl_info spl_infos[] = {
> > -     { "rk3036", "RK30", 0x1000, false, false },
> > -     { "rk3188", "RK31", 0x8000 - 0x800, true, false },
> > -     { "rk3288", "RK32", 0x8000, false, false },
> > -     { "rk3328", "RK32", 0x8000 - 0x1000, false, false },
> > +     { "rk3036", "RK30", 0x1000, false, true },
> > +     { "rk3188", "RK31", 0x8000 - 0x800, true, true },
> > +     { "rk3288", "RK32", 0x8000, false, true },
> > +     { "rk3328", "RK32", 0x8000 - 0x1000, false, true },
> >       { "rk3399", "RK33", 0x20000, false, true },
> > };
> >
> > --
> > 1.9.1
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>

[U-Boot] [RFC PATCH 5/5] rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

[Dr. Philipp Tomsich]

> On 01 Jun 2017, at 03:11, Andy Yan <andyshrk@gmail.com> wrote:
> 
> Hi Philipp:
> 
> 2017-05-31 18:58 GMT+08:00 Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
> Now that there are no remaining cases where (spl_boot0 == false), we can
> drop the spl_boot0 field from struct spl_info and also remove the if-check
> (leaving only the code-path for true in place).
> 
> 
> 
>   I think we still have one case: use mkimage to pack ddr.bin  from rockchip as spl.bin.

Excellent point! I completely missed that use-case.

However, then we’d need to implement support (an additional commandline flag?) for
communicating this into mkimage: with the current state of the code, the new code path
(i.e. assuming its a pre-padded image) will be always be taken and ddr.bin can’t be processed.

The alternative would be to clearly document that a ddr.bin needs to be prepadded and then
pad the ddr.bin manually before passing it to mkimage.

I have no preference either way, but would recommend/request that if we leave the code-path
in, then there should be a way to invoke it.

Best regards,
Philipp.

[U-Boot] [RFC PATCH 5/5] rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

[Kever Yang]

Philipp and Andy,


I will remove the "spl_boot0 == false" case in next version.

Rockchip ddr.bin is always with pre-paded TAG.

Thanks,
- Kever
On 06/01/2017 03:41 PM, Dr. Philipp Tomsich wrote:
>> On 01 Jun 2017, at 03:11, Andy Yan <andyshrk@gmail.com> wrote:
>>
>> Hi Philipp:
>>
>> 2017-05-31 18:58 GMT+08:00 Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>> Now that there are no remaining cases where (spl_boot0 == false), we can
>> drop the spl_boot0 field from struct spl_info and also remove the if-check
>> (leaving only the code-path for true in place).
>>
>>
>>
>>    I think we still have one case: use mkimage to pack ddr.bin  from rockchip as spl.bin.
> Excellent point! I completely missed that use-case.
>
> However, then we’d need to implement support (an additional commandline flag?) for
> communicating this into mkimage: with the current state of the code, the new code path
> (i.e. assuming its a pre-padded image) will be always be taken and ddr.bin can’t be processed.
>
> The alternative would be to clearly document that a ddr.bin needs to be prepadded and then
> pad the ddr.bin manually before passing it to mkimage.
>
> I have no preference either way, but would recommend/request that if we leave the code-path
> in, then there should be a way to invoke it.
>
> Best regards,
> Philipp.
>
>

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Simon Glass]

Hi Kever,

On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>
> I think the boot0 hook is suppose to add some data in the very beginning
> of the SPL image, am I right?
>
> Rockchip SoCs bootrom design is like this:
> - First 2KB or 4KB internal memory is for bootrom stack and heap;
> - Then the first 4-byte suppose to be a TAG like 'RK33';
> - The the following memory address end with '0004' is the first
>   instruction load and running by bootrom;
>
> Example for RK3288:
> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write to
> media device after mkimage like this:
>
> ff704000: 32334b52 00000000 00000000 00000000    RK32............
> ff704010: 00000000 00000000 00000000 00000000    ................
> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>
> Where the first instruction from bootrom is '00000000', which is a
> undefined instruction.
> The '_start' and 'reset' have to align to 0x20 for the requirement of
> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>
> We can use the boot0 hook to fix this issue, after this patch set,
> the SPL_TEXT_BASE is ff704000 and image write to media device after
> mkimage like this:
>
> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>
> The first instruction from bootrom is a 'b reset', and memory of
> '008'~'01c' are filled with 'nop' instruction.
>
> This patch set does not provide patch for socfpga, bcm and sunxi SoCs which also
> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
> compatible with those three platforms.
>
>
>
> Kever Yang (5):
>   armv7: move boot hook before '_start'
>   rockchip: boot0: align to 0x20 for armv7 '_start'
>   rockchip: enable BOOT0_HOOK for SoCs
>   rockchip: configs: use aligned address for SPL_TEXT_BASE
>   rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>
>  arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>  arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>  arch/arm/mach-rockchip/Kconfig             |  3 +++
>  include/configs/rk3036_common.h            |  2 +-
>  include/configs/rk3288_common.h            |  2 +-
>  tools/rkcommon.c                           |  8 ++++----
>  6 files changed, 27 insertions(+), 16 deletions(-)
>
> --
> 1.9.1
>

Do will still need this series now that (I think) we have a fix for
the return-to-brom feature in u-boot-rockchip/master?

Regards,
Simon

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Kever Yang]

Hi Andre, Steve, Marek,

     Could you help to check how to make it work with this patch on 
sunxi, bcm and socfpga platform?

Thanks,
- Kever
On 05/31/2017 06:50 PM, Kever Yang wrote:
> The boot0 hook suppose to add some data before the SPL data,
> let's move it at very begining and before '_start'.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
>   arch/arm/lib/vectors.S | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index f53b1e9..b4cd825 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -35,6 +35,16 @@
>   
>   	.section ".vectors", "ax"
>   
> +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
> +/*
> + * Various SoCs need something special and SoC-specific up front in
> + * order to boot, allow them to set that in their boot0.h file and then
> + * use it here.
> + */
> +#include <asm/arch/boot0.h>
> +
> +#endif
> +
>   /*
>    *************************************************************************
>    *
> @@ -60,15 +70,6 @@ _start:
>   	ldr	pc, _irq
>   	ldr	pc, _fiq
>   
> -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
> -/*
> - * Various SoCs need something special and SoC-specific up front in
> - * order to boot, allow them to set that in their boot0.h file and then
> - * use it here.
> - */
> -#include <asm/arch/boot0.h>
> -#endif
> -
>   /*
>    *************************************************************************
>    *

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Kever Yang]

Simon,


On 06/07/2017 05:08 AM, Simon Glass wrote:
> Hi Kever,
>
> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>> I think the boot0 hook is suppose to add some data in the very beginning
>> of the SPL image, am I right?
>>
>> Rockchip SoCs bootrom design is like this:
>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>> - The the following memory address end with '0004' is the first
>>    instruction load and running by bootrom;
>>
>> Example for RK3288:
>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write to
>> media device after mkimage like this:
>>
>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>> ff704010: 00000000 00000000 00000000 00000000    ................
>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>
>> Where the first instruction from bootrom is '00000000', which is a
>> undefined instruction.
>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>
>> We can use the boot0 hook to fix this issue, after this patch set,
>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>> mkimage like this:
>>
>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>
>> The first instruction from bootrom is a 'b reset', and memory of
>> '008'~'01c' are filled with 'nop' instruction.
>>
>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs which also
>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>> compatible with those three platforms.
>>
>>
>>
>> Kever Yang (5):
>>    armv7: move boot hook before '_start'
>>    rockchip: boot0: align to 0x20 for armv7 '_start'
>>    rockchip: enable BOOT0_HOOK for SoCs
>>    rockchip: configs: use aligned address for SPL_TEXT_BASE
>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>
>>   arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>   arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>   arch/arm/mach-rockchip/Kconfig             |  3 +++
>>   include/configs/rk3036_common.h            |  2 +-
>>   include/configs/rk3288_common.h            |  2 +-
>>   tools/rkcommon.c                           |  8 ++++----
>>   6 files changed, 27 insertions(+), 16 deletions(-)
>>
>> --
>> 1.9.1
>>
> Do will still need this series now that (I think) we have a fix for
> the return-to-brom feature in u-boot-rockchip/master?

Could you point me out exactly which fix do you talking about?

This is not about return-to-brom, it's about the first instruction from 
Bootrom to SPL.
So this is need for all Rockchip armv7 SoCs.

Thanks,
- Kever
> Regards,
> Simon
>

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Simon Glass]

Hi Kever,

On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
> Simon,
>
>
>
> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>
>> Hi Kever,
>>
>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>
>>> I think the boot0 hook is suppose to add some data in the very beginning
>>> of the SPL image, am I right?
>>>
>>> Rockchip SoCs bootrom design is like this:
>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>> - The the following memory address end with '0004' is the first
>>>    instruction load and running by bootrom;
>>>
>>> Example for RK3288:
>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write to
>>> media device after mkimage like this:
>>>
>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>
>>> Where the first instruction from bootrom is '00000000', which is a
>>> undefined instruction.
>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>
>>> We can use the boot0 hook to fix this issue, after this patch set,
>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>> mkimage like this:
>>>
>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>
>>> The first instruction from bootrom is a 'b reset', and memory of
>>> '008'~'01c' are filled with 'nop' instruction.
>>>
>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>> which also
>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>> compatible with those three platforms.
>>>
>>>
>>>
>>> Kever Yang (5):
>>>    armv7: move boot hook before '_start'
>>>    rockchip: boot0: align to 0x20 for armv7 '_start'
>>>    rockchip: enable BOOT0_HOOK for SoCs
>>>    rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>
>>>   arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>   arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>   arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>   include/configs/rk3036_common.h            |  2 +-
>>>   include/configs/rk3288_common.h            |  2 +-
>>>   tools/rkcommon.c                           |  8 ++++----
>>>   6 files changed, 27 insertions(+), 16 deletions(-)
>>>
>>> --
>>> 1.9.1
>>>
>> Do will still need this series now that (I think) we have a fix for
>> the return-to-brom feature in u-boot-rockchip/master?
>
>
> Could you point me out exactly which fix do you talking about?

These ones:

a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the boot ROM
ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
99c700c794 rockchip: mkimage: add support for verify_header/print_header

>
> This is not about return-to-brom, it's about the first instruction from
> Bootrom to SPL.
> So this is need for all Rockchip armv7 SoCs.

OK, how did we survive before? What has changed to make this series needed?

Regards,
Simon

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Marek Vasut]

On 06/07/2017 04:28 AM, Kever Yang wrote:
> Hi Andre, Steve, Marek,
> 
>     Could you help to check how to make it work with this patch on
> sunxi, bcm and socfpga platform?

The socfpga expects the hook at that exact position (0x40 I think) , so
if you moved it somewhere, you broke socfpga.

> Thanks,
> - Kever
> On 05/31/2017 06:50 PM, Kever Yang wrote:
>> The boot0 hook suppose to add some data before the SPL data,
>> let's move it at very begining and before '_start'.
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>>   arch/arm/lib/vectors.S | 19 ++++++++++---------
>>   1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
>> index f53b1e9..b4cd825 100644
>> --- a/arch/arm/lib/vectors.S
>> +++ b/arch/arm/lib/vectors.S
>> @@ -35,6 +35,16 @@
>>         .section ".vectors", "ax"
>>   +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>> +/*
>> + * Various SoCs need something special and SoC-specific up front in
>> + * order to boot, allow them to set that in their boot0.h file and then
>> + * use it here.
>> + */
>> +#include <asm/arch/boot0.h>
>> +
>> +#endif
>> +
>>   /*
>>   
>> *************************************************************************
>>    *
>> @@ -60,15 +70,6 @@ _start:
>>       ldr    pc, _irq
>>       ldr    pc, _fiq
>>   -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>> -/*
>> - * Various SoCs need something special and SoC-specific up front in
>> - * order to boot, allow them to set that in their boot0.h file and then
>> - * use it here.
>> - */
>> -#include <asm/arch/boot0.h>
>> -#endif
>> -
>>   /*
>>   
>> *************************************************************************
>>    *
> 
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Kever Yang]

Simon,


On 06/07/2017 11:15 AM, Simon Glass wrote:
> Hi Kever,
>
> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>> Simon,
>>
>>
>>
>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>> Hi Kever,
>>>
>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> I think the boot0 hook is suppose to add some data in the very beginning
>>>> of the SPL image, am I right?
>>>>
>>>> Rockchip SoCs bootrom design is like this:
>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>> - The the following memory address end with '0004' is the first
>>>>     instruction load and running by bootrom;
>>>>
>>>> Example for RK3288:
>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write to
>>>> media device after mkimage like this:
>>>>
>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>
>>>> Where the first instruction from bootrom is '00000000', which is a
>>>> undefined instruction.
>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>
>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>> mkimage like this:
>>>>
>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>
>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>
>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>> which also
>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>> compatible with those three platforms.
>>>>
>>>>
>>>>
>>>> Kever Yang (5):
>>>>     armv7: move boot hook before '_start'
>>>>     rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>     rockchip: enable BOOT0_HOOK for SoCs
>>>>     rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>     rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>
>>>>    arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>    arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>    arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>    include/configs/rk3036_common.h            |  2 +-
>>>>    include/configs/rk3288_common.h            |  2 +-
>>>>    tools/rkcommon.c                           |  8 ++++----
>>>>    6 files changed, 27 insertions(+), 16 deletions(-)
>>>>
>>>> --
>>>> 1.9.1
>>>>
>>> Do will still need this series now that (I think) we have a fix for
>>> the return-to-brom feature in u-boot-rockchip/master?
>>
>> Could you point me out exactly which fix do you talking about?
> These ones:
>
> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the boot ROM
> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>
>> This is not about return-to-brom, it's about the first instruction from
>> Bootrom to SPL.
>> So this is need for all Rockchip armv7 SoCs.
> OK, how did we survive before? What has changed to make this series needed?

After check with JTAG, I find that I'm wrong with cmd code '00000000',
this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.

I still want this patch set applied because it's better to make all the 
Rockchip's
SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
can replace each other easily and work with one mkimage tool.

Thanks,
- Kever
>
> Regards,
> Simon
>

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Simon Glass]

Hi Kever,

On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
> Simon,
>
>
>
> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>
>> Hi Kever,
>>
>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>
>>> Simon,
>>>
>>>
>>>
>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>
>>>> Hi Kever,
>>>>
>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>
>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>> beginning
>>>>> of the SPL image, am I right?
>>>>>
>>>>> Rockchip SoCs bootrom design is like this:
>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>> - The the following memory address end with '0004' is the first
>>>>>     instruction load and running by bootrom;
>>>>>
>>>>> Example for RK3288:
>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>> to
>>>>> media device after mkimage like this:
>>>>>
>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>
>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>> undefined instruction.
>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>
>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>> mkimage like this:
>>>>>
>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>
>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>
>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>> which also
>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>> compatible with those three platforms.
>>>>>
>>>>>
>>>>>
>>>>> Kever Yang (5):
>>>>>     armv7: move boot hook before '_start'
>>>>>     rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>     rockchip: enable BOOT0_HOOK for SoCs
>>>>>     rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>     rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>
>>>>>    arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>    arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>    arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>    include/configs/rk3036_common.h            |  2 +-
>>>>>    include/configs/rk3288_common.h            |  2 +-
>>>>>    tools/rkcommon.c                           |  8 ++++----
>>>>>    6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>
>>>>> --
>>>>> 1.9.1
>>>>>
>>>> Do will still need this series now that (I think) we have a fix for
>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>
>>>
>>> Could you point me out exactly which fix do you talking about?
>>
>> These ones:
>>
>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>> boot ROM
>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>
>>> This is not about return-to-brom, it's about the first instruction from
>>> Bootrom to SPL.
>>> So this is need for all Rockchip armv7 SoCs.
>>
>> OK, how did we survive before? What has changed to make this series
>> needed?
>
>
> After check with JTAG, I find that I'm wrong with cmd code '00000000',
> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>
> I still want this patch set applied because it's better to make all the
> Rockchip's
> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
> can replace each other easily and work with one mkimage tool.

I'm not sure how to apply this since on the other thread[1] Marek says
it will break socfpga.

Would it be possible to change the DDR binary to not include the padding?

Regards,
Simon

[1] armv7: move boot hook before '_start'

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Dr. Philipp Tomsich]

Simon,

> On 09 Jun 2017, at 14:27, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Kever,
> 
> On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
>> Simon,
>> 
>> 
>> 
>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>> 
>>> Hi Kever,
>>> 
>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> 
>>>> Simon,
>>>> 
>>>> 
>>>> 
>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>> 
>>>>> Hi Kever,
>>>>> 
>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>> 
>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>> beginning
>>>>>> of the SPL image, am I right?
>>>>>> 
>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>> - The the following memory address end with '0004' is the first
>>>>>>    instruction load and running by bootrom;
>>>>>> 
>>>>>> Example for RK3288:
>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>> to
>>>>>> media device after mkimage like this:
>>>>>> 
>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>> 
>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>> undefined instruction.
>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>> 
>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>> mkimage like this:
>>>>>> 
>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>> 
>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>> 
>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>> which also
>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>> compatible with those three platforms.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> Kever Yang (5):
>>>>>>    armv7: move boot hook before '_start'
>>>>>>    rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>    rockchip: enable BOOT0_HOOK for SoCs
>>>>>>    rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>> 
>>>>>>   arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>   arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>   arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>   include/configs/rk3036_common.h            |  2 +-
>>>>>>   include/configs/rk3288_common.h            |  2 +-
>>>>>>   tools/rkcommon.c                           |  8 ++++----
>>>>>>   6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>> 
>>>>>> --
>>>>>> 1.9.1
>>>>>> 
>>>>> Do will still need this series now that (I think) we have a fix for
>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>> 
>>>> 
>>>> Could you point me out exactly which fix do you talking about?
>>> 
>>> These ones:
>>> 
>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>> boot ROM
>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>> 
>>>> This is not about return-to-brom, it's about the first instruction from
>>>> Bootrom to SPL.
>>>> So this is need for all Rockchip armv7 SoCs.
>>> 
>>> OK, how did we survive before? What has changed to make this series
>>> needed?
>> 
>> 
>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>> 
>> I still want this patch set applied because it's better to make all the
>> Rockchip's
>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>> can replace each other easily and work with one mkimage tool.
> 
> I'm not sure how to apply this since on the other thread[1] Marek says
> it will break socfpga.

To me it looks as if we need to fix the BOOT0 handling across all ARMv7
platforms, as it looks as if the current implementation and its documentation
contradict each other.

Here’s how BOOT0 was intended:

1. from Kconfig:
          If the SoC's BOOT0 requires a header area filled with (magic)
          values, then choose this option, and create a define called
          ARM_SOC_BOOT0_HOOK which contains the required assembler
          preprocessor code.

2. from the code in arch/arm/lib/vectors.S:
/*
 * Various SoCs need something special and SoC-specific up front in
 * order to boot, allow them to set that in their boot0.h file and then
 * use it here.
 */

Can we just resolve this by changing arch/arm/lib/vectors.S to replace
the entire content starting after the “_start:” label with the BOOT0 hook,
if one is defined?
This would then make it the responsibiliy of the respective BOOT0 hook
to appropriately insert the vectors before or after its other magic and
allow all architectures to do whatever their boot ROM requires...

> Would it be possible to change the DDR binary to not include the padding?

I would rather have all SPL binaries for Rockchip prepadded (and even
insert the appropriate ‘RK30’ .. ‘RK33’ word from asm/arch-rockchip/boot0.h
instead of from mkimage) as this is how the Rockchip ROM defines
a SPL payload… which in turn makes the usage of the boot0-hook exactly
what the docs for the SOC_BOOT0_HOOK want.

> [1] armv7: move boot hook before '_start'


Regards,
Philipp.

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Simon Glass]

Hi Philipp,

On 9 June 2017 at 06:59, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Simon,
>
>> On 09 Jun 2017, at 14:27, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Kever,
>>
>> On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> Simon,
>>>
>>>
>>>
>>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>>>
>>>> Hi Kever,
>>>>
>>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>
>>>>> Simon,
>>>>>
>>>>>
>>>>>
>>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>>>
>>>>>> Hi Kever,
>>>>>>
>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>>
>>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>>> beginning
>>>>>>> of the SPL image, am I right?
>>>>>>>
>>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>>> - The the following memory address end with '0004' is the first
>>>>>>>    instruction load and running by bootrom;
>>>>>>>
>>>>>>> Example for RK3288:
>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>>> to
>>>>>>> media device after mkimage like this:
>>>>>>>
>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>
>>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>>> undefined instruction.
>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>>
>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>>> mkimage like this:
>>>>>>>
>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>
>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>>
>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>>> which also
>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>>> compatible with those three platforms.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Kever Yang (5):
>>>>>>>    armv7: move boot hook before '_start'
>>>>>>>    rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>>    rockchip: enable BOOT0_HOOK for SoCs
>>>>>>>    rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>>
>>>>>>>   arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>>   arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>>   arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>>   include/configs/rk3036_common.h            |  2 +-
>>>>>>>   include/configs/rk3288_common.h            |  2 +-
>>>>>>>   tools/rkcommon.c                           |  8 ++++----
>>>>>>>   6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>>
>>>>>>> --
>>>>>>> 1.9.1
>>>>>>>
>>>>>> Do will still need this series now that (I think) we have a fix for
>>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>>
>>>>>
>>>>> Could you point me out exactly which fix do you talking about?
>>>>
>>>> These ones:
>>>>
>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>>> boot ROM
>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>>>
>>>>> This is not about return-to-brom, it's about the first instruction from
>>>>> Bootrom to SPL.
>>>>> So this is need for all Rockchip armv7 SoCs.
>>>>
>>>> OK, how did we survive before? What has changed to make this series
>>>> needed?
>>>
>>>
>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>>
>>> I still want this patch set applied because it's better to make all the
>>> Rockchip's
>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>>> can replace each other easily and work with one mkimage tool.
>>
>> I'm not sure how to apply this since on the other thread[1] Marek says
>> it will break socfpga.
>
> To me it looks as if we need to fix the BOOT0 handling across all ARMv7
> platforms, as it looks as if the current implementation and its documentation
> contradict each other.
>
> Here’s how BOOT0 was intended:
>
> 1. from Kconfig:
>           If the SoC's BOOT0 requires a header area filled with (magic)
>           values, then choose this option, and create a define called
>           ARM_SOC_BOOT0_HOOK which contains the required assembler
>           preprocessor code.
>
> 2. from the code in arch/arm/lib/vectors.S:
> /*
>  * Various SoCs need something special and SoC-specific up front in
>  * order to boot, allow them to set that in their boot0.h file and then
>  * use it here.
>  */
>
> Can we just resolve this by changing arch/arm/lib/vectors.S to replace
> the entire content starting after the “_start:” label with the BOOT0 hook,
> if one is defined?
> This would then make it the responsibiliy of the respective BOOT0 hook
> to appropriately insert the vectors before or after its other magic and
> allow all architectures to do whatever their boot ROM requires...
>
>> Would it be possible to change the DDR binary to not include the padding?
>
> I would rather have all SPL binaries for Rockchip prepadded (and even
> insert the appropriate ‘RK30’ .. ‘RK33’ word from asm/arch-rockchip/boot0.h
> instead of from mkimage) as this is how the Rockchip ROM defines
> a SPL payload… which in turn makes the usage of the boot0-hook exactly
> what the docs for the SOC_BOOT0_HOOK want.
>
>> [1] armv7: move boot hook before '_start'

Well OK. So how many SoCs are affected by this? Is it just socfpga and
sunxi? How do we move them over?

Regards,
Simon

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Kever Yang]

Hi Simon,


On 06/09/2017 08:27 PM, Simon Glass wrote:
> Hi Kever,
>
> On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
>> Simon,
>>
>>
>>
>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>> Hi Kever,
>>>
>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> Simon,
>>>>
>>>>
>>>>
>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>> Hi Kever,
>>>>>
>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>> beginning
>>>>>> of the SPL image, am I right?
>>>>>>
>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>> - The the following memory address end with '0004' is the first
>>>>>>      instruction load and running by bootrom;
>>>>>>
>>>>>> Example for RK3288:
>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>> to
>>>>>> media device after mkimage like this:
>>>>>>
>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>
>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>> undefined instruction.
>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>
>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>> mkimage like this:
>>>>>>
>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>
>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>
>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>> which also
>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>> compatible with those three platforms.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Kever Yang (5):
>>>>>>      armv7: move boot hook before '_start'
>>>>>>      rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>      rockchip: enable BOOT0_HOOK for SoCs
>>>>>>      rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>      rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>
>>>>>>     arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>     arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>     arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>     include/configs/rk3036_common.h            |  2 +-
>>>>>>     include/configs/rk3288_common.h            |  2 +-
>>>>>>     tools/rkcommon.c                           |  8 ++++----
>>>>>>     6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 1.9.1
>>>>>>
>>>>> Do will still need this series now that (I think) we have a fix for
>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>
>>>> Could you point me out exactly which fix do you talking about?
>>> These ones:
>>>
>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>> boot ROM
>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>>
>>>> This is not about return-to-brom, it's about the first instruction from
>>>> Bootrom to SPL.
>>>> So this is need for all Rockchip armv7 SoCs.
>>> OK, how did we survive before? What has changed to make this series
>>> needed?
>>
>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>
>> I still want this patch set applied because it's better to make all the
>> Rockchip's
>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>> can replace each other easily and work with one mkimage tool.
> I'm not sure how to apply this since on the other thread[1] Marek says
> it will break socfpga.

I know this must break socfpga platform, and as I have said in cover letter,
I think this patch also break bcm and sunxi platform, I add 'RFC' for 
this patch
set because I don't know the detail requirement of hook0 for other 3 
platform,
I'm not asking if this patch set break other 3 platforms or not, but I'm 
asking for
the solution for those 3 platform so that we can apply this patch set.

I think my patch is correct in what HOOK0 wants to do, just like 
Philipp's opinion
in another mail.
I hope to get help from maintainer for socfpga, bcm, sunxi platform.

Thanks,
- Kever
>
> Would it be possible to change the DDR binary to not include the padding?
>
> Regards,
> Simon
>
> [1] armv7: move boot hook before '_start'
>

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Kever Yang]

Hi Marek,


On 06/07/2017 02:28 PM, Marek Vasut wrote:
> On 06/07/2017 04:28 AM, Kever Yang wrote:
>> Hi Andre, Steve, Marek,
>>
>>      Could you help to check how to make it work with this patch on
>> sunxi, bcm and socfpga platform?
> The socfpga expects the hook at that exact position (0x40 I think) , so
> if you moved it somewhere, you broke socfpga.

I know this break socfpga, and that's why I cc you for help, could you
help the take a look if we can have a solution on socfpga to "make it
work with this patch"?

Comments from Philipp[0] show that its reasonable for this patch set,
I don't know how the boot0-hook works in socfpga and also sunxi and bcm,
could you help me to fix the hook content upon this patch?

Thanks,
- Kever
[0]https://www.mail-archive.com/u-boot at lists.denx.de/msg252823.html
>
>> Thanks,
>> - Kever
>> On 05/31/2017 06:50 PM, Kever Yang wrote:
>>> The boot0 hook suppose to add some data before the SPL data,
>>> let's move it at very begining and before '_start'.
>>>
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> ---
>>>
>>>    arch/arm/lib/vectors.S | 19 ++++++++++---------
>>>    1 file changed, 10 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
>>> index f53b1e9..b4cd825 100644
>>> --- a/arch/arm/lib/vectors.S
>>> +++ b/arch/arm/lib/vectors.S
>>> @@ -35,6 +35,16 @@
>>>          .section ".vectors", "ax"
>>>    +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>>> +/*
>>> + * Various SoCs need something special and SoC-specific up front in
>>> + * order to boot, allow them to set that in their boot0.h file and then
>>> + * use it here.
>>> + */
>>> +#include <asm/arch/boot0.h>
>>> +
>>> +#endif
>>> +
>>>    /*
>>>    
>>> *************************************************************************
>>>     *
>>> @@ -60,15 +70,6 @@ _start:
>>>        ldr    pc, _irq
>>>        ldr    pc, _fiq
>>>    -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>>> -/*
>>> - * Various SoCs need something special and SoC-specific up front in
>>> - * order to boot, allow them to set that in their boot0.h file and then
>>> - * use it here.
>>> - */
>>> -#include <asm/arch/boot0.h>
>>> -#endif
>>> -
>>>    /*
>>>    
>>> *************************************************************************
>>>     *
>>
>

[U-Boot] [U-Boot, RFC, 2/5] rockchip: boot0: align to 0x20 for armv7 '_start'

[Kever Yang]

Hi Philipp,


On 06/10/2017 02:47 AM, Philipp Tomsich wrote:
>
> On Wed, 31 May 2017, Kever Yang wrote:
>
>> The '_start' is using as vector table base address, and will write
>> to VBAR register, need to align to 0x20 for armv7.
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> arch/arm/include/asm/arch-rockchip/boot0.h | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h 
>> b/arch/arm/include/asm/arch-rockchip/boot0.h
>> index 7346876..88f0ff6 100644
>> --- a/arch/arm/include/asm/arch-rockchip/boot0.h
>> +++ b/arch/arm/include/asm/arch-rockchip/boot0.h
>> @@ -13,10 +13,17 @@
>>  */
>>
>> #ifdef CONFIG_SPL_BUILD
>> -    .space 0x4         /* space for the 'RK33' */
>> +    .space 0x4         /* space for Rockchip SoC tag like 'RK33' */
>> #endif
>>     b reset
>>
>> +/* For armv7, the addr '_start' will used as vector start address
>> + * and write to VBAR register, which need to aligned to 0x20.
>> + */
>> +#ifdef CONFIG_CPU_V7
>> +    .align(5)
>> +#endif
>> +
>> #if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD)
>>     .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM    /* space for the ATF 
>> data */
>> #endif
>>
>
> You'll need to adapt this for whatever fix we decide on for those
> platforms that need their boot0-hook after the vector-table (e.g.
> socfpga).

Thanks very much for your review and comments, I know we need fix other 
platforms
like socfpga, bcm and sunxi, that's why this patch set prefixed with 'RFC'.
It looks like the U-Boot mailing list is not very active and there is 
not response from
bcm or sunxi although I CCed the author who commit the patch for boot0-hook
in other platform, and Marek only replied with "you broke socfpga" 
without a solution.

Maybe I need to wait more time.

Thanks,
- Kever
>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Dr. Philipp Tomsich]

> On 13 Jun 2017, at 03:40, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Hi Simon,
> 
> 
> On 06/09/2017 08:27 PM, Simon Glass wrote:
>> Hi Kever,
>> 
>> On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> Simon,
>>> 
>>> 
>>> 
>>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>>> Hi Kever,
>>>> 
>>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>> Simon,
>>>>> 
>>>>> 
>>>>> 
>>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>>> Hi Kever,
>>>>>> 
>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>>> beginning
>>>>>>> of the SPL image, am I right?
>>>>>>> 
>>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>>> - The the following memory address end with '0004' is the first
>>>>>>>     instruction load and running by bootrom;
>>>>>>> 
>>>>>>> Example for RK3288:
>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>>> to
>>>>>>> media device after mkimage like this:
>>>>>>> 
>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>> 
>>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>>> undefined instruction.
>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>> 
>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>>> mkimage like this:
>>>>>>> 
>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>> 
>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>> 
>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>>> which also
>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>>> compatible with those three platforms.
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> Kever Yang (5):
>>>>>>>     armv7: move boot hook before '_start'
>>>>>>>     rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>>     rockchip: enable BOOT0_HOOK for SoCs
>>>>>>>     rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>>     rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>> 
>>>>>>>    arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>>    arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>>    arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>>    include/configs/rk3036_common.h            |  2 +-
>>>>>>>    include/configs/rk3288_common.h            |  2 +-
>>>>>>>    tools/rkcommon.c                           |  8 ++++----
>>>>>>>    6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>> 
>>>>>>> --
>>>>>>> 1.9.1
>>>>>>> 
>>>>>> Do will still need this series now that (I think) we have a fix for
>>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>> 
>>>>> Could you point me out exactly which fix do you talking about?
>>>> These ones:
>>>> 
>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>>> boot ROM
>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>>> 
>>>>> This is not about return-to-brom, it's about the first instruction from
>>>>> Bootrom to SPL.
>>>>> So this is need for all Rockchip armv7 SoCs.
>>>> OK, how did we survive before? What has changed to make this series
>>>> needed?
>>> 
>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>> 
>>> I still want this patch set applied because it's better to make all the
>>> Rockchip's
>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>>> can replace each other easily and work with one mkimage tool.
>> I'm not sure how to apply this since on the other thread[1] Marek says
>> it will break socfpga.
> 
> I know this must break socfpga platform, and as I have said in cover letter,
> I think this patch also break bcm and sunxi platform, I add 'RFC' for this patch
> set because I don't know the detail requirement of hook0 for other 3 platform,
> I'm not asking if this patch set break other 3 platforms or not, but I'm asking for
> the solution for those 3 platform so that we can apply this patch set.

The following platforms have a boot0-hook implemented:

./arch/arm/include/asm/arch-rockchip/boot0.h
./arch/arm/include/asm/arch-bcm235xx/boot0.h
./arch/arm/include/asm/arch-sunxi/boot0.h
./arch/arm/include/asm/arch-bcm281xx/boot0.h
./arch/arm/mach-socfpga/include/mach/boot0.h

> I think my patch is correct in what HOOK0 wants to do, just like Philipp's opinion
> in another mail.
> I hope to get help from maintainer for socfpga, bcm, sunxi platform.
> 
> Thanks,
> - Kever
>> 
>> Would it be possible to change the DDR binary to not include the padding?
>> 
>> Regards,
>> Simon
>> 
>> [1] armv7: move boot hook before '_start'

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Marek Vasut]

On 06/13/2017 03:51 AM, Kever Yang wrote:
> Hi Marek,

Hi,

> On 06/07/2017 02:28 PM, Marek Vasut wrote:
>> On 06/07/2017 04:28 AM, Kever Yang wrote:
>>> Hi Andre, Steve, Marek,
>>>
>>>      Could you help to check how to make it work with this patch on
>>> sunxi, bcm and socfpga platform?
>> The socfpga expects the hook at that exact position (0x40 I think) , so
>> if you moved it somewhere, you broke socfpga.
> 
> I know this break socfpga, and that's why I cc you for help, could you
> help the take a look if we can have a solution on socfpga to "make it
> work with this patch"?

Can you generate the same u-boot binary with this patch ? If so, then it
will work on socfpga. SoCFPGA expects that small piece of stuff at
offset 0x40 , so if you move this boot0 hook, it will break. HTH

> Comments from Philipp[0] show that its reasonable for this patch set,
> I don't know how the boot0-hook works in socfpga and also sunxi and bcm,
> could you help me to fix the hook content upon this patch?
> 
> Thanks,
> - Kever
> [0]https://www.mail-archive.com/u-boot at lists.denx.de/msg252823.html
>>
>>> Thanks,
>>> - Kever
>>> On 05/31/2017 06:50 PM, Kever Yang wrote:
>>>> The boot0 hook suppose to add some data before the SPL data,
>>>> let's move it at very begining and before '_start'.
>>>>
>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>> ---
>>>>
>>>>    arch/arm/lib/vectors.S | 19 ++++++++++---------
>>>>    1 file changed, 10 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
>>>> index f53b1e9..b4cd825 100644
>>>> --- a/arch/arm/lib/vectors.S
>>>> +++ b/arch/arm/lib/vectors.S
>>>> @@ -35,6 +35,16 @@
>>>>          .section ".vectors", "ax"
>>>>    +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>>>> +/*
>>>> + * Various SoCs need something special and SoC-specific up front in
>>>> + * order to boot, allow them to set that in their boot0.h file and
>>>> then
>>>> + * use it here.
>>>> + */
>>>> +#include <asm/arch/boot0.h>
>>>> +
>>>> +#endif
>>>> +
>>>>    /*
>>>>   
>>>> *************************************************************************
>>>>
>>>>     *
>>>> @@ -60,15 +70,6 @@ _start:
>>>>        ldr    pc, _irq
>>>>        ldr    pc, _fiq
>>>>    -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>>>> -/*
>>>> - * Various SoCs need something special and SoC-specific up front in
>>>> - * order to boot, allow them to set that in their boot0.h file and
>>>> then
>>>> - * use it here.
>>>> - */
>>>> -#include <asm/arch/boot0.h>
>>>> -#endif
>>>> -
>>>>    /*
>>>>   
>>>> *************************************************************************
>>>>
>>>>     *
>>>
>>
> 
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Dr. Philipp Tomsich]

Marek,

> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de> wrote:
> 
> On 06/13/2017 03:51 AM, Kever Yang wrote:
>> Hi Marek,
> 
> Hi,
> 
>> On 06/07/2017 02:28 PM, Marek Vasut wrote:
>>> On 06/07/2017 04:28 AM, Kever Yang wrote:
>>>> Hi Andre, Steve, Marek,
>>>> 
>>>>     Could you help to check how to make it work with this patch on
>>>> sunxi, bcm and socfpga platform?
>>> The socfpga expects the hook at that exact position (0x40 I think) , so
>>> if you moved it somewhere, you broke socfpga.
>> 
>> I know this break socfpga, and that's why I cc you for help, could you
>> help the take a look if we can have a solution on socfpga to "make it
>> work with this patch"?
> 
> Can you generate the same u-boot binary with this patch ? If so, then it
> will work on socfpga. SoCFPGA expects that small piece of stuff at
> offset 0x40 , so if you move this boot0 hook, it will break. HTH

So the SoCFPGA-implementation relies on the boot0 hook to always start
at 0x40?  Then this needs fixing anyway… or it will break once someone
else touches the armv7 startup code.

That said, I read your comment to mean that the following changes would
work for SoCFPGA:
1.	arch/arm/lib/vectors.S:
		#if CONFIG_IS_ENABLED(BOOT0_HOOK)
			… pull in the boot0 hook, which needs to contain the vectors
		#else
			b reset
			… remaining vectors … 
		#endif
2.	boot0.h (for socfpga):
		b reset
		… remaining vectors …
		/* now at 0x40 */
		… whatever currently lives in the boot0.h for socfpga ...

Ok?

> 
>> Comments from Philipp[0] show that its reasonable for this patch set,
>> I don't know how the boot0-hook works in socfpga and also sunxi and bcm,
>> could you help me to fix the hook content upon this patch?
>> 
>> Thanks,
>> - Kever
>> [0]https://www.mail-archive.com/u-boot at lists.denx.de/msg252823.html
>>> 
>>>> Thanks,
>>>> - Kever
>>>> On 05/31/2017 06:50 PM, Kever Yang wrote:
>>>>> The boot0 hook suppose to add some data before the SPL data,
>>>>> let's move it at very begining and before '_start'.
>>>>> 
>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>>> ---
>>>>> 
>>>>>   arch/arm/lib/vectors.S | 19 ++++++++++---------
>>>>>   1 file changed, 10 insertions(+), 9 deletions(-)
>>>>> 
>>>>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
>>>>> index f53b1e9..b4cd825 100644
>>>>> --- a/arch/arm/lib/vectors.S
>>>>> +++ b/arch/arm/lib/vectors.S
>>>>> @@ -35,6 +35,16 @@
>>>>>         .section ".vectors", "ax"
>>>>>   +#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>>>>> +/*
>>>>> + * Various SoCs need something special and SoC-specific up front in
>>>>> + * order to boot, allow them to set that in their boot0.h file and
>>>>> then
>>>>> + * use it here.
>>>>> + */
>>>>> +#include <asm/arch/boot0.h>
>>>>> +
>>>>> +#endif
>>>>> +
>>>>>   /*
>>>>> 
>>>>> *************************************************************************
>>>>> 
>>>>>    *
>>>>> @@ -60,15 +70,6 @@ _start:
>>>>>       ldr    pc, _irq
>>>>>       ldr    pc, _fiq
>>>>>   -#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
>>>>> -/*
>>>>> - * Various SoCs need something special and SoC-specific up front in
>>>>> - * order to boot, allow them to set that in their boot0.h file and
>>>>> then
>>>>> - * use it here.
>>>>> - */
>>>>> -#include <asm/arch/boot0.h>
>>>>> -#endif
>>>>> -
>>>>>   /*
>>>>> 
>>>>> *************************************************************************
>>>>> 
>>>>>    *
>>>> 
>>> 
>> 
>> 
> 
> 
> -- 
> Best regards,
> Marek Vasut

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Marek Vasut]

On 06/13/2017 11:44 AM, Dr. Philipp Tomsich wrote:
> Marek,
> 
>> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de
>> <mailto:marex@denx.de>> wrote:
>>
>> On 06/13/2017 03:51 AM, Kever Yang wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On 06/07/2017 02:28 PM, Marek Vasut wrote:
>>>> On 06/07/2017 04:28 AM, Kever Yang wrote:
>>>>> Hi Andre, Steve, Marek,
>>>>>
>>>>>     Could you help to check how to make it work with this patch on
>>>>> sunxi, bcm and socfpga platform?
>>>> The socfpga expects the hook at that exact position (0x40 I think) , so
>>>> if you moved it somewhere, you broke socfpga.
>>>
>>> I know this break socfpga, and that's why I cc you for help, could you
>>> help the take a look if we can have a solution on socfpga to "make it
>>> work with this patch"?
>>
>> Can you generate the same u-boot binary with this patch ? If so, then it
>> will work on socfpga. SoCFPGA expects that small piece of stuff at
>> offset 0x40 , so if you move this boot0 hook, it will break. HTH
> 
> So the SoCFPGA-implementation relies on the boot0 hook to always start
> at 0x40?  Then this needs fixing anyway… or it will break once someone
> else touches the armv7 startup code.

I think it's not just the SoCFPGA.

> That said, I read your comment to mean that the following changes would
> work for SoCFPGA:
> 1.arch/arm/lib/vectors.S:
> #if CONFIG_IS_ENABLED(BOOT0_HOOK)
> … pull in the boot0 hook, which needs to contain the vectors
> #else
> b reset
> … remaining vectors … 
> #endif
> 2.boot0.h (for socfpga):
> b reset
> … remaining vectors …
> /* now at 0x40 */
> … whatever currently lives in the boot0.h for socfpga ...
> 
> Ok?

I'm not sure I understand what you're trying to say. IIRC the SoCFPGA
bootrom checks the content at 0x40-0x48 in SRAM and then jumps to 0x50
if the content is valid. Otherwise it reloads the content from
SD/SPI/NAND...

[...]

btw can you please disable the HTML email ?

-- 
Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Dr. Philipp Tomsich]

> On 13 Jun 2017, at 11:47, Marek Vasut <marex@denx.de> wrote:
> 
> On 06/13/2017 11:44 AM, Dr. Philipp Tomsich wrote:
>> Marek,
>> 
>>> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de
>>> <mailto:marex@denx.de>> wrote:
>>> 
>>> On 06/13/2017 03:51 AM, Kever Yang wrote:
>>>> Hi Marek,
>>> 
>>> Hi,
>>> 
>>>> On 06/07/2017 02:28 PM, Marek Vasut wrote:
>>>>> On 06/07/2017 04:28 AM, Kever Yang wrote:
>>>>>> Hi Andre, Steve, Marek,
>>>>>> 
>>>>>>    Could you help to check how to make it work with this patch on
>>>>>> sunxi, bcm and socfpga platform?
>>>>> The socfpga expects the hook at that exact position (0x40 I think) , so
>>>>> if you moved it somewhere, you broke socfpga.
>>>> 
>>>> I know this break socfpga, and that's why I cc you for help, could you
>>>> help the take a look if we can have a solution on socfpga to "make it
>>>> work with this patch"?
>>> 
>>> Can you generate the same u-boot binary with this patch ? If so, then it
>>> will work on socfpga. SoCFPGA expects that small piece of stuff at
>>> offset 0x40 , so if you move this boot0 hook, it will break. HTH
>> 
>> So the SoCFPGA-implementation relies on the boot0 hook to always start
>> at 0x40?  Then this needs fixing anyway… or it will break once someone
>> else touches the armv7 startup code.
> 
> I think it's not just the SoCFPGA.

What I am saying is that the boot0-hook is intended to add a header
before the actual U-Boot code (as Andre had originally stated in his
commit message and as various comments around the boot0 hook
also suggest): 

    arm/arm64: implement a boot header capability
    
    Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0)
    require a header before the actual U-Boot binary to both check its
    validity and to find other data to load. Sometimes this header may
    only be a few bytes of information, and sometimes this might simply
    be space that needs to be reserved for a post-processing tool.

However, the actual start of the boot0-hook in the U-Boot today is right
after the vector-table (which is 0x40 long), which has a few interesting
consequences for various architectures:
- sunxi: doesn’t matter
- socfpga: boot0 hook relies on something being in front of it, as it uses
	a ".balign 64, …” to advance to 0x40
- rockchip: comes too late to allow inserting the word-sized field at the
	very start (and to move the vectors back by a word).

Now having looked at the implementation in
	arch/arm/mach-socfpga/include/mach/boot0.h
it is clear why Kever’s change breaks this code: .balign will not forward
to 0x40 (after all, 0x0 is also 64-byte aligned).

Once the boot0 hook is at the beginning of the binary (as always intended),
this will need to become a .space or .fill directive.

>> That said, I read your comment to mean that the following changes would
>> work for SoCFPGA:
>> 1.arch/arm/lib/vectors.S:
>> #if CONFIG_IS_ENABLED(BOOT0_HOOK)
>> … pull in the boot0 hook, which needs to contain the vectors
>> #else
>> b reset
>> … remaining vectors … 
>> #endif
>> 2.boot0.h (for socfpga):
>> b reset
>> … remaining vectors …
>> /* now at 0x40 */
>> … whatever currently lives in the boot0.h for socfpga ...
>> 
>> Ok?
> 
> I'm not sure I understand what you're trying to say. IIRC the SoCFPGA
> bootrom checks the content at 0x40-0x48 in SRAM and then jumps to 0x50
> if the content is valid. Otherwise it reloads the content from
> SD/SPI/NAND…

Thanks for the background info. With this, it should be possible to create
a new version of the patch that also correctly handles socfpga.

Regards,
Philipp.

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Jagan Teki]

On Tue, Jun 13, 2017 at 1:14 PM, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>> On 13 Jun 2017, at 03:40, Kever Yang <kever.yang@rock-chips.com> wrote:
>>
>> Hi Simon,
>>
>>
>> On 06/09/2017 08:27 PM, Simon Glass wrote:
>>> Hi Kever,
>>>
>>> On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> Simon,
>>>>
>>>>
>>>>
>>>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>>>> Hi Kever,
>>>>>
>>>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>> Simon,
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>>>> Hi Kever,
>>>>>>>
>>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>>>> beginning
>>>>>>>> of the SPL image, am I right?
>>>>>>>>
>>>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>>>> - The the following memory address end with '0004' is the first
>>>>>>>>     instruction load and running by bootrom;
>>>>>>>>
>>>>>>>> Example for RK3288:
>>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>>>> to
>>>>>>>> media device after mkimage like this:
>>>>>>>>
>>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>>
>>>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>>>> undefined instruction.
>>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>>>
>>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>>>> mkimage like this:
>>>>>>>>
>>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>>
>>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>>>
>>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>>>> which also
>>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>>>> compatible with those three platforms.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Kever Yang (5):
>>>>>>>>     armv7: move boot hook before '_start'
>>>>>>>>     rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>>>     rockchip: enable BOOT0_HOOK for SoCs
>>>>>>>>     rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>>>     rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>>>
>>>>>>>>    arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>>>    arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>>>    arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>>>    include/configs/rk3036_common.h            |  2 +-
>>>>>>>>    include/configs/rk3288_common.h            |  2 +-
>>>>>>>>    tools/rkcommon.c                           |  8 ++++----
>>>>>>>>    6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>>>
>>>>>>>> --
>>>>>>>> 1.9.1
>>>>>>>>
>>>>>>> Do will still need this series now that (I think) we have a fix for
>>>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>>>
>>>>>> Could you point me out exactly which fix do you talking about?
>>>>> These ones:
>>>>>
>>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>>>> boot ROM
>>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>>>>
>>>>>> This is not about return-to-brom, it's about the first instruction from
>>>>>> Bootrom to SPL.
>>>>>> So this is need for all Rockchip armv7 SoCs.
>>>>> OK, how did we survive before? What has changed to make this series
>>>>> needed?
>>>>
>>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>>>
>>>> I still want this patch set applied because it's better to make all the
>>>> Rockchip's
>>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>>>> can replace each other easily and work with one mkimage tool.
>>> I'm not sure how to apply this since on the other thread[1] Marek says
>>> it will break socfpga.
>>
>> I know this must break socfpga platform, and as I have said in cover letter,
>> I think this patch also break bcm and sunxi platform, I add 'RFC' for this patch
>> set because I don't know the detail requirement of hook0 for other 3 platform,
>> I'm not asking if this patch set break other 3 platforms or not, but I'm asking for
>> the solution for those 3 platform so that we can apply this patch set.
>
> The following platforms have a boot0-hook implemented:
>
> ./arch/arm/include/asm/arch-rockchip/boot0.h
> ./arch/arm/include/asm/arch-bcm235xx/boot0.h
> ./arch/arm/include/asm/arch-sunxi/boot0.h
> ./arch/arm/include/asm/arch-bcm281xx/boot0.h
> ./arch/arm/mach-socfpga/include/mach/boot0.h
>
>> I think my patch is correct in what HOOK0 wants to do, just like Philipp's opinion
>> in another mail.
>> I hope to get help from maintainer for socfpga, bcm, sunxi platform.

for sunxi32 boot0 hook does normal execution (b reset) but the image
changed 4 bytes with this change.

w/o the change:
0000000 0016 ea00 4765 4e4f 422e 3054 cf32 4cc6
0000010 6000 0000 5053 024c 0000 0000 0000 0000

with the change:
0000000 0016 ea00 4765 4e4f 422e 3054 da15 4b6f
0000010 6000 0000 5053 024c 0000 0000 0000 0000

of-course board not booting.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Jagan Teki]

On Fri, Jun 9, 2017 at 6:29 PM, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Simon,
>
>> On 09 Jun 2017, at 14:27, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Kever,
>>
>> On 7 June 2017 at 04:55, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> Simon,
>>>
>>>
>>>
>>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>>>
>>>> Hi Kever,
>>>>
>>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>
>>>>> Simon,
>>>>>
>>>>>
>>>>>
>>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>>>
>>>>>> Hi Kever,
>>>>>>
>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>>
>>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>>> beginning
>>>>>>> of the SPL image, am I right?
>>>>>>>
>>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>>> - The the following memory address end with '0004' is the first
>>>>>>>    instruction load and running by bootrom;
>>>>>>>
>>>>>>> Example for RK3288:
>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>>> to
>>>>>>> media device after mkimage like this:
>>>>>>>
>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>
>>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>>> undefined instruction.
>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>>
>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>>> mkimage like this:
>>>>>>>
>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>
>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>>
>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>>> which also
>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>>> compatible with those three platforms.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Kever Yang (5):
>>>>>>>    armv7: move boot hook before '_start'
>>>>>>>    rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>>    rockchip: enable BOOT0_HOOK for SoCs
>>>>>>>    rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>>
>>>>>>>   arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>>   arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>>   arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>>   include/configs/rk3036_common.h            |  2 +-
>>>>>>>   include/configs/rk3288_common.h            |  2 +-
>>>>>>>   tools/rkcommon.c                           |  8 ++++----
>>>>>>>   6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>>
>>>>>>> --
>>>>>>> 1.9.1
>>>>>>>
>>>>>> Do will still need this series now that (I think) we have a fix for
>>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>>
>>>>>
>>>>> Could you point me out exactly which fix do you talking about?
>>>>
>>>> These ones:
>>>>
>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>>> boot ROM
>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>>>
>>>>> This is not about return-to-brom, it's about the first instruction from
>>>>> Bootrom to SPL.
>>>>> So this is need for all Rockchip armv7 SoCs.
>>>>
>>>> OK, how did we survive before? What has changed to make this series
>>>> needed?
>>>
>>>
>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>>
>>> I still want this patch set applied because it's better to make all the
>>> Rockchip's
>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>>> can replace each other easily and work with one mkimage tool.
>>
>> I'm not sure how to apply this since on the other thread[1] Marek says
>> it will break socfpga.
>
> To me it looks as if we need to fix the BOOT0 handling across all ARMv7
> platforms, as it looks as if the current implementation and its documentation
> contradict each other.
>
> Here’s how BOOT0 was intended:
>
> 1. from Kconfig:
>           If the SoC's BOOT0 requires a header area filled with (magic)
>           values, then choose this option, and create a define called
>           ARM_SOC_BOOT0_HOOK which contains the required assembler
>           preprocessor code.
>
> 2. from the code in arch/arm/lib/vectors.S:
> /*
>  * Various SoCs need something special and SoC-specific up front in
>  * order to boot, allow them to set that in their boot0.h file and then
>  * use it here.
>  */
>
> Can we just resolve this by changing arch/arm/lib/vectors.S to replace
> the entire content starting after the “_start:” label with the BOOT0 hook,
> if one is defined?
> This would then make it the responsibiliy of the respective BOOT0 hook
> to appropriately insert the vectors before or after its other magic and
> allow all architectures to do whatever their boot ROM requires...

I don't think placing boot0 hook after _start will resolve (I thought
you mentioned the same here) and look like placing boot0 hook
insertion before or after _start results the same based on the
generated image.I've checked this and find the same 4-byte change in
hexdump.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Dr. Philipp Tomsich]

Jagan,

> On 13 Jun 2017, at 19:48, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> 
>>>>>>> 
>>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>>> 
>>>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>>>> beginning
>>>>>>>> of the SPL image, am I right?
>>>>>>>> 
>>>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>>>> - The the following memory address end with '0004' is the first
>>>>>>>>   instruction load and running by bootrom;
>>>>>>>> 
>>>>>>>> Example for RK3288:
>>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>>>> to
>>>>>>>> media device after mkimage like this:
>>>>>>>> 
>>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>> 
>>>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>>>> undefined instruction.
>>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>>> 
>>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>>>> mkimage like this:
>>>>>>>> 
>>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>> 
>>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>>> 
>>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>>>> which also
>>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>>>> compatible with those three platforms.
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Kever Yang (5):
>>>>>>>>   armv7: move boot hook before '_start'
>>>>>>>>   rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>>>   rockchip: enable BOOT0_HOOK for SoCs
>>>>>>>>   rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>>>   rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>>> 
>>>>>>>>  arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>>>  arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>>>  arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>>>  include/configs/rk3036_common.h            |  2 +-
>>>>>>>>  include/configs/rk3288_common.h            |  2 +-
>>>>>>>>  tools/rkcommon.c                           |  8 ++++----
>>>>>>>>  6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>>> 
>>>>>>>> --
>>>>>>>> 1.9.1
>>>>>>>> 
>>>>>>> Do will still need this series now that (I think) we have a fix for
>>>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>>> 
>>>>>> 
>>>>>> Could you point me out exactly which fix do you talking about?
>>>>> 
>>>>> These ones:
>>>>> 
>>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>>>> boot ROM
>>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>>>> 
>>>>>> This is not about return-to-brom, it's about the first instruction from
>>>>>> Bootrom to SPL.
>>>>>> So this is need for all Rockchip armv7 SoCs.
>>>>> 
>>>>> OK, how did we survive before? What has changed to make this series
>>>>> needed?
>>>> 
>>>> 
>>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>>> 
>>>> I still want this patch set applied because it's better to make all the
>>>> Rockchip's
>>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>>>> can replace each other easily and work with one mkimage tool.
>>> 
>>> I'm not sure how to apply this since on the other thread[1] Marek says
>>> it will break socfpga.
>> 
>> To me it looks as if we need to fix the BOOT0 handling across all ARMv7
>> platforms, as it looks as if the current implementation and its documentation
>> contradict each other.
>> 
>> Here’s how BOOT0 was intended:
>> 
>> 1. from Kconfig:
>>          If the SoC's BOOT0 requires a header area filled with (magic)
>>          values, then choose this option, and create a define called
>>          ARM_SOC_BOOT0_HOOK which contains the required assembler
>>          preprocessor code.
>> 
>> 2. from the code in arch/arm/lib/vectors.S:
>> /*
>> * Various SoCs need something special and SoC-specific up front in
>> * order to boot, allow them to set that in their boot0.h file and then
>> * use it here.
>> */
>> 
>> Can we just resolve this by changing arch/arm/lib/vectors.S to replace
>> the entire content starting after the “_start:” label with the BOOT0 hook,
>> if one is defined?
>> This would then make it the responsibiliy of the respective BOOT0 hook
>> to appropriately insert the vectors before or after its other magic and
>> allow all architectures to do whatever their boot ROM requires...
> 
> I don't think placing boot0 hook after _start will resolve (I thought
> you mentioned the same here) and look like placing boot0 hook
> insertion before or after _start results the same based on the
> generated image.I've checked this and find the same 4-byte change in
> hexdump.

I’ll look at what’s going on there. Which defconfig did you try with?

Thanks,
Philipp.

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Jagan Teki]

On Jun 13, 2017 11:37 PM, "Dr. Philipp Tomsich" <
philipp.tomsich@theobroma-systems.com> wrote:

Jagan,

> On 13 Jun 2017, at 19:48, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>
>>>>>>>
>>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang@rock-chips.com>
wrote:
>>>>>>>>
>>>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>>>> beginning
>>>>>>>> of the SPL image, am I right?
>>>>>>>>
>>>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>>>> - The the following memory address end with '0004' is the first
>>>>>>>>   instruction load and running by bootrom;
>>>>>>>>
>>>>>>>> Example for RK3288:
>>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image
write
>>>>>>>> to
>>>>>>>> media device after mkimage like this:
>>>>>>>>
>>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>>
>>>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>>>> undefined instruction.
>>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement
of
>>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>>>>
>>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>>>> mkimage like this:
>>>>>>>>
>>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>>>>
>>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>>>>
>>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi
SoCs
>>>>>>>> which also
>>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to
make it
>>>>>>>> compatible with those three platforms.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Kever Yang (5):
>>>>>>>>   armv7: move boot hook before '_start'
>>>>>>>>   rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>>>   rockchip: enable BOOT0_HOOK for SoCs
>>>>>>>>   rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>>>   rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>>>>
>>>>>>>>  arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>>>  arch/arm/lib/vectors.S                     | 19
++++++++++---------
>>>>>>>>  arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>>>  include/configs/rk3036_common.h            |  2 +-
>>>>>>>>  include/configs/rk3288_common.h            |  2 +-
>>>>>>>>  tools/rkcommon.c                           |  8 ++++----
>>>>>>>>  6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>>>>
>>>>>>>> --
>>>>>>>> 1.9.1
>>>>>>>>
>>>>>>> Do will still need this series now that (I think) we have a fix for
>>>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>>>>
>>>>>>
>>>>>> Could you point me out exactly which fix do you talking about?
>>>>>
>>>>> These ones:
>>>>>
>>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing
the
>>>>> boot ROM
>>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>>>> 99c700c794 rockchip: mkimage: add support for
verify_header/print_header
>>>>>
>>>>>> This is not about return-to-brom, it's about the first instruction
from
>>>>>> Bootrom to SPL.
>>>>>> So this is need for all Rockchip armv7 SoCs.
>>>>>
>>>>> OK, how did we survive before? What has changed to make this series
>>>>> needed?
>>>>
>>>>
>>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>>>>
>>>> I still want this patch set applied because it's better to make all the
>>>> Rockchip's
>>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr
binary
>>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set,
we
>>>> can replace each other easily and work with one mkimage tool.
>>>
>>> I'm not sure how to apply this since on the other thread[1] Marek says
>>> it will break socfpga.
>>
>> To me it looks as if we need to fix the BOOT0 handling across all ARMv7
>> platforms, as it looks as if the current implementation and its
documentation
>> contradict each other.
>>
>> Here’s how BOOT0 was intended:
>>
>> 1. from Kconfig:
>>          If the SoC's BOOT0 requires a header area filled with (magic)
>>          values, then choose this option, and create a define called
>>          ARM_SOC_BOOT0_HOOK which contains the required assembler
>>          preprocessor code.
>>
>> 2. from the code in arch/arm/lib/vectors.S:
>> /*
>> * Various SoCs need something special and SoC-specific up front in
>> * order to boot, allow them to set that in their boot0.h file and then
>> * use it here.
>> */
>>
>> Can we just resolve this by changing arch/arm/lib/vectors.S to replace
>> the entire content starting after the “_start:” label with the BOOT0
hook,
>> if one is defined?
>> This would then make it the responsibiliy of the respective BOOT0 hook
>> to appropriately insert the vectors before or after its other magic and
>> allow all architectures to do whatever their boot ROM requires...
>
> I don't think placing boot0 hook after _start will resolve (I thought
> you mentioned the same here) and look like placing boot0 hook
> insertion before or after _start results the same based on the
> generated image.I've checked this and find the same 4-byte change in
> hexdump.

I’ll look at what’s going on there. Which defconfig did you try with?


orangepi_pc

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Dr. Philipp Tomsich]

> On 13 Jun 2017, at 20:14, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> 
> 
> 
> On Jun 13, 2017 11:37 PM, "Dr. Philipp Tomsich" <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
> Jagan,
> 
> > On 13 Jun 2017, at 19:48, Jagan Teki <jagannadh.teki at gmail.com <mailto:jagannadh.teki@gmail.com>> wrote:
> >
> >>>>>>>
> >>>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote:
> >>>>>>>>
> >>>>>>>> I think the boot0 hook is suppose to add some data in the very
> >>>>>>>> beginning
> >>>>>>>> of the SPL image, am I right?
> >>>>>>>>
> >>>>>>>> Rockchip SoCs bootrom design is like this:
> >>>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
> >>>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
> >>>>>>>> - The the following memory address end with '0004' is the first
> >>>>>>>>   instruction load and running by bootrom;
> >>>>>>>>
> >>>>>>>> Example for RK3288:
> >>>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
> >>>>>>>> to
> >>>>>>>> media device after mkimage like this:
> >>>>>>>>
> >>>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
> >>>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
> >>>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
> >>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
> >>>>>>>>
> >>>>>>>> Where the first instruction from bootrom is '00000000', which is a
> >>>>>>>> undefined instruction.
> >>>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
> >>>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
> >>>>>>>>
> >>>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
> >>>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
> >>>>>>>> mkimage like this:
> >>>>>>>>
> >>>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
> >>>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
> >>>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
> >>>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
> >>>>>>>>
> >>>>>>>> The first instruction from bootrom is a 'b reset', and memory of
> >>>>>>>> '008'~'01c' are filled with 'nop' instruction.
> >>>>>>>>
> >>>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
> >>>>>>>> which also
> >>>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
> >>>>>>>> compatible with those three platforms.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Kever Yang (5):
> >>>>>>>>   armv7: move boot hook before '_start'
> >>>>>>>>   rockchip: boot0: align to 0x20 for armv7 '_start'
> >>>>>>>>   rockchip: enable BOOT0_HOOK for SoCs
> >>>>>>>>   rockchip: configs: use aligned address for SPL_TEXT_BASE
> >>>>>>>>   rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
> >>>>>>>>
> >>>>>>>>  arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
> >>>>>>>>  arch/arm/lib/vectors.S                     | 19 ++++++++++---------
> >>>>>>>>  arch/arm/mach-rockchip/Kconfig             |  3 +++
> >>>>>>>>  include/configs/rk3036_common.h            |  2 +-
> >>>>>>>>  include/configs/rk3288_common.h            |  2 +-
> >>>>>>>>  tools/rkcommon.c                           |  8 ++++----
> >>>>>>>>  6 files changed, 27 insertions(+), 16 deletions(-)
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> 1.9.1
> >>>>>>>>
> >>>>>>> Do will still need this series now that (I think) we have a fix for
> >>>>>>> the return-to-brom feature in u-boot-rockchip/master?
> >>>>>>
> >>>>>>
> >>>>>> Could you point me out exactly which fix do you talking about?
> >>>>>
> >>>>> These ones:
> >>>>>
> >>>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
> >>>>> boot ROM
> >>>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
> >>>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
> >>>>>
> >>>>>> This is not about return-to-brom, it's about the first instruction from
> >>>>>> Bootrom to SPL.
> >>>>>> So this is need for all Rockchip armv7 SoCs.
> >>>>>
> >>>>> OK, how did we survive before? What has changed to make this series
> >>>>> needed?
> >>>>
> >>>>
> >>>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
> >>>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
> >>>>
> >>>> I still want this patch set applied because it's better to make all the
> >>>> Rockchip's
> >>>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
> >>>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
> >>>> can replace each other easily and work with one mkimage tool.
> >>>
> >>> I'm not sure how to apply this since on the other thread[1] Marek says
> >>> it will break socfpga.
> >>
> >> To me it looks as if we need to fix the BOOT0 handling across all ARMv7
> >> platforms, as it looks as if the current implementation and its documentation
> >> contradict each other.
> >>
> >> Here’s how BOOT0 was intended:
> >>
> >> 1. from Kconfig:
> >>          If the SoC's BOOT0 requires a header area filled with (magic)
> >>          values, then choose this option, and create a define called
> >>          ARM_SOC_BOOT0_HOOK which contains the required assembler
> >>          preprocessor code.
> >>
> >> 2. from the code in arch/arm/lib/vectors.S:
> >> /*
> >> * Various SoCs need something special and SoC-specific up front in
> >> * order to boot, allow them to set that in their boot0.h file and then
> >> * use it here.
> >> */
> >>
> >> Can we just resolve this by changing arch/arm/lib/vectors.S to replace
> >> the entire content starting after the “_start:” label with the BOOT0 hook,
> >> if one is defined?
> >> This would then make it the responsibiliy of the respective BOOT0 hook
> >> to appropriately insert the vectors before or after its other magic and
> >> allow all architectures to do whatever their boot ROM requires...
> >
> > I don't think placing boot0 hook after _start will resolve (I thought
> > you mentioned the same here) and look like placing boot0 hook
> > insertion before or after _start results the same based on the
> > generated image.I've checked this and find the same 4-byte change in
> > hexdump.
> 
> I’ll look at what’s going on there. Which defconfig did you try with?
> 
> orangepi_pc

I must be doing something wrong, as this doesn’t set the BOOT0_HOOK
(this was against the mailine master @ 8cb3ce64f936f5dedbcfc1935c5caf31bb682474):

ptomsich at android:~/rk3399-spl/boot0/u-boot$ make CROSS_COMPILE=arm-unknown-eabi- ARCH=arm -j8 orangepi_pc_defconfig && grep BOOT0 .config
#
# configuration written to .config
#
# CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK is not set
# CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER is not set

[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

[Dr. Philipp Tomsich]

Jagan,

> On 13 Jun 2017, at 19:10, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> 
>>> I know this must break socfpga platform, and as I have said in cover letter,
>>> I think this patch also break bcm and sunxi platform, I add 'RFC' for this patch
>>> set because I don't know the detail requirement of hook0 for other 3 platform,
>>> I'm not asking if this patch set break other 3 platforms or not, but I'm asking for
>>> the solution for those 3 platform so that we can apply this patch set.
>> 
>> The following platforms have a boot0-hook implemented:
>> 
>> ./arch/arm/include/asm/arch-rockchip/boot0.h
>> ./arch/arm/include/asm/arch-bcm235xx/boot0.h
>> ./arch/arm/include/asm/arch-sunxi/boot0.h
>> ./arch/arm/include/asm/arch-bcm281xx/boot0.h
>> ./arch/arm/mach-socfpga/include/mach/boot0.h
>> 
>>> I think my patch is correct in what HOOK0 wants to do, just like Philipp's opinion
>>> in another mail.
>>> I hope to get help from maintainer for socfpga, bcm, sunxi platform.
> 
> for sunxi32 boot0 hook does normal execution (b reset) but the image
> changed 4 bytes with this change.
> 
> w/o the change:
> 0000000 0016 ea00 4765 4e4f 422e 3054 cf32 4cc6
> 0000010 6000 0000 5053 024c 0000 0000 0000 0000
> 
> with the change:
> 0000000 0016 ea00 4765 4e4f 422e 3054 da15 4b6f
> 0000010 6000 0000 5053 024c 0000 0000 0000 0000
> 
> of-course board not booting.

You may be looking at the wrong part of the image, as this is
part of the header (the code from vectors.S starts at 0x60):

00000000  16 00 00 ea 65 47 4f 4e  2e 42 54 30 24 45 26 97  |....eGON.BT0$E&.|
00000010  00 60 00 00 53 50 4c 02  00 00 00 00 00 00 00 00  |.`..SPL.........|
00000020  2c 00 00 00 00 00 00 00  00 00 00 00 73 75 6e 38  |,...........sun8|
00000030  69 2d 68 33 2d 6f 72 61  6e 67 65 70 69 2d 70 63  |i-h3-orangepi-pc|
00000040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000060  0f 00 00 ea 14 f0 9f e5  14 f0 9f e5 14 f0 9f e5  |................|
00000070  14 f0 9f e5 14 f0 9f e5  14 f0 9f e5 14 f0 9f e5  |................|
00000080  a0 00 00 00 a0 00 00 00  a0 00 00 00 a0 00 00 00  |................|
00000090  a0 00 00 00 a0 00 00 00  a0 00 00 00 ef be ad de  |…………….|

When looking at the header-definition in spl.h, the changed word is the length:

/* boot head definition from sun4i boot code */
struct boot_file_head {
        uint32_t b_instruction; /* one intruction jumping to real code */
        uint8_t magic[8];       /* ="eGON.BT0" or "eGON.BT1", not C-style str */
        uint32_t check_sum;     /* generated by PC */
        uint32_t length;        /* generated by PC */

With Kever’s change I’d expect something to happen at 0x60.
And if it moves code around there, it’s likely to break the mksunxiboot.c
implementation, which builds the instruction:
        /* fill the header */
        img.header.b_instruction =      /* b instruction */
                0xEA000000 |    /* jump to the first instr after the header */
                ((sizeof(struct boot_file_head) / sizeof(int) - 2)
                 & 0x00FFFFFF);

In fact, I now wonder whether most of the header chould already
be generated by the boot0-hook, such that only the length and
the checksum would need to be patched in by mksunxiboot.c… 
…but that is me taking the second step before the first ;-)

—Phil.

[U-Boot] [RFC PATCH 1/5] armv7: move boot hook before '_start'

[Simon Glass]

Hi,

On 13 June 2017 at 04:31, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>> On 13 Jun 2017, at 11:47, Marek Vasut <marex@denx.de> wrote:
>>
>> On 06/13/2017 11:44 AM, Dr. Philipp Tomsich wrote:
>>> Marek,
>>>
>>>> On 13 Jun 2017, at 10:42, Marek Vasut <marex@denx.de
>>>> <mailto:marex@denx.de>> wrote:
>>>>
>>>> On 06/13/2017 03:51 AM, Kever Yang wrote:
>>>>> Hi Marek,
>>>>
>>>> Hi,
>>>>
>>>>> On 06/07/2017 02:28 PM, Marek Vasut wrote:
>>>>>> On 06/07/2017 04:28 AM, Kever Yang wrote:
>>>>>>> Hi Andre, Steve, Marek,
>>>>>>>
>>>>>>>    Could you help to check how to make it work with this patch on
>>>>>>> sunxi, bcm and socfpga platform?
>>>>>> The socfpga expects the hook at that exact position (0x40 I think) , so
>>>>>> if you moved it somewhere, you broke socfpga.
>>>>>
>>>>> I know this break socfpga, and that's why I cc you for help, could you
>>>>> help the take a look if we can have a solution on socfpga to "make it
>>>>> work with this patch"?
>>>>
>>>> Can you generate the same u-boot binary with this patch ? If so, then it
>>>> will work on socfpga. SoCFPGA expects that small piece of stuff at
>>>> offset 0x40 , so if you move this boot0 hook, it will break. HTH
>>>
>>> So the SoCFPGA-implementation relies on the boot0 hook to always start
>>> at 0x40?  Then this needs fixing anyway… or it will break once someone
>>> else touches the armv7 startup code.
>>
>> I think it's not just the SoCFPGA.
>
> What I am saying is that the boot0-hook is intended to add a header
> before the actual U-Boot code (as Andre had originally stated in his
> commit message and as various comments around the boot0 hook
> also suggest):
>
>     arm/arm64: implement a boot header capability
>
>     Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0)
>     require a header before the actual U-Boot binary to both check its
>     validity and to find other data to load. Sometimes this header may
>     only be a few bytes of information, and sometimes this might simply
>     be space that needs to be reserved for a post-processing tool.
>
> However, the actual start of the boot0-hook in the U-Boot today is right
> after the vector-table (which is 0x40 long), which has a few interesting
> consequences for various architectures:
> - sunxi: doesn’t matter
> - socfpga: boot0 hook relies on something being in front of it, as it uses
>         a ".balign 64, …” to advance to 0x40
> - rockchip: comes too late to allow inserting the word-sized field at the
>         very start (and to move the vectors back by a word).
>
> Now having looked at the implementation in
>         arch/arm/mach-socfpga/include/mach/boot0.h
> it is clear why Kever’s change breaks this code: .balign will not forward
> to 0x40 (after all, 0x0 is also 64-byte aligned).
>
> Once the boot0 hook is at the beginning of the binary (as always intended),
> this will need to become a .space or .fill directive.
>
>>> That said, I read your comment to mean that the following changes would
>>> work for SoCFPGA:
>>> 1.arch/arm/lib/vectors.S:
>>> #if CONFIG_IS_ENABLED(BOOT0_HOOK)
>>> … pull in the boot0 hook, which needs to contain the vectors
>>> #else
>>> b reset
>>> … remaining vectors …
>>> #endif
>>> 2.boot0.h (for socfpga):
>>> b reset
>>> … remaining vectors …
>>> /* now at 0x40 */
>>> … whatever currently lives in the boot0.h for socfpga ...
>>>
>>> Ok?
>>
>> I'm not sure I understand what you're trying to say. IIRC the SoCFPGA
>> bootrom checks the content at 0x40-0x48 in SRAM and then jumps to 0x50
>> if the content is valid. Otherwise it reloads the content from
>> SD/SPI/NAND…
>
> Thanks for the background info. With this, it should be possible to create
> a new version of the patch that also correctly handles socfpga.

Sounds like this is figured out, great!

- Simon