All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.