Config fragments

[Simon Glass]

Hi Andrew,

On Tue, 29 Aug 2023 at 11:05, Andrew Davis <afd at ti.com> wrote:
>
> On 8/29/23 11:47 AM, Simon Glass wrote:
> > Hi Andrew,
> >
> > On Wed, 23 Aug 2023 at 10:44, Andrew Davis <afd at ti.com> wrote:
> >>
> >> On 8/23/23 10:30 AM, Simon Glass wrote:
> >>> Hi,
> >>>
> >>> Up until 2023.04 it has been possible to build all the defconfigs but
> >>> with 2023.07 that changed. Tom mentioned this to me recently.
> >>>
> >>> Up until 2023.04 we can enumerate all the board configs that can be
> >>> built. We can build any of them using a single name and a single
> >>> defconfig. The boards.cfg file which buildman creates contains a full
> >>> list of things that can be built.
> >>>
> >>>   From 2023.07 this changes and we now have random .config files
> >>> sprinkled about the place. I say random because they are not connected
> >>> to anything. For example, here is
> >>> board/ti/am62x/MAINTAINERS:
> >>>
> >>> AM62x BOARD
> >>> M: Dave Gerlach <d-gerlach at ti.com>
> >>> M: Tom Rini <trini at konsulko.com>
> >>> S: Maintained
> >>> F: board/ti/am62x/
> >>> F: include/configs/am62x_evm.h
> >>> F: configs/am62x_evm_r5_defconfig
> >>> F: configs/am62x_evm_a53_defconfig
> >>>
> >>> BEAGLEPLAY BOARD
> >>> M:     Nishanth Menon <nm at ti.com>
> >>> M:     Robert Nelson <robertcnelson at gmail.com>
> >>> M:     Tom Rini <trini at konsulko.com>
> >>> S:     Maintained
> >>> N:     beagleplay
> >>>
> >>> In most cases the MAINTAINERS file tells us about each board and until
> >>> [1] I had assumed that was the case. With that patch reverted, these
> >>> are the only 'orphaned' MAINTAINERS entries (buildman
> >>> --maintainer-check):
> >>>
> >>> WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
> >>> ending at line 8
> >>> WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at line 44
> >>> WARNING: orphaned defconfig in
> >>> board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
> >>> WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at line 6
> >>> WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 15
> >>>
> >>> But Tom advised me that MAINTAINERS is not intended for this purpose,
> >>> so the check has been dropped. I am not suggesting we bring it back,
> >>> necessarily, but if we did, we could use it to specify the .config and
> >>> defconfig files which are used by each board.
> >>>
> >>> *Failing that*, I suggest a new 'name.brd' file in the board directory
> >>> to contain this information, e.g.
> >>>
> >>> # Contents of board/ti/am62x/beagleplay.brd:
> >>> beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
> >>> beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
> >>>
> >>> This could be parsed by buildman and other tools, to produce a list of
> >>> available boards and to build each using a single name (e.g. make
> >>> beagleplay-r5_defconfig)
> >>>
> >>> What do people think?
> >>>
> >>
> >> How about instead of needing this new 'name.brd' files, we look into
> >> "include" directives inside these configs? So we could have a file:
> >>
> >> beagleplay_r5_defconfig
> >>
> >> in the normal configs/ directory, but with the contents:
> >>
> >> #include "configs/am62x_evm_r5_defconfig"
> >>
> >> CONFIG_DEFAULT_DEVICE_TREE="k3-am625-beagleplay"
> >> CONFIG_OF_LIST="k3-am625-beagleplay"
> >> ...
> >>
> >> The # is already a comment line so these should be safe
> >> for existing tools, and then we have in our Makefile
> >> an automatic pass through the C preprocessor.
> >>
> >> Some boards have can build with and without the fragments,
> >> so to have complete CI coverage, we have dummy defconfigs
> >> that have both the base and a fragment include:
> >>
> >> #include "configs/am62x_evm_r5_defconfig"
> >> #include "board/ti/am62x/high_security.config"
> >>
> >> Something like that, then as Heinrich mentioned, simply
> >> enumerating configs/*defconfig should yield all valid
> >> combinations for building/testing.
> >
> > I do agree it would be nice to have this information in the file it
> > relates to. But wouldn't that involve changing kconf and other tools?
> > What tools will parse those files? We really want 'make xxx_defconfig'
> > to work for these new 'combined config' boards and my understanding is
> > that kconf is used here.
>
> The U-Boot Makefile passes the provided xxx_defconfig into the kconfig
> scripts. All I'm suggesting is to have that Makefile run the passed
> in defconfig through the C preprocessor before handing it off to kconfig.

Could you take a look to see how hard that would be?

>
> For existing defconfigs the preprocessor will make no changes to the file.
> For the config fragments, the `#include` lines will be substituted with the
> contents, the result will be a normal defconfig file that can then also be
> passed into kconfig.

OK

>
> No changes to kconfig scripts, or anything other than the Makefile, are needed.
>
> >
> > Your proposal certainly allows each 'combined config' to have a name -
> > it is just the filename of the defconfig file.
> >
>
> Exactly, this keeps all current CI tooling/flows the exact same.

Indeed.

Regards,
SImon