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

[Bin Meng]

Hi Simon,

On Thu, May 28, 2015 at 10:55 AM, Simon Glass <sjg at chromium.org> wrote:
> 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.

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