[U-Boot] [PATCH 1/2] configs: Resync with savedefconfig
Masahiro Yamada
yamada.masahiro at socionext.com
Wed Sep 14 04:27:36 CEST 2016
2016-09-13 3:28 GMT+09:00 Stephen Warren <swarren at wwwdotorg.org>:
> On 09/12/2016 08:23 AM, Tom Rini wrote:
>>
>> On Mon, Sep 12, 2016 at 11:24:35AM +0900, Masahiro Yamada wrote:
>>>
>>> 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
>>
>>
>> I am of the opinion, at least currently, that we need to make wider use
>> of "default X" in choice statements and that configs/ should be treated
>> as generated files. I am frankly surprised that the Kernel stopped
>> doing this because I would have sworn that wasn't true in the past (one
>> shouldn't push unrelated "cleanups" to defconfig files but you also
>> better put stuff where savedefconfig puts things as otherwise it would
>> be the old time conflict hell madness of pre-Kconfig days).
>
>
> Can we modify Kconfig in U-Boot (maybe upstream too) to require a default
> value for choices? Doing this by code review is likely to miss some.
I do not think we need that.
The savedefconfig re-sync problem happens anytime we change the default
in Kconfig entries.
Any CONFIG has its implied default.
For boolean/tristate CONFIGs, "default n" is implied
unless default is explicitly described.
Likewise, for choices, the first entry is the implied default.
I do not think this is so strange.
People should recognize adding/deleting the first entry of choices
means changing the default.
As I mentioned, we should be careful when we do so.
--
Best Regards
Masahiro Yamada
More information about the U-Boot
mailing list