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 ?

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

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.

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

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

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.

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!

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

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'

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!