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

Mon Sep 12 16:23:45 CEST 2016

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).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160912/d95144b3/attachment.sig>