Procedure for the send review on u-boot patch

Procedure for the send review on u-boot patch

[logananth hcl]

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

Hai All,

In openbmc/u-boot repo(branch - v2016.07-aspeed-openbmc)
https://github.com/openbmc/u-boot/tree/v2016.07-aspeed-openbmc

I want to send a review on this particular branch in u-boot through
upstream.
Is there any specific procedure to upstream it,
kindly provide your suggestions.


Thanks,
Logananth.s

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

Re: Procedure for the send review on u-boot patch

[Joel Stanley]

Hello,

On Thu, 13 Jan 2022 at 08:52, logananth hcl <logananth13.hcl@gmail.com> wrote:
>
> Hai All,
>
> In openbmc/u-boot repo(branch - v2016.07-aspeed-openbmc)
> https://github.com/openbmc/u-boot/tree/v2016.07-aspeed-openbmc
>
> I want to send a review on this particular branch in u-boot through upstream.
> Is there any specific procedure to upstream it,
> kindly provide your suggestions.

I strongly recommend using the newer v2019.04 based branch for any new
system you're bringing up.

However, if you insist on the older branch, you can send patches to
the list for it. Indicate in the subject that they are for the old
branch:

git format-patch --subject-prefix "PATCH u-boot v2017.07-aspeed-openbmc"

Cheers,

Joel

>
>
> Thanks,
> Logananth.s

Re: Procedure for the send review on u-boot patch

[Patrick Williams]

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

On Thu, Jan 13, 2022 at 10:40:40PM +0000, Joel Stanley wrote:
> Hello,
> 
> On Thu, 13 Jan 2022 at 08:52, logananth hcl <logananth13.hcl@gmail.com> wrote:
> >
> > Hai All,
> >
> > In openbmc/u-boot repo(branch - v2016.07-aspeed-openbmc)
> > https://github.com/openbmc/u-boot/tree/v2016.07-aspeed-openbmc
> >
> > I want to send a review on this particular branch in u-boot through upstream.
> > Is there any specific procedure to upstream it,
> > kindly provide your suggestions.
> 
> I strongly recommend using the newer v2019.04 based branch for any new
> system you're bringing up.

It doesn't seem like anyone is using this branch for any Aspeed system though.

```
$ git grep "v2019.04"
meta-aspeed/recipes-bsp/u-boot/u-boot-common-aspeed-sdk_2019.04.inc:PV = "v2019.04+git${SRCPV}"
```

I think everyone doing AST2400/AST2500 work has been copying from existing
systems and still pointing to the v2016.07 branch that we have working in the
tree.

> However, if you insist on the older branch, you can send patches to
> the list for it. Indicate in the subject that they are for the old
> branch:
> 
> git format-patch --subject-prefix "PATCH u-boot v2017.07-aspeed-openbmc"

I think the question is two-fold:

1) How do we get the code into the tree that everyone is using? (Which you've
answered)

2) What is the best way to get feedback from upstream on the proper way to do
this?

Is most of the Aspeed content fully upstream now?  I thought it wasn't and was
still in yet another Aspeed U-Boot tree.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Procedure for the send review on u-boot patch

[Zev Weiss]

On Fri, Jan 14, 2022 at 10:45:50AM PST, Patrick Williams wrote:
>On Thu, Jan 13, 2022 at 10:40:40PM +0000, Joel Stanley wrote:
>> Hello,
>>
>> On Thu, 13 Jan 2022 at 08:52, logananth hcl <logananth13.hcl@gmail.com> wrote:
>> >
>> > Hai All,
>> >
>> > In openbmc/u-boot repo(branch - v2016.07-aspeed-openbmc)
>> > https://github.com/openbmc/u-boot/tree/v2016.07-aspeed-openbmc
>> >
>> > I want to send a review on this particular branch in u-boot through upstream.
>> > Is there any specific procedure to upstream it,
>> > kindly provide your suggestions.
>>
>> I strongly recommend using the newer v2019.04 based branch for any new
>> system you're bringing up.
>
>It doesn't seem like anyone is using this branch for any Aspeed system though.
>
>```
>$ git grep "v2019.04"
>meta-aspeed/recipes-bsp/u-boot/u-boot-common-aspeed-sdk_2019.04.inc:PV = "v2019.04+git${SRCPV}"
>```
>
>I think everyone doing AST2400/AST2500 work has been copying from existing
>systems and still pointing to the v2016.07 branch that we have working in the
>tree.
>

I think g200a, fp5280g2, and vegman are all using the new branch?  And
if I'm interpreting the below correctly, also any aspeed-g6 systems that
haven't overridden the defaults?

```
$ git grep 'PREFERRED_PROVIDER.*u-boot-aspeed-sdk'
meta-aspeed/conf/machine/include/aspeed.inc:PREFERRED_PROVIDER_virtual/bootloader:aspeed-g6 ?= "u-boot-aspeed-sdk"
meta-aspeed/conf/machine/include/aspeed.inc:PREFERRED_PROVIDER_u-boot:aspeed-g6 ?= "u-boot-aspeed-sdk"
meta-bytedance/meta-g220a/conf/machine/g220a.conf:PREFERRED_PROVIDER_virtual/bootloader = "u-boot-aspeed-sdk"
meta-bytedance/meta-g220a/conf/machine/g220a.conf:PREFERRED_PROVIDER_u-boot = "u-boot-aspeed-sdk"
meta-inspur/meta-fp5280g2/conf/machine/fp5280g2.conf:PREFERRED_PROVIDER_virtual/bootloader = "u-boot-aspeed-sdk"
meta-inspur/meta-fp5280g2/conf/machine/fp5280g2.conf:PREFERRED_PROVIDER_u-boot = "u-boot-aspeed-sdk"
meta-yadro/meta-vegman/conf/machine/include/vegman.inc:PREFERRED_PROVIDER_virtual/bootloader = "u-boot-aspeed-sdk"
meta-yadro/meta-vegman/conf/machine/include/vegman.inc:PREFERRED_PROVIDER_u-boot = "u-boot-aspeed-sdk"
```

FWIW, I've also got another port in the works (Delta AHE-50DC Open19
powershelf) that I'm hoping to start upstreaming in the next few weeks
that so far seems to be working well on the 2019.04 branch internally.

That said, on some other systems I've experimented with switching to the
newer branch has caused various problems (which I haven't had time to
fully track down & debug).


Zev

Re: Procedure for the send review on u-boot patch

[Patrick Williams]

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

On Fri, Jan 14, 2022 at 10:02:40PM +0000, Zev Weiss wrote:
> On Fri, Jan 14, 2022 at 10:45:50AM PST, Patrick Williams wrote:

> >```
> >$ git grep "v2019.04"
> >meta-aspeed/recipes-bsp/u-boot/u-boot-common-aspeed-sdk_2019.04.inc:PV = "v2019.04+git${SRCPV}"
> >```
> >
> >I think everyone doing AST2400/AST2500 work has been copying from existing
> >systems and still pointing to the v2016.07 branch that we have working in the
> >tree.
> >
> 
> I think g200a, fp5280g2, and vegman are all using the new branch?  And
> if I'm interpreting the below correctly, also any aspeed-g6 systems that
> haven't overridden the defaults?

Thanks, my git-grep failed me and for some reason I thought 'sdk' means
something else w.r.t. u-boot.  You appear to be correct.

Logananth, we might want to upgrade Yv2/Tiogapass to this newer branch if
possible then.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Procedure for the send review on u-boot patch

[Lei Yu]

On Sat, Jan 15, 2022 at 2:46 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Thu, Jan 13, 2022 at 10:40:40PM +0000, Joel Stanley wrote:
> > Hello,
> >
> > On Thu, 13 Jan 2022 at 08:52, logananth hcl <logananth13.hcl@gmail.com> wrote:
> > >
> > > Hai All,
> > >
> > > In openbmc/u-boot repo(branch - v2016.07-aspeed-openbmc)
> > > https://github.com/openbmc/u-boot/tree/v2016.07-aspeed-openbmc
> > >
> > > I want to send a review on this particular branch in u-boot through upstream.
> > > Is there any specific procedure to upstream it,
> > > kindly provide your suggestions.
> >
> > I strongly recommend using the newer v2019.04 based branch for any new
> > system you're bringing up.
>
> It doesn't seem like anyone is using this branch for any Aspeed system though.
>
> ```
> $ git grep "v2019.04"
> meta-aspeed/recipes-bsp/u-boot/u-boot-common-aspeed-sdk_2019.04.inc:PV = "v2019.04+git${SRCPV}"
> ```
>
> I think everyone doing AST2400/AST2500 work has been copying from existing
> systems and still pointing to the v2016.07 branch that we have working in the
> tree.
>

Doing `git grep u-boot-aspeed-sdk` and there are a few systems using
the new u-boot.

Just a note: the new u-boot requires a different layout by default, it
matches the `openbmc-flash-layout-64.dtsi` but differs from the 32MiB
`openbmc-flash-layout.dtsi`.
So one needs to adjust the `bootcmd` if switching from v2016.07 to v2019.04.


-- 
BRs,
Lei YU

Re: Procedure for the send review on u-boot patch

[Joel Stanley]

On Fri, 14 Jan 2022 at 18:45, Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Thu, Jan 13, 2022 at 10:40:40PM +0000, Joel Stanley wrote:
> > Hello,
> >
> > On Thu, 13 Jan 2022 at 08:52, logananth hcl <logananth13.hcl@gmail.com> wrote:
> > >
> > > Hai All,
> > >
> > > In openbmc/u-boot repo(branch - v2016.07-aspeed-openbmc)
> > > https://github.com/openbmc/u-boot/tree/v2016.07-aspeed-openbmc
> > >
> > > I want to send a review on this particular branch in u-boot through upstream.
> > > Is there any specific procedure to upstream it,
> > > kindly provide your suggestions.
> >
> > I strongly recommend using the newer v2019.04 based branch for any new
> > system you're bringing up.
>
> It doesn't seem like anyone is using this branch for any Aspeed system though.

- Bytedance use it for g22a
- Yadro use it for the vegman series of machines
- inspur use it for the fp5280g2 (I learn this when grepping!)

$ git grep PREFERRED_PROVIDER_u-boot |grep u-boot-aspeed-sdk
meta-aspeed/conf/machine/include/aspeed.inc:PREFERRED_PROVIDER_u-boot:aspeed-g6
?= "u-boot-aspeed-sdk"
meta-bytedance/meta-g220a/conf/machine/g220a.conf:PREFERRED_PROVIDER_u-boot
= "u-boot-aspeed-sdk"
meta-inspur/meta-fp5280g2/conf/machine/fp5280g2.conf:PREFERRED_PROVIDER_u-boot
= "u-boot-aspeed-sdk"
meta-yadro/meta-vegman/conf/machine/include/vegman.inc:PREFERRED_PROVIDER_u-boot
= "u-boot-aspeed-sdk"

(I just noticed that others have pointed this out too).

>
> ```
> $ git grep "v2019.04"
> meta-aspeed/recipes-bsp/u-boot/u-boot-common-aspeed-sdk_2019.04.inc:PV = "v2019.04+git${SRCPV}"
> ```
>
> I think everyone doing AST2400/AST2500 work has been copying from existing
> systems and still pointing to the v2016.07 branch that we have working in the
> tree.

Where possible I review for this. If there's a way for gerrit to tell
me when a new machine is added I'd enforce it more often.

I've mentioned on the list before that I'd welcome a change that flips
the default to the new tree. That would require setting
PREFERRED_PROVIDER_u-boot for all existing aspeed 2400/2500 machines
that don't have it set, and then changing meta-aspeed.

>
> > However, if you insist on the older branch, you can send patches to
> > the list for it. Indicate in the subject that they are for the old
> > branch:
> >
> > git format-patch --subject-prefix "PATCH u-boot v2017.07-aspeed-openbmc"
>
> I think the question is two-fold:
>
> 1) How do we get the code into the tree that everyone is using? (Which you've
> answered)
>
> 2) What is the best way to get feedback from upstream on the proper way to do
> this?
>
> Is most of the Aspeed content fully upstream now?  I thought it wasn't and was
> still in yet another Aspeed U-Boot tree.

More of it is upstream. The 2600 patches are on list, and could go in
at any time. The 2400 support could be posted and merged too.

Aspeed are making progress there. I would encourage companies to help
get support merged by having someone work on it, or funding an expert
to do so.

Cheers,

Joel

Re: Procedure for the send review on u-boot patch

[Joel Stanley]

On Mon, 17 Jan 2022 at 06:54, Joel Stanley <joel@jms.id.au> wrote:

> > I think everyone doing AST2400/AST2500 work has been copying from existing
> > systems and still pointing to the v2016.07 branch that we have working in the
> > tree.
>
> Where possible I review for this. If there's a way for gerrit to tell
> me when a new machine is added I'd enforce it more often.
>
> I've mentioned on the list before that I'd welcome a change that flips
> the default to the new tree. That would require setting
> PREFERRED_PROVIDER_u-boot for all existing aspeed 2400/2500 machines
> that don't have it set, and then changing meta-aspeed.

I've given this a go:

https://gerrit.openbmc-project.xyz/q/topic:aspeed-u-boot-default

I did some boot tests of yosemitev2 and romulus in qemu and all looked fine.

Next step would be to convert the EVB to the new u-boot, and perhaps
some of the prominent machines that people use to base their new
systems, on so they start on the right track.

Re: Procedure for the send review on u-boot patch

[Patrick Williams]

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

On Mon, Jan 17, 2022 at 06:54:48AM +0000, Joel Stanley wrote:
> On Fri, 14 Jan 2022 at 18:45, Patrick Williams <patrick@stwcx.xyz> wrote:
> > On Thu, Jan 13, 2022 at 10:40:40PM +0000, Joel Stanley wrote:
> > > I strongly recommend using the newer v2019.04 based branch for any new
> > > system you're bringing up.

I was under the mistaken assumption that the aspeed-sdk was not hosted and
maintained by us.  In u-boot-common-aspeed-sdk_2019.04.inc I see:

HOMEPAGE = "https://github.com/AspeedTech-BMC/u-boot"

... but ...

SRC_URI = "git://git@github.com/openbmc/u-boot.git;nobranch=1;protocol=https"

Should we align these to avoid any confusion?  I assume we should adjust the
HOMEPAGE to point at our repository?

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Procedure for the send review on u-boot patch

[Joel Stanley]

On Tue, 18 Jan 2022 at 17:40, Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Mon, Jan 17, 2022 at 06:54:48AM +0000, Joel Stanley wrote:
> > On Fri, 14 Jan 2022 at 18:45, Patrick Williams <patrick@stwcx.xyz> wrote:
> > > On Thu, Jan 13, 2022 at 10:40:40PM +0000, Joel Stanley wrote:
> > > > I strongly recommend using the newer v2019.04 based branch for any new
> > > > system you're bringing up.
>
> I was under the mistaken assumption that the aspeed-sdk was not hosted and
> maintained by us.  In u-boot-common-aspeed-sdk_2019.04.inc I see:
>
> HOMEPAGE = "https://github.com/AspeedTech-BMC/u-boot"
>
> ... but ...
>
> SRC_URI = "git://git@github.com/openbmc/u-boot.git;nobranch=1;protocol=https"
>
> Should we align these to avoid any confusion?  I assume we should adjust the
> HOMEPAGE to point at our repository?

The upstream for our branch is that homepage URL. Ideally we would put
all of our patches into the SDK (I often send Aspeed pull requests to
do this), and un-fork the repository.

If you found the homepage URL confusing we could add a note to
explain, and change it. I'll leave it to you.