[U-Boot] pine64 and "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"

[André Przywara]

On 04/08/16 20:58, Siarhei Siamashka wrote:
> On Thu, 4 Aug 2016 11:40:25 -0400
> Tom Rini <trini at konsulko.com> wrote:

Hi,

thanks Siarhei for the answer and the explanation!

....

> 
>> On Thu, Aug 04, 2016 at 11:36:01AM -0400, Tom Rini wrote:
>>> On Thu, Aug 04, 2016 at 04:14:21PM +0100, Andre Przywara wrote:  
>>>> Hi,
>>>>
>>>> On 04/08/16 16:01, Tom Rini wrote:  
>>>>> Hey guys,
>>>>>
>>>>> I just started trying out my Pine64 1GB and mainline U-Boot and I've
>>>>> found that:
>>>>> commit 1a83fb4a17d959d7b037999ab7ed7e62429abe34
>>>>> Author: Siarhei Siamashka <siarhei.siamashka at gmail.com>
>>>>> Date:   Tue May 31 01:48:06 2016 +0300
>>>>>
>>>>>     sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80
>>>>>
>>>>> is breaking boot for me.  boot0 seems to run fine and then the last bit
>>>>> of output that I get is:
>>>>> boot0: start load other image
>>>>> boot0: Loading BL3-1
>>>>> Loading file 0 at address 0x40000000,size 0x00000200 success
>>>>> boot0: Loading scp
>>>>> Loading file 2 at address 0x00040000,size 0x0000c200 success
>>>>> set arisc reset to de-assert state
>>>>> Ready to disable icache.
>>>>> Jump to secend Boot.
>>>>> NOTICE:  BL3-1: Running in SRAM A2 (@0x44000)
>>>>> NOTICE:  Configuring SPC Controller
>>>>> NOTICE:  BL3-1: v1.0(debug):d697594
>>>>> NOTICE:  BL3-1: Built : 09:15:57, Aug  4 2016
>>>>> NOTICE:  Configuring AXP PMIC
>>>>> NOTICE:  PMIC: already configured for RSB
>>>>> NOTICE:  PMIC: setup successful
>>>>> INFO:    BL3-1: Initializing runtime services
>>>>> INFO:    BL3-1: Preparing for EL3 exit to normal world
>>>>> INFO:    BL3-1: Next image address: 0x4a000000, SPSR: 0x3c9
>>>>>
>>>>> I'm using d697594 from
>>>>> https://github.com/apritzel/arm-trusted-firmware.git for ATF and
>>>>> boot0.img from the 20160701 Debian "longsleep" image.  Any ideas?  
>>>>
>>>> Yes, we were experiencing similar issues, basically it's stuck in
>>>> start.S, as far as I could quickly check.
>>>> Siarhei and I were doubtful that this commit (which we eyed at before)
>>>> could be the culprit, as it _should_ affect only SPL code, which we
>>>> don't use atm.  
> 
> Well, it does affect the main U-Boot binary too, as we checked some
> days ago on IRC. I was about to send some patches to address this issue.
> 
>>> It's not touching SPL code tho.  You're changing CONFIG_SYS_INIT_SP_ADDR
>>> which is the start of _main in arch/arm/lib/crt0_64.S
>>>   
>>
>> Or to be extra clear:
>> @@ -100,7 +100,7 @@
>>   * the 1 actually activates the mapping of the first 32 KiB to 0x00000000.
>>   */
>>  #define CONFIG_SYS_INIT_RAM_ADDR       0x10000
>> -#define CONFIG_SYS_INIT_RAM_SIZE       0x08000 /* FIXME: 40 KiB ? */
>> +#define CONFIG_SYS_INIT_RAM_SIZE       0xA000  /* 40 KiB */
>>  #else
>>  #define CONFIG_SYS_INIT_RAM_ADDR       0x0
>>  #define CONFIG_SYS_INIT_RAM_SIZE       0x8000  /* 32 KiB */
>>
>> is changing CONFIG_SYS_INIT_SP_ADDR which is in U-Boot proper.  To
>> change the SPL stack pointer you use CONFIG_SPL_STACK which is what the
>> second hunk tweaks.
> 
> Yes, except that apparently there is a bug in
> "arch/arm/cpu/armv7/lowlevel_init.S" and it also uses
> CONFIG_SYS_INIT_SP_ADDR for the SPL.
> 
> It is safe to revert this patch right now and come up with a better fix
> later. The patch only tries to ensure the availability of more space
> for the SPL, because the SPL code size was bigger than 24K on
> Pine64 before switching to tiny printf.

OK, I must have missed that part. So it indeed looks like reverting is
the best option for now.

> The root cause of all these troubles is that on Allwinner A64 the
> SRAM C is apparently temporarily leased from the Cedar VE accelerator
> during the boot time. This was a relatively recent discovery:
> 
> http://irclog.whitequark.org/linux-sunxi/2016-06-29#16862925;

Ah right, I was thinking that this VE leasing only affects the last part
of the SRAM, which we found not working at all.
But this explanation (SRAM not meant to be accessed by the CPU) makes
some sense.

> And an ugly part of it is that accessing SRAM C directly by the CPU
> is unreliable if AHB1 is clocked at 200MHz (default when running the
> Allwinner's Linux kernel). But the SRAM C works fine with AHB1 clocked
> at 100MHz (which is, for example the default clock speed in the
> FEL mode). So after AHB1 is reclocked (by boot0), nobody can safely
> access SRAM C anymore.

Which would explain why I have no issues with the (experimental) SPL
setup I use for FEL booting everyday (because it clocks AHB1 at 100 MHz
quite early). And that "unreliable if higher than 100 MHz" would also
explain why it occasionally works for me.

So what about this for now:
1) We revert this patch to fix U-Boot proper with boot0.
2) We may think about increasing the AHB1 clock speed again, basically
reverting that other patch which was just a prerequisite for this one.
I was always wondering if a halved AHB1 has side effect on Linux later on.
3) In the (upcoming) SPL support we either find some other space for the
stack (SRAM A2?) or clock AHB1 to 100 MHz _only_ for the SPL runtime,
when we desperately need the SRAM C.

Does that make some sense?

Cheers,
Andre.