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

[Bin Meng]

Hi Joe,

On Thu, May 28, 2015 at 7:51 AM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> Hi Simon,
>
> On Wed, May 27, 2015 at 6:25 PM, 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.
>>>

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