[U-Boot] [RFC] Rearrange CONFIG_* macros

[Tom Rini]

On Fri, Sep 30, 2016 at 04:20:10AM +0000, york sun wrote:
> On 09/29/2016 08:59 PM, Masahiro Yamada wrote:
> > 2016-09-30 0:47 GMT+09:00 york sun <york.sun at nxp.com>:
> >> On 09/28/2016 12:17 PM, Simon Glass wrote:
> >>> +Masahiro, who may know
> >>
> >> Masahiro,
> >>
> >> Need some advise on this. How do we deal with duplicated config?
> >>
> >> York
> >>
> >>>
> >>> On 28 September 2016 at 12:57, york sun <york.sun at nxp.com> wrote:
> >>>> On 09/27/2016 10:55 AM, Simon Glass wrote:
> >>>>> Hi York,
> >>>>>
> >>>>> On 27 September 2016 at 11:39, york sun <york.sun at nxp.com> wrote:
> >>>>>> Guys,
> >>>>>>
> >>>>>> I want to discuss to rearrange the current CONFIG_* macros used by
> >>>>>> Freescale Layerscape SoCs. We have been encouraged to use CONFIG_SYS_*
> >>>>>> macros to define hardware setting, and other CONFIG_* macros for user
> >>>>>> setting. Clearly this has changed. Kconfig options are used for most
> >>>>>> user settings from now on. I am OK with it. But for existing
> >>>>>> CONFIG_SYS_* macros, we need a plan to either move them out of CONFIG_*
> >>>>>> name space, or move some to Kconfig, if that makes sense. Since most our
> >>>>>> CONFIG_SYS_* macros are cross platforms (including armv7 and armv8), if
> >>>>>> using Kconfig option, do we add another mach-fsl folder to host the
> >>>>>> Kconfig, or somewhere else? We also have macros cross ARM and PowerPC.
> >>>>>> So we either move them out of arch/, or duplicate them for both arch.
> >>>>>>
> >>>>>> If we move them to another name space, for example SYS_FSL_*, do we want
> >>>>>> to move them out of config.h?
> >>>>>>
> >>>>>> Any suggestion/comment?
> >>>>>
> >>>>> I wonder if some of these (the ones that represent actual values
> >>>>> rather than enabling functions) can move to device tree, or tables?
> >>>>>
> >>>>
> >>>> While I try to move some options to Kconfig, I have an issue of
> >>>> duplicated names. For example, the MAX_CPUS is used in arch/x86/Kconfig.
> >>>> I know I can put another config with the same name, for example
> >>>> arch/arm/cpu/armv8/fsl-layerscape/Kconfig. The funny thing is when I
> >>>> search for this config option under menuconfig, it shows the location of
> >>>> x86, but showing defined in arch/arm/cpu/armv8/fsl-layerscape/Kconfig.
> >>>> Same thing happens when I try to move an option which is used by both
> >>>> PowerPC and ARM. This is not related to a common driver so I cannot put
> >>>> the option into a driver Kconfig (for example CONFIG_SYS_HAS_SERDES).
> >>>>
> >>>> Is there a solution for this?
> >>>>
> >
> > Right.
> > The "Help" in menuconfig is just to display the path
> > to the first Kconfig entry found.
> > The result looks funny if we have multiple entries for the same option name.
> >
> >
> > I am not pretty sure about this particular case,
> > but there should be general solutions.
> >
> >
> > (1)  Rename the option to avoid name space conflict, e.g.
> >
> >     MAX_CPUS   ->  FSL_MAX_CPUS      (for Freescale SoCs)
> >     MAX_CPUS   ->  X86_MAX_CPUS      (for x86)
> >
> 
> It is a solution, but I don't like it for the reason described below[1].
> 
> >
> > (2) Unify the Kconfig entry
> >
> >   As in Linux, we do not generally create multiple entries for the same option.
> >
> >   If the option is sensible only for some platforms, the general practice is
> >     to use   "depends on HAVE_..." to hide the option on unrelated platforms.
> >
> >
> >   If X86 and FSL are the only platforms that have interest in this option,
> >   we can save HAVE_... and put them directly in "depends on".
> >
> >     config MAX_CPUS
> >          int "Maximum number of CPUs permitted"
> >          depends on (SMP && X86) || VENDOR_FSL
> >
> >   I am not quite sure about "VENDOR_FSL",
> >    so please choose your favorite one like
> >   SOC_FSL, PLAT_FSL, ARCH_FSL, or whatever.
> 
> This sounds like a bandage. I can go with this if there is no better 
> solution.

If MAX_CPUS has the same meaning in both cases, this isn't imho a
bandage but one of the cleanups that was hoped for in moving to Kconfig.
We see that a problem is encountered in more than one area more easily
and instead of letting N solutions accumulate we get one.

> Checking Linux Kconfig, I noticed the NR_CPUS are used in all archs. The 
> difference is only one arch Kconfig is sourced by
> 
> source "arch/$SRCARCH/Kconfig"
> 
> I tried to do the same for U-Boot, but that will require we put ARCH at 
> the command line of environmental variable. I think we moved away from 
> this syntax. It also has some trouble to to use aarch64 because we put 
> the actual code under arch/arm.

Yeah, we don't want to move in this direction.

[snip]
> [1] We have macros shared between PowerPC and ARM for Freescale QorIQ 
> SoCs because they share similar designs at SoC level. Some options can 
> be moved to driver Kconfig to avoid duplication. But there are many 
> options are tied to hardware and they are not related to a common 
> driver. Using the same macro simplifies the code and is easier to 
> maintain. It is preferred to either PowerPC Kconfig, or ARM Kconfig to 
> avoid duplication if we can.

I think this is honestly going to be the hard part, deciding what
belongs in a Kconfig'd area and what belongs in a different namespace
but that in the end we'll have clearer code for PowerPC and ARM going
forward.

-- 
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/20160930/eb5f2a42/attachment.sig>