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

[Tom Rini <trini@konsulko.com>]

On Mon, Sep 12, 2016 at 01:32:54PM +0900, Masahiro Yamada wrote:
> Hi Simon,
> 
> 
> 2016-09-12 13:16 GMT+09:00 Simon Glass <sjg@chromium.org>:
> > Hi,
> >
> > On 6 September 2016 at 09:54, Tom Rini <trini@konsulko.com> wrote:
> >> 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@socionext.com> wrote:
> >>> > 2016-09-02 23:35 GMT+09:00 Tom Rini <trini@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 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.
> >>
> >>> > 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.
> >> 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.
> >>
> >>> > [3] use pre-processor to support #include <...>
> >>> >
> >>> >
> >>> >
> >>> >
> >>> >
> >>> > BTW, SPL_GPIO_SUPPORT is optional in this case?
> >>> >
> >>> > U-Boot proper supports a command line interface,
> >>> > while SPL is usually run in a non-interactive mode.
> >>> >
> >>> > So, what SPL needs is generally mandatory,
> >>> > so we can "select" it,  I guess.
> >>>
> >>> I found a lot of cases where 90% of the boards with the same SoC  had
> >>> the same setting for this (and a few other) options, but some boards
> >>> did not. So I did not want to use select, since then it is impossible
> >>> to unselect.
> >>
> >> Maybe this is where [1] above will work best and we can select
> >> ARCH_WANT_.. (or BOARD_WANT_...) from TARGET_... and leave some things
> >> as questions.
> >>
> >>> This series is actually really tricky. I'm looking forward to putting
> >>> it to bed. We need to make these conversions easier.
> >>
> >> Agreed!
> >
> > I am not sure about the WANT business, but I am sure that I don't want
> > to work through all the SoCs and figure how how they should set it.
> > I'd like to get this series in since i think it is a good starting
> > point for improving things. Changing to WANT will be easier after that
> > if we want to. Also I feel this should be the job of SoC maintainers.
> 
> 
> OK, I agree.
> 
> 
> But, you added
> 
> config SPL_GPIO_SUPPORT
>       default y
> 
> for some platforms, so it looks like you already figured out
> how the default should be set for them.
> 
> 
> Anyway, I know this task is too much burden.
> How about moving CONFIG_SPL_GPIO_SUPPORT=y to defconfigs verbatim for now?
> 
> 
> If SoC maintainers are unhappy about duplicated CONFIG defines in
> their defconfigs,
> they can work on the WANT refactoring later.

Yes, how about this so we can get it in, and SoC clean up to follow by
SoC custodians ?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160912/49c23d80/attachment.sig>