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

[Simon Glass]

HI Bin,

On May 27, 2015 8:41 PM, "Bin Meng" <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> 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.
> >
>
> 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