RE: [PATCH] ARM: use "depends on" for SoC configs instead of "if" after prompt

[yamada.masahiro]

Hi Arnd,

 
> On Monday 16 November 2015 12:06:10 Masahiro Yamada wrote:
> > Many ARM sub-architectures use prompts followed by "if" conditional,
> > but it is wrong.
> >
> > Please notice the difference between
> >
> >     config ARCH_FOO
> >             bool "Foo SoCs" if ARCH_MULTI_V7
> >
> > and
> >
> >     config ARCH_FOO
> >             bool "Foo SoCs"
> >             depends on ARCH_MULTI_V7
> >
> > These two are *not* equivalent!
> >
> > In the former statement, it is not ARCH_FOO, but its prompt that
> > depends on ARCH_MULTI_V7.  So, it is completely valid that ARCH_FOO is
> > selected by another, but ARCH_MULTI_V7 is still disabled. As it is not
> > unmet dependency, Kconfig never warns.  This is probably not what you
> > want.
> 
> Did you encounter a case where someone actually did a 'select' on one of
> those symbols? I probably introduced a lot of them and did not expect that
> to happen.

No, for ARM sub-architectures.
But, yes for the ARM core part.


For example, the following entry in arch/arm/Kconfig is suspicous.

config PCI
        bool "PCI support" if MIGHT_HAVE_PCI
        help
          Find out whether you have a PCI motherboard. PCI is the name of a
          bus system, i.e. the way the CPU talks to the other stuff inside
          your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
          VESA. If you have PCI, say Y, otherwise N.




Try "make ARCH=arm footbridge_defconfig" and check the .config file.

It defines CONFIG_PCI=y, but not CONFIG_MIGHT_HAVE_PCI.
I am not sure this is a sane .config or not.

But, anyway, Kconfig does not complain about it.


We have similar issues for CPU_V6, CPU_V6K, CPU_V7, etc.
The config is selected, but the "if" conditional is unmet.
(I decided to postpone this problem because it would take some time to understand
complicated dependency.)

The use of a prompt followed by "if" without correct understanding is dangerous
because it could hide the unmet dependency problem.

I want to eliminate the potential problem by this patch
before somebody introduce insane dependency.


> 
> > diff --git a/arch/arm/mach-integrator/Kconfig
> > b/arch/arm/mach-integrator/Kconfig
> > index 02d0834..2fa9d11 100644
> > --- a/arch/arm/mach-integrator/Kconfig
> > +++ b/arch/arm/mach-integrator/Kconfig
> > @@ -1,5 +1,6 @@
> >  config ARCH_INTEGRATOR
> > -	bool "ARM Ltd. Integrator family" if (ARCH_MULTI_V4T ||
> ARCH_MULTI_V5 || ARCH_MULTI_V6)
> > +	bool "ARM Ltd. Integrator family"
> > +	depends on ARCH_MULTI_V4T || ARCH_MULTI_V5 || ARCH_MULTI_V6
> >  	select ARM_AMBA
> >  	select ARM_PATCH_PHYS_VIRT if MMU
> >  	select AUTO_ZRELADDR
> 
> There is one related change that I would like to see, and that is to convert
> all top-level 'config' statements that have sub-options into 'menuconfig'
> statements for consistency. At the moment, the platform menu has a mix of
> platform-selection and platform-specific options, and I'd like to make that
> more consistent.

I agree, but in another patch (or series)?



Best Regards
Mashairo Yamada

N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå