<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 4, 2016 at 10:46 AM, Maxim Sloyko <span dir="ltr"><<a href="mailto:maxims@google.com" target="_blank">maxims@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="gmail-h5">On Fri, Nov 4, 2016 at 8:54 AM, Cédric Le Goater <span dir="ltr"><<a href="mailto:clg@kaod.org" target="_blank">clg@kaod.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello !<br>
<span><br>
On 11/03/2016 09:44 PM, Maxim Sloyko wrote:<br>
> Hi Cedric,<br>
><br>
> My manager (Rick Alther) mentioned that you were working on some<br>
> minimal "upstreamable" commit for Aspeed in U-Boot. How far were<br>
> you able to get?<br>
<br>
</span>Please check :<br>
<br>
<a href="https://github.com/openbmc/u-boot/wiki" rel="noreferrer" target="_blank">https://github.com/openbmc/u-b<wbr>oot/wiki</a><br>
<br>
This is where we track the ugly details and here is my tree :<br>
<br>
<a href="https://github.com/legoater/u-boot/commits/v2016.11-aspeed-openbmc" rel="noreferrer" target="_blank">https://github.com/legoater/<wbr>u-boot/commits/v2016.11-<wbr>aspeed-openbmc</a><br>
<br>
Patch one is the "minimum" we have been working on with Joel these<br>
last 6 months to :<br>
<br>
* make it compatible with upstream<br>
* extract all exoticisms<br>
* extract funky drivers<br>
* isolate ugly hacks<br>
<br>
I think we are at a point where we need to rewrite bits of lowlevel_init<br>
or move them later on in the code. This big asm routines does :<br>
<br>
* LPC patching for ast2300 (we can kill that)<br>
* uart init (if SDRAM calibration is done, easy we can keep)<br>
* SDRAM calibration (we could use static calibration values instead)<br>
* SDRAM size calculation (may be we can move that in a C part)<br>
* video clock setting (one reg setting)<br>
* JTAG master (one reg setting)<br>
* RMII/RGMII clock setting (one reg setting)<br>
* GPIO massive reset (not sure if this is useful, should ask Andrew)<br>
* SPI Calibration (I have done in C so we can move)<br></blockquote><div><br></div></div></div><div>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.</div><div><br></div><div>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 --</div><div>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. </div></div></div></div></blockquote><div><br></div><div>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)</div><div><br></div><div><div>--- a/arch/arm/mach-aspeed/platform_g5.S</div><div>+++ b/arch/arm/mach-aspeed/platform_g5.S</div><div>@@ -242,12 +242,12 @@ TIME_TABLE_DDR4_1600:</div><div> Calibration Macro End</div><div> ******************************************************************************/</div><div> </div><div>-.globl lowlevel_init</div><div>-lowlevel_init:</div><div>+.globl board_early_init_f</div><div>+board_early_init_f:<br></div><div><br></div><div>// Here I'm just renaming lowlevel_init to board_early_init_f</div><div> </div><div> init_dram:</div><div> /* save lr */</div><div>- mov r4, lr</div><div>+<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>push {r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, lr}</div><div><br></div><div>We are inside board_init_f, so we have stack at this point. Let's use it.</div><div> </div><div> /* Clear AHB bus lock condition */</div><div> ldr r0, =0x1e600000</div><div>@@ -1991,9 +1991,6 @@ set_D2PLL:</div><div> ldr r1, =0xEA</div><div> str r1, [r0]</div><div> </div><div>- /* restore lr */</div><div>- mov lr, r4</div><div>-</div><div>- /* back to arch calling code */</div><div>- mov pc, lr</div><div>+<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>mov r0, #0</div><div>+<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>pop {r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, pc}</div><div><br></div><div>// basically, return 0</div><div> </div><div>diff --git a/board/aspeed/zaius-bmc/zaius-bmc.c b/board/aspeed/zaius-bmc/zaius-bmc.c</div><div>index 04909d1..0052767 100644</div><div>--- a/board/aspeed/zaius-bmc/zaius-bmc.c</div><div>+++ b/board/aspeed/zaius-bmc/zaius-bmc.c</div><div>@@ -56,6 +56,8 @@ void show_boot_progress(int progress)</div><div> }</div><div> #endif</div><div> </div><div>+void lowlevel_init() {}</div><div>+</div><div><br></div><div>// The function is not necessary, but it is still called, so needs to be present in the binary.</div><div><br></div><div> int board_init(void)</div><div> {</div><div> <span class="gmail-Apple-tab-span" style="white-space:pre"> </span>/* adress of boot parameters */</div><div>diff --git a/include/configs/ast-common.h b/include/configs/ast-common.h</div><div>index 0c7d8fe..b9c87d6 100644</div><div>--- a/include/configs/ast-common.h</div><div>+++ b/include/configs/ast-common.h</div><div>@@ -33,10 +33,10 @@</div><div> #define CONFIG_SYS_DCACHE_OFF<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>1</div><div><br></div><div>// Config changes -- this is the most important part. U-Boot will use these to setup stack and heap for us.</div><div> </div><div> #define CONFIG_SYS_SDRAM_BASE<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>AST_DRAM_BASE</div><div>-#define CONFIG_SYS_INIT_RAM_ADDR<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>CONFIG_SYS_SDRAM_BASE</div><div>-#define CONFIG_SYS_INIT_RAM_SIZE<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(32*1024)</div><div>+#define CONFIG_SYS_INIT_RAM_ADDR<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(AST_SRAM_BASE)</div><div>+#define CONFIG_SYS_INIT_RAM_SIZE<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(36*1024)</div><div> #define CONFIG_SYS_INIT_RAM_END<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE)</div><div>-#define CONFIG_SYS_INIT_SP_ADDR <span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(CONFIG_SYS_INIT_RAM_END - GENERATED_GBL_DATA_SIZE)</div><div>+#define CONFIG_SYS_INIT_SP_ADDR <span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(CONFIG_SYS_INIT_RAM_END)</div><div> </div><div> #define CONFIG_NR_DRAM_BANKS<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>1</div><div> </div><div>diff --git a/include/configs/zaius-bmc.h b/include/configs/zaius-bmc.h</div><div>index cf4b1ff..65a76fb 100644</div><div>--- a/include/configs/zaius-bmc.h</div><div>+++ b/include/configs/zaius-bmc.h</div><div>@@ -36,6 +36,9 @@</div><div> #define CONFIG_EEPROM_I2C_ADDR<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>(0x50)</div><div> #define CONFIG_ID_EEPROM</div><div><br></div><div>// Don't forget to enable the call to board_early_init_f, it's behind #ifdef in board_f.c</div><div> </div><div>+#define CONFIG_SYS_INIT_SP_OFFSET (CONFIG_SYS_GBL_DATA_OFFSET)</div><div>+#define CONFIG_BOARD_EARLY_INIT_F</div><div>+</div><div> /* platform.S settings */</div><div> #define CONFIG_DRAM_ECC</div><div> #define<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>CONFIG_DRAM_ECC_SIZE<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>0x20000000</div></div><div><br></div><div>Compile, flash and U-Boot booted to a prompt on the eval board. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span><br>
> This is also what I was thinking of doing, so we should be able to join forces on this.<br>
><br>
> I talked to Simon Glass recently (he's an active contributor to mainline U-Boot)<br>
> and basically that's the path that he also recommended -- just getting minimal amount<br>
> of code in, that can just boot to a prompt. He also said that DRAM driver would have<br>
> to be part of it.<br>
<br>
</span>yes. See my comments above.<br>
<span><br>
> So, the way I see it, there are two big chunks of work here:<br>
> 1. Setting up the whole structure. This would include actually adding a board, minimum<br>
> amount of supporting code, debug serial console init,<br>
<br>
</span>yes. that is patch one above, in which we still need to cleanup<br>
stuff in the .S part.<br>
<span><br>
> 2. DDR3/DDR4 driver. This is the biggest part of what we have in platform.S now. A lot<br>
> of work, but relatively straightforward, just rewrite ~1.5k lines of assembly in C.<br>
<br>
</span>I am not sure this is feasible, AFAIK, you can not call C. I might<br>
be wrong.<br></blockquote><div><br></div></span><div>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</div><div><br></div><div>mov sp, =TOP_OF_SRAM</div><div><br></div><div>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.</div><span class="gmail-"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span><br>
> Personally, I don't have preference on who does what, as long as we don't step on each<br>
> other's toes. It would probably be easier for me to take (1), because I can easily talk<br>
> to two people who are in our time zone and have a lot of mainline U-Boot experience.<br>
><br>
> Thoughts?<br>
<br>
</span>you could send patches on top of my tree to cleanup what ever<br>
part you want in lowlevel_init. when lowlevel_init reaches a<br>
minimum acceptable level, with a full patchset working on top<br>
of it, I think we can send for review. Then, we will work on<br>
the next details :<br>
<br>
* extra settings<br>
* spi/nor driver<br>
* ftgmac100 extensions for Aspeed<br>
* NCSI support<br>
* DTS support<br>
* etc.<br>
<br>
There is also a massive hack on the ast2500 mmu disablement in<br>
the core part of u-boot that I don't understand. So we could<br>
try to fix now or ask help when the patchset is sent.<br>
<br>
<br>
Cheers,<br>
<br>
C.<br>
</blockquote></span></div><span class="gmail-HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div class="gmail-m_2076816628761673642gmail_signature"><div><b>M</b>axim <b>S</b>loyko</div></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div><b>M</b>axim <b>S</b>loyko</div></div>
</div></div>