diff mbox

[1/1] thermal: Exynos: Add missing dependency

Message ID 1352875704-2178-1-git-send-email-sachin.kamat@linaro.org (mailing list archive)
State Superseded, archived
Delegated to: Zhang Rui
Headers show

Commit Message

Sachin Kamat Nov. 14, 2012, 6:48 a.m. UTC
CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
for dependencies gives the following compilation warnings:
warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)

Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested using exynos4_defconfig on linux-next tree of 20121114.
---
 drivers/thermal/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Zhang Rui Nov. 15, 2012, 12:11 a.m. UTC | #1
Hi, Sachin,

thanks for catching the problem.

On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
> for dependencies gives the following compilation warnings:
> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
> 
Amit,

how is exynos driver supposed to work?
do you want the exynos driver still be loaded without CPU_THERMAL?
If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
and CPU_THERMAL will select CPU_FREQ_TABLE instead.

IMO, either of the above solution will be more proper to fix this
warning.

thanks,
rui

> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> Build tested using exynos4_defconfig on linux-next tree of 20121114.
> ---
>  drivers/thermal/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 266c15e..197b7db 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -50,7 +50,7 @@ config RCAR_THERMAL
>  
>  config EXYNOS_THERMAL
>  	tristate "Temperature sensor on Samsung EXYNOS"
> -	depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> +	depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
>  	select CPU_FREQ_TABLE
>  	help
>  	  If you say yes here you get support for TMU (Thermal Managment


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sachin Kamat Nov. 15, 2012, 3:22 a.m. UTC | #2
Hi Zhang,

Thanks for your review comments.

On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
> Hi, Sachin,
>
> thanks for catching the problem.
>
> On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
>> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
>> for dependencies gives the following compilation warnings:
>> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
>> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
>> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>>
> Amit,
>
> how is exynos driver supposed to work?
> do you want the exynos driver still be loaded without CPU_THERMAL?
> If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
> If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
> and CPU_THERMAL will select CPU_FREQ_TABLE instead.
>
> IMO, either of the above solution will be more proper to fix this
> warning.

I will discuss with Amit and send an updated patch to fix this.


>
> thanks,
> rui
>
>> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> Build tested using exynos4_defconfig on linux-next tree of 20121114.
>> ---
>>  drivers/thermal/Kconfig |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 266c15e..197b7db 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -50,7 +50,7 @@ config RCAR_THERMAL
>>
>>  config EXYNOS_THERMAL
>>       tristate "Temperature sensor on Samsung EXYNOS"
>> -     depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>> +     depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
>>       select CPU_FREQ_TABLE
>>       help
>>         If you say yes here you get support for TMU (Thermal Managment
>
>
Zhang Rui Nov. 15, 2012, 4:14 a.m. UTC | #3
On Thu, 2012-11-15 at 08:52 +0530, Sachin Kamat wrote:
> Hi Zhang,
> 
> Thanks for your review comments.
> 
> On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
> > Hi, Sachin,
> >
> > thanks for catching the problem.
> >
> > On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
> >> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
> >> for dependencies gives the following compilation warnings:
> >> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
> >> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
> >> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
> >>
> > Amit,
> >
> > how is exynos driver supposed to work?
> > do you want the exynos driver still be loaded without CPU_THERMAL?
> > If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
> > If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
> > and CPU_THERMAL will select CPU_FREQ_TABLE instead.
> >
> > IMO, either of the above solution will be more proper to fix this
> > warning.
> 
> I will discuss with Amit and send an updated patch to fix this.
> 
I checked the code of exynos driver, it fails to load if no
cpufreq_cooling device found, which means it depends on CPU_THERMAL.

I'm not sure if this is the behavior you want, but IMO, exynos driver
should register its critical trip point to the generic thermal layer
instead, when CPU_THERMAL is not set.

thanks,
rui
> 
> >
> > thanks,
> > rui
> >
> >> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> >> ---
> >> Build tested using exynos4_defconfig on linux-next tree of 20121114.
> >> ---
> >>  drivers/thermal/Kconfig |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >> index 266c15e..197b7db 100644
> >> --- a/drivers/thermal/Kconfig
> >> +++ b/drivers/thermal/Kconfig
> >> @@ -50,7 +50,7 @@ config RCAR_THERMAL
> >>
> >>  config EXYNOS_THERMAL
> >>       tristate "Temperature sensor on Samsung EXYNOS"
> >> -     depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> >> +     depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
> >>       select CPU_FREQ_TABLE
> >>       help
> >>         If you say yes here you get support for TMU (Thermal Managment
> >
> >
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 266c15e..197b7db 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -50,7 +50,7 @@  config RCAR_THERMAL
 
 config EXYNOS_THERMAL
 	tristate "Temperature sensor on Samsung EXYNOS"
-	depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
+	depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
 	select CPU_FREQ_TABLE
 	help
 	  If you say yes here you get support for TMU (Thermal Managment