U-Boot upstreaming

Maxim Sloyko maxims at google.com
Sat Nov 5 10:51:48 AEDT 2016


On Fri, Nov 4, 2016 at 10:46 AM, Maxim Sloyko <maxims at google.com> wrote:

>
>
> On Fri, Nov 4, 2016 at 8:54 AM, Cédric Le Goater <clg at kaod.org> wrote:
>
>> Hello !
>>
>> On 11/03/2016 09:44 PM, Maxim Sloyko wrote:
>> > Hi Cedric,
>> >
>> > My manager (Rick Alther) mentioned that you were working on some
>> > minimal "upstreamable" commit for Aspeed in U-Boot. How far were
>> > you able to get?
>>
>> Please check :
>>
>>   https://github.com/openbmc/u-boot/wiki
>>
>> This is where we track the ugly details and here is my tree :
>>
>>    https://github.com/legoater/u-boot/commits/v2016.11-aspeed-openbmc
>>
>> Patch one is the "minimum" we have been working on with Joel these
>> last 6 months to :
>>
>>  * make it compatible with upstream
>>  * extract all exoticisms
>>  * extract funky drivers
>>  * isolate ugly hacks
>>
>> I think we are at a point where we need to rewrite bits of lowlevel_init
>> or move them later on in the code. This big asm routines does :
>>
>>  * LPC patching for ast2300 (we can kill that)
>>  * uart init (if SDRAM calibration is done, easy we can keep)
>>  * SDRAM calibration (we could use static calibration values instead)
>>  * SDRAM size calculation (may be we can move that in a C part)
>>  * video clock setting (one reg setting)
>>  * JTAG master (one reg setting)
>>  * RMII/RGMII clock setting (one reg setting)
>>  * GPIO massive reset (not sure if this is useful, should ask Andrew)
>>  * SPI Calibration (I have done in C so we can move)
>>
>
> Yes, I looked at it too and it does not look like any of that has any
> business being in lowlevel_init, let alone being implemented in asm.
>
> Did you try moving this whole function further down the init sequence?
> board_early_init_f might be a good candidate. It might cause problems
> because it does not restore registers properly, but should be an easy fix --
> at this point we would already have stack setup for us (need to be
> configured via CONFIG_SYS_INIT_SP_ADDR to point to the top of SRAM) and
> even have a basic malloc (need to be configured). This would make calling
> into C somewhat easier.
>

OK, so I actually tried this and it works! I did it on my branch so that I
can just quickly give it a try and don't really have time to prepare proper
patch today (this being Friday and all), but this is the summary of
required changes: (the patch is also attached, just in case, but you won't
be able to apply it)

--- a/arch/arm/mach-aspeed/platform_g5.S
+++ b/arch/arm/mach-aspeed/platform_g5.S
@@ -242,12 +242,12 @@ TIME_TABLE_DDR4_1600:
  Calibration Macro End

******************************************************************************/

-.globl lowlevel_init
-lowlevel_init:
+.globl board_early_init_f
+board_early_init_f:

// Here I'm just renaming lowlevel_init to board_early_init_f

 init_dram:
     /* save lr */
-    mov   r4, lr
+ push {r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, lr}

We are inside board_init_f, so we have stack at this point. Let's use it.

     /* Clear AHB bus lock condition */
     ldr   r0, =0x1e600000
@@ -1991,9 +1991,6 @@ set_D2PLL:
     ldr   r1, =0xEA
     str   r1, [r0]

-    /* restore lr */
-    mov   lr, r4
-
-    /* back to arch calling code */
-    mov   pc, lr
+ mov r0, #0
+ pop {r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, pc}

// basically, return 0

diff --git a/board/aspeed/zaius-bmc/zaius-bmc.c
b/board/aspeed/zaius-bmc/zaius-bmc.c
index 04909d1..0052767 100644
--- a/board/aspeed/zaius-bmc/zaius-bmc.c
+++ b/board/aspeed/zaius-bmc/zaius-bmc.c
@@ -56,6 +56,8 @@ void show_boot_progress(int progress)
 }
 #endif

+void lowlevel_init() {}
+

// The function is not necessary, but it is still called, so needs to be
present in the binary.

 int board_init(void)
 {
  /* adress of boot parameters */
diff --git a/include/configs/ast-common.h b/include/configs/ast-common.h
index 0c7d8fe..b9c87d6 100644
--- a/include/configs/ast-common.h
+++ b/include/configs/ast-common.h
@@ -33,10 +33,10 @@
 #define CONFIG_SYS_DCACHE_OFF 1

// Config changes -- this is the most important part. U-Boot will use these
to setup stack and heap for us.

 #define CONFIG_SYS_SDRAM_BASE AST_DRAM_BASE
-#define CONFIG_SYS_INIT_RAM_ADDR CONFIG_SYS_SDRAM_BASE
-#define CONFIG_SYS_INIT_RAM_SIZE (32*1024)
+#define CONFIG_SYS_INIT_RAM_ADDR (AST_SRAM_BASE)
+#define CONFIG_SYS_INIT_RAM_SIZE (36*1024)
 #define CONFIG_SYS_INIT_RAM_END (CONFIG_SYS_INIT_RAM_ADDR +
CONFIG_SYS_INIT_RAM_SIZE)
-#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_INIT_RAM_END -
GENERATED_GBL_DATA_SIZE)
+#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_INIT_RAM_END)

 #define CONFIG_NR_DRAM_BANKS 1

diff --git a/include/configs/zaius-bmc.h b/include/configs/zaius-bmc.h
index cf4b1ff..65a76fb 100644
--- a/include/configs/zaius-bmc.h
+++ b/include/configs/zaius-bmc.h
@@ -36,6 +36,9 @@
 #define CONFIG_EEPROM_I2C_ADDR (0x50)
 #define CONFIG_ID_EEPROM

// Don't forget to enable the call to board_early_init_f, it's behind
#ifdef in board_f.c

+#define CONFIG_SYS_INIT_SP_OFFSET (CONFIG_SYS_GBL_DATA_OFFSET)
+#define CONFIG_BOARD_EARLY_INIT_F
+
 /* platform.S settings */
 #define CONFIG_DRAM_ECC
 #define CONFIG_DRAM_ECC_SIZE 0x20000000

Compile, flash and U-Boot booted to a prompt on the eval board.


>
>>
>> > This is also what I was thinking of doing, so we should be able to join
>> forces on this.
>> >
>> > I talked to Simon Glass recently (he's an active contributor to
>> mainline U-Boot)
>> > and basically that's the path that he also recommended -- just getting
>> minimal amount
>> > of code in, that can just boot to a prompt. He also said that DRAM
>> driver would have
>> > to be part of it.
>>
>> yes. See my comments above.
>>
>> > So, the way I see it, there are two big chunks of work here:
>> >  1. Setting up the whole structure. This would include actually adding
>> a board, minimum
>> > amount of supporting code, debug serial console init,
>>
>> yes. that is patch one above, in which we still need to cleanup
>> stuff in the .S part.
>>
>> >  2. DDR3/DDR4 driver. This is the biggest part of what we have in
>> platform.S now. A lot
>> > of work, but relatively straightforward, just rewrite ~1.5k lines of
>> assembly in C.
>>
>> I am not sure this is feasible, AFAIK, you can not call C. I might
>> be wrong.
>>
>
> You can. I tried it and it works. We just need stack (see above). Even if
> moving the whole lowlevel_init further down will prove too hard, we can at
> least call
>
> mov sp, =TOP_OF_SRAM
>
> at top of lowlevel_init. after this you can write (and call into) pretty
> complicated C functions. This is what I've tried in my early experiments.
>
>
>>
>> > Personally, I don't have preference on who does what, as long as we
>> don't step on each
>> > other's toes. It would probably be easier for me to take (1), because I
>> can easily talk
>> > to two people who are in our time zone and have a lot of mainline
>> U-Boot experience.
>> >
>> > Thoughts?
>>
>> you could send patches on top of my tree to cleanup what ever
>> part you want in lowlevel_init. when lowlevel_init reaches a
>> minimum acceptable level, with a full patchset working on top
>> of it, I think we can send for review. Then, we will work on
>> the next details :
>>
>>  * extra settings
>>  * spi/nor driver
>>  * ftgmac100 extensions for Aspeed
>>  * NCSI support
>>  * DTS support
>>  * etc.
>>
>> There is also a massive hack on the ast2500 mmu disablement in
>> the core part of u-boot that I don't understand. So we could
>> try to fix now or ask help when the patchset is sent.
>>
>>
>> Cheers,
>>
>> C.
>>
>
>
>
> --
> *M*axim *S*loyko
>



-- 
*M*axim *S*loyko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20161104/eced99c9/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: board_early_init_f.patch
Type: text/x-patch
Size: 2582 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20161104/eced99c9/attachment-0001.bin>


More information about the openbmc mailing list