diff mbox

[V3,1/8] cpufreq: cpufreq-cpu0: remove dependency on thermal

Message ID c73c825beda0207867bd29e40fece217cbf15925.1400736536.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar May 22, 2014, 5:37 a.m. UTC
cpufreq-cpu0 uses thermal framework to register a cooling device, but doesn't
depend on it as there are dummy calls provided by thermal layer when
CONFIG_THERMAL=n. So, we don't really need to mention thermal as a dependency
for cpufreq-cpu0 in Kconfig.

Remove it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eduardo Valentin May 22, 2014, 2:52 p.m. UTC | #1
Hello Viresh,

On Thu, May 22, 2014 at 11:07:25AM +0530, Viresh Kumar wrote:
> cpufreq-cpu0 uses thermal framework to register a cooling device, but doesn't
> depend on it as there are dummy calls provided by thermal layer when
> CONFIG_THERMAL=n. So, we don't really need to mention thermal as a dependency
> for cpufreq-cpu0 in Kconfig.


I see your point.
> 
> Remove it.

However, on CPUs that needs thermal managment, it makes sense to have
such dependency, from functional perspective. Mainly because scaling
frequency and voltage up would be allowed only when thermal management
is enabled.

> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index 1fbe11f..4310997 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -185,7 +185,7 @@ config CPU_FREQ_GOV_CONSERVATIVE
>  
>  config GENERIC_CPUFREQ_CPU0
>  	tristate "Generic CPU0 cpufreq driver"
> -	depends on HAVE_CLK && REGULATOR && OF && THERMAL && CPU_THERMAL
> +	depends on HAVE_CLK && REGULATOR && OF
>  	select PM_OPP
>  	help
>  	  This adds a generic cpufreq driver for CPU0 frequency management.
> -- 
> 2.0.0.rc2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
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
Viresh Kumar May 23, 2014, 4:33 a.m. UTC | #2
On 22 May 2014 20:22, Eduardo Valentin <edubezval@gmail.com> wrote:
> However, on CPUs that needs thermal managment, it makes sense to have
> such dependency, from functional perspective. Mainly because scaling
> frequency and voltage up would be allowed only when thermal management
> is enabled.

AFAIK, dependencies in KCONFIG are only for fixing compilation time issues.
As some APIs wouldn't be available without enabling some config options..

If drivers fail at runtime because some API returned error, fix it for your
platform instead and not bug KCONFIG for that.

Thought we might consider some runtime dependencies here as well. For
example regulators. There probably are dummy routine available for cases
where CONFIG_REGULATOR (or whatever) isn't enabled and driver would
still compile, but it is guaranteed to fail as we don't continue when we get
errors from regulator APIs..

Though I still feel that this driver should still support platforms without
regulators (atleast in software, they might always have them on board :))..
And so dependencies for regulators may also die out one day..

The dependencies here mean: "This driver would never ever work/compile
if the dependencies aren't met.."
--
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
Eduardo Valentin May 23, 2014, 1:21 p.m. UTC | #3
Hi Viresh,

On Fri, May 23, 2014 at 10:03:27AM +0530, Viresh Kumar wrote:
> On 22 May 2014 20:22, Eduardo Valentin <edubezval@gmail.com> wrote:
> > However, on CPUs that needs thermal managment, it makes sense to have
> > such dependency, from functional perspective. Mainly because scaling
> > frequency and voltage up would be allowed only when thermal management
> > is enabled.
> 
> AFAIK, dependencies in KCONFIG are only for fixing compilation time issues.


Actually, they also impose module loading sequencing. 

> As some APIs wouldn't be available without enabling some config options..
> 
> If drivers fail at runtime because some API returned error, fix it for your
> platform instead and not bug KCONFIG for that.

Agreed, but I don't think this is the point of this thread, as you
already stated in your patch description.

> 
> Thought we might consider some runtime dependencies here as well. For
> example regulators. There probably are dummy routine available for cases
> where CONFIG_REGULATOR (or whatever) isn't enabled and driver would
> still compile, but it is guaranteed to fail as we don't continue when we get
> errors from regulator APIs..

I agree. We need to have runtime dependency, and that is the major
concern on my behalf. The problem of Kconfig dependency is that it
imposes sequencing only on module loading, not at boot sequencing.

Another way around is returning -EPROBE_DEFER when some API is not ready at device probing for instance.

> 
> Though I still feel that this driver should still support platforms without
> regulators (atleast in software, they might always have them on board :))..
> And so dependencies for regulators may also die out one day..
> 
> The dependencies here mean: "This driver would never ever work/compile
> if the dependencies aren't met.."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Viresh Kumar May 23, 2014, 1:28 p.m. UTC | #4
> On 23-May-2014, at 6:51 pm, Eduardo Valentin <edubezval@gmail.com> wrote:
> I agree. We need to have runtime dependency, and that is the major
> concern on my behalf. The problem of Kconfig dependency is that it
> imposes sequencing only on module loading, not at boot sequencing.
> 
> Another way around is returning -EPROBE_DEFER when some API is not ready at device probing for instance.

Okay, coming back to this patch. This driver is usable without thermal.
And so the dependencies better be dropped ?--
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
Pavel Machek May 24, 2014, 12:59 p.m. UTC | #5
On Fri 2014-05-23 10:03:27, Viresh Kumar wrote:
> On 22 May 2014 20:22, Eduardo Valentin <edubezval@gmail.com> wrote:
> > However, on CPUs that needs thermal managment, it makes sense to have
> > such dependency, from functional perspective. Mainly because scaling
> > frequency and voltage up would be allowed only when thermal management
> > is enabled.
> 
> AFAIK, dependencies in KCONFIG are only for fixing compilation time issues.

I do not think that's correct.
--
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
Viresh Kumar May 26, 2014, 4:04 a.m. UTC | #6
On 24 May 2014 18:29, Pavel Machek <pavel@ucw.cz> wrote:
> On Fri 2014-05-23 10:03:27, Viresh Kumar wrote:
>> On 22 May 2014 20:22, Eduardo Valentin <edubezval@gmail.com> wrote:
>> > However, on CPUs that needs thermal managment, it makes sense to have
>> > such dependency, from functional perspective. Mainly because scaling
>> > frequency and voltage up would be allowed only when thermal management
>> > is enabled.
>>
>> AFAIK, dependencies in KCONFIG are only for fixing compilation time issues.
>
> I do not think that's correct.

Yeah, that what I accepted later as well.. Dependency is whatever without which
the module is unusable.

And cpufreq-cpu0 is usable without THERMAL and so this dependency should
go away..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 1fbe11f..4310997 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -185,7 +185,7 @@  config CPU_FREQ_GOV_CONSERVATIVE
 
 config GENERIC_CPUFREQ_CPU0
 	tristate "Generic CPU0 cpufreq driver"
-	depends on HAVE_CLK && REGULATOR && OF && THERMAL && CPU_THERMAL
+	depends on HAVE_CLK && REGULATOR && OF
 	select PM_OPP
 	help
 	  This adds a generic cpufreq driver for CPU0 frequency management.