[U-Boot] [PATCH v3 5/7] kconfig: switch to single .config configuration

Masahiro Yamada
2015-02-25T06:14:12Z

Hi Scott,

On Tue, 24 Feb 2015 18:17:59 -0600
Scott Wood <scottwood@freescale.com> wrote:

> On Tue, 2015-02-24 at 16:20 +0900, Masahiro Yamada wrote:
> > Hi Scott,
> >
> >
> > On Mon, 23 Feb 2015 19:22:51 -0600
> > Scott Wood <scottwood@freescale.com> wrote:
> >
> > > On Fri, 2015-02-20 at 14:24 +0900, Masahiro Yamada wrote:
> > > > When Kconfig for U-boot was examined, one of the biggest issues was
> > > > how to support multiple images (Normal, SPL, TPL). There were
> > > > actually two options, "single .config" and "multiple .config".
> > > > After some discussions and thought experiments, I chose the latter,
> > > > i.e. to create ".config", "spl/.config", "tpl/.config" for Normal,
> > > > SPL, TPL, respectively.
> > > >
> > > > It is true that the "multiple .config" strategy provided us the
> > > > maximum flexibility and helped to avoid duplicating CONFIGs among
> > > > Normal, SPL, TPL, but I have noticed some fatal problems:
> > > >
> > > > [1] It is impossible to share CONFIG options across the images.
> > > > If you change the configuration of Main image, you often have to
> > > > adjust some SPL configurations correspondingly. Currently, we
> > > > cannot handle the dependencies between them. It means one of the
> > > > biggest advantages of Kconfig is lost.
> > >
> > > Sharing can happen in the defconfig with "+S:"...
> >
> > Yes, it can as for "make *_defconfig".
> >
> > If we modify some options in .config for example by "make menuconfig",
> > we also modify some in spl/.config correspondingly.
> >
> > Users are responsible for configure .config and spl/.config in sync
> > in the sane combination.
> >
> >
> >
> > > What sort of dependencies are people wanting? Would it be possible to
> > > modify kconfig to import SPL .config into the main config (or vice
> > > versa?) with a name prefix so that dependencies could happen, without
> > > sacrificing the ability to set symbols independently?
> >
> > To have independent symboles coexist in a single .config,
> > I can only suggest to duplicate options like
> > CONFIG_FOO=0x100
> > CONFIG_SPL_FOO=0x200
> > CONFIG_TPL_FOO=0x300
>
> What I meant was a way to keep the configs separate, but automatically
> import the CONFIG_FOO from the SPL .config as CONFIG_SPL_FOO (or some
> other prefix that doesn't conflict with SPL-specific options).

What is the benefit of doing this?

> > > Or as Ian suggested, have only the main config be user-editable, but
> > > still let select/depends turn certain things on/off for the
> > > auto-generated SPL config.
> >
> > I guess it is possible for boolean options,
> > but impossible to set hex/int options independently.
>
> How many hex/int options are there, that need to be different in SPL
> versus the main U-Boot? Having a few CONFIG_SPL_xxx for those is better
> than having a bunch.

OK.
But, I do not think we need to tweak the Kconfig just for saving boolean options.

> > BTW, Ian's idea had been already achieved by include/config_uncmd_spl.h
>
> So, the answer is to avoid kconfig and go back to using the preprocessor
> for configuration? :-(

I am not saying I prefer the preprocessor.

Indeed, include/config_uncmd_spl.h is ugly,
so I'd like to propose a better solution.

If we introduce CONFIG_SPL_DM, for example, the ifdef conditional in source files
will be like this:

#if (!defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM)) || \
(defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_DM))

[Driver Model Code]

#else
[Non Driver Model Code]
#endif

This is too ugly to be written in each conditional.

So, I want to describe like this:

#if IS_ENABLED_CONFIG(DM)
[Driver Model Code]
#else
[Non Driver Model Code]
#endif

I will post some patches later on.

> > > > [2] It is too painful to change both ".config" and "spl/.config".
> > > > Sunxi guys started to work around this problem by creating a new
> > > > configuration target. Commit cbdd9a9737cc (sunxi: kconfig: Add
> > > > %_felconfig rule to enable FEL build of sunxi platforms.) added
> > > > "make *_felconfig" to enable CONFIG_SPL_FEL on both images.
> > > > Changing the configuration of multiple images in one command is a
> > > > generic demand. The current implementation cannot propose any
> > > > good solution about this.
> > >
> > > How about defconfig fragments? Instead of having script infrastructure
> > > specifically for CONFIG_SPL_FEL, merge a fragment containing
> > > "+S:CONFIG_SPL_FEL".
> >
> > Do you mean something like this?
> > U-boot proper : common/.config + .config
> > SPL : common/.config + spl/.config
> > TPL : common/.config + tpl/.config
>
> No, I meant having a fragment containing only "+S:CONFIG_SPL_FEL" that
> could be merged into any other config.

So, the fragment is something like the _common_ .config, right?

> > > > [3] Kconfig files are getting ugly and difficult to understand.
> > > > Commit b724bd7d6349 (dm: Kconfig: Move CONFIG_SYS_MALLOC_F_LEN to
> > > > Kconfig) has sprinkled "if !SPL_BUILD" over the Kconfig files.
> > >
> > > It seems like the root cause of this sprinkling is wanting to use
> > > default y to avoid touching a bunch of defconfig files, but not wanting
> > > to do the default y at the toplevel Kconfig. Maybe better tooling for
> > > bulk defconfig updates would help.
> >
> > Yes. If we could move the default settings into defconfig files
> > (and defconfig is just for that purpose), this problem would go away.
> > But, in the duscussion with Simon and Alexey, we understood
> > maintaining many defconfigs in sync is a pain.
>
> I think that's a problem that needs to be solved regardless of SPL.

Agree.
I think we can live the defaults in Kconfig, but I am still searching for a different solution.

> > > In any case, couldn't you do
> > > CONFIG_SPL_DM currently, by making DM depend on "!SPL_BUILD || SPL_DM",
> > > without fundamentally changing the SPL kconfig infrastructure?
> >
> > As for the Driver Model options, the dependency descriptions will get ugly,
> > but we won't carry them so long.
> > In a long run, all the boards will be converted and eventually CONFIG_DM
> > will bocome the default.
>
> ...so it's not a very good example of why the current situation must
> change.

Right, but we still have many other options that can be enabled on SPL.

> > > Why do symbols like LOCALVERSION and CC_OPTIMIZE_FOR_SIZE depend on !
> > > SPL_BUILD?
> >
> > These two options are used by the top-level Makefile
> > and it is automatically propagated to spl/*.
> >
> > It is harmless to define them again in spl/.config, but meaningless.
>
> ...so not all of the existing !SPL_BUILD instances in Kconfig need to be
> there.
>
> > > > [4] The build system got more complicated than it should be.
> > > > To adjust Linux-originated Kconfig to U-Boot, the helper script
> > > > "scripts/multiconfig.sh" was introduced. Writing a complicated
> > > > text processor is a shell script sometimes caused problems.
> > > >
> > > > Now I believe the "single .config" will serve us better. With it,
> > > > all the problems above would go away. Instead, we will have to add
> > > > some CONFIG_SPL_* (and CONFIG_TPL_*) options such as CONFIG_SPL_DM,
> > > > but we will not have much. Anyway, this is what we do now in
> > > > scripts/Makefile.spl.
> > >
> > > I had been hoping that the split configs would let us get rid of many of
> > > the CONFIG_SPL_* options that we already have.
> > >
> > > How will TPL be handled? Are you going to duplicate all the SPL
> > > symbols? Or just avoid ever kconfigizing them?
> >
> > Not all, but I expect some duplicated CONFIG_TPL_* such as CONFIG_TPL_TEXT_BASE.
>
> I'm not talking about TEXT_BASE. I'm talking about stuff like this:

We have to add some CONFIG_TPL_*, but we will just have 20.

> #ifdef CONFIG_TPL_BUILD
> #define CONFIG_SPL_NAND_BOOT
> #define CONFIG_SPL_FLUSH_IMAGE
> #define CONFIG_SPL_ENV_SUPPORT
> #define CONFIG_SPL_NAND_INIT
> #define CONFIG_SPL_SERIAL_SUPPORT
> #define CONFIG_SPL_LIBGENERIC_SUPPORT
> #define CONFIG_SPL_LIBCOMMON_SUPPORT
> #define CONFIG_SPL_I2C_SUPPORT
> #define CONFIG_SPL_NAND_SUPPORT
> #define CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
> #define CONFIG_SPL_COMMON_INIT_DDR
> #define CONFIG_SPL_MAX_SIZE (128 << 10)
> #define CONFIG_SPL_TEXT_BASE 0xf8f81000
> #define CONFIG_SYS_MPC85XX_NO_RESETVEC
> #define CONFIG_SYS_NAND_U_BOOT_SIZE (832 << 10)
> #define CONFIG_SYS_NAND_U_BOOT_DST (0x11000000)
> #define CONFIG_SYS_NAND_U_BOOT_START (0x11000000)
> #define CONFIG_SYS_NAND_U_BOOT_OFFS ((128 + 128) << 10)
> #elif defined(CONFIG_SPL_BUILD)
> #define CONFIG_SPL_INIT_MINIMAL
> #define CONFIG_SPL_SERIAL_SUPPORT
> #define CONFIG_SPL_NAND_SUPPORT
> #define CONFIG_SPL_FLUSH_IMAGE
> #define CONFIG_SPL_TEXT_BASE 0xff800000
> #define CONFIG_SPL_MAX_SIZE 4096
> #define CONFIG_SYS_NAND_U_BOOT_SIZE (128 << 10)
> #define CONFIG_SYS_NAND_U_BOOT_DST 0xf8f80000
> #define CONFIG_SYS_NAND_U_BOOT_START 0xf8f80000
> #define CONFIG_SYS_NAND_U_BOOT_OFFS (128 << 10)
> #endif
>
> If symbols like CONFIG_SPL_I2C_SUPPORT or CONFIG_SPL_COMMON_INIT_DDR get
> kconfigized, how would you handle them being in TPL but not SPL?

We can add CONFIG_TPL_* if necessary.
As I said, if we swap the order of SPL and TPL, we will be able to save
CONFIG_TPL_* defines.

> > Currently, U-Boot runs SPL, TPL, and U-Boot proper in this order, but
> > in hindsight, it might have been better to run
> > TPL, SPL, and U-Boot proper, in this order.
>
> TPL is just makefile infrastructure for inserting an extra stage. It
> doesn't refer to the contents.
>
> > In 4KB memory footprint, it is impossible to include Driver Model.
> > It would be a really ad-hoc implementation.
>
> "Is", not "would be". And this applies to some SPL targets without TPL
> as well.
>
> > In the former order, we need CONFIG_TPL_DM,
> > but in the latter, we can save it.
> >
> > I know TPL means "Third Program Loader", but
> > can we perhaps swap the order
> > if we assume TPL is the abbreviation of "Tiny Program Loader" ?
>
> If you redefine TPL to mean SPL that doesn't use certain code, you'll
> end up with targets that have TPL but no SPL. Are you sure this is
> simplifying anything?

Sorry, I can't get it.
What I expect is like follows:

CONFIG_TPL still depends on CONFIG_SPL.

We have three options for the boot procedure:

[1] U-Boot-proper (CONFIG_SPL is not defined)

[2] SPL + U-Boot-proper (CONFIG_SPL is defined)

[3] TPL + SPL + U-Boot-proper (CONFIG_SPL and CONFIG_TPL are defined)

The image size: TPL < SPL < U-Boot-proper

Driver Model and some other features are available on SPL if CONFIG_SPL_* is defined.

Almost no systematic infrastructure is available on TPL, so we will have
very small number of CONFIG_TPL_*.

> > > > - Add some entries to include/config_uncmd_spl.h and the new file
> > > > scripts/Makefile.uncmd_spl. Some CONFIG options that are not
> > > > supported on SPL must be disabled because one .config is shared
> > > > between SPL and U-Boot proper going forward. I know this is not
> > > > a beautiful solution and I think we can do better, but let's see
> > > > how much we will have to describe them.
> > >
> > > How is uncmd_spl better than "!SPL_BUILD"?
> >
> > We can use Kconfig as it is in Linux.
>
> Not after this patch.
>

Right, I need to do more cleanups for that.

Best Regards
Masahiro Yamada