[PATCH] cpuidle: big.LITTLE: add MCPM dependency

[PATCH] cpuidle: big.LITTLE: add MCPM dependency

[Arnd Bergmann]

662322fcb6d ("cpuidle: big.LITTLE: Add ARCH_EXYNOS entry in config")
made it possible for the big-little cpuidle driver to run on exynos,
which may or may not include MCPM support at compile time, so we
run into a link error when it is disabled:

drivers/built-in.o: In function `bl_enter_powerdown':
:(.text+0x1889a0): undefined reference to `mcpm_cpu_powered_up'
drivers/built-in.o: In function `bl_powerdown_finisher':
:(.text+0x1889e8): undefined reference to `mcpm_set_entry_vector'
:(.text+0x1889ec): undefined reference to `mcpm_cpu_suspend'

This adds an explicit dependency to CONFIG_MCPM to avoid that
case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Chander Kashyap <chander.kashyap@linaro.org>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
---
I believe the broken commit is only present in the samsung/for-next
tree (through v3.17-next/cpuidle-exynos), so it should be fixed there.

On a side note, I wonder if we should have platform dependencies at
all, or just the MCPM dependency by itself.

diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 2f6b33ea6e08..459b7c91407a 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -10,6 +10,7 @@ config ARM_ARMADA_370_XP_CPUIDLE
 config ARM_BIG_LITTLE_CPUIDLE
 	bool "Support for ARM big.LITTLE processors"
 	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
+	depends on MCPM
 	select ARM_CPU_SUSPEND
 	select CPU_IDLE_MULTIPLE_DRIVERS
 	help

[PATCH] cpuidle: big.LITTLE: add MCPM dependency

[Nicolas Pitre]

On Mon, 14 Jul 2014, Arnd Bergmann wrote:

> 662322fcb6d ("cpuidle: big.LITTLE: Add ARCH_EXYNOS entry in config")
> made it possible for the big-little cpuidle driver to run on exynos,
> which may or may not include MCPM support at compile time, so we
> run into a link error when it is disabled:
> 
> drivers/built-in.o: In function `bl_enter_powerdown':
> :(.text+0x1889a0): undefined reference to `mcpm_cpu_powered_up'
> drivers/built-in.o: In function `bl_powerdown_finisher':
> :(.text+0x1889e8): undefined reference to `mcpm_set_entry_vector'
> :(.text+0x1889ec): undefined reference to `mcpm_cpu_suspend'
> 
> This adds an explicit dependency to CONFIG_MCPM to avoid that
> case.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Chander Kashyap <chander.kashyap@linaro.org>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> I believe the broken commit is only present in the samsung/for-next
> tree (through v3.17-next/cpuidle-exynos), so it should be fixed there.
> 
> On a side note, I wonder if we should have platform dependencies at
> all, or just the MCPM dependency by itself.

This was discussed in some other thread already but I'm too lazy to dig 
a reference to it.

Only a dependency on MCPM alone is needed here.  And then:

Acked-by: Nicolas Pitre <nico@linaro.org>

> 
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 2f6b33ea6e08..459b7c91407a 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -10,6 +10,7 @@ config ARM_ARMADA_370_XP_CPUIDLE
>  config ARM_BIG_LITTLE_CPUIDLE
>  	bool "Support for ARM big.LITTLE processors"
>  	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
> +	depends on MCPM
>  	select ARM_CPU_SUSPEND
>  	select CPU_IDLE_MULTIPLE_DRIVERS
>  	help
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

[PATCH] cpuidle: big.LITTLE: add MCPM dependency

[Daniel Lezcano]

On 07/14/2014 01:23 PM, Arnd Bergmann wrote:
> 662322fcb6d ("cpuidle: big.LITTLE: Add ARCH_EXYNOS entry in config")
> made it possible for the big-little cpuidle driver to run on exynos,
> which may or may not include MCPM support at compile time, so we
> run into a link error when it is disabled:
>
> drivers/built-in.o: In function `bl_enter_powerdown':
> :(.text+0x1889a0): undefined reference to `mcpm_cpu_powered_up'
> drivers/built-in.o: In function `bl_powerdown_finisher':
> :(.text+0x1889e8): undefined reference to `mcpm_set_entry_vector'
> :(.text+0x1889ec): undefined reference to `mcpm_cpu_suspend'
>
> This adds an explicit dependency to CONFIG_MCPM to avoid that
> case.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Chander Kashyap <chander.kashyap@linaro.org>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> I believe the broken commit is only present in the samsung/for-next
> tree (through v3.17-next/cpuidle-exynos), so it should be fixed there.
>
> On a side note, I wonder if we should have platform dependencies at
> all, or just the MCPM dependency by itself.
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 2f6b33ea6e08..459b7c91407a 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -10,6 +10,7 @@ config ARM_ARMADA_370_XP_CPUIDLE
>   config ARM_BIG_LITTLE_CPUIDLE
>   	bool "Support for ARM big.LITTLE processors"
>   	depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
> +	depends on MCPM

Why do we want to depend on MCPM ? If I want to enable the cpuidle 
driver, the Kconfig should select the dependencies instead of letting 
the user to figure out what dependency is missing in order to enable the 
cpuidle option, no ?

+	select MCPM

>   	select ARM_CPU_SUSPEND
>   	select CPU_IDLE_MULTIPLE_DRIVERS
>   	help
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH] cpuidle: big.LITTLE: add MCPM dependency

[Arnd Bergmann]

On Monday 14 July 2014 20:44:50 Daniel Lezcano wrote:
> > --- a/drivers/cpuidle/Kconfig.arm
> > +++ b/drivers/cpuidle/Kconfig.arm
> > @@ -10,6 +10,7 @@ config ARM_ARMADA_370_XP_CPUIDLE
> >   config ARM_BIG_LITTLE_CPUIDLE
> >       bool "Support for ARM big.LITTLE processors"
> >       depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS
> > +     depends on MCPM
> 
> Why do we want to depend on MCPM ? If I want to enable the cpuidle 
> driver, the Kconfig should select the dependencies instead of letting 
> the user to figure out what dependency is missing in order to enable the 
> cpuidle option, no ?
> 
> +       select MCPM
> 

MCPM is a user-selectable option, which doesn't mix well with 'select'.

If you want to use 'select' here, better also change

	bool "Multi-Cluster Power Management"

to just

	bool

in the ARM global Kconfig. The normal way to deal with these dependencies
though is to have platforms 'select' the features they allow and have
device drivers 'depends on' the same symbol.

	Arnd