Patchwork [v4,2/5] ARM: SMP: Refactor Kconfig to be more maintainable

login
register
mail settings
Submitter Dave Martin
Date Dec. 13, 2011, 5:56 p.m.
Message ID <1323798993-8727-3-git-send-email-dave.martin@linaro.org>
Download mbox | patch
Permalink /patch/131158/
State New
Headers show

Comments

Dave Martin - Dec. 13, 2011, 5:56 p.m.
Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
bothersome to maintain and likely to lead to merge conflicts.

This patch moves the knowledge of which platforms are SMP-capable
to the individual machines.  To enable this, a new HAVE_SMP config
option is introduced to allow machines to indicate that they can
run in a SMP configuration.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/Kconfig               |   15 +++++++++++----
 arch/arm/mach-exynos/Kconfig   |    1 +
 arch/arm/mach-imx/Kconfig      |    1 +
 arch/arm/mach-msm/Kconfig      |    1 +
 arch/arm/mach-omap2/Kconfig    |    1 +
 arch/arm/mach-realview/Kconfig |    4 ++++
 arch/arm/mach-vexpress/Kconfig |    1 +
 7 files changed, 20 insertions(+), 4 deletions(-)
David Brown - Dec. 13, 2011, 8:11 p.m.
On Tue, Dec 13, 2011 at 05:56:30PM +0000, Dave Martin wrote:
> Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
> bothersome to maintain and likely to lead to merge conflicts.

> diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> index ebde97f..e6beaff 100644
> --- a/arch/arm/mach-msm/Kconfig
> +++ b/arch/arm/mach-msm/Kconfig
> @@ -67,6 +67,7 @@ config MSM_SOC_REV_A
>  	bool
>  config  ARCH_MSM_SCORPIONMP
>  	bool
> +	select HAVE_SMP
>  
>  config  ARCH_MSM_ARM11
>  	bool

The ARCH_MSM_SCORPIONMP config's only purpose was to enable SMP higher
up.  We might as well eliminate ARCH_MSM_SCORPIONMP entirely, and just
select HAVE_SMP in ARCH_MSM8X60 and ARCH_MSM8960.

Also, be sure to run ./scripts/get_maintainer.pl on your patch so that
the proper people get addressed by the message.  Otherwise, people
might miss patches.

David
Dave Martin - Dec. 14, 2011, 11:36 a.m.
On Tue, Dec 13, 2011 at 12:11:37PM -0800, David Brown wrote:
> On Tue, Dec 13, 2011 at 05:56:30PM +0000, Dave Martin wrote:
> > Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
> > bothersome to maintain and likely to lead to merge conflicts.
> 
> > diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> > index ebde97f..e6beaff 100644
> > --- a/arch/arm/mach-msm/Kconfig
> > +++ b/arch/arm/mach-msm/Kconfig
> > @@ -67,6 +67,7 @@ config MSM_SOC_REV_A
> >  	bool
> >  config  ARCH_MSM_SCORPIONMP
> >  	bool
> > +	select HAVE_SMP
> >  
> >  config  ARCH_MSM_ARM11
> >  	bool
> 
> The ARCH_MSM_SCORPIONMP config's only purpose was to enable SMP higher
> up.  We might as well eliminate ARCH_MSM_SCORPIONMP entirely, and just
> select HAVE_SMP in ARCH_MSM8X60 and ARCH_MSM8960.

First and foremost, I'm just refactoring with this series.  I've
included other trivial changes suggested by other people where the
effect is clear and straightforward.


Removing ARCH_MSM_SCORPIONMP is not quite trivial, though:

arch/arm/mach-msm/timer.c:
#ifdef CONFIG_ARCH_MSM_SCORPIONMP
	writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
#endif

This suggests that ARCH_MSM_SCORPIONMP may mean more than just
"HAVE_SMP".

Also,

arch/arm/Kconfig:config HAVE_SMP
	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP

...and

arch/arm/Kconfig:config LOCAL_TIMERS
	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)


Now, we could list the affected MSM boards longhand in those
dependencies, but that's just reintroducing some of the exact kind of
kconfig clunkiness I'm trying to remove: those lists are obviously
liable to grow over time.

Configs like this also look like they may be incompatible with the
single kernel binary goal: if any of the boards supported by the kernel
have the ARM SCU and/or TWD unit, surely we should be able to enable the
support in the kernel?


If you can see a nice way to resolve those issues though, feel free to
propose a patch and I'll append it to the series.

> 
> Also, be sure to run ./scripts/get_maintainer.pl on your patch so that
> the proper people get addressed by the message.  Otherwise, people
> might miss patches.

Argh, my metadata for that series was mangled -- I put together a long
CC list based on but it never got used :(

Thanks for highlighting that... I'll repost.  This probably explains why
I didn't get much feedback.

Cheers
---Dave
Linus Walleij - Dec. 14, 2011, 5:31 p.m.
On Tue, Dec 13, 2011 at 6:56 PM, Dave Martin <dave.martin@linaro.org> wrote:

> Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
> bothersome to maintain and likely to lead to merge conflicts.
>
> This patch moves the knowledge of which platforms are SMP-capable
> to the individual machines.  To enable this, a new HAVE_SMP config
> option is introduced to allow machines to indicate that they can
> run in a SMP configuration.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

For "my" platforms...

Yours,
Linus Walleij
David Brown - Dec. 14, 2011, 5:46 p.m.
On Wed, Dec 14, 2011 at 11:36:55AM +0000, Dave Martin wrote:
> On Tue, Dec 13, 2011 at 12:11:37PM -0800, David Brown wrote:
> > On Tue, Dec 13, 2011 at 05:56:30PM +0000, Dave Martin wrote:
> > > Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
> > > bothersome to maintain and likely to lead to merge conflicts.
> > 
> > > diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> > > index ebde97f..e6beaff 100644
> > > --- a/arch/arm/mach-msm/Kconfig
> > > +++ b/arch/arm/mach-msm/Kconfig
> > > @@ -67,6 +67,7 @@ config MSM_SOC_REV_A
> > >  	bool
> > >  config  ARCH_MSM_SCORPIONMP
> > >  	bool
> > > +	select HAVE_SMP
> > >  
> > >  config  ARCH_MSM_ARM11
> > >  	bool
> > 
> > The ARCH_MSM_SCORPIONMP config's only purpose was to enable SMP higher
> > up.  We might as well eliminate ARCH_MSM_SCORPIONMP entirely, and just
> > select HAVE_SMP in ARCH_MSM8X60 and ARCH_MSM8960.
> 
> First and foremost, I'm just refactoring with this series.  I've
> included other trivial changes suggested by other people where the
> effect is clear and straightforward.

In that light, your patch is probably the right approach.  I can clean
our stuff up and sent out other patches later.

Thanks,
David
Dave Martin - Dec. 14, 2011, 6:20 p.m.
On Wed, Dec 14, 2011 at 09:46:18AM -0800, David Brown wrote:
> On Wed, Dec 14, 2011 at 11:36:55AM +0000, Dave Martin wrote:
> > On Tue, Dec 13, 2011 at 12:11:37PM -0800, David Brown wrote:
> > > On Tue, Dec 13, 2011 at 05:56:30PM +0000, Dave Martin wrote:
> > > > Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
> > > > bothersome to maintain and likely to lead to merge conflicts.
> > > 
> > > > diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> > > > index ebde97f..e6beaff 100644
> > > > --- a/arch/arm/mach-msm/Kconfig
> > > > +++ b/arch/arm/mach-msm/Kconfig
> > > > @@ -67,6 +67,7 @@ config MSM_SOC_REV_A
> > > >  	bool
> > > >  config  ARCH_MSM_SCORPIONMP
> > > >  	bool
> > > > +	select HAVE_SMP
> > > >  
> > > >  config  ARCH_MSM_ARM11
> > > >  	bool
> > > 
> > > The ARCH_MSM_SCORPIONMP config's only purpose was to enable SMP higher
> > > up.  We might as well eliminate ARCH_MSM_SCORPIONMP entirely, and just
> > > select HAVE_SMP in ARCH_MSM8X60 and ARCH_MSM8960.
> > 
> > First and foremost, I'm just refactoring with this series.  I've
> > included other trivial changes suggested by other people where the
> > effect is clear and straightforward.
> 
> In that light, your patch is probably the right approach.  I can clean
> our stuff up and sent out other patches later.

OK, thanks for confirming.

Cheers
---Dave
Olof Johansson - Dec. 18, 2011, 4:22 a.m.
On Tue, Dec 13, 2011 at 9:56 AM, Dave Martin <dave.martin@linaro.org> wrote:
> Making SMP depend on (huge list of MACH_ and ARCH_ configs) is
> bothersome to maintain and likely to lead to merge conflicts.
>
> This patch moves the knowledge of which platforms are SMP-capable
> to the individual machines.  To enable this, a new HAVE_SMP config
> option is introduced to allow machines to indicate that they can
> run in a SMP configuration.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

Acked-by: Olof Johansson <olof@lixom.net>  (Tegra parts)

Thanks for doing this, I've been itching over it as well but never got
around to fixing it.


-Olof

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 16a4b9e..d33eb39 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -344,6 +344,7 @@  config ARCH_HIGHBANK
 	select CPU_V7
 	select GENERIC_CLOCKEVENTS
 	select HAVE_ARM_SCU
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select USE_OF
 	help
@@ -636,6 +637,7 @@  config ARCH_TEGRA
 	select GENERIC_GPIO
 	select HAVE_CLK
 	select HAVE_SCHED_CLOCK
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select ARCH_HAS_CPUFREQ
 	help
@@ -706,6 +708,7 @@  config ARCH_SHMOBILE
 	select HAVE_CLK
 	select CLKDEV_LOOKUP
 	select HAVE_MACH_CLKDEV
+	select HAVE_SMP
 	select GENERIC_CLOCKEVENTS
 	select MIGHT_HAVE_CACHE_L2X0
 	select NO_IOPORT
@@ -909,6 +912,7 @@  config ARCH_U8500
 	select CLKDEV_LOOKUP
 	select ARCH_REQUIRE_GPIOLIB
 	select ARCH_HAS_CPUFREQ
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
 	  Support for ST-Ericsson's Ux500 architecture
@@ -1430,14 +1434,17 @@  menu "Kernel Features"
 
 source "kernel/time/Kconfig"
 
+config HAVE_SMP
+	bool
+	help
+	  This option should be selected by machines which have an SMP-
+	  capable CPU.
+
 config SMP
 	bool "Symmetric Multi-Processing"
 	depends on CPU_V6K || CPU_V7
 	depends on GENERIC_CLOCKEVENTS
-	depends on REALVIEW_EB_ARM11MP || REALVIEW_EB_A9MP || \
-		 MACH_REALVIEW_PB11MP || MACH_REALVIEW_PBX || ARCH_OMAP4 || \
-		 ARCH_EXYNOS4 || ARCH_TEGRA || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || \
-		 ARCH_MSM_SCORPIONMP || ARCH_SHMOBILE || ARCH_HIGHBANK || SOC_IMX6Q
+	depends on HAVE_SMP
 	depends on MMU
 	select USE_GENERIC_SMP_HELPERS
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 7f2347b..e1efbca 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -17,6 +17,7 @@  choice
 
 config ARCH_EXYNOS4
 	bool "SAMSUNG EXYNOS4"
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
 	  Samsung EXYNOS4 SoCs based systems
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 5f7f9c2..29a3d61 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -615,6 +615,7 @@  config SOC_IMX6Q
 	select HAVE_IMX_GPC
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
+	select HAVE_SMP
 	select USE_OF
 
 	help
diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index ebde97f..e6beaff 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -67,6 +67,7 @@  config MSM_SOC_REV_A
 	bool
 config  ARCH_MSM_SCORPIONMP
 	bool
+	select HAVE_SMP
 
 config  ARCH_MSM_ARM11
 	bool
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index c841578..bb1b670 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -43,6 +43,7 @@  config ARCH_OMAP4
 	depends on ARCH_OMAP2PLUS
 	select CPU_V7
 	select ARM_GIC
+	select HAVE_SMP
 	select LOCAL_TIMERS if SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select PL310_ERRATA_588369
diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
index 3dd620f..c593be4 100644
--- a/arch/arm/mach-realview/Kconfig
+++ b/arch/arm/mach-realview/Kconfig
@@ -12,6 +12,7 @@  config REALVIEW_EB_A9MP
 	bool "Support Multicore Cortex-A9 Tile"
 	depends on MACH_REALVIEW_EB
 	select CPU_V7
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
 	  Enable support for the Cortex-A9MPCore tile fitted to the
@@ -22,6 +23,7 @@  config REALVIEW_EB_ARM11MP
 	depends on MACH_REALVIEW_EB
 	select CPU_V6K
 	select ARCH_HAS_BARRIERS if SMP
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	help
 	  Enable support for the ARM11MPCore tile fitted to the Realview(R)
@@ -41,6 +43,7 @@  config MACH_REALVIEW_PB11MP
 	select CPU_V6K
 	select ARM_GIC
 	select HAVE_PATA_PLATFORM
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select ARCH_HAS_BARRIERS if SMP
 	help
@@ -82,6 +85,7 @@  config MACH_REALVIEW_PBX
 	bool "Support RealView(R) Platform Baseboard Explore"
 	select ARM_GIC
 	select HAVE_PATA_PLATFORM
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select ARCH_SPARSEMEM_ENABLE if CPU_V7 && !REALVIEW_HIGH_PHYS_OFFSET
 	select ZONE_DMA if SPARSEMEM
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index a8aefc8..9b3d0fb 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -8,6 +8,7 @@  config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
+	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 
 endmenu