diff mbox

U-Boot upstreaming

Message ID CAFR_W8q+A+V3RyUGO-cNqqYLnzOA_jJaj-eMiYFbtJcN5uN=Dg@mail.gmail.com
State Deferred, archived
Headers show

Commit Message

Maxim Sloyko Nov. 4, 2016, 11:51 p.m. UTC 
On Fri, Nov 4, 2016 at 10:46 AM, Maxim Sloyko <maxims@google.com> wrote:

>
>
> On Fri, Nov 4, 2016 at 8:54 AM, Cédric Le Goater <clg@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)

 /* 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
>

Comments

Cédric Le Goater Nov. 7, 2016, 5:18 a.m. UTC | #1 
Hello,

> 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)

nice one :) 

but do we need any of this if we are to remove lowlevel_init ?

> --- a/include/configs/ast-common.h
> +++ b/include/configs/ast-common.h
> @@ -33,10 +33,10 @@
>  #define CONFIG_SYS_DCACHE_OFF1
> 
> // Config changes -- this is the most important part. U-Boot will use these to setup stack and heap for us.
>  
>  #define CONFIG_SYS_SDRAM_BASEAST_DRAM_BASE
> -#define CONFIG_SYS_INIT_RAM_ADDRCONFIG_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)

That is a fix I have included in my branch.

Thanks,

C.

[ is it possible to configure gmail to wrap lines at a reasonable 
  width ? ]
Maxim Sloyko Nov. 7, 2016, 6:08 p.m. UTC | #2 
On Sun, Nov 6, 2016 at 9:18 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello,
>
> > 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)
>
> nice one :)
>
> but do we need any of this if we are to remove lowlevel_init ?


The most important part is setting up the stack, which is basically
configuration of CONFIG_SYS_INIT_RAM_* and CONFIG_SYS_INIT_SP_*
This part is needed for the code that is going to end up being called
from board_init_f function, which includes dram_init and early serial
console init.

>
>
> > --- a/include/configs/ast-common.h
> > +++ b/include/configs/ast-common.h
> > @@ -33,10 +33,10 @@
> >  #define CONFIG_SYS_DCACHE_OFF1
> >
> > // Config changes -- this is the most important part. U-Boot will use these to setup stack and heap for us.
> >
> >  #define CONFIG_SYS_SDRAM_BASEAST_DRAM_BASE
> > -#define CONFIG_SYS_INIT_RAM_ADDRCONFIG_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)


You will also need

#define CONFIG_SYS_INIT_SP_OFFSET  (GENERATED_GBL_DATA_SIZE)

or just leave CONFIG_SYS_INIT_SP_ADDR defined as it is, but I guess
that's less idiomatic.

>
>
> That is a fix I have included in my branch.
>
> Thanks,
>
> C.
>
> [ is it possible to configure gmail to wrap lines at a reasonable
>   width ? ]
>

This is just a difference between plain text mode & "Rich Text" mode.
This one should be wrapped normally (that is, if you trust the
documentation).
diff mbox

Patch

commit 5e08b6fc8b72f8240373e15f828279db27cb06ff
Author: Maxim Sloyko <maxims@google.com>
Date:   Fri Nov 4 16:23:28 2016 -0700

    How to move lowlevel_init to board_early_init_f

diff --git a/arch/arm/mach-aspeed/platform_g5.S b/arch/arm/mach-aspeed/platform_g5.S
index c810f44..6f72f1c 100644
--- 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:
 
 init_dram:
     /* save lr */
-    mov   r4, lr
+	push {r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, lr}
 
     /* 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}
 
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() {}
+
 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
 
 #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
 
+#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