Hi Tom, On Tue, 14 Feb 2023 at 09:31, Tom Rini <[email protected]> wrote: > > On Sun, Feb 12, 2023 at 04:16:04PM -0700, Simon Glass wrote: > > > At present kconfig writes out several files, including: > > > > auto.conf - CONFIG settings used by make > > autoconf.h - header file used by C code > > > > This works well but is a bit ugly in places, for example requiring the use > > of a SPL_TPL_ macro in Makefiles to distinguish between options intended > > for SPL and U-Boot proper. > > > > Update the kconfig tool to also output separate files for each phase: e.g. > > auto_spl.conf and autoconf_spl.h > > > > These are similar to the existing files, but drop the SPL_ prefix so that > > SPL_TPL_ is not needed. It also allows the CONFIG_IS_ENABLED() macro to be > > simplified, in a later patch, eventually replacing it with IS_ENABLED(). > > > > When CONFIG_FOO is used within SPL, it means that FOO is enabled in that > > SPL phase. For example if CONFIG_SPL_FOO is enabled in the Kconfig, that > > means that CONFIG_FOO will be enabled in the SPL phase. So the SPL builds > > can just use CONFIG_FOO to check it. There is no need to use > > CONFIG_SPL_FOO or CONFIG_IS_ENABLED() anymore. > > > > This of course means that if there is a need to access a PPL symbol from > > an SPL build, there is no way to do it. To copy with that, we need a > > CONFIG_PPL_FOO to be visibilty to all SPL builds. > > > > So this change also adds new PPL_ output for U-Boot proper (Primary > > Program Loader). So every CONFIG_FOO that is enabled in PPL also has a > > CONFIG_PPL_FOO > > > > This allows SPL to access the TEXT_BASE for U-Boot proper, for example, so > > it knows where to load it. There are about 30 places where this is needed, > > in addition to TEXT_BASE. The environment has the same problem, adding > > another dozen or so caes in include/config_distro_bootcmd.h but it has > > been decided to ignore that for now. > > > > The feature is controlled by an environment variable, since it seems to be > > bad form to add flags to the conf tool. > > > > Rebuild the autoconf files if the split config is not present. This allows > > building this commit as part of a chain, without generating build errors. > > > > These changes may benefit from some reworking to send upstream, e.g. to > > use a struct for the 'arg' parameter. > > > > Signed-off-by: Simon Glass <[email protected]> > > This patch, I think, is where my largest problem is. We go from being > able to say "if CONFIG_SPL_FOO is undefined, it is false" to "we must > define CONFIG_SPL_FOO to false". There's around 150 cases of this, with > the series. Why can we not extend the PPL logic (which I'm not super > happy with, but, I understand and I think an audit of everything > not-TEXT_BASE should be fairly straight forward), to say that if > CONFIG_FOO exists and CONFIG_SPL_FOO does not exist, say CONFIG_SPL_FOO > is now false.
Well it is all about choices. We don't have to add a CONFIG_SPL_xxx to tell Kconfig that the PPL symbol controls all phases. We can use the conf_nospl file instead. But then the entire description is not in Kconfig. Of course, we might expect that some of those things in conf_nospl might end up needing to be controlled in SPL, so perhaps that file would shrink? Not sure about that, though. It isn't just SPL , BTW. We might have any xPL symbol defined. I suppose you are thinking of something like: #define CONFIG_IS_ENABLED(x) IS_ENABLED(CONFIG_ ## xpl_prefix ## x) || ( IS_ENABLED(CONFIG_ ## x) && !IS_ENABLED(CONFIG_ SPL_ ## x) && !IS_ENABLED(CONFIG_TPL_ ##x) ..) But how do we deal with Makefiles? We still end up with the SPL_TPL_ stuff. Yes, adding PPL moves a step forward and reduces the audit to only uses of PPL, instead of the whole Kconfig. This series does get us closer to separate configs (it is really easy to see what is enabled in each build just by looking at the auto_xpl.conf files) and I think those 150 exceptions are not a big price to pay. Ultimately I am coming to the view that we should extend the Kconfig language to support multiple build phases, using a 'phase' property. Zephyr is going to need it too, fairly soon[1]. Regards, Simon [1] https://github.com/zephyrproject-rtos/zephyr/issues/54534
