| Message ID | 4345351.RJRQd7ghCF@wuerfel (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html