[U-Boot] [PATCH 1/2] configs: Resync with savedefconfig

[Masahiro Yamada]

2016-09-10 7:25 GMT+09:00 Stephen Warren <swarren at wwwdotorg.org>:
> On 09/09/2016 03:14 PM, Tom Rini wrote:
>>
>> On Fri, Sep 09, 2016 at 03:06:01PM -0600, Stephen Warren wrote:
>>>
>>> On 09/09/2016 01:11 PM, Tom Rini wrote:
>>>>
>>>> On Fri, Sep 09, 2016 at 01:09:43PM -0600, Stephen Warren wrote:
>>>>>
>>>>> On 09/09/2016 01:02 PM, Tom Rini wrote:
>>>>>>
>>>>>> On Fri, Sep 09, 2016 at 10:21:45AM -0600, Stephen Warren wrote:
>>>>>>>
>>>>>>> On 09/08/2016 07:19 PM, Tom Rini wrote:
>>>>>>>>
>>>>>>>> Signed-off-by: Tom Rini <trini at konsulko.com>
>>>>>>
>>>>>> /bin/bash: ess: command not found
>>>>>>>>
>>>>>>>> diff --git a/configs/harmony_defconfig b/configs/harmony_defconfig
>>>>>>>
>>>>>>>
>>>>>>>> -CONFIG_USE_PRIVATE_LIBGCC=y
>>>>>>>
>>>>>>>
>>>>>>> I assume that's because =y is the default for that now?
>>>>>>
>>>>>>
>>>>>> Yes.
>>>>>
>>>>>
>>>>>>>> diff --git a/configs/p2771-0000-500_defconfig
>>>>>>>> b/configs/p2771-0000-500_defconfig
>>>>>>>
>>>>>>>
>>>>>>>> -CONFIG_TARGET_P2771_0000=y
>>>>>>>
>>>>>>>
>>>>>>> I think we need to keep those two. Those two defconfigs are slightly
>>>>>>> different configurations of U-Boot's p2771-0000 board/target.
>>>>>>
>>>>>>
>>>>>> OK, then you need to submit a patch to fix the underlying problem, if
>>>>>> there is one, really.  Keep in mind that what this is, is re-running
>>>>>> savedefconfig for each target.  So if something odd drops out that
>>>>>> means
>>>>>> that it wasn't having any effect before.
>>>>>
>>>>>
>>>>> I don't believe there is any underlying problem. The Kconfig option
>>>>> in question is defined in arch/arm/mach-tegra/tegra186/Kconfig, when
>>>>> building for p2771-0000-000 the option makes it into .config just
>>>>> fine, and the value is used by board/nvidia/p2771-0000/Kconfig.
>>>>> Isn't this a bug in savedefconfig? I'm CC'ing Masahiro to get some
>>>>> insight.
>>>>
>>>>
>>>> It is the default value then and isn't saved is I believe the answer.
>>>
>>>
>>> I don't think it's the default; nothing selects that option and it
>>> has no "default y".
>>>
>>> Or maybe since the value is inside a choice, and is the only entry
>>> that's there, that does make it the default? If so, that seems a
>>> little fragile; what if someone comes along and adds a new board
>>> into the list, and puts it before the existing entry (e.g. to
>>> maintain alphabetical sorting). That would changing the meaning of
>>> the current defconfig file if the first entry is picked as default.
>>> If so, I'm surprised if nobody has had that issue yet.
>>
>>
>> It's all correct and I think we have had this hit once or twice when
>> adding or changing the order.  It is a side effect of how Kconfig just
>> works.
>
>
> I'm not convinced. If I apply your patch, and also the following:
>
>> diff --git a/arch/arm/mach-tegra/tegra186/Kconfig
>> b/arch/arm/mach-tegra/tegra186/Kconfig
>> index 97cf23f31f80..05f691ed3ede 100644
>> --- a/arch/arm/mach-tegra/tegra186/Kconfig
>> +++ b/arch/arm/mach-tegra/tegra186/Kconfig
>> @@ -7,6 +7,14 @@ if TEGRA186
>>  choice
>>         prompt "Tegra186 board select"
>>
>> +config TARGET_E9999_1234
>> +       bool "NVIDIA Tegra186 E9999-1234 board"
>> +       help
>> +         P2771-0000 is a P3310 CPU board married to a P2597 I/O board.
>> The
>> +         combination contains SoC, DRAM, eMMC, SD card slot, HDMI, USB
>> +         micro-B port, Ethernet, USB3 host port, SATA, PCIe, and two GPIO
>> +         expansion headers.
>> +
>>  config TARGET_P2771_0000
>>         bool "NVIDIA Tegra186 P2771-0000 board"
>>         help
>
>
> ... then I get a build failure for p2771-0000-000_defconfig since .config
> ends up containing CONFIG_TARGET_E9999_1234 rather than
> CONFIG_TARGET_P2771_0000. I think we want to avoid that don't we? If we
> don't, savedefconfig in the face of choice options seems rather too fragile,
> or we need to force people to update *_defconfig any time any Kconfig choice
> is edited.


We have discussed this problem several times before.

  - The first entry in a "choice" is the default, unless otherwise specified.
  - "make savedefconfig" drops all the defaults to create a minimum
set of configs.

So, we need to be careful when we insert a new entry at the first
or delete the first entry in a "choice".

We are supposed to not re-sync defconfigs in Linux Kernel, so we are not hit
by this issue much there, but we still be careful when we edit the
first entry of a choice.
This potential fragileness exists in any projects that use Kconfig.


In U-Boot, unfortunately, we need defconfig resync periodically.

As you noticed, "update *_defconfig any time any Kconfig choice is edited."
is one solution.


Or, you can specify a default explicitly, like I do in
arch/arm/mach-uniphier/Kconfig:


choice
        prompt "UniPhier SoC select"
        default ARCH_UNIPHIER_PRO4


-- 
Best Regards
Masahiro Yamada