diff mbox

[U-Boot] x86: Fix regression build issue of coreboot-x86_defconfig

Message ID BLU436-SMTP3895F22E5587DC7922C897BFCB0@phx.gbl
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Bin Meng May 27, 2015, 3:55 p.m. UTC
Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
This commit reverts the change.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 configs/coreboot-x86_defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Bin Meng May 27, 2015, 4:01 p.m. UTC | #1
Hi Simon,

On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
> This commit reverts the change.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  configs/coreboot-x86_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
> index 66f94d0..799853f 100644
> --- a/configs/coreboot-x86_defconfig
> +++ b/configs/coreboot-x86_defconfig
> @@ -1,4 +1,5 @@
>  CONFIG_X86=y
> +CONFIG_VENDOR_COREBOOT=y
>  CONFIG_TARGET_COREBOOT=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_DM_PCI=y
> --

Please apply this patch after commit
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
and before commit
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
I've verified the build with buildman on a new 'testing' branch with
insertion of this patch.

Regards,
Bin
Joe Hershberger May 27, 2015, 4:13 p.m. UTC | #2
Hi Bin,

On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>> This commit reverts the change.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  configs/coreboot-x86_defconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>> index 66f94d0..799853f 100644
>> --- a/configs/coreboot-x86_defconfig
>> +++ b/configs/coreboot-x86_defconfig
>> @@ -1,4 +1,5 @@
>>  CONFIG_X86=y
>> +CONFIG_VENDOR_COREBOOT=y
>>  CONFIG_TARGET_COREBOOT=y
>>  CONFIG_OF_CONTROL=y
>>  CONFIG_DM_PCI=y
>> --
>
> Please apply this patch after commit
> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
> and before commit
> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
> I've verified the build with buildman on a new 'testing' branch with
> insertion of this patch.

This should be squashed as part of
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc

You need to remember to run savedefconfig when changing Kconfig or defconfig.

Cheers,
-Joe
Bin Meng May 27, 2015, 4:21 p.m. UTC | #3
Hi Joe,

On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> Hi Bin,
>
> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>>> This commit reverts the change.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  configs/coreboot-x86_defconfig | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>>> index 66f94d0..799853f 100644
>>> --- a/configs/coreboot-x86_defconfig
>>> +++ b/configs/coreboot-x86_defconfig
>>> @@ -1,4 +1,5 @@
>>>  CONFIG_X86=y
>>> +CONFIG_VENDOR_COREBOOT=y
>>>  CONFIG_TARGET_COREBOOT=y
>>>  CONFIG_OF_CONTROL=y
>>>  CONFIG_DM_PCI=y
>>> --
>>
>> Please apply this patch after commit
>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>> and before commit
>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>> I've verified the build with buildman on a new 'testing' branch with
>> insertion of this patch.
>
> This should be squashed as part of
> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>
> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>

I still don't get it. commit 65c4ac0 introduced
'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
to clean up the defconfig. I suspect there was something wrong with
'savedefconfig'?

Regards,
Bin
Joe Hershberger May 27, 2015, 4:27 p.m. UTC | #4
Hi Bin,

On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Joe,
>
> On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
> <joe.hershberger@gmail.com> wrote:
>> Hi Bin,
>>
>> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>>>> This commit reverts the change.
>>>>
>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>> ---
>>>>
>>>>  configs/coreboot-x86_defconfig | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>>>> index 66f94d0..799853f 100644
>>>> --- a/configs/coreboot-x86_defconfig
>>>> +++ b/configs/coreboot-x86_defconfig
>>>> @@ -1,4 +1,5 @@
>>>>  CONFIG_X86=y
>>>> +CONFIG_VENDOR_COREBOOT=y
>>>>  CONFIG_TARGET_COREBOOT=y
>>>>  CONFIG_OF_CONTROL=y
>>>>  CONFIG_DM_PCI=y
>>>> --
>>>
>>> Please apply this patch after commit
>>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>>> and before commit
>>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>>> I've verified the build with buildman on a new 'testing' branch with
>>> insertion of this patch.
>>
>> This should be squashed as part of
>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>>
>> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>>
>
> I still don't get it. commit 65c4ac0 introduced
> 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
> to clean up the defconfig. I suspect there was something wrong with
> 'savedefconfig'?

No, savedefconfig is doing exactly what it should. Before your patch,
CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
Therefore savedefconfig sees it as redundant to specify that in the
defconfig as well, so it removed it. When you change that explicit
default to something else, it is up to you to change the defconfigs of
the old and new default boards.

Your other option is to stop defining a default in the Kconfig and
instead mark the choice as "optional" (like I did for many other
selections like this that had no default explicitly - Kconfig
otherwise treats the first entry as default in that case) in which
case all defconfigs must have a specified vendor.

Cheers,
-Joe
Simon Glass May 27, 2015, 11:25 p.m. UTC | #5
Hi,

On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>
> Hi Bin,
>
> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> > Hi Joe,
> >
> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
> > <joe.hershberger@gmail.com> wrote:
> >> Hi Bin,
> >>
> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >>> Hi Simon,
> >>>
> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
> >>>> This commit reverts the change.
> >>>>
> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >>>> ---
> >>>>
> >>>>  configs/coreboot-x86_defconfig | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
> >>>> index 66f94d0..799853f 100644
> >>>> --- a/configs/coreboot-x86_defconfig
> >>>> +++ b/configs/coreboot-x86_defconfig
> >>>> @@ -1,4 +1,5 @@
> >>>>  CONFIG_X86=y
> >>>> +CONFIG_VENDOR_COREBOOT=y
> >>>>  CONFIG_TARGET_COREBOOT=y
> >>>>  CONFIG_OF_CONTROL=y
> >>>>  CONFIG_DM_PCI=y
> >>>> --
> >>>
> >>> Please apply this patch after commit
> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
> >>> and before commit
> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
> >>> I've verified the build with buildman on a new 'testing' branch with
> >>> insertion of this patch.
> >>
> >> This should be squashed as part of
> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
> >>
> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
> >>
> >
> > I still don't get it. commit 65c4ac0 introduced
> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
> > to clean up the defconfig. I suspect there was something wrong with
> > 'savedefconfig'?
>
> No, savedefconfig is doing exactly what it should. Before your patch,
> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
> Therefore savedefconfig sees it as redundant to specify that in the
> defconfig as well, so it removed it. When you change that explicit
> default to something else, it is up to you to change the defconfigs of
> the old and new default boards.
>
> Your other option is to stop defining a default in the Kconfig and
> instead mark the choice as "optional" (like I did for many other
> selections like this that had no default explicitly - Kconfig
> otherwise treats the first entry as default in that case) in which
> case all defconfigs must have a specified vendor.

OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
OK I'll pull it into master.

Regards,
Simon
Joe Hershberger May 27, 2015, 11:51 p.m. UTC | #6
Hi Simon,

On Wed, May 27, 2015 at 6:25 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>>
>> Hi Bin,
>>
>> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > Hi Joe,
>> >
>> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>> > <joe.hershberger@gmail.com> wrote:
>> >> Hi Bin,
>> >>
>> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >>> Hi Simon,
>> >>>
>> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>> >>>> This commit reverts the change.
>> >>>>
>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> >>>> ---
>> >>>>
>> >>>>  configs/coreboot-x86_defconfig | 1 +
>> >>>>  1 file changed, 1 insertion(+)
>> >>>>
>> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>> >>>> index 66f94d0..799853f 100644
>> >>>> --- a/configs/coreboot-x86_defconfig
>> >>>> +++ b/configs/coreboot-x86_defconfig
>> >>>> @@ -1,4 +1,5 @@
>> >>>>  CONFIG_X86=y
>> >>>> +CONFIG_VENDOR_COREBOOT=y
>> >>>>  CONFIG_TARGET_COREBOOT=y
>> >>>>  CONFIG_OF_CONTROL=y
>> >>>>  CONFIG_DM_PCI=y
>> >>>> --
>> >>>
>> >>> Please apply this patch after commit
>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>> >>> and before commit
>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>> >>> I've verified the build with buildman on a new 'testing' branch with
>> >>> insertion of this patch.
>> >>
>> >> This should be squashed as part of
>> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>> >>
>> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>> >>
>> >
>> > I still don't get it. commit 65c4ac0 introduced
>> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>> > to clean up the defconfig. I suspect there was something wrong with
>> > 'savedefconfig'?
>>
>> No, savedefconfig is doing exactly what it should. Before your patch,
>> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>> Therefore savedefconfig sees it as redundant to specify that in the
>> defconfig as well, so it removed it. When you change that explicit
>> default to something else, it is up to you to change the defconfigs of
>> the old and new default boards.
>>
>> Your other option is to stop defining a default in the Kconfig and
>> instead mark the choice as "optional" (like I did for many other
>> selections like this that had no default explicitly - Kconfig
>> otherwise treats the first entry as default in that case) in which
>> case all defconfigs must have a specified vendor.
>
> OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
> OK I'll pull it into master.

It should also include removing the (now) redundant
CONFIG_VENDOR_EMULATION from that defconfig. That is best done by
running savedefconfig on that defconfig after applying the "x86: Make
QEMU the default vendor" patch.

Bin, are you in the middle of this?

Cheers,
-Joe
Bin Meng May 28, 2015, 2:41 a.m. UTC | #7
Hi Simon,

On Thu, May 28, 2015 at 7:25 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>>
>> Hi Bin,
>>
>> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > Hi Joe,
>> >
>> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>> > <joe.hershberger@gmail.com> wrote:
>> >> Hi Bin,
>> >>
>> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >>> Hi Simon,
>> >>>
>> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>> >>>> This commit reverts the change.
>> >>>>
>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> >>>> ---
>> >>>>
>> >>>>  configs/coreboot-x86_defconfig | 1 +
>> >>>>  1 file changed, 1 insertion(+)
>> >>>>
>> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>> >>>> index 66f94d0..799853f 100644
>> >>>> --- a/configs/coreboot-x86_defconfig
>> >>>> +++ b/configs/coreboot-x86_defconfig
>> >>>> @@ -1,4 +1,5 @@
>> >>>>  CONFIG_X86=y
>> >>>> +CONFIG_VENDOR_COREBOOT=y
>> >>>>  CONFIG_TARGET_COREBOOT=y
>> >>>>  CONFIG_OF_CONTROL=y
>> >>>>  CONFIG_DM_PCI=y
>> >>>> --
>> >>>
>> >>> Please apply this patch after commit
>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>> >>> and before commit
>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>> >>> I've verified the build with buildman on a new 'testing' branch with
>> >>> insertion of this patch.
>> >>
>> >> This should be squashed as part of
>> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>> >>
>> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>> >>
>> >
>> > I still don't get it. commit 65c4ac0 introduced
>> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>> > to clean up the defconfig. I suspect there was something wrong with
>> > 'savedefconfig'?
>>
>> No, savedefconfig is doing exactly what it should. Before your patch,
>> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>> Therefore savedefconfig sees it as redundant to specify that in the
>> defconfig as well, so it removed it. When you change that explicit
>> default to something else, it is up to you to change the defconfigs of
>> the old and new default boards.
>>
>> Your other option is to stop defining a default in the Kconfig and
>> instead mark the choice as "optional" (like I did for many other
>> selections like this that had no default explicitly - Kconfig
>> otherwise treats the first entry as default in that case) in which
>> case all defconfigs must have a specified vendor.
>
> OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
> OK I'll pull it into master.
>

It looks OK. Another thing, I noticed this patch [1] is not in the
testing branch. Did you apply it to somewhere else?

[1] http://patchwork.ozlabs.org/patch/472988/

Regards,
Bin
Simon Glass May 28, 2015, 2:55 a.m. UTC | #8
HI Bin,

On May 27, 2015 8:41 PM, "Bin Meng" <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Thu, May 28, 2015 at 7:25 AM, Simon Glass <sjg@chromium.org> wrote:
> > Hi,
> >
> > On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com>
wrote:
> >>
> >> Hi Bin,
> >>
> >> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >> > Hi Joe,
> >> >
> >> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
> >> > <joe.hershberger@gmail.com> wrote:
> >> >> Hi Bin,
> >> >>
> >> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com>
wrote:
> >> >>> Hi Simon,
> >> >>>
> >> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com>
wrote:
> >> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig"
accidentally
> >> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from
configs/coreboot-x86_defconfig.
> >> >>>> This commit reverts the change.
> >> >>>>
> >> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> >>>> ---
> >> >>>>
> >> >>>>  configs/coreboot-x86_defconfig | 1 +
> >> >>>>  1 file changed, 1 insertion(+)
> >> >>>>
> >> >>>> diff --git a/configs/coreboot-x86_defconfig
b/configs/coreboot-x86_defconfig
> >> >>>> index 66f94d0..799853f 100644
> >> >>>> --- a/configs/coreboot-x86_defconfig
> >> >>>> +++ b/configs/coreboot-x86_defconfig
> >> >>>> @@ -1,4 +1,5 @@
> >> >>>>  CONFIG_X86=y
> >> >>>> +CONFIG_VENDOR_COREBOOT=y
> >> >>>>  CONFIG_TARGET_COREBOOT=y
> >> >>>>  CONFIG_OF_CONTROL=y
> >> >>>>  CONFIG_DM_PCI=y
> >> >>>> --
> >> >>>
> >> >>> Please apply this patch after commit
> >> >>>
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
> >> >>> and before commit
> >> >>>
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
.
> >> >>> I've verified the build with buildman on a new 'testing' branch
with
> >> >>> insertion of this patch.
> >> >>
> >> >> This should be squashed as part of
> >> >>
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
> >> >>
> >> >> You need to remember to run savedefconfig when changing Kconfig or
defconfig.
> >> >>
> >> >
> >> > I still don't get it. commit 65c4ac0 introduced
> >> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
> >> > to clean up the defconfig. I suspect there was something wrong with
> >> > 'savedefconfig'?
> >>
> >> No, savedefconfig is doing exactly what it should. Before your patch,
> >> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
> >> Therefore savedefconfig sees it as redundant to specify that in the
> >> defconfig as well, so it removed it. When you change that explicit
> >> default to something else, it is up to you to change the defconfigs of
> >> the old and new default boards.
> >>
> >> Your other option is to stop defining a default in the Kconfig and
> >> instead mark the choice as "optional" (like I did for many other
> >> selections like this that had no default explicitly - Kconfig
> >> otherwise treats the first entry as default in that case) in which
> >> case all defconfigs must have a specified vendor.
> >
> > OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
> > OK I'll pull it into master.
> >
>
> It looks OK. Another thing, I noticed this patch [1] is not in the
> testing branch. Did you apply it to somewhere else?

I was worried that it was submitted after the merge window and affects
other code.
>
> [1] http://patchwork.ozlabs.org/patch/472988/
>
> Regards,
> Bin

Regards,
Simon
Bin Meng May 28, 2015, 3:04 a.m. UTC | #9
Hi Joe,

On Thu, May 28, 2015 at 7:51 AM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Wed, May 27, 2015 at 6:25 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi,
>>
>> On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>>>
>>> Hi Bin,
>>>
>>> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> > Hi Joe,
>>> >
>>> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>>> > <joe.hershberger@gmail.com> wrote:
>>> >> Hi Bin,
>>> >>
>>> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> >>> Hi Simon,
>>> >>>
>>> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>>> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>>> >>>> This commit reverts the change.
>>> >>>>
>>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> >>>> ---
>>> >>>>
>>> >>>>  configs/coreboot-x86_defconfig | 1 +
>>> >>>>  1 file changed, 1 insertion(+)
>>> >>>>
>>> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>>> >>>> index 66f94d0..799853f 100644
>>> >>>> --- a/configs/coreboot-x86_defconfig
>>> >>>> +++ b/configs/coreboot-x86_defconfig
>>> >>>> @@ -1,4 +1,5 @@
>>> >>>>  CONFIG_X86=y
>>> >>>> +CONFIG_VENDOR_COREBOOT=y
>>> >>>>  CONFIG_TARGET_COREBOOT=y
>>> >>>>  CONFIG_OF_CONTROL=y
>>> >>>>  CONFIG_DM_PCI=y
>>> >>>> --
>>> >>>
>>> >>> Please apply this patch after commit
>>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>>> >>> and before commit
>>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>>> >>> I've verified the build with buildman on a new 'testing' branch with
>>> >>> insertion of this patch.
>>> >>
>>> >> This should be squashed as part of
>>> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>>> >>
>>> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>>> >>
>>> >
>>> > I still don't get it. commit 65c4ac0 introduced
>>> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>>> > to clean up the defconfig. I suspect there was something wrong with
>>> > 'savedefconfig'?
>>>
>>> No, savedefconfig is doing exactly what it should. Before your patch,
>>> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>>> Therefore savedefconfig sees it as redundant to specify that in the
>>> defconfig as well, so it removed it. When you change that explicit
>>> default to something else, it is up to you to change the defconfigs of
>>> the old and new default boards.
>>>

Thanks for the explanation. Now it is clear to me.

>>> Your other option is to stop defining a default in the Kconfig and
>>> instead mark the choice as "optional" (like I did for many other
>>> selections like this that had no default explicitly - Kconfig
>>> otherwise treats the first entry as default in that case) in which
>>> case all defconfigs must have a specified vendor.
>>
>> OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
>> OK I'll pull it into master.
>
> It should also include removing the (now) redundant
> CONFIG_VENDOR_EMULATION from that defconfig. That is best done by
> running savedefconfig on that defconfig after applying the "x86: Make
> QEMU the default vendor" patch.
>
> Bin, are you in the middle of this?
>

Yes, now we should remove CONFIG_VENDOR_EMULATION from
configs/qemu-x86_defconfig. But if removing this, I feel this is
inconsistent with other x86 boards' defconfig. Is this really a
must-have to replace the defconfig with the one generated by
'savedefconfig'? If so, can we make it part of checkpatch.pl so that
each time we submit the patch which contains modifications to the
defconfig we can make sure the file is clean?

Regards,
Bin
Bin Meng May 28, 2015, 3:10 a.m. UTC | #10
Hi Simon,

On Thu, May 28, 2015 at 10:55 AM, Simon Glass <sjg@chromium.org> wrote:
> HI Bin,
>
> On May 27, 2015 8:41 PM, "Bin Meng" <bmeng.cn@gmail.com> wrote:
>>
>> Hi Simon,
>>
>> On Thu, May 28, 2015 at 7:25 AM, Simon Glass <sjg@chromium.org> wrote:
>> > Hi,
>> >
>> > On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com>
>> > wrote:
>> >>
>> >> Hi Bin,
>> >>
>> >> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >> > Hi Joe,
>> >> >
>> >> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>> >> > <joe.hershberger@gmail.com> wrote:
>> >> >> Hi Bin,
>> >> >>
>> >> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com>
>> >> >> wrote:
>> >> >>> Hi Simon,
>> >> >>>
>> >> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com>
>> >> >>> wrote:
>> >> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig"
>> >> >>>> accidentally
>> >> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from
>> >> >>>> configs/coreboot-x86_defconfig.
>> >> >>>> This commit reverts the change.
>> >> >>>>
>> >> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> >> >>>> ---
>> >> >>>>
>> >> >>>>  configs/coreboot-x86_defconfig | 1 +
>> >> >>>>  1 file changed, 1 insertion(+)
>> >> >>>>
>> >> >>>> diff --git a/configs/coreboot-x86_defconfig
>> >> >>>> b/configs/coreboot-x86_defconfig
>> >> >>>> index 66f94d0..799853f 100644
>> >> >>>> --- a/configs/coreboot-x86_defconfig
>> >> >>>> +++ b/configs/coreboot-x86_defconfig
>> >> >>>> @@ -1,4 +1,5 @@
>> >> >>>>  CONFIG_X86=y
>> >> >>>> +CONFIG_VENDOR_COREBOOT=y
>> >> >>>>  CONFIG_TARGET_COREBOOT=y
>> >> >>>>  CONFIG_OF_CONTROL=y
>> >> >>>>  CONFIG_DM_PCI=y
>> >> >>>> --
>> >> >>>
>> >> >>> Please apply this patch after commit
>> >> >>>
>> >> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>> >> >>> and before commit
>> >> >>>
>> >> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>> >> >>> I've verified the build with buildman on a new 'testing' branch
>> >> >>> with
>> >> >>> insertion of this patch.
>> >> >>
>> >> >> This should be squashed as part of
>> >> >>
>> >> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>> >> >>
>> >> >> You need to remember to run savedefconfig when changing Kconfig or
>> >> >> defconfig.
>> >> >>
>> >> >
>> >> > I still don't get it. commit 65c4ac0 introduced
>> >> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>> >> > to clean up the defconfig. I suspect there was something wrong with
>> >> > 'savedefconfig'?
>> >>
>> >> No, savedefconfig is doing exactly what it should. Before your patch,
>> >> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>> >> Therefore savedefconfig sees it as redundant to specify that in the
>> >> defconfig as well, so it removed it. When you change that explicit
>> >> default to something else, it is up to you to change the defconfigs of
>> >> the old and new default boards.
>> >>
>> >> Your other option is to stop defining a default in the Kconfig and
>> >> instead mark the choice as "optional" (like I did for many other
>> >> selections like this that had no default explicitly - Kconfig
>> >> otherwise treats the first entry as default in that case) in which
>> >> case all defconfigs must have a specified vendor.
>> >
>> > OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
>> > OK I'll pull it into master.
>> >
>>
>> It looks OK. Another thing, I noticed this patch [1] is not in the
>> testing branch. Did you apply it to somewhere else?
>
> I was worried that it was submitted after the merge window and affects other
> code.

OK, understood. But I think it is safe to remove given no board is using it.

>>
>> [1] http://patchwork.ozlabs.org/patch/472988/
>>

Regards,
Bin
Bin Meng May 28, 2015, 3:05 p.m. UTC | #11
Hi Simon, Hi Joe,

On Thu, May 28, 2015 at 7:25 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi,
>
> On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger at gmail.com> wrote:
>>
>> Hi Bin,
>>
>> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> > Hi Joe,
>> >
>> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>> > <joe.hershberger at gmail.com> wrote:
>> >> Hi Bin,
>> >>
>> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> >>> Hi Simon,
>> >>>
>> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>> >>>> This commit reverts the change.
>> >>>>
>> >>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> >>>> ---
>> >>>>
>> >>>>  configs/coreboot-x86_defconfig | 1 +
>> >>>>  1 file changed, 1 insertion(+)
>> >>>>
>> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>> >>>> index 66f94d0..799853f 100644
>> >>>> --- a/configs/coreboot-x86_defconfig
>> >>>> +++ b/configs/coreboot-x86_defconfig
>> >>>> @@ -1,4 +1,5 @@
>> >>>>  CONFIG_X86=y
>> >>>> +CONFIG_VENDOR_COREBOOT=y
>> >>>>  CONFIG_TARGET_COREBOOT=y
>> >>>>  CONFIG_OF_CONTROL=y
>> >>>>  CONFIG_DM_PCI=y
>> >>>> --
>> >>>
>> >>> Please apply this patch after commit
>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>> >>> and before commit
>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>> >>> I've verified the build with buildman on a new 'testing' branch with
>> >>> insertion of this patch.
>> >>
>> >> This should be squashed as part of
>> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>> >>
>> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>> >>
>> >
>> > I still don't get it. commit 65c4ac0 introduced
>> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>> > to clean up the defconfig. I suspect there was something wrong with
>> > 'savedefconfig'?
>>
>> No, savedefconfig is doing exactly what it should. Before your patch,
>> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>> Therefore savedefconfig sees it as redundant to specify that in the
>> defconfig as well, so it removed it. When you change that explicit
>> default to something else, it is up to you to change the defconfigs of
>> the old and new default boards.
>>
>> Your other option is to stop defining a default in the Kconfig and
>> instead mark the choice as "optional" (like I did for many other
>> selections like this that had no default explicitly - Kconfig
>> otherwise treats the first entry as default in that case) in which
>> case all defconfigs must have a specified vendor.
>
> OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
> OK I'll pull it into master.
>

I just found u-boot.rom created from u-boot-x86/testing branch does
not have working network any more on QEMU. It is working on the
u-boot-x86/master branch.

=> set serverip 10.10.0.100;set ipaddr 10.10.0.108;set netmask
255.255.255.0;set gatewayip 10.10.0.100
=> run ramboot
*** ERROR: `serverip' not set
*** ERROR: `serverip' not set

But 'print' says the env indeed has the 'serverip'. I think there
should be something in the upstream that caused this after the
u-boot-x86/testing branch rebase. Do you know where might be the
problem?

Regards,
Bin
Andy Pont May 28, 2015, 3:22 p.m. UTC | #12
Hi Bin,
 
> I just found u-boot.rom created from u-boot-x86/testing branch does
> not have working network any more on QEMU. It is working on the
> u-boot-x86/master branch.
> 
> => set serverip 10.10.0.100;set ipaddr 10.10.0.108;set netmask
> 255.255.255.0;set gatewayip 10.10.0.100
> => run ramboot
> *** ERROR: `serverip' not set
> *** ERROR: `serverip' not set
> 
> But 'print' says the env indeed has the 'serverip'. I think there
> should be something in the upstream that caused this after the
> u-boot-x86/testing branch rebase. Do you know where might be the
> problem?

This looks very similar to an issue that got patched on the i.MX6 yesterday
by Stefano which said:

commit fd3056337e6fcc140f400e11edd33f6f1cb37de1
"Use env callbacks for net variables" has a side effect on i.MX6 boards
because they do not set CONFIG_NET: the ip address results not set, but it
is stored in the environment."

Might be something to check...

Regards,

Andy.
Simon Glass May 28, 2015, 11:24 p.m. UTC | #13
Hi Bin,

On 28 May 2015 at 09:05, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon, Hi Joe,
>
> On Thu, May 28, 2015 at 7:25 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi,
>>
>> On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger at gmail.com> wrote:
>>>
>>> Hi Bin,
>>>
>>> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> > Hi Joe,
>>> >
>>> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>>> > <joe.hershberger at gmail.com> wrote:
>>> >> Hi Bin,
>>> >>
>>> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> >>> Hi Simon,
>>> >>>
>>> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>>> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>>> >>>> This commit reverts the change.
>>> >>>>
>>> >>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> >>>> ---
>>> >>>>
>>> >>>>  configs/coreboot-x86_defconfig | 1 +
>>> >>>>  1 file changed, 1 insertion(+)
>>> >>>>
>>> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>>> >>>> index 66f94d0..799853f 100644
>>> >>>> --- a/configs/coreboot-x86_defconfig
>>> >>>> +++ b/configs/coreboot-x86_defconfig
>>> >>>> @@ -1,4 +1,5 @@
>>> >>>>  CONFIG_X86=y
>>> >>>> +CONFIG_VENDOR_COREBOOT=y
>>> >>>>  CONFIG_TARGET_COREBOOT=y
>>> >>>>  CONFIG_OF_CONTROL=y
>>> >>>>  CONFIG_DM_PCI=y
>>> >>>> --
>>> >>>
>>> >>> Please apply this patch after commit
>>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>>> >>> and before commit
>>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>>> >>> I've verified the build with buildman on a new 'testing' branch with
>>> >>> insertion of this patch.
>>> >>
>>> >> This should be squashed as part of
>>> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>>> >>
>>> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>>> >>
>>> >
>>> > I still don't get it. commit 65c4ac0 introduced
>>> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>>> > to clean up the defconfig. I suspect there was something wrong with
>>> > 'savedefconfig'?
>>>
>>> No, savedefconfig is doing exactly what it should. Before your patch,
>>> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>>> Therefore savedefconfig sees it as redundant to specify that in the
>>> defconfig as well, so it removed it. When you change that explicit
>>> default to something else, it is up to you to change the defconfigs of
>>> the old and new default boards.
>>>
>>> Your other option is to stop defining a default in the Kconfig and
>>> instead mark the choice as "optional" (like I did for many other
>>> selections like this that had no default explicitly - Kconfig
>>> otherwise treats the first entry as default in that case) in which
>>> case all defconfigs must have a specified vendor.
>>
>> OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
>> OK I'll pull it into master.
>>
>
> I just found u-boot.rom created from u-boot-x86/testing branch does
> not have working network any more on QEMU. It is working on the
> u-boot-x86/master branch.
>
> => set serverip 10.10.0.100;set ipaddr 10.10.0.108;set netmask
> 255.255.255.0;set gatewayip 10.10.0.100
> => run ramboot
> *** ERROR: `serverip' not set
> *** ERROR: `serverip' not set
>
> But 'print' says the env indeed has the 'serverip'. I think there
> should be something in the upstream that caused this after the
> u-boot-x86/testing branch rebase. Do you know where might be the
> problem?

I'll hold off pushing to master for now.

Regards,
Simon
Bin Meng May 29, 2015, 3:03 a.m. UTC | #14
Hi Andy,

On Thu, May 28, 2015 at 11:22 PM, Andy Pont <andy.pont@sdcsystems.com> wrote:
> Hi Bin,
>
>> I just found u-boot.rom created from u-boot-x86/testing branch does
>> not have working network any more on QEMU. It is working on the
>> u-boot-x86/master branch.
>>
>> => set serverip 10.10.0.100;set ipaddr 10.10.0.108;set netmask
>> 255.255.255.0;set gatewayip 10.10.0.100
>> => run ramboot
>> *** ERROR: `serverip' not set
>> *** ERROR: `serverip' not set
>>
>> But 'print' says the env indeed has the 'serverip'. I think there
>> should be something in the upstream that caused this after the
>> u-boot-x86/testing branch rebase. Do you know where might be the
>> problem?
>
> This looks very similar to an issue that got patched on the i.MX6 yesterday
> by Stefano which said:
>
> commit fd3056337e6fcc140f400e11edd33f6f1cb37de1
> "Use env callbacks for net variables" has a side effect on i.MX6 boards
> because they do not set CONFIG_NET: the ip address results not set, but it
> is stored in the environment."
>
> Might be something to check...
>

Thanks for the hint! Indeed there are something wrong in the upstream
which broke the networking on several x86 boards.

Regards,
Bin
Bin Meng May 29, 2015, 7:23 a.m. UTC | #15
Hi Simon,

On Fri, May 29, 2015 at 7:24 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 28 May 2015 at 09:05, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon, Hi Joe,
>>
>> On Thu, May 28, 2015 at 7:25 AM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi,
>>>
>>> On 27 May 2015 at 10:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>>>>
>>>> Hi Bin,
>>>>
>>>> On Wed, May 27, 2015 at 11:21 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> > Hi Joe,
>>>> >
>>>> > On Thu, May 28, 2015 at 12:13 AM, Joe Hershberger
>>>> > <joe.hershberger@gmail.com> wrote:
>>>> >> Hi Bin,
>>>> >>
>>>> >> On Wed, May 27, 2015 at 11:01 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> >>> Hi Simon,
>>>> >>>
>>>> >>> On Wed, May 27, 2015 at 11:55 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> >>>> Commit bd328eb "Clean all defconfigs with savedefconfig" accidentally
>>>> >>>> removed 'CONFIG_VENDOR_COREBOOT=y' from configs/coreboot-x86_defconfig.
>>>> >>>> This commit reverts the change.
>>>> >>>>
>>>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>> >>>> ---
>>>> >>>>
>>>> >>>>  configs/coreboot-x86_defconfig | 1 +
>>>> >>>>  1 file changed, 1 insertion(+)
>>>> >>>>
>>>> >>>> diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>>>> >>>> index 66f94d0..799853f 100644
>>>> >>>> --- a/configs/coreboot-x86_defconfig
>>>> >>>> +++ b/configs/coreboot-x86_defconfig
>>>> >>>> @@ -1,4 +1,5 @@
>>>> >>>>  CONFIG_X86=y
>>>> >>>> +CONFIG_VENDOR_COREBOOT=y
>>>> >>>>  CONFIG_TARGET_COREBOOT=y
>>>> >>>>  CONFIG_OF_CONTROL=y
>>>> >>>>  CONFIG_DM_PCI=y
>>>> >>>> --
>>>> >>>
>>>> >>> Please apply this patch after commit
>>>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=3506805839e14a67b0971b02c7784e37b85d5fbf
>>>> >>> and before commit
>>>> >>> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc.
>>>> >>> I've verified the build with buildman on a new 'testing' branch with
>>>> >>> insertion of this patch.
>>>> >>
>>>> >> This should be squashed as part of
>>>> >> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=05cab1d6da2a84911fa0ec0ffa8fa038adef4dbc
>>>> >>
>>>> >> You need to remember to run savedefconfig when changing Kconfig or defconfig.
>>>> >>
>>>> >
>>>> > I still don't get it. commit 65c4ac0 introduced
>>>> > 'CONFIG_VENDOR_COREBOOT=y' and was applied before your commit bd328eb
>>>> > to clean up the defconfig. I suspect there was something wrong with
>>>> > 'savedefconfig'?
>>>>
>>>> No, savedefconfig is doing exactly what it should. Before your patch,
>>>> CONFIG_VENDOR_COREBOOT was the default, explicitly in the Kconfig.
>>>> Therefore savedefconfig sees it as redundant to specify that in the
>>>> defconfig as well, so it removed it. When you change that explicit
>>>> default to something else, it is up to you to change the defconfigs of
>>>> the old and new default boards.
>>>>
>>>> Your other option is to stop defining a default in the Kconfig and
>>>> instead mark the choice as "optional" (like I did for many other
>>>> selections like this that had no default explicitly - Kconfig
>>>> otherwise treats the first entry as default in that case) in which
>>>> case all defconfigs must have a specified vendor.
>>>
>>> OK I've squashed that in and pushed to u-boot-x86/testing. If it looks
>>> OK I'll pull it into master.
>>>
>>
>> I just found u-boot.rom created from u-boot-x86/testing branch does
>> not have working network any more on QEMU. It is working on the
>> u-boot-x86/master branch.
>>
>> => set serverip 10.10.0.100;set ipaddr 10.10.0.108;set netmask
>> 255.255.255.0;set gatewayip 10.10.0.100
>> => run ramboot
>> *** ERROR: `serverip' not set
>> *** ERROR: `serverip' not set
>>
>> But 'print' says the env indeed has the 'serverip'. I think there
>> should be something in the upstream that caused this after the
>> u-boot-x86/testing branch rebase. Do you know where might be the
>> problem?
>
> I'll hold off pushing to master for now.
>

You may need to wait for Joe to submit a patch to fix this networking
issue. Please see
http://u-boot.10912.n7.nabble.com/PATCH-imx-missing-CONFIG-NET-after-consolidation-patches-td215762.html
for the discussion.

Regards,
Bin
diff mbox

Patch

diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
index 66f94d0..799853f 100644
--- a/configs/coreboot-x86_defconfig
+++ b/configs/coreboot-x86_defconfig
@@ -1,4 +1,5 @@ 
 CONFIG_X86=y
+CONFIG_VENDOR_COREBOOT=y
 CONFIG_TARGET_COREBOOT=y
 CONFIG_OF_CONTROL=y
 CONFIG_DM_PCI=y