public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3 5/7] kconfig: switch to single .config configuration
Date: Tue, 24 Feb 2015 18:17:59 -0600	[thread overview]
Message-ID: <1424823479.4698.38.camel@freescale.com> (raw)
In-Reply-To: <20150224162048.63A7.AA925319@jp.panasonic.com>

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

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

> 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? :-(

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

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

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

> > 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:

#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?

> 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?

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

-Scott

  reply	other threads:[~2015-02-25  0:17 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-20  5:24 [U-Boot] [PATCH v3 0/7] kconfig: turnaround into single .config Masahiro Yamada
2015-02-20  5:24 ` [U-Boot] [PATCH v3 1/7] ARM: UniPhier: set CONFIG_SYS_MALLOC_F to the global default value Masahiro Yamada
2015-02-20  5:24 ` [U-Boot] [PATCH v3 2/7] malloc_f: fix broken .config caused by CONFIG_SYS_MALLOC_F Masahiro Yamada
2015-02-20 17:11   ` Simon Glass
2015-02-20  5:24 ` [U-Boot] [PATCH v3 3/7] kconfig: Adjust ordering so that defaults work as expected Masahiro Yamada
2015-02-20  5:24 ` [U-Boot] [PATCH v3 4/7] malloc_f: add ARCH_MALLOC_F_LEN to specify SoC-default malloc_f_len Masahiro Yamada
2015-02-20 17:11   ` Simon Glass
2015-02-20 17:11   ` Masahiro YAMADA
2015-02-20  5:24 ` [U-Boot] [PATCH v3 5/7] kconfig: switch to single .config configuration Masahiro Yamada
2015-02-20 16:59   ` Simon Glass
2015-02-24  1:22   ` Scott Wood
2015-02-24  7:20     ` Masahiro Yamada
2015-02-25  0:17       ` Scott Wood [this message]
2015-02-25  6:14         ` Masahiro Yamada
2015-02-25 13:40           ` Simon Glass
2015-02-25 23:29           ` Scott Wood
2015-02-24 16:42     ` Simon Glass
2015-02-20  5:25 ` [U-Boot] [PATCH v3 6/7] kconfig: remove unneeded dependency on !SPL_BUILD Masahiro Yamada
2015-02-20 17:06   ` Simon Glass
2015-02-20 17:54     ` Stephen Warren
2015-02-20 18:39       ` Simon Glass
2015-02-21  0:54         ` Masahiro YAMADA
2015-02-21  2:28           ` Simon Glass
2015-02-21  2:37             ` Masahiro YAMADA
2015-02-23 14:02               ` Simon Glass
2015-02-23 17:33                 ` Stephen Warren
2015-02-23 17:44                   ` Simon Glass
2015-02-24  5:05                     ` Masahiro Yamada
2015-02-24 16:45                       ` Stephen Warren
2015-02-24 13:36                 ` Masahiro Yamada
2015-02-20  5:25 ` [U-Boot] [PATCH v3 7/7] malloc_f: enable SYS_MALLOC_F by default if DM is on Masahiro Yamada
2015-02-20 17:08   ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1424823479.4698.38.camel@freescale.com \
    --to=scottwood@freescale.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox