U-Boot upstreaming

U-Boot upstreaming

[Maxim Sloyko]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

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?

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.

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,
 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.

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?

-- 
*M*axim *S*loyko

[-- Attachment #2: Type: text/html, Size: 1532 bytes --]

Re: U-Boot upstreaming

[Joel Stanley]

Hi Maxim,

On Fri, Nov 4, 2016 at 7:14 AM, Maxim Sloyko <maxims@google.com> 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?
>
> 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.

I agree. The downside here is Cedric has done a lot of work in
cleaning up the Aspeed SDK code already, so we don't want to throw
away the good stuff he's done.

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

I have a tree where I've started pulling the build scaffolding -
kconfig, makefiles, etc - out of the patch we have.

The second part of it is writing a clean serial driver. The hardware a
pretty standard layout, so I imagine we don't need to do much here.
Perhaps doing some calculations based on the clock speed to set a baud
rate.

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

Our concern with rewriting this is future maintainability. If we
rewrite it, it becomes harder to incorporate fixes and new hardware
support from Aspeed SDKs.

On the other hand, there's no way we can upstream it as-is. So lets
re-write it in C, and where we can document parts of the
reimplementation with reference to the assembly so making updates is
possible.

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

I think we've got a good handle on (1), however I'll let Cedric
comment as to what he would prefer to work on.

Note that this work is continuing on from some work done by Teddy
several months ago. He has a tree here:

 https://github.com/theopolis/u-boot-openbmc/tree/v2016.03-openbmc-r3

Cheers,

Joel

Re: U-Boot upstreaming

[Cédric Le Goater]

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)

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

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

Re: U-Boot upstreaming

[Cédric Le Goater]

On 11/04/2016 01:09 AM, Joel Stanley wrote:
> Hi Maxim,
> 
> On Fri, Nov 4, 2016 at 7:14 AM, Maxim Sloyko <maxims@google.com> 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?
>>
>> 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.
> 
> I agree. The downside here is Cedric has done a lot of work in
> cleaning up the Aspeed SDK code already, so we don't want to throw
> away the good stuff he's done.

half of that work is yours, Joel.
 
>> 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,
> 
> I have a tree where I've started pulling the build scaffolding -
> kconfig, makefiles, etc - out of the patch we have.
> 
> The second part of it is writing a clean serial driver. The hardware a
> pretty standard layout, so I imagine we don't need to do much here.
> Perhaps doing some calculations based on the clock speed to set a baud
> rate.
> 
>>  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.
> 
> Our concern with rewriting this is future maintainability. If we
> rewrite it, it becomes harder to incorporate fixes and new hardware
> support from Aspeed SDKs.

Yes that is one big concern I think. The platform*.s files are 
exactly the one shipped by Aspeed.

> On the other hand, there's no way we can upstream it as-is. So lets
> re-write it in C, and where we can document parts of the
> reimplementation with reference to the assembly so making updates is
> possible.
> 
>>
>> 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.

sure. 

> I think we've got a good handle on (1), however I'll let Cedric
> comment as to what he would prefer to work on.

My main concern is keeping a full patchset working with the HW 
we have. I don't think there is much value in pushing mainline 
some code we don't use because it is too far away from our 
current need. So whatever we send should be used in our openbmc
tree, but not in the stable branch of course.

As said in the other email, I think we are at point where we 
should dismantle lowlevel_init in little parts. 

Maxim, may be you could take a look at the SDRAM calibration ? 

This is the biggest part when this is done, the rest should 
flow. 

Cheers,

C.

> Note that this work is continuing on from some work done by Teddy
> several months ago. He has a tree here:
> 
>  https://github.com/theopolis/u-boot-openbmc/tree/v2016.03-openbmc-r3
> 
> Cheers,
> 
> Joel

Re: U-Boot upstreaming

[Maxim Sloyko]

[-- Attachment #1: Type: text/plain, Size: 4154 bytes --]

On Fri, Nov 4, 2016 at 9:08 AM, Cédric Le Goater <clg@kaod.org> wrote:

> On 11/04/2016 01:09 AM, Joel Stanley wrote:
> > Hi Maxim,
> >
> > On Fri, Nov 4, 2016 at 7:14 AM, Maxim Sloyko <maxims@google.com> 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?
> >>
> >> 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.
> >
> > I agree. The downside here is Cedric has done a lot of work in
> > cleaning up the Aspeed SDK code already, so we don't want to throw
> > away the good stuff he's done.
>

Why would that be thrown away? The idea is to prepare minimum amount of
stuff required for the first commit upstream.
The rest won't be thrown away, it will just follow.


>
> half of that work is yours, Joel.
>
> >> 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,
> >
> > I have a tree where I've started pulling the build scaffolding -
> > kconfig, makefiles, etc - out of the patch we have.
> >
> > The second part of it is writing a clean serial driver. The hardware a
> > pretty standard layout, so I imagine we don't need to do much here.
> > Perhaps doing some calculations based on the clock speed to set a baud
> > rate.
> >
> >>  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.
> >
> > Our concern with rewriting this is future maintainability. If we
> > rewrite it, it becomes harder to incorporate fixes and new hardware
> > support from Aspeed SDKs.
>
> Yes that is one big concern I think. The platform*.s files are
> exactly the one shipped by Aspeed.
>

Since they are not helping, I don't see any other way. We will just have to
look at their diffs and incorporate those fixes in our tree. I would take
this option any day over having to make fixes in platform.s, if we need to
change something.


>
> > On the other hand, there's no way we can upstream it as-is. So lets
> > re-write it in C, and where we can document parts of the
> > reimplementation with reference to the assembly so making updates is
> > possible.
> >
> >>
> >> 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.
>
> sure.
>
> > I think we've got a good handle on (1), however I'll let Cedric
> > comment as to what he would prefer to work on.
>
> My main concern is keeping a full patchset working with the HW
> we have. I don't think there is much value in pushing mainline
> some code we don't use because it is too far away from our
> current need. So whatever we send should be used in our openbmc
> tree, but not in the stable branch of course.
>
> As said in the other email, I think we are at point where we
> should dismantle lowlevel_init in little parts.
>
> Maxim, may be you could take a look at the SDRAM calibration ?
>
> This is the biggest part when this is done, the rest should
> flow.
>

Sounds good to me, I'll start pulling out SDRAM part out.


>
> Cheers,
>
> C.
>
> > Note that this work is continuing on from some work done by Teddy
> > several months ago. He has a tree here:
> >
> >  https://github.com/theopolis/u-boot-openbmc/tree/v2016.03-openbmc-r3
> >
> > Cheers,
> >
> > Joel
>
>


-- 
*M*axim *S*loyko

[-- Attachment #2: Type: text/html, Size: 5818 bytes --]

Re: U-Boot upstreaming

[Cédric Le Goater]

On 11/04/2016 06:21 PM, Maxim Sloyko wrote:
> 
> 
> On Fri, Nov 4, 2016 at 9:08 AM, Cédric Le Goater <clg@kaod.org <mailto:clg@kaod.org>> wrote:
> 
>     On 11/04/2016 01:09 AM, Joel Stanley wrote:
>     > Hi Maxim,
>     >
>     > On Fri, Nov 4, 2016 at 7:14 AM, Maxim Sloyko <maxims@google.com <mailto:maxims@google.com>> 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?
>     >>
>     >> 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.
>     >
>     > I agree. The downside here is Cedric has done a lot of work in
>     > cleaning up the Aspeed SDK code already, so we don't want to throw
>     > away the good stuff he's done.
> 
> 
> Why would that be thrown away? The idea is to prepare minimum amount of stuff required for the first commit upstream.
> The rest won't be thrown away, it will just follow.
>  
> 
> 
>     half of that work is yours, Joel.
> 
>     >> 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,
>     >
>     > I have a tree where I've started pulling the build scaffolding -
>     > kconfig, makefiles, etc - out of the patch we have.
>     >
>     > The second part of it is writing a clean serial driver. The hardware a
>     > pretty standard layout, so I imagine we don't need to do much here.
>     > Perhaps doing some calculations based on the clock speed to set a baud
>     > rate.
>     >
>     >>  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.
>     >
>     > Our concern with rewriting this is future maintainability. If we
>     > rewrite it, it becomes harder to incorporate fixes and new hardware
>     > support from Aspeed SDKs.
> 
>     Yes that is one big concern I think. The platform*.s files are
>     exactly the one shipped by Aspeed.
> 
> 
> Since they are not helping, I don't see any other way. We will just have to look at their diffs and incorporate those fixes in our tree. I would take this option any day over having to make fixes in platform.s, if we need to change something.
>  
> 
> 
>     > On the other hand, there's no way we can upstream it as-is. So lets
>     > re-write it in C, and where we can document parts of the
>     > reimplementation with reference to the assembly so making updates is
>     > possible.
>     >
>     >>
>     >> 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.
> 
>     sure.
> 
>     > I think we've got a good handle on (1), however I'll let Cedric
>     > comment as to what he would prefer to work on.
> 
>     My main concern is keeping a full patchset working with the HW
>     we have. I don't think there is much value in pushing mainline
>     some code we don't use because it is too far away from our
>     current need. So whatever we send should be used in our openbmc
>     tree, but not in the stable branch of course.
> 
>     As said in the other email, I think we are at point where we
>     should dismantle lowlevel_init in little parts.
> 
>     Maxim, may be you could take a look at the SDRAM calibration ?
> 
>     This is the biggest part when this is done, the rest should
>     flow.
> 
> 
> Sounds good to me, I'll start pulling out SDRAM part out.

Thanks a lot. 

If that helps, openbmc/qemu now supports the SDRAM calibration 
on the AST2400 so it can be used to track how the registers are 
set. It won't replace real HW.

Cheers,

C. 

Re: U-Boot upstreaming

[Maxim Sloyko]

[-- Attachment #1: Type: text/plain, Size: 4375 bytes --]

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.


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

[-- Attachment #2: Type: text/html, Size: 5759 bytes --]

Re: U-Boot upstreaming

[Maxim Sloyko]

[-- Attachment #1.1: Type: text/plain, Size: 7853 bytes --]

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)

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

[-- Attachment #1.2: Type: text/html, Size: 11977 bytes --]

[-- Attachment #2: board_early_init_f.patch --]
[-- Type: text/x-patch, Size: 2582 bytes --]

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

Re: U-Boot upstreaming

[Cédric Le Goater]

On 11/04/2016 06:46 PM, Maxim Sloyko wrote:
> 
> 
> On Fri, Nov 4, 2016 at 8:54 AM, Cédric Le Goater <clg@kaod.org <mailto: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 <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 <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.

I agree that the biggest part goes to dram_init and the rest can be 
moved in different areas.

> Did you try moving this whole function further down the init sequence? 

I don't think there are any early inits needed in this file.

I have moved all the relevant code to C routines. lowlevel_init 
is now empty, resets, inits are done in C, console is OK. For DRAM 
calibration, which is the hairy part, I tried to use pre-calculated 
values from an another system but the sequence is not good enough 
and so the DRAM controller has issues.

Here is my dev v2016.11-rc3 branch for you to see :

	https://github.com/legoater/u-boot/commits/v2016.11-aspeed-openbmc-sdmc

and the DRAM part :

	https://github.com/legoater/u-boot/commit/b3fb9523e1f37a7057d5b3bee3122ef7a78dafb4

beware, it can brick your system. Mine has died a couple of time 
when accessing RAM. But it is a good base to start rewriting the 
DRAM calibration.

Cheers,

C. 

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

Re: U-Boot upstreaming

[Cédric Le Goater]

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 ? ] 

Re: U-Boot upstreaming

[Maxim Sloyko]

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

-- 
Maxim Sloyko

Re: U-Boot upstreaming

[Maxim Sloyko]

On Thu, Nov 3, 2016 at 5:09 PM, Joel Stanley <joel@jms.id.au> wrote:
> Hi Maxim,
>
> On Fri, Nov 4, 2016 at 7:14 AM, Maxim Sloyko <maxims@google.com> 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?
>>
>> 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.
>
> I agree. The downside here is Cedric has done a lot of work in
> cleaning up the Aspeed SDK code already, so we don't want to throw
> away the good stuff he's done.
>
>>
>> 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,
>
> I have a tree where I've started pulling the build scaffolding -
> kconfig, makefiles, etc - out of the patch we have.
>
> The second part of it is writing a clean serial driver. The hardware a
> pretty standard layout, so I imagine we don't need to do much here.
> Perhaps doing some calculations based on the clock speed to set a baud
> rate.

To set clock speed properly we would also need clock drivers, which
might also be needed by SDRAM driver. We also might need watchdog
driver to properly implement reset_cpu (via sysreset driver,
probably). And all of these need a device tree.

It's great that things are moving out of the assembly file, but we
also need to move everything in the "u-boot way" direction.

Creating a device tree and implementing some basic drivers (clocks,
WDT) would be a good place to start.

>
>>  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.
>
> Our concern with rewriting this is future maintainability. If we
> rewrite it, it becomes harder to incorporate fixes and new hardware
> support from Aspeed SDKs.
>
> On the other hand, there's no way we can upstream it as-is. So lets
> re-write it in C, and where we can document parts of the
> reimplementation with reference to the assembly so making updates is
> possible.
>
>>
>> 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.
>
> I think we've got a good handle on (1), however I'll let Cedric
> comment as to what he would prefer to work on.
>
> Note that this work is continuing on from some work done by Teddy
> several months ago. He has a tree here:
>
>  https://github.com/theopolis/u-boot-openbmc/tree/v2016.03-openbmc-r3
>
> Cheers,
>
> Joel



-- 
Maxim Sloyko