[U-Boot] rk3288: broken boot

[U-Boot] rk3288: broken boot

[Jagan Teki]

Hi Philipp/Kever,

Issue observed on rk3288 TPL supported board (vyasa), worked on
previous release v2017.11.

Tried to bisect but i couldn't do it because of changes in many files any help?

thanks!
-- 
Jagan Teki
Senior Linux Kernel Engineer | Amarula Solutions
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] rk3288: broken boot

[Dr. Philipp Tomsich]

Jagan,

> On 15 Dec 2017, at 08:08, Jagan Teki <jagan@amarulasolutions.com> wrote:
> 
> Hi Philipp/Kever,
> 
> Issue observed on rk3288 TPL supported board (vyasa), worked on
> previous release v2017.11.

What exactly is the issue (“broken boot” is not very specific)?

> Tried to bisect but i couldn't do it because of changes in many files any help?

I don’t have a RK3288-based target system.
So I won’t be able to do a bysect myself.

Regards,
Philipp.

> 
> thanks!
> -- 
> Jagan Teki
> Senior Linux Kernel Engineer | Amarula Solutions
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.

[U-Boot] rk3288: broken boot

[Kever Yang]

Hi Jagan,

     Could you enable global DEBUG and share the log?

Thanks,

- Kever

On 12/15/2017 03:08 PM, Jagan Teki wrote:
> Hi Philipp/Kever,
>
> Issue observed on rk3288 TPL supported board (vyasa), worked on
> previous release v2017.11.
>
> Tried to bisect but i couldn't do it because of changes in many files any help?
>
> thanks!

[U-Boot] rk3288: broken boot

[Jagan Teki]

On Fri, Dec 15, 2017 at 2:45 PM, Kever Yang <kever.yang@rock-chips.com> wrote:
> Hi Jagan,
>
>     Could you enable global DEBUG and share the log?
>
> Thanks,
>
> - Kever
>
>
> On 12/15/2017 03:08 PM, Jagan Teki wrote:
>>
>> Hi Philipp/Kever,
>>
>> Issue observed on rk3288 TPL supported board (vyasa), worked on
>> previous release v2017.11.
>>
>> Tried to bisect but i couldn't do it because of changes in many files any
>> help?

I don't understand how rk3288 boards were working since from 2
releases, I've bisect this and found the bad

d962e5dadc2cbc21bffd375f29665e5042879e66 is the first bad commit
commit d962e5dadc2cbc21bffd375f29665e5042879e66
Author: Kever Yang <kever.yang@rock-chips.com>
Date:   Tue Oct 10 16:21:04 2017 +0200

    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs

    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>
    [Updated:]
    Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Look like the boot magic can't find it in 0x800 and it can't overwrite
4 bytes, so we need to explicitly specify 0x804 as header size

--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -357,7 +357,7 @@ int rkcommon_vrec_header(struct image_tool_params *params,
         * 4 bytes of these images can safely be overwritten using the
         * boot magic.
         */
-       tparams->header_size = RK_SPL_HDR_START;
+       tparams->header_size = RK_SPL_HDR_START + 4;

        /* Allocate, clear and install the header */
        tparams->hdr = malloc(tparams->header_size);

Please let me know for comments.

Jagan.

[U-Boot] rk3288: broken boot

[Dr. Philipp Tomsich]

Jagan,

The boot0-header should insert the additional 4 bytes needed… the resulting
input format to mkimage is thus compatible with how the ddr.bin images by
Rockchip where originally formatted  (i.e. prepadded).

See arch/arm/include/asm/arch-rockchip/boot0.h for the insertion of these
additional 4 bytes… in the case of U-Boot we pad with a branch (unlike the
0-bytes in the prepadded legacy images), so the binary works even when
loaded in via JTAG.

For ARMv8 the insertion of the boot0 header should happen via
	arch/arm/lib/vectors.S

Please check that the padding happens as expected.
If that works, you way want to check that boot0.h is correct (unfortunately
if contains quite a few #ifdef directives) for the 3288 use-case...

Regards,
Philipp.

> On 13 Feb 2018, at 11:46, Jagan Teki <jagan@amarulasolutions.com> wrote:
> 
> On Fri, Dec 15, 2017 at 2:45 PM, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote:
>> Hi Jagan,
>> 
>>    Could you enable global DEBUG and share the log?
>> 
>> Thanks,
>> 
>> - Kever
>> 
>> 
>> On 12/15/2017 03:08 PM, Jagan Teki wrote:
>>> 
>>> Hi Philipp/Kever,
>>> 
>>> Issue observed on rk3288 TPL supported board (vyasa), worked on
>>> previous release v2017.11.
>>> 
>>> Tried to bisect but i couldn't do it because of changes in many files any
>>> help?
> 
> I don't understand how rk3288 boards were working since from 2
> releases, I've bisect this and found the bad
> 
> d962e5dadc2cbc21bffd375f29665e5042879e66 is the first bad commit
> commit d962e5dadc2cbc21bffd375f29665e5042879e66
> Author: Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>>
> Date:   Tue Oct 10 16:21:04 2017 +0200
> 
>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
> 
>    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 at rock-chips.com <mailto:kever.yang@rock-chips.com>>
>    [Updated:]
>    Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>>
> 
> Look like the boot magic can't find it in 0x800 and it can't overwrite
> 4 bytes, so we need to explicitly specify 0x804 as header size
> 
> --- a/tools/rkcommon.c
> +++ b/tools/rkcommon.c
> @@ -357,7 +357,7 @@ int rkcommon_vrec_header(struct image_tool_params *params,
>         * 4 bytes of these images can safely be overwritten using the
>         * boot magic.
>         */
> -       tparams->header_size = RK_SPL_HDR_START;
> +       tparams->header_size = RK_SPL_HDR_START + 4;
> 
>        /* Allocate, clear and install the header */
>        tparams->hdr = malloc(tparams->header_size);
> 
> Please let me know for comments.
> 
> Jagan.

[U-Boot] rk3288: broken boot

[Dr. Philipp Tomsich]

I verified the build for evb-rk3288_defconfig and everything looks fine:

spl/u-boot-spl.bin:     file format binary


Disassembly of section .data:

00000000 <.data>:
       0:       eaffffff        b       0x4	<— this is the extra branch inserted (to be overwritten by mkimage)
       4:       ea000016        b       0x64
        ...
      20:       ea00000f        b       0x64
      24:       e59ff014        ldr     pc, [pc, #20]   ; 0x40
      28:       e59ff014        ldr     pc, [pc, #20]   ; 0x44
      2c:       e59ff014        ldr     pc, [pc, #20]   ; 0x48
      30:       e59ff014        ldr     pc, [pc, #20]   ; 0x4c
      34:       e59ff014        ldr     pc, [pc, #20]   ; 0x50
      38:       e59ff014        ldr     pc, [pc, #20]   ; 0x54
      3c:       e59ff014        ldr     pc, [pc, #20]   ; 0x58


> On 13 Feb 2018, at 12:15, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
> 
> Jagan,
> 
> The boot0-header should insert the additional 4 bytes needed… the resulting
> input format to mkimage is thus compatible with how the ddr.bin images by
> Rockchip where originally formatted  (i.e. prepadded).
> 
> See arch/arm/include/asm/arch-rockchip/boot0.h for the insertion of these
> additional 4 bytes… in the case of U-Boot we pad with a branch (unlike the
> 0-bytes in the prepadded legacy images), so the binary works even when
> loaded in via JTAG.
> 
> For ARMv8 the insertion of the boot0 header should happen via
> 	arch/arm/lib/vectors.S
> 
> Please check that the padding happens as expected.
> If that works, you way want to check that boot0.h is correct (unfortunately
> if contains quite a few #ifdef directives) for the 3288 use-case...
> 
> Regards,
> Philipp.
> 
>> On 13 Feb 2018, at 11:46, Jagan Teki <jagan at amarulasolutions.com <mailto:jagan@amarulasolutions.com>> wrote:
>> 
>> On Fri, Dec 15, 2017 at 2:45 PM, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote:
>>> Hi Jagan,
>>> 
>>>    Could you enable global DEBUG and share the log?
>>> 
>>> Thanks,
>>> 
>>> - Kever
>>> 
>>> 
>>> On 12/15/2017 03:08 PM, Jagan Teki wrote:
>>>> 
>>>> Hi Philipp/Kever,
>>>> 
>>>> Issue observed on rk3288 TPL supported board (vyasa), worked on
>>>> previous release v2017.11.
>>>> 
>>>> Tried to bisect but i couldn't do it because of changes in many files any
>>>> help?
>> 
>> I don't understand how rk3288 boards were working since from 2
>> releases, I've bisect this and found the bad
>> 
>> d962e5dadc2cbc21bffd375f29665e5042879e66 is the first bad commit
>> commit d962e5dadc2cbc21bffd375f29665e5042879e66
>> Author: Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>>
>> Date:   Tue Oct 10 16:21:04 2017 +0200
>> 
>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>> 
>>    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 at rock-chips.com <mailto:kever.yang@rock-chips.com>>
>>    [Updated:]
>>    Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>>
>> 
>> Look like the boot magic can't find it in 0x800 and it can't overwrite
>> 4 bytes, so we need to explicitly specify 0x804 as header size
>> 
>> --- a/tools/rkcommon.c
>> +++ b/tools/rkcommon.c
>> @@ -357,7 +357,7 @@ int rkcommon_vrec_header(struct image_tool_params *params,
>>         * 4 bytes of these images can safely be overwritten using the
>>         * boot magic.
>>         */
>> -       tparams->header_size = RK_SPL_HDR_START;
>> +       tparams->header_size = RK_SPL_HDR_START + 4;
>> 
>>        /* Allocate, clear and install the header */
>>        tparams->hdr = malloc(tparams->header_size);
>> 
>> Please let me know for comments.
>> 
>> Jagan.
> 

[U-Boot] rk3288: broken boot

[Jagan Teki]

On Tue, Feb 13, 2018 at 5:29 PM, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> I verified the build for evb-rk3288_defconfig and everything looks fine:
>
> spl/u-boot-spl.bin:     file format binary
>
>
> Disassembly of section .data:
>
> 00000000 <.data>:
>        0:       eaffffff        b       0x4     <— this is the extra branch inserted (to be overwritten by mkimage)
>        4:       ea000016        b       0x64

I saw this on SPL, but I'm using TPL where I'm using mkimage to tp binary

>         ...
>       20:       ea00000f        b       0x64
>       24:       e59ff014        ldr     pc, [pc, #20]   ; 0x40
>       28:       e59ff014        ldr     pc, [pc, #20]   ; 0x44
>       2c:       e59ff014        ldr     pc, [pc, #20]   ; 0x48
>       30:       e59ff014        ldr     pc, [pc, #20]   ; 0x4c
>       34:       e59ff014        ldr     pc, [pc, #20]   ; 0x50
>       38:       e59ff014        ldr     pc, [pc, #20]   ; 0x54
>       3c:       e59ff014        ldr     pc, [pc, #20]   ; 0x58

I can see the difference of size by 8 bytes with spl/u-boot-spl-dtb.bin

with tparams->header_size = RK_SPL_HDR_START + 4; (working scenario)
$ ls -l spl/u-boot-spl-dtb.bin
-rw-r--r-- 1 root root 37571

normal build have
$ ls -l spl/u-boot-spl-dtb.bin
-rw-r--r-- 1 root root 37563

[U-Boot] rk3288: broken boot

[Dr. Philipp Tomsich]

Jagan,

> On 13 Feb 2018, at 13:20, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> 
> On Tue, Feb 13, 2018 at 5:29 PM, Dr. Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> I verified the build for evb-rk3288_defconfig and everything looks fine:
>> 
>> spl/u-boot-spl.bin:     file format binary
>> 
>> 
>> Disassembly of section .data:
>> 
>> 00000000 <.data>:
>>       0:       eaffffff        b       0x4     <— this is the extra branch inserted (to be overwritten by mkimage)
>>       4:       ea000016        b       0x64
> 
> I saw this on SPL, but I'm using TPL where I'm using mkimage to tp binary
> 
>>        ...
>>      20:       ea00000f        b       0x64
>>      24:       e59ff014        ldr     pc, [pc, #20]   ; 0x40
>>      28:       e59ff014        ldr     pc, [pc, #20]   ; 0x44
>>      2c:       e59ff014        ldr     pc, [pc, #20]   ; 0x48
>>      30:       e59ff014        ldr     pc, [pc, #20]   ; 0x4c
>>      34:       e59ff014        ldr     pc, [pc, #20]   ; 0x50
>>      38:       e59ff014        ldr     pc, [pc, #20]   ; 0x54
>>      3c:       e59ff014        ldr     pc, [pc, #20]   ; 0x58
> 
> I can see the difference of size by 8 bytes with spl/u-boot-spl-dtb.bin
> 
> with tparams->header_size = RK_SPL_HDR_START + 4; (working scenario)
> $ ls -l spl/u-boot-spl-dtb.bin
> -rw-r--r-- 1 root root 37571
> 
> normal build have
> $ ls -l spl/u-boot-spl-dtb.bin
> -rw-r--r-- 1 root root 37563

In order to find the issue, I’d recommend to look at both the ELF file
and the binary for your TPL stage. Disassembling the binary will show
whether the extra branch is indeed added as the first instruction and
if the second instruction is a branch to the reset entry-point. 

Regards,
Philipp.

[U-Boot] rk3288: broken boot

[Jagan Teki]

On Wed, Feb 14, 2018 at 5:38 AM, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Jagan,
>
>> On 13 Feb 2018, at 13:20, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>
>> On Tue, Feb 13, 2018 at 5:29 PM, Dr. Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com> wrote:
>>> I verified the build for evb-rk3288_defconfig and everything looks fine:
>>>
>>> spl/u-boot-spl.bin:     file format binary
>>>
>>>
>>> Disassembly of section .data:
>>>
>>> 00000000 <.data>:
>>>       0:       eaffffff        b       0x4     <— this is the extra branch inserted (to be overwritten by mkimage)
>>>       4:       ea000016        b       0x64
>>
>> I saw this on SPL, but I'm using TPL where I'm using mkimage to tp binary
>>
>>>        ...
>>>      20:       ea00000f        b       0x64
>>>      24:       e59ff014        ldr     pc, [pc, #20]   ; 0x40
>>>      28:       e59ff014        ldr     pc, [pc, #20]   ; 0x44
>>>      2c:       e59ff014        ldr     pc, [pc, #20]   ; 0x48
>>>      30:       e59ff014        ldr     pc, [pc, #20]   ; 0x4c
>>>      34:       e59ff014        ldr     pc, [pc, #20]   ; 0x50
>>>      38:       e59ff014        ldr     pc, [pc, #20]   ; 0x54
>>>      3c:       e59ff014        ldr     pc, [pc, #20]   ; 0x58
>>
>> I can see the difference of size by 8 bytes with spl/u-boot-spl-dtb.bin
>>
>> with tparams->header_size = RK_SPL_HDR_START + 4; (working scenario)
>> $ ls -l spl/u-boot-spl-dtb.bin
>> -rw-r--r-- 1 root root 37571
>>
>> normal build have
>> $ ls -l spl/u-boot-spl-dtb.bin
>> -rw-r--r-- 1 root root 37563
>
> In order to find the issue, I’d recommend to look at both the ELF file
> and the binary for your TPL stage. Disassembling the binary will show
> whether the extra branch is indeed added as the first instruction and
> if the second instruction is a branch to the reset entry-point.

This is what I found with mkimage tpl, the extra 0xeaffffff branch
inserted start with 0x820 in case of working, but branch insertion did
4 bytes prior to 0x820 [1]

https://paste.ubuntu.com/p/BbZrvJhqJD/

[U-Boot] rk3288: broken boot

[Dr. Philipp Tomsich]

> On 14 Feb 2018, at 11:47, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> 
> On Wed, Feb 14, 2018 at 5:38 AM, Dr. Philipp Tomsich
> <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
>> Jagan,
>> 
>>> On 13 Feb 2018, at 13:20, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> 
>>> On Tue, Feb 13, 2018 at 5:29 PM, Dr. Philipp Tomsich
>>> <philipp.tomsich@theobroma-systems.com> wrote:
>>>> I verified the build for evb-rk3288_defconfig and everything looks fine:
>>>> 
>>>> spl/u-boot-spl.bin:     file format binary
>>>> 
>>>> 
>>>> Disassembly of section .data:
>>>> 
>>>> 00000000 <.data>:
>>>>      0:       eaffffff        b       0x4     <— this is the extra branch inserted (to be overwritten by mkimage)
>>>>      4:       ea000016        b       0x64
>>> 
>>> I saw this on SPL, but I'm using TPL where I'm using mkimage to tp binary
>>> 
>>>>       ...
>>>>     20:       ea00000f        b       0x64
>>>>     24:       e59ff014        ldr     pc, [pc, #20]   ; 0x40
>>>>     28:       e59ff014        ldr     pc, [pc, #20]   ; 0x44
>>>>     2c:       e59ff014        ldr     pc, [pc, #20]   ; 0x48
>>>>     30:       e59ff014        ldr     pc, [pc, #20]   ; 0x4c
>>>>     34:       e59ff014        ldr     pc, [pc, #20]   ; 0x50
>>>>     38:       e59ff014        ldr     pc, [pc, #20]   ; 0x54
>>>>     3c:       e59ff014        ldr     pc, [pc, #20]   ; 0x58
>>> 
>>> I can see the difference of size by 8 bytes with spl/u-boot-spl-dtb.bin
>>> 
>>> with tparams->header_size = RK_SPL_HDR_START + 4; (working scenario)
>>> $ ls -l spl/u-boot-spl-dtb.bin
>>> -rw-r--r-- 1 root root 37571
>>> 
>>> normal build have
>>> $ ls -l spl/u-boot-spl-dtb.bin
>>> -rw-r--r-- 1 root root 37563
>> 
>> In order to find the issue, I’d recommend to look at both the ELF file
>> and the binary for your TPL stage. Disassembling the binary will show
>> whether the extra branch is indeed added as the first instruction and
>> if the second instruction is a branch to the reset entry-point.
> 
> This is what I found with mkimage tpl, the extra 0xeaffffff branch
> inserted start with 0x820 in case of working, but branch insertion did
> 4 bytes prior to 0x820 [1]
> 
> https://paste.ubuntu.com/p/BbZrvJhqJD/ <https://paste.ubuntu.com/p/BbZrvJhqJD/>
This output looks very wrong (even the one that works for you).

There shouldn’t be all the 0-bytes at the beginning and the ea000016 (‘b 0x64’)
should be at offset 4 (i.e. 0x804); the 0xeaffffff should have been at offset 0 (and
been overwritten with the ‘RK32’ at 0x800).

Apparently the tpl/u-boot-tpl.bin you feed into mkimage is already wrong: you
should check your ld-script, the ELF-file and the objcopy.  This looks as if some
part of the process is adding 7 words of padding.  The assembly injected from
the boot0 header needs to be at the start of the binary… if it is not, then something
has gone wrong before.

Thanks,
Philipp.