[U-Boot] [PATCH v2 21/44] Convert CONFIG_SPL_GPIO_SUPPORT to Kconfig

[Masahiro Yamada]

2016-09-07 0:54 GMT+09:00 Tom Rini <trini at konsulko.com>:
> On Mon, Sep 05, 2016 at 07:04:45PM -0600, Simon Glass wrote:
>> Hi Masahiro,
>>
>> On 4 September 2016 at 20:40, Masahiro Yamada
>> <yamada.masahiro at socionext.com> wrote:
>> > 2016-09-02 23:35 GMT+09:00 Tom Rini <trini at konsulko.com>:
>> >
>> >>> >> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
>> >>> >> index c25fcf3..d4a5bc9 100644
>> >>> >> --- a/arch/arm/mach-exynos/Kconfig
>> >>> >> +++ b/arch/arm/mach-exynos/Kconfig
>> >>> >> @@ -61,6 +61,9 @@ endif
>> >>> >>
>> >>> >>  if ARCH_EXYNOS5
>> >>> >>
>> >>> >> +config SPL_GPIO_SUPPORT
>> >>> >> +       default y
>> >>> >> +
>> >>> >
>> >>> >
>> >>> > As we discussed before,
>> >>> > we decided to not do this.
>> >>>
>> >>> Tom was keen to avoid changing every defconfig file. It is there
>> >>> another way to express common defaults?
>> >>
>> >> I was thinking in the Kconfig with the entry for SPL_GPIO_SUPPORT, for
>> >> optional stuff and select in the board, etc, Kconfig for non-optional
>> >> stuff.  Now, I realize that optional vs non-optional is more the domain
>> >> of the individual SoC custodians, so we'll have some clean up afterwards
>> >> that isn't on you (well, aside from the SoCs you know like rockchip ;)).
>> >
>> > config SPL_GPIO_SUPPORT
>> >        default y
>> >
>> > is incorrect because it could violate
>> > the dependency in the proper Kconfig entry in spl/Kconfig.
>>
>> But only if options depended on by this are not set, right?
>>
>> >
>> >
>> >
>> >
>> > Basically, we are supposed to use "select FOO" when it is mandatory,
>> > or add CONFIG_FOO=y in a defconfig when the board wants it, but
>> > can still make it optional.
>>
>> Yes, but this is not mandatory. It is a default, and some boards will
>> unset it. I did this in response to Tom's comment about trying to cut
>> down the defconfig patch size.
>
> Well, here is where it is tricky.  For example, in SPL when we bring in
> the GPIO code, it's because it's required to enable DDR or know which
> board rev we're on.  So it needs to be select'd or people will get
> failure to build problems.  Other things like filesystems should be an
> option.

I agree.


>> > I know our pain comes from that "include" is not supported by Kconfig.
>>
>> How would that help? Why don't we implement it?
>
> Well, if we could more literally translate the various *common*.h files
> into a Kconfig file that was "included" it would be easier to say that
> various CONFIG variables are just a bool (or hex) and then
> board/ti/am335x/Kconfig would 'include
> include/kconfig/ti_am33xx_common.k' and get all of the stuff it
> used to get from include/configs/ti_am335x_common.h.

Yes, exactly.

The problem is we can not sync defconfigs any more.
Actually, we can only sync *common* defconfigs,
but we cannot per-board ones.


>> > What I can suggest now is:
>> >
>> >
>> >
>> > [1]  Add ARCH_WANT_* to specify a SoC-common default.
>> >
>> >
>> >        config SPL_GPIO_SUPPORT
>> >                 bool "GPIO support for SPL"
>> >                 default  ARCH_WANT_SPL_GPIO_SUPPORT
>> >
>> >
>> >        config ARCH_WANT_SPL_GPIO_SUPPORT
>> >                 bool
>> >
>> >
>> >        config ARCH_EXYNOS5
>> >                select ARCH_WANT_SPL_GPIO_SUPPORT
>> >
>> >
>> > Linux used to have ARCH_WANT_OPTIONAL_GPIOLIB to do similar things.
>> > (they stopped this way, though)
>
> This may be better.
>
>> > [2] Support multi boards with one defconfig
>> >     (barebox supports multi-platform like Linux does.)
>
> Unless I missed something, this is just kicking the problem up a level
> frankly.  They just allow (and we could / can / do, depending on the
> SoC) one full "barebox" to be loaded by the board-specific preloader.

Right.
Barebox proper can be multi-platform
because it supports driver model and device tree.

On the other hand, barebox PBL (= correspond to U-Boot SPL)
is a per-board image.
(More precisely, per-board and per-boot-mode image)


> We can, should, and hopefully will once DM is 'done', have this be an
> option.  But that's a different problem set from how do we configure the
> board specific part of a build.


As far as I know, things are roughly like this:

The build system of barebox generates a *big* PBL object first
by compiling every source files for all of enabled boards,
enabled boot modes, like boardA_nand, boardA_mmc, boardB_nand, boardC, etc.
Then gives a different entry point for each board/boot-mode
to the linker script.

The objects unreachable from the board-specific entry point
are ripped off by the GCC garbage collection.
For example, the  board_a_init()
is unreachable from the entry point of board B/C,
so contained only in the board A PBL.

Finally, it gets small per-board/per-boot-mode PBL images.


So, there is no CONFIG options to switch the boot-flow of PBL.

For U-Boot,
we enable CONFIG_SPL_NAND_SUPPORT for  boardA_nand_defconfig,
and CONFIG_SPL_MMC_SUPPORT for boardA_mmc_defconfig.


In barebox, each of boardA_nand, boardA_mmc, whatever
has its own entry point.

nand_load() is reachable from boardA_nand entry, but
not from boardA_mmc entry.  Only needed objects are linked.


-- 
Best Regards
Masahiro Yamada