RE: [next] kbuild: use $(srctree) instead of KBUILD_SRC to check out-of-tree build

[yamada.masahiro]

Hi Sergey,

> -Original Message-
> From: Sergey Senozhatsky [mailto:sergey.senozhatsky.w...@gmail.com]
> Sent: Friday, April 19, 2019 5:34 PM
> To: Yamada, Masahiro/山田 真弘 
> Cc: linux-kernel@vger.kernel.org; Sergey Senozhatsky
> 
> Subject: [next] kbuild: use $(srctree) instead of KBUILD_SRC to check
> out-of-tree build
> 
> Hello Masahiro,
> 
> It seems that commit a9a49c2ad9b9b8 (linux-next) breaks ./scripts/tags.sh
> on my system. It scans, basically, my entire drive now (including the
> root partition):
> 
> $ ./scripts/tags.sh tags


I did not know this script is run directly
because I expected people were using 'make tags'.

It is easy to fix, though.


diff --git a/scripts/tags.sh b/scripts/tags.sh
index 6a551805db52..70e14c67bde7 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -19,7 +19,7 @@ ignore="$ignore ( -name *.mod.c ) -prune -o"
 # Do not use full path if we do not use O=.. builds
 # Use make O=. {tags|cscope}
 # to force full paths for a non-O= build
-if [ "${srctree}" = "." ]; then
+if [ "${srctree}" = "." -o -z "${srctree}" ]; then
tree=
 else
tree=${srctree}/



> find: ‘/arch/*.[chS]/’: No such file or directory
> find: ‘/include’: No such file or directory
> find: ‘/boot/lost+found’: Permission denied
> find: ‘/etc/pacman.d/gnupg/openpgp-revocs.d’: Permission denied
> find: ‘/etc/pacman.d/gnupg/private-keys-v1.d’: Permission denied
> find: ‘/etc/pacman.d/gnupg/crls.d’: Permission denied
> find: ‘/etc/polkit-1/rules.d’: Permission denied
> find: ‘/etc/sudoers.d’: Permission denied
> find: ‘/etc/audisp/plugins.d’: Permission denied
> find: ‘/home/lost+found’: Permission denied
> find: ‘/home/sk’: Permission denied
> find: ‘/lost+found’: Permission denied
> ctags: Warning: --extra option is obsolete; use --extras instead
> [..]
> 
> 
> With reverted a9a49c2ad9b9b8 tags scans kernel source tree only
> 
> ./scripts/tags.sh tags
> find: ‘arch/*.[chS]/’: No such file or directory
> ctags: Warning: --extra option is obsolete; use --extras instead
> [..]
> 
>   -ss

RE: [PATCH] kbuild: [bin]deb-pkg: add DPKG_FLAGS variable

[yamada.masahiro]

> -Original Message-
> From: Kacper Kołodziej [mailto:kac...@kolodziej.it]
> Sent: Tuesday, February 05, 2019 9:38 PM
> To: Yamada, Masahiro/山田 真弘 ;
> michal.l...@markovi.net
> Cc: linux-kbu...@vger.kernel.org; linux-kernel@vger.kernel.org;
> kac...@kolodziej.it
> Subject: [PATCH] kbuild: [bin]deb-pkg: add DPKG_FLAGS variable
> 
> DPKG_FLAGS variable lets user to add more flags to dpkg-buildpackage
> command in deb-pkg and bindeb-pkg.
> 
> Signed-off-by: Kacper Kołodziej 
> ---

Applied to linux-kbuild.
Thanks.



>  scripts/package/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/package/Makefile b/scripts/package/Makefile
> index 453fecee62f0..6963eacb323c 100644
> --- a/scripts/package/Makefile
> +++ b/scripts/package/Makefile
> @@ -72,11 +72,11 @@ deb-pkg: FORCE
>   $(call cmd,src_tar,$(KDEB_SOURCENAME))
>   origversion=$$(dpkg-parsechangelog -SVersion |sed
> 's/-[^-]*$$//');\
>   mv
> $(KDEB_SOURCENAME).tar.gz ../$(KDEB_SOURCENAME)_$${origversion}.orig.t
> ar.gz
> - +dpkg-buildpackage -r$(KBUILD_PKG_ROOTCMD) -a$$(cat debian/arch)
> -i.git -us -uc
> + +dpkg-buildpackage -r$(KBUILD_PKG_ROOTCMD) -a$$(cat debian/arch)
> $(DPKG_FLAGS) -i.git -us -uc
> 
>  bindeb-pkg: FORCE
>   $(CONFIG_SHELL) $(srctree)/scripts/package/mkdebian
> - +dpkg-buildpackage -r$(KBUILD_PKG_ROOTCMD) -a$$(cat debian/arch)
> -b -nc -uc
> + +dpkg-buildpackage -r$(KBUILD_PKG_ROOTCMD) -a$$(cat debian/arch)
> $(DPKG_FLAGS) -b -nc -uc
> 
>  intdeb-pkg: FORCE
>   +$(CONFIG_SHELL) $(srctree)/scripts/package/builddeb
> --
> 2.20.1

RE: [PATCH 2/3] kbuild: add real-prereqs shorthand for $(filter-out FORCE, $^)

[yamada.masahiro]

Hi Alexey,

> What is this series made on top of? This does not apply on top of master
> from
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git


If you want to apply this series cleanly, please apply the following first:

https://patchwork.kernel.org/patch/10761625/
https://patchwork.kernel.org/patch/10767193/



The conflicts are not significant.
You can use 'git am -C1' if you want to apply this series
directly to Linus tree.

Masahiro

RE: [PATCH] arm64: dts: Remove inconsistent use of 'arm,armv8' compatible string

[yamada.masahiro]

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Thursday, January 10, 2019 5:30 AM
> To: a...@kernel.org
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Andreas
> Färber ; Manivannan Sadhasivam
> ; Tsahee Zidenberg
> ; Antoine Tenart ;
> Maxime Ripard ; Chen-Yu Tsai ;
> Dinh Nguyen ; Kevin Hilman ;
> Duc Dang ; Liviu Dudau ; Sudeep Holla
> ; Lorenzo Pieralisi ;
> Ray Jui ; Scott Branden ;
> bcm-kernel-feedback-l...@broadcom.com; Robert Richter
> ; Jayachandran C ; Kukjin
> Kim ; Krzysztof Kozlowski ; Wei Xu
> ; Chanho Min ; Jason Cooper
> ; Andrew Lunn ; Gregory Clement
> ; Sebastian Hesselbarth
> ; Matthias Brugger
> ; Thierry Reding ;
> Jonathan Hunter ; Andy Gross
> ; David Brown ; Simon
> Horman ; Magnus Damm ; Heiko
> Stuebner ; Yamada, Masahiro/山田 真弘
> ; Orson Zhai ;
> Baolin Wang ; Chunyan Zhang
> ; Jisheng Zhang ;
> Tero Kristo ; Nishanth Menon ; Jun Nie
> ; Shawn Guo ;
> linux-arm-ker...@lists.infradead.org;
> linux-amlo...@lists.infradead.org; linux-samsung-...@vger.kernel.org;
> linux-media...@lists.infradead.org; linux-te...@vger.kernel.org;
> linux-arm-...@vger.kernel.org; linux-renesas-...@vger.kernel.org;
> linux-rockc...@lists.infradead.org; Mark Rutland ;
> Will Deacon ; Michal Simek 
> Subject: [PATCH] arm64: dts: Remove inconsistent use of 'arm,armv8'
> compatible string
> 
> The 'arm,armv8' compatible string is only for software models. It adds
> little value otherwise and is inconsistently used as a fallback on some
> platforms. Remove it from those platforms.
> 
> This fixes warnings generated by the DT schema.
> 
> Reported-by: Michal Simek 
> Cc: Mark Rutland 
> Cc: Will Deacon 
> Signed-off-by: Rob Herring 
> ---
> armsoc folks, Please apply directly.
> 
> Note that there's still a case left in msm8998 that needs a new
> compatible string.
> 
> Rob

[> ] 

>  .../boot/dts/socionext/uniphier-ld11.dtsi |   4 +-
>  .../boot/dts/socionext/uniphier-ld20.dtsi |   8 +-
>  .../boot/dts/socionext/uniphier-pxs3.dtsi |   8 +-


For uniphier,

Acked-by: Masahiro Yamada 


BTW, do you have a plan to fix files under
Documentation/devicetree/bindings/ ?

RE: [PATCH] Use short unix-style option instead of --longname.

[yamada.masahiro]

Hi.

> -Original Message-
> From: Rob Landley [mailto:r...@landley.net]
> Sent: Thursday, July 05, 2018 4:06 AM
> To: triv...@kernel.org; Yamada, Masahiro/山田 真弘
> ; Kees Cook ; Ulf
> Magnusson 
> Subject: [PATCH] Use short unix-style option instead of --longname.
> 
> From: Rob Landley 
> 
> Use short unix-style option instead of --longname.

This is the repeat of the subject.
Instead, could you describe the reason of this patch here please?


> Signed-off-by: Rob Landley 
> ---
> 
> Avoids warning messages with the latest release of toybox, which never
> bothered
> to implement the --longopts nothing was using.

I am fine with this patch,
but I want the information below the --- line
to be recorded in git.


I think WHY is the most important part in the commit log.

If the commit log only mentions 'Use short unix-style option instead of 
--longname'
it is not easy to know why this commit is necessary.





>  init/Kconfig |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 041f3a0..192d400 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2,9 +2,9 @@ config DEFCONFIG_LIST
>   string
>   depends on !UML
>   option defconfig_list
> - default "/lib/modules/$(shell,uname --release)/.config"
> + default "/lib/modules/$(shell,uname -r)/.config"
>   default "/etc/kernel-config"
> - default "/boot/config-$(shell,uname --release)"
> + default "/boot/config-$(shell,uname -r)"
>   default ARCH_DEFCONFIG
>   default "arch/$(ARCH)/defconfig"

RE: [PATCH] Use short unix-style option instead of --longname.

[yamada.masahiro]

Hi.

> -Original Message-
> From: Rob Landley [mailto:r...@landley.net]
> Sent: Thursday, July 05, 2018 4:06 AM
> To: triv...@kernel.org; Yamada, Masahiro/山田 真弘
> ; Kees Cook ; Ulf
> Magnusson 
> Subject: [PATCH] Use short unix-style option instead of --longname.
> 
> From: Rob Landley 
> 
> Use short unix-style option instead of --longname.

This is the repeat of the subject.
Instead, could you describe the reason of this patch here please?


> Signed-off-by: Rob Landley 
> ---
> 
> Avoids warning messages with the latest release of toybox, which never
> bothered
> to implement the --longopts nothing was using.

I am fine with this patch,
but I want the information below the --- line
to be recorded in git.


I think WHY is the most important part in the commit log.

If the commit log only mentions 'Use short unix-style option instead of 
--longname'
it is not easy to know why this commit is necessary.





>  init/Kconfig |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 041f3a0..192d400 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2,9 +2,9 @@ config DEFCONFIG_LIST
>   string
>   depends on !UML
>   option defconfig_list
> - default "/lib/modules/$(shell,uname --release)/.config"
> + default "/lib/modules/$(shell,uname -r)/.config"
>   default "/etc/kernel-config"
> - default "/boot/config-$(shell,uname --release)"
> + default "/boot/config-$(shell,uname -r)"
>   default ARCH_DEFCONFIG
>   default "arch/$(ARCH)/defconfig"

RE: linux-next: build failure after merge of most trees

[yamada.masahiro]

Hi Stephen,


> -Original Message-
> From: Stephen Rothwell [mailto:s...@canb.auug.org.au]
> Sent: Friday, June 23, 2017 3:43 PM
> To: David Miller 
> Cc: npig...@gmail.com; linux-n...@vger.kernel.org;
> linux-kernel@vger.kernel.org; Yamada, Masahiro/山田 真弘
> ; amo...@gmail.com
> Subject: Re: linux-next: build failure after merge of most trees
> 
> Hi all,
> 
> On Thu, 22 Jun 2017 10:56:48 -0400 (EDT) David Miller 
> wrote:
> >
> > Feel free to merge it into your series:
> >
> > 
[snip]
> 
> I added that to linux-next today and it fixed the build problem.

Thanks for taking care of this.


Sorry, I missed to pick up this for today's release.
I will insert this before Nicholas' series.

RE: linux-next: build failure after merge of most trees

[yamada.masahiro]

Hi Stephen,


> -Original Message-
> From: Stephen Rothwell [mailto:s...@canb.auug.org.au]
> Sent: Friday, June 23, 2017 3:43 PM
> To: David Miller 
> Cc: npig...@gmail.com; linux-n...@vger.kernel.org;
> linux-kernel@vger.kernel.org; Yamada, Masahiro/山田 真弘
> ; amo...@gmail.com
> Subject: Re: linux-next: build failure after merge of most trees
> 
> Hi all,
> 
> On Thu, 22 Jun 2017 10:56:48 -0400 (EDT) David Miller 
> wrote:
> >
> > Feel free to merge it into your series:
> >
> > 
[snip]
> 
> I added that to linux-next today and it fixed the build problem.

Thanks for taking care of this.


Sorry, I missed to pick up this for today's release.
I will insert this before Nicholas' series.

RE: [PATCH 1/2] watchdog: uniphier: add UniPhier watchdog driver

[yamada.masahiro]

Hi Hayashibara-san


> a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> new file mode 100644
> index 000..a59d1ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> @@ -0,0 +1,20 @@
> +UniPhier watchdog timer controller
> +
> +This UniPhier watchdog timer controller must be under sysctrl compatible
> node.
> +
> +Required properties:
> +- compatible: should be "socionext,uniphier-wdt"
> +
> +Example:
> +
> + sysctrl@6184 {
> + compatible = "socionext,uniphier-ld11-sysctrl",
> +  "simple-mfd", "syscon";
> + reg = <0x6184 0x4000>;
> +
> + sys_wdt: watchdog {
> + compatible = "socionext,uniphier-wdt";
> + }
> +
> + other nodes ...
> + };


Next time, please include
devicet...@vger.kernel.org
(and Rob) in your CC list for binding review.

I think scripts/get_maintainer.pl will do it correctly for you.





> +#include 

Is this used?


Thanks.

RE: [PATCH 1/2] watchdog: uniphier: add UniPhier watchdog driver

[yamada.masahiro]

Hi Hayashibara-san


> a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> new file mode 100644
> index 000..a59d1ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> @@ -0,0 +1,20 @@
> +UniPhier watchdog timer controller
> +
> +This UniPhier watchdog timer controller must be under sysctrl compatible
> node.
> +
> +Required properties:
> +- compatible: should be "socionext,uniphier-wdt"
> +
> +Example:
> +
> + sysctrl@6184 {
> + compatible = "socionext,uniphier-ld11-sysctrl",
> +  "simple-mfd", "syscon";
> + reg = <0x6184 0x4000>;
> +
> + sys_wdt: watchdog {
> + compatible = "socionext,uniphier-wdt";
> + }
> +
> + other nodes ...
> + };


Next time, please include
devicet...@vger.kernel.org
(and Rob) in your CC list for binding review.

I think scripts/get_maintainer.pl will do it correctly for you.





> +#include 

Is this used?


Thanks.

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

[yamada.masahiro]

Hi Arnd,

Could you apply this patch with Acked-by storm
so that other subarchtectures can base on it?

Also, the following series:

1/2: ARM: hisi: do not export smp_operations structures
2/2: ARM: use const and __initconst for smp_operations





> -Original Message-
> From: Thierry Reding [mailto:thierry.red...@gmail.com]
> Sent: Wednesday, November 25, 2015 12:59 AM
> To: Yamada, Masahiro
> Cc: linux-arm-ker...@lists.infradead.org; a...@kernel.org; Michal Marek;
> Arnd Bergmann; Olof Johansson; linux-kbu...@vger.kernel.org; Jamie Iles;
> Andy Gross; Sascha Hauer; Gregory Clement; linux...@vger.kernel.org;
> Tsahee Zidenberg; linux-arm-...@vger.kernel.org; Linus Walleij; Kumar
> Gala; Jean-Christophe Plagniol-Villard; Dinh Nguyen; Barry Song; Shiraz
> Hashim; Ray Jui; Patrice Chotard; ker...@stlinux.com; Sören Brinkmann;
> Simon Horman; Michal Simek; Heiko Stuebner;
> linux-rockc...@lists.infradead.org; linux-...@vger.kernel.org; David
> Brown; Scott Branden; Viresh Kumar; Alexandre Belloni; Kyungmin Park;
> Krzysztof Kozlowski; bcm-kernel-feedback-l...@broadcom.com; Florian
> Fainelli; Alexandre Courbot; Matthias Brugger; Kukjin Kim; Rob Herring;
> linux-kernel@vger.kernel.org; spear-de...@list.st.com; Sebastian
> Hesselbarth; Sudeep Holla; Andrew Lunn; Russell King;
> linux-samsung-...@vger.kernel.org; linux-media...@lists.infradead.org;
> linux-te...@vger.kernel.org; Maxime Coquelin; Srinivas Kandagatla; Shawn
> Guo; Liviu Dudau; Jun Nie; Lorenzo Pieralisi; Nicolas Ferre; Jason Cooper;
> Stephen Warren; Krzysztof Halasa; Wei Xu; Magnus Damm; Chen-Yu Tsai; Maxime
> Ripard
> Subject: Re: [PATCH] ARM: use "depends on" for SoC configs instead of "if"
> after prompt
> 
> On Mon, Nov 16, 2015 at 12:06:10PM +0900, Masahiro Yamada wrote:
> [...]
> >  arch/arm/mach-tegra/Kconfig  |  3 ++-
> 
> There will be a slight conflict between this and a patch in the tegra tree,
> but nothing that the ARM SoC maintainers can't handle, so:
> 
> Acked-by: Thierry Reding 

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

[yamada.masahiro]

Hi Arnd,

Could you apply this patch with Acked-by storm
so that other subarchtectures can base on it?

Also, the following series:

1/2: ARM: hisi: do not export smp_operations structures
2/2: ARM: use const and __initconst for smp_operations





> -Original Message-
> From: Thierry Reding [mailto:thierry.red...@gmail.com]
> Sent: Wednesday, November 25, 2015 12:59 AM
> To: Yamada, Masahiro
> Cc: linux-arm-ker...@lists.infradead.org; a...@kernel.org; Michal Marek;
> Arnd Bergmann; Olof Johansson; linux-kbu...@vger.kernel.org; Jamie Iles;
> Andy Gross; Sascha Hauer; Gregory Clement; linux...@vger.kernel.org;
> Tsahee Zidenberg; linux-arm-...@vger.kernel.org; Linus Walleij; Kumar
> Gala; Jean-Christophe Plagniol-Villard; Dinh Nguyen; Barry Song; Shiraz
> Hashim; Ray Jui; Patrice Chotard; ker...@stlinux.com; Sören Brinkmann;
> Simon Horman; Michal Simek; Heiko Stuebner;
> linux-rockc...@lists.infradead.org; linux-...@vger.kernel.org; David
> Brown; Scott Branden; Viresh Kumar; Alexandre Belloni; Kyungmin Park;
> Krzysztof Kozlowski; bcm-kernel-feedback-l...@broadcom.com; Florian
> Fainelli; Alexandre Courbot; Matthias Brugger; Kukjin Kim; Rob Herring;
> linux-kernel@vger.kernel.org; spear-de...@list.st.com; Sebastian
> Hesselbarth; Sudeep Holla; Andrew Lunn; Russell King;
> linux-samsung-...@vger.kernel.org; linux-media...@lists.infradead.org;
> linux-te...@vger.kernel.org; Maxime Coquelin; Srinivas Kandagatla; Shawn
> Guo; Liviu Dudau; Jun Nie; Lorenzo Pieralisi; Nicolas Ferre; Jason Cooper;
> Stephen Warren; Krzysztof Halasa; Wei Xu; Magnus Damm; Chen-Yu Tsai; Maxime
> Ripard
> Subject: Re: [PATCH] ARM: use "depends on" for SoC configs instead of "if"
> after prompt
> 
> On Mon, Nov 16, 2015 at 12:06:10PM +0900, Masahiro Yamada wrote:
> [...]
> >  arch/arm/mach-tegra/Kconfig  |  3 ++-
> 
> There will be a slight conflict between this and a patch in the tegra tree,
> but nothing that the ARM SoC maintainers can't handle, so:
> 
> Acked-by: Thierry Reding 

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�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

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�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i