All of lore.kernel.org
 help / color / mirror / Atom feed
* [PM-WIP_CPUFREQ][PATCH 0/6 v2] Cleanups for cpufreq
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
@ 2011-05-18  7:37 ` Nishanth Menon
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit Nishanth Menon
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

Rev 2 of cleanups for cpufreq to include a couple of bugfixes we found as part of
additional dvfs testing and code review.

Testing done: OMAP4SDP4430 +.39-rc7- I think the only other major platform would be
omap2 - but I doubt it was functional previously - if someone has a chance to give
it a whirl, please do, as I dont have access to an OMAP2 platform atm :(.

Applies on top of:
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
branch: pm-wip/cpufreq

Changes since V1: http://marc.info/?l=linux-omap&m=130532085617487&w=2
	Patch 5 was reworked - return condition handling + lesser spam error msg
	Patch 6 was added - caught as part of kmemleak check
	
Nishanth Menon (6):
  OMAP2+: cpufreq: free up table on exit
  OMAP2+: cpufreq: handle invalid cpufreq table
  OMAP2+: cpufreq: minor comment cleanup
  OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available
  OMAP2+: cpufreq: use cpufreq_frequency_table_target
  OMAP2+: cpufreq: fix freq_table leak

 arch/arm/mach-omap2/omap2plus-cpufreq.c |   81 ++++++++++++++++++++++++++-----
 1 files changed, 69 insertions(+), 12 deletions(-)

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 0/6 v2] Cleanups for cpufreq Nishanth Menon
@ 2011-05-18  7:37 ` Nishanth Menon
  2011-05-19 10:26   ` Kevin Hilman
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 2/6 v2] OMAP2+: cpufreq: handle invalid cpufreq table Nishanth Menon
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

freq_table allocated by opp_init_cpufreq_table in omap_cpu_init
needs to be freed in omap_cpu_exit.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index d53ce23..e38ebb8 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -26,6 +26,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/opp.h>
+#include <linux/slab.h>
 #include <linux/cpu.h>
 
 #include <asm/system.h>
@@ -216,6 +217,8 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 static int omap_cpu_exit(struct cpufreq_policy *policy)
 {
 	clk_exit_cpufreq_table(&freq_table);
+	kfree(freq_table);
+	freq_table = NULL;
 	clk_put(mpu_clk);
 	return 0;
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PM-WIP_CPUFREQ][PATCH 2/6 v2] OMAP2+: cpufreq: handle invalid cpufreq table
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 0/6 v2] Cleanups for cpufreq Nishanth Menon
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit Nishanth Menon
@ 2011-05-18  7:37 ` Nishanth Menon
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup Nishanth Menon
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

Handle the case when cpufreq_frequency_table_cpuinfo fails. freq_table
that we passed failed the internal test of cpufreq generic driver,
so we should'nt be using the freq_table as such. Instead, warn and
fallback to clock functions for validation and operation.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index e38ebb8..6e3666a 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -182,10 +182,18 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 	if (freq_table) {
 		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
-		if (!result)
+		if (!result) {
 			cpufreq_frequency_table_get_attr(freq_table,
 							policy->cpu);
-	} else {
+		} else {
+			WARN(true, "%s: fallback to clk_round(freq_table=%d)\n",
+					__func__, result);
+			kfree(freq_table);
+			freq_table = NULL;
+		}
+	}
+
+	if (!freq_table) {
 		policy->cpuinfo.min_freq = clk_round_rate(mpu_clk, 0) / 1000;
 		policy->cpuinfo.max_freq = clk_round_rate(mpu_clk,
 							VERY_HI_RATE) / 1000;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
                   ` (2 preceding siblings ...)
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 2/6 v2] OMAP2+: cpufreq: handle invalid cpufreq table Nishanth Menon
@ 2011-05-18  7:37 ` Nishanth Menon
  2011-05-18 20:08   ` Todd Poynor
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available Nishanth Menon
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

this should probably get squashed in..

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 6e3666a..45f1e9e 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -84,8 +84,10 @@ static int omap_target(struct cpufreq_policy *policy,
 	if (is_smp() && (num_online_cpus() < NR_CPUS))
 		return ret;
 
-	/* Ensure desired rate is within allowed range.  Some govenors
-	 * (ondemand) will just pass target_freq=0 to get the minimum. */
+	/*
+	 * Ensure desired rate is within allowed range.  Some govenors
+	 * (ondemand) will just pass target_freq=0 to get the minimum.
+	 */
 	if (target_freq < policy->min)
 		target_freq = policy->min;
 	if (target_freq > policy->max)
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
                   ` (3 preceding siblings ...)
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup Nishanth Menon
@ 2011-05-18  7:37 ` Nishanth Menon
  2011-05-19 13:12   ` Kevin Hilman
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 5/6 v2] OMAP2+: cpufreq: use cpufreq_frequency_table_target Nishanth Menon
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 6/6 v2] OMAP2+: cpufreq: fix freq_table leak Nishanth Menon
  6 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

OMAP2 does not use OPP tables at the moment for DVFS. Currently,
we depend on opp table initialization to give us the freq_table,
which makes sense for OMAP3+. for OMAP2, we should be using
clk_init_cpufreq_table - so if the opp based frequency table
initilization fails, fall back to clk_init_cpufreq_table to give
us the table.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 45f1e9e..854f4b3 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -180,7 +180,13 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 		pr_warning("%s: unable to get the mpu device\n", __func__);
 		return -EINVAL;
 	}
-	opp_init_cpufreq_table(mpu_dev, &freq_table);
+
+	/*
+	 * if we dont get cpufreq table using opp, use traditional omap2 lookup
+	 * as a fallback
+	 */
+	if (opp_init_cpufreq_table(mpu_dev, &freq_table))
+		clk_init_cpufreq_table(&freq_table);
 
 	if (freq_table) {
 		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
@@ -188,6 +194,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 			cpufreq_frequency_table_get_attr(freq_table,
 							policy->cpu);
 		} else {
+			clk_exit_cpufreq_table(&freq_table);
 			WARN(true, "%s: fallback to clk_round(freq_table=%d)\n",
 					__func__, result);
 			kfree(freq_table);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PM-WIP_CPUFREQ][PATCH 5/6 v2] OMAP2+: cpufreq: use cpufreq_frequency_table_target
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
                   ` (4 preceding siblings ...)
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available Nishanth Menon
@ 2011-05-18  7:37 ` Nishanth Menon
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 6/6 v2] OMAP2+: cpufreq: fix freq_table leak Nishanth Menon
  6 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

Use cpufreq_frequency_table_target for finding the proper target
instead of seeing if the frequency requested is divisible alone.
if we have a frequency table, we should restrict ourselves to
selecting the "approved" frequencies alone and only in the case
where the frequency table is not available should we attempt at
closest roundable clock frequency.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |   38 ++++++++++++++++++++++--------
 1 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 854f4b3..d0b4f97 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -77,24 +77,42 @@ static int omap_target(struct cpufreq_policy *policy,
 		       unsigned int target_freq,
 		       unsigned int relation)
 {
-	int i, ret = 0;
+	unsigned int i;
+	int ret = 0;
 	struct cpufreq_freqs freqs;
 
 	/* Changes not allowed until all CPUs are online */
 	if (is_smp() && (num_online_cpus() < NR_CPUS))
 		return ret;
 
-	/*
-	 * Ensure desired rate is within allowed range.  Some govenors
-	 * (ondemand) will just pass target_freq=0 to get the minimum.
-	 */
-	if (target_freq < policy->min)
-		target_freq = policy->min;
-	if (target_freq > policy->max)
-		target_freq = policy->max;
+	if (freq_table) {
+		ret = cpufreq_frequency_table_target(policy, freq_table,
+				target_freq, relation, &i);
+		if (ret) {
+			pr_debug("%s: cpu%d: no freq match for %d(ret=%d)\n",
+				__func__, policy->cpu, target_freq, ret);
+			return ret;
+		}
+		freqs.new = freq_table[i].frequency;
+	} else {
+		/*
+		 * Ensure desired rate is within allowed range. Some govenors
+		 * (ondemand) will just pass target_freq=0 to get the minimum.
+		 */
+		if (target_freq < policy->min)
+			target_freq = policy->min;
+		if (target_freq > policy->max)
+			target_freq = policy->max;
+
+		freqs.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
+	}
+	if (!freqs.new) {
+		pr_err("%s: cpu%d: no match for freq %d\n", __func__,
+			policy->cpu, target_freq);
+		return -EINVAL;
+	}
 
 	freqs.old = omap_getspeed(policy->cpu);
-	freqs.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
 	freqs.cpu = policy->cpu;
 
 	if (freqs.old == freqs.new)
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PM-WIP_CPUFREQ][PATCH 6/6 v2] OMAP2+: cpufreq: fix freq_table leak
       [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
                   ` (5 preceding siblings ...)
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 5/6 v2] OMAP2+: cpufreq: use cpufreq_frequency_table_target Nishanth Menon
@ 2011-05-18  7:37 ` Nishanth Menon
  6 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2011-05-18  7:37 UTC (permalink / raw)
  To: kevin; +Cc: linux-omap, Nishanth Menon

Since we have two cpus the cpuinit call for cpu1 causes
freq_table of cpu0 to be overwritten. instead, we maintain
a counter to keep track of cpus who use the cpufreq table
allocate it once(one freq table for all CPUs) and free them
once the last user is done with it.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |   33 ++++++++++++++++++++++++------
 1 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index d0b4f97..fc3d0fb 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -42,6 +42,9 @@
 #define VERY_HI_RATE	900000000
 
 static struct cpufreq_frequency_table *freq_table;
+static int freq_table_users;
+static DEFINE_MUTEX(freq_table_lock);
+
 static struct clk *mpu_clk;
 
 static int omap_verify_speed(struct cpufreq_policy *policy)
@@ -172,6 +175,18 @@ skip_lpj:
 	return ret;
 }
 
+static void freq_table_free(void)
+{
+	if (!freq_table_users)
+		return;
+	freq_table_users--;
+	if (freq_table_users)
+		return;
+	clk_exit_cpufreq_table(&freq_table);
+	kfree(freq_table);
+	freq_table = NULL;
+}
+
 static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 {
 	int result = 0;
@@ -199,14 +214,18 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 		return -EINVAL;
 	}
 
+	mutex_lock(&freq_table_lock);
 	/*
 	 * if we dont get cpufreq table using opp, use traditional omap2 lookup
 	 * as a fallback
 	 */
-	if (opp_init_cpufreq_table(mpu_dev, &freq_table))
-		clk_init_cpufreq_table(&freq_table);
+	if (!freq_table) {
+		if (opp_init_cpufreq_table(mpu_dev, &freq_table))
+			clk_init_cpufreq_table(&freq_table);
+	}
 
 	if (freq_table) {
+		freq_table_users++;
 		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
 		if (!result) {
 			cpufreq_frequency_table_get_attr(freq_table,
@@ -215,10 +234,10 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 			clk_exit_cpufreq_table(&freq_table);
 			WARN(true, "%s: fallback to clk_round(freq_table=%d)\n",
 					__func__, result);
-			kfree(freq_table);
-			freq_table = NULL;
+			freq_table_free();
 		}
 	}
+	mutex_unlock(&freq_table_lock);
 
 	if (!freq_table) {
 		policy->cpuinfo.min_freq = clk_round_rate(mpu_clk, 0) / 1000;
@@ -251,9 +270,9 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 static int omap_cpu_exit(struct cpufreq_policy *policy)
 {
-	clk_exit_cpufreq_table(&freq_table);
-	kfree(freq_table);
-	freq_table = NULL;
+	mutex_lock(&freq_table_lock);
+	freq_table_free();
+	mutex_unlock(&freq_table_lock);
 	clk_put(mpu_clk);
 	return 0;
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup Nishanth Menon
@ 2011-05-18 20:08   ` Todd Poynor
  2011-05-18 20:34     ` Menon, Nishanth
  0 siblings, 1 reply; 15+ messages in thread
From: Todd Poynor @ 2011-05-18 20:08 UTC (permalink / raw)
  To: Nishanth Menon; +Cc: kevin, linux-omap

On Wed, May 18, 2011 at 02:37:43AM -0500, Nishanth Menon wrote:
> this should probably get squashed in..
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index 6e3666a..45f1e9e 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -84,8 +84,10 @@ static int omap_target(struct cpufreq_policy *policy,
>  	if (is_smp() && (num_online_cpus() < NR_CPUS))
>  		return ret;
>  
> -	/* Ensure desired rate is within allowed range.  Some govenors
> -	 * (ondemand) will just pass target_freq=0 to get the minimum. */
> +	/*
> +	 * Ensure desired rate is within allowed range.  Some govenors
> +	 * (ondemand) will just pass target_freq=0 to get the minimum.

So long as the comment is being touched up, could fixup the typo for
"governor" (and also copied to a new block of code in another patch
in the series).

> +	 */
>  	if (target_freq < policy->min)
>  		target_freq = policy->min;
>  	if (target_freq > policy->max)
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup
  2011-05-18 20:08   ` Todd Poynor
@ 2011-05-18 20:34     ` Menon, Nishanth
  0 siblings, 0 replies; 15+ messages in thread
From: Menon, Nishanth @ 2011-05-18 20:34 UTC (permalink / raw)
  To: Todd Poynor; +Cc: kevin, linux-omap

On Wed, May 18, 2011 at 15:08, Todd Poynor <toddpoynor@google.com> wrote:
> On Wed, May 18, 2011 at 02:37:43AM -0500, Nishanth Menon wrote:
>> this should probably get squashed in..
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    6 ++++--
>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> index 6e3666a..45f1e9e 100644
>> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> @@ -84,8 +84,10 @@ static int omap_target(struct cpufreq_policy *policy,
>>       if (is_smp() && (num_online_cpus() < NR_CPUS))
>>               return ret;
>>
>> -     /* Ensure desired rate is within allowed range.  Some govenors
>> -      * (ondemand) will just pass target_freq=0 to get the minimum. */
>> +     /*
>> +      * Ensure desired rate is within allowed range.  Some govenors
>> +      * (ondemand) will just pass target_freq=0 to get the minimum.
>
> So long as the comment is being touched up, could fixup the typo for
> "governor" (and also copied to a new block of code in another patch
> in the series).

Will do. thanks for catching it :).

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit Nishanth Menon
@ 2011-05-19 10:26   ` Kevin Hilman
  2011-05-19 13:48     ` Menon, Nishanth
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-05-19 10:26 UTC (permalink / raw)
  To: Nishanth Menon; +Cc: linux-omap

Nishanth Menon <nm@ti.com> writes:

> freq_table allocated by opp_init_cpufreq_table in omap_cpu_init
> needs to be freed in omap_cpu_exit.

Actually it needs to be freed by a corresponding OPP layer function.

IOW, what happens if the OPP core code switches from using kmalloc to
static tables, or something else?    The cleanup should be done by the
same layer that does the init/alloc.

Kevin

> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index d53ce23..e38ebb8 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -26,6 +26,7 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/opp.h>
> +#include <linux/slab.h>
>  #include <linux/cpu.h>
>  
>  #include <asm/system.h>
> @@ -216,6 +217,8 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
>  static int omap_cpu_exit(struct cpufreq_policy *policy)
>  {
>  	clk_exit_cpufreq_table(&freq_table);
> +	kfree(freq_table);
> +	freq_table = NULL;
>  	clk_put(mpu_clk);
>  	return 0;
>  }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available
  2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available Nishanth Menon
@ 2011-05-19 13:12   ` Kevin Hilman
  2011-05-19 13:51     ` Menon, Nishanth
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-05-19 13:12 UTC (permalink / raw)
  To: Nishanth Menon; +Cc: linux-omap

Nishanth Menon <nm@ti.com> writes:

> OMAP2 does not use OPP tables at the moment for DVFS. Currently,
> we depend on opp table initialization to give us the freq_table,
> which makes sense for OMAP3+. for OMAP2, we should be using
> clk_init_cpufreq_table - so if the opp based frequency table
> initilization fails, fall back to clk_init_cpufreq_table to give
> us the table.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

This is a good approach, but for readability, the OPP version and the
clk version should probably be separated into separate functions, along
with their error handling.

Minor: please capitalize acronyms: OPP, CPU, OMAP, etc...

Kevin

> ---
>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index 45f1e9e..854f4b3 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -180,7 +180,13 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
>  		pr_warning("%s: unable to get the mpu device\n", __func__);
>  		return -EINVAL;
>  	}
> -	opp_init_cpufreq_table(mpu_dev, &freq_table);
> +
> +	/*
> +	 * if we dont get cpufreq table using opp, use traditional omap2 lookup
> +	 * as a fallback
> +	 */
> +	if (opp_init_cpufreq_table(mpu_dev, &freq_table))
> +		clk_init_cpufreq_table(&freq_table);
>  
>  	if (freq_table) {
>  		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> @@ -188,6 +194,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
>  			cpufreq_frequency_table_get_attr(freq_table,
>  							policy->cpu);
>  		} else {
> +			clk_exit_cpufreq_table(&freq_table);
>  			WARN(true, "%s: fallback to clk_round(freq_table=%d)\n",
>  					__func__, result);
>  			kfree(freq_table);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit
  2011-05-19 10:26   ` Kevin Hilman
@ 2011-05-19 13:48     ` Menon, Nishanth
  0 siblings, 0 replies; 15+ messages in thread
From: Menon, Nishanth @ 2011-05-19 13:48 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: linux-omap

On Thu, May 19, 2011 at 05:26, Kevin Hilman <khilman@ti.com> wrote:
> Nishanth Menon <nm@ti.com> writes:
>
>> freq_table allocated by opp_init_cpufreq_table in omap_cpu_init
>> needs to be freed in omap_cpu_exit.
>
> Actually it needs to be freed by a corresponding OPP layer function.
>
> IOW, what happens if the OPP core code switches from using kmalloc to
> static tables, or something else?    The cleanup should be done by the
> same layer that does the init/alloc.

Agreed. We can introduce a new api in OPP layer to free up as well. I
will post a patch for it.
Regards,
Nishanth Menon

> Kevin
>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> index d53ce23..e38ebb8 100644
>> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/clk.h>
>>  #include <linux/io.h>
>>  #include <linux/opp.h>
>> +#include <linux/slab.h>
>>  #include <linux/cpu.h>
>>
>>  #include <asm/system.h>
>> @@ -216,6 +217,8 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
>>  static int omap_cpu_exit(struct cpufreq_policy *policy)
>>  {
>>       clk_exit_cpufreq_table(&freq_table);
>> +     kfree(freq_table);
>> +     freq_table = NULL;
>>       clk_put(mpu_clk);
>>       return 0;
>>  }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available
  2011-05-19 13:12   ` Kevin Hilman
@ 2011-05-19 13:51     ` Menon, Nishanth
  2011-05-25  0:01       ` Kevin Hilman
  0 siblings, 1 reply; 15+ messages in thread
From: Menon, Nishanth @ 2011-05-19 13:51 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: linux-omap

On Thu, May 19, 2011 at 08:12, Kevin Hilman <khilman@ti.com> wrote:
> Nishanth Menon <nm@ti.com> writes:
>
>> OMAP2 does not use OPP tables at the moment for DVFS. Currently,
>> we depend on opp table initialization to give us the freq_table,
>> which makes sense for OMAP3+. for OMAP2, we should be using
>> clk_init_cpufreq_table - so if the opp based frequency table
>> initilization fails, fall back to clk_init_cpufreq_table to give
>> us the table.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>
> This is a good approach, but for readability, the OPP version and the
> clk version should probably be separated into separate functions, along
> with their error handling.
Was thinking more of the lines of splitting the file up. OMAP3+ all
have OPPs defined. only one pending is OMAP2
Either we introduce OPPs to OMAP2 OR we split it up and depend the OPP
based stuff on ARCH_HAS_OPP and CPUFREQ

>
> Minor: please capitalize acronyms: OPP, CPU, OMAP, etc...

will do.
Regards,
Nishanth Menon

>
> Kevin
>
>> ---
>>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    9 ++++++++-
>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> index 45f1e9e..854f4b3 100644
>> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> @@ -180,7 +180,13 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
>>               pr_warning("%s: unable to get the mpu device\n", __func__);
>>               return -EINVAL;
>>       }
>> -     opp_init_cpufreq_table(mpu_dev, &freq_table);
>> +
>> +     /*
>> +      * if we dont get cpufreq table using opp, use traditional omap2 lookup
>> +      * as a fallback
>> +      */
>> +     if (opp_init_cpufreq_table(mpu_dev, &freq_table))
>> +             clk_init_cpufreq_table(&freq_table);
>>
>>       if (freq_table) {
>>               result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
>> @@ -188,6 +194,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
>>                       cpufreq_frequency_table_get_attr(freq_table,
>>                                                       policy->cpu);
>>               } else {
>> +                     clk_exit_cpufreq_table(&freq_table);
>>                       WARN(true, "%s: fallback to clk_round(freq_table=%d)\n",
>>                                       __func__, result);
>>                       kfree(freq_table);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available
  2011-05-19 13:51     ` Menon, Nishanth
@ 2011-05-25  0:01       ` Kevin Hilman
  2011-05-25  7:44         ` Menon, Nishanth
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2011-05-25  0:01 UTC (permalink / raw)
  To: Menon, Nishanth; +Cc: linux-omap

"Menon, Nishanth" <nm@ti.com> writes:

> On Thu, May 19, 2011 at 08:12, Kevin Hilman <khilman@ti.com> wrote:
>> Nishanth Menon <nm@ti.com> writes:
>>
>>> OMAP2 does not use OPP tables at the moment for DVFS. Currently,
>>> we depend on opp table initialization to give us the freq_table,
>>> which makes sense for OMAP3+. for OMAP2, we should be using
>>> clk_init_cpufreq_table - so if the opp based frequency table
>>> initilization fails, fall back to clk_init_cpufreq_table to give
>>> us the table.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>
>> This is a good approach, but for readability, the OPP version and the
>> clk version should probably be separated into separate functions, along
>> with their error handling.
>
> Was thinking more of the lines of splitting the file up. OMAP3+ all
> have OPPs defined. only one pending is OMAP2
> Either we introduce OPPs to OMAP2 OR we split it up and depend the OPP
> based stuff on ARCH_HAS_OPP and CPUFREQ

Let's take the latter approach, and just focus on a single OPP-based driver.

When someone wants to add DVFS for OMAP2, all that's necessary is to add
the OPPs.

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available
  2011-05-25  0:01       ` Kevin Hilman
@ 2011-05-25  7:44         ` Menon, Nishanth
  0 siblings, 0 replies; 15+ messages in thread
From: Menon, Nishanth @ 2011-05-25  7:44 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: linux-omap

On Tue, May 24, 2011 at 17:01, Kevin Hilman <khilman@ti.com> wrote:
> "Menon, Nishanth" <nm@ti.com> writes:
>
>> On Thu, May 19, 2011 at 08:12, Kevin Hilman <khilman@ti.com> wrote:
>>> Nishanth Menon <nm@ti.com> writes:
>>>
>>>> OMAP2 does not use OPP tables at the moment for DVFS. Currently,
>>>> we depend on opp table initialization to give us the freq_table,
>>>> which makes sense for OMAP3+. for OMAP2, we should be using
>>>> clk_init_cpufreq_table - so if the opp based frequency table
>>>> initilization fails, fall back to clk_init_cpufreq_table to give
>>>> us the table.
>>>>
>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>
>>> This is a good approach, but for readability, the OPP version and the
>>> clk version should probably be separated into separate functions, along
>>> with their error handling.
>>
>> Was thinking more of the lines of splitting the file up. OMAP3+ all
>> have OPPs defined. only one pending is OMAP2
>> Either we introduce OPPs to OMAP2 OR we split it up and depend the OPP
>> based stuff on ARCH_HAS_OPP and CPUFREQ
>
> Let's take the latter approach, and just focus on a single OPP-based driver.
>
> When someone wants to add DVFS for OMAP2, all that's necessary is to add
> the OPPs.
yes, I have isolated the code to do that earlier today.. hopefully I
should get time to post this out asap.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-05-25  7:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <[PM-WIP_CPUFREQ][PATCH 0/5] Cleanups for cpufreq>
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 0/6 v2] Cleanups for cpufreq Nishanth Menon
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 1/6 v2] OMAP2+: cpufreq: free up table on exit Nishanth Menon
2011-05-19 10:26   ` Kevin Hilman
2011-05-19 13:48     ` Menon, Nishanth
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 2/6 v2] OMAP2+: cpufreq: handle invalid cpufreq table Nishanth Menon
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 3/6 v2] OMAP2+: cpufreq: minor comment cleanup Nishanth Menon
2011-05-18 20:08   ` Todd Poynor
2011-05-18 20:34     ` Menon, Nishanth
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 4/6 v2] OMAP2: cpufreq: use clk_init_cpufreq_table if OPPs not available Nishanth Menon
2011-05-19 13:12   ` Kevin Hilman
2011-05-19 13:51     ` Menon, Nishanth
2011-05-25  0:01       ` Kevin Hilman
2011-05-25  7:44         ` Menon, Nishanth
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 5/6 v2] OMAP2+: cpufreq: use cpufreq_frequency_table_target Nishanth Menon
2011-05-18  7:37 ` [PM-WIP_CPUFREQ][PATCH 6/6 v2] OMAP2+: cpufreq: fix freq_table leak Nishanth Menon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.