[PATCH] cpufreq: OMAP: specify range for voltage scaling

[PATCH] cpufreq: OMAP: specify range for voltage scaling

[Afzal Mohammed]

Specify voltage in ranges for regulator. Range
used is tolerance specified for OPP.

This helps to achieve DVFS with a wider range of
regulators.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
Hi,

Tolerance specified here is that of AM335X, least value
of tolerance that I could find so far for OMAP family

This applies on top of Kevin Hilman's patch (v2),
"cpufreq: OMAP: scale voltage along with frequency"
http://www.spinics.net/lists/linux-omap/msg65002.html

Regards
Afzal

 drivers/cpufreq/omap-cpufreq.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 10b8e23..3cea51b 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -38,6 +38,9 @@
 
 #include <mach/hardware.h>
 
+/* OPP tolerance in percentage */
+#define	OPP_TOLERANCE	4
+
 #ifdef CONFIG_SMP
 struct lpj_info {
 	unsigned long	ref;
@@ -81,7 +84,7 @@ static int omap_target(struct cpufreq_policy *policy,
 	int r, ret = 0;
 	struct cpufreq_freqs freqs;
 	struct opp *opp;
-	unsigned long freq, volt = 0, volt_old = 0;
+	unsigned long freq, volt = 0, volt_old = 0, tol = 0;
 
 	if (!freq_table) {
 		dev_err(mpu_dev, "%s: cpu%d: no freq table!\n", __func__,
@@ -125,6 +128,7 @@ static int omap_target(struct cpufreq_policy *policy,
 			return -EINVAL;
 		}
 		volt = opp_get_voltage(opp);
+		tol = volt * OPP_TOLERANCE / 100;
 		volt_old = regulator_get_voltage(mpu_reg);
 	}
 
@@ -134,7 +138,7 @@ static int omap_target(struct cpufreq_policy *policy,
 
 	/* scaling up?  scale voltage before frequency */
 	if (mpu_reg && (freqs.new > freqs.old)) {
-		r = regulator_set_voltage(mpu_reg, volt, volt);
+		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
 		if (r < 0) {
 			dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
 				 __func__);
@@ -147,7 +151,7 @@ static int omap_target(struct cpufreq_policy *policy,
 
 	/* scaling down?  scale voltage after frequency */
 	if (mpu_reg && (freqs.new < freqs.old)) {
-		r = regulator_set_voltage(mpu_reg, volt, volt);
+		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
 		if (r < 0) {
 			dev_warn(mpu_dev, "%s: unable to scale voltage down.\n",
 				 __func__);
-- 
1.7.1

Re: [PATCH] cpufreq: OMAP: specify range for voltage scaling

[Kevin Hilman]

Afzal Mohammed <afzal@ti.com> writes:

> Specify voltage in ranges for regulator. Range
> used is tolerance specified for OPP.
>
> This helps to achieve DVFS with a wider range of
> regulators.
>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Afzal Mohammed <afzal@ti.com>

Thanks, will queue this with the CPUfreq changes for MPU DVFS.

Kevin

> ---
> Hi,
>
> Tolerance specified here is that of AM335X, least value
> of tolerance that I could find so far for OMAP family
>
> This applies on top of Kevin Hilman's patch (v2),
> "cpufreq: OMAP: scale voltage along with frequency"
> http://www.spinics.net/lists/linux-omap/msg65002.html
>
> Regards
> Afzal
>
>  drivers/cpufreq/omap-cpufreq.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> index 10b8e23..3cea51b 100644
> --- a/drivers/cpufreq/omap-cpufreq.c
> +++ b/drivers/cpufreq/omap-cpufreq.c
> @@ -38,6 +38,9 @@
>  
>  #include <mach/hardware.h>
>  
> +/* OPP tolerance in percentage */
> +#define	OPP_TOLERANCE	4
> +
>  #ifdef CONFIG_SMP
>  struct lpj_info {
>  	unsigned long	ref;
> @@ -81,7 +84,7 @@ static int omap_target(struct cpufreq_policy *policy,
>  	int r, ret = 0;
>  	struct cpufreq_freqs freqs;
>  	struct opp *opp;
> -	unsigned long freq, volt = 0, volt_old = 0;
> +	unsigned long freq, volt = 0, volt_old = 0, tol = 0;
>  
>  	if (!freq_table) {
>  		dev_err(mpu_dev, "%s: cpu%d: no freq table!\n", __func__,
> @@ -125,6 +128,7 @@ static int omap_target(struct cpufreq_policy *policy,
>  			return -EINVAL;
>  		}
>  		volt = opp_get_voltage(opp);
> +		tol = volt * OPP_TOLERANCE / 100;
>  		volt_old = regulator_get_voltage(mpu_reg);
>  	}
>  
> @@ -134,7 +138,7 @@ static int omap_target(struct cpufreq_policy *policy,
>  
>  	/* scaling up?  scale voltage before frequency */
>  	if (mpu_reg && (freqs.new > freqs.old)) {
> -		r = regulator_set_voltage(mpu_reg, volt, volt);
> +		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
>  		if (r < 0) {
>  			dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
>  				 __func__);
> @@ -147,7 +151,7 @@ static int omap_target(struct cpufreq_policy *policy,
>  
>  	/* scaling down?  scale voltage after frequency */
>  	if (mpu_reg && (freqs.new < freqs.old)) {
> -		r = regulator_set_voltage(mpu_reg, volt, volt);
> +		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
>  		if (r < 0) {
>  			dev_warn(mpu_dev, "%s: unable to scale voltage down.\n",
>  				 __func__);

Re: [PATCH] cpufreq: OMAP: specify range for voltage scaling

[Kevin Hilman]

+Tero

Kevin Hilman <khilman@ti.com> writes:

> Afzal Mohammed <afzal@ti.com> writes:
>
>> Specify voltage in ranges for regulator. Range
>> used is tolerance specified for OPP.
>>
>> This helps to achieve DVFS with a wider range of
>> regulators.
>>
>> Cc: Kevin Hilman <khilman@ti.com>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Signed-off-by: Afzal Mohammed <afzal@ti.com>
>
> Thanks, will queue this with the CPUfreq changes for MPU DVFS.

Actually, not quite yet...

After some testing with the SMPS regulators, this won't quite work with
the current SMPS regulators.  Does this actually work with the
regulators you're using?

For OMAPs using VC/VP for voltage scaling, the TWL regulator passes on
the voltage requested directly to the voltage layer.  When using voltage
- tolerance, this results in voltages that the voltage layer doesn't
know about because they do not match any of the voltages from the known
OPPs.

The problem that we have is that while the regulators can support a
broad range of voltages (from min to max with a some step), the on-chip
voltage domains cannot, which is why we have defined the OPPs which are
known to work.

Tero, for the SMPS regulators, would it be possible to configure the
regulators so that only a discrete set voltages are availble to pick
from?  These should be initialied from the OPP layer.

Somehow we need to support something like $SUBJECT patch in order to
support a broad range of regulators.   The OMAP voltagedomain
limitations need to configured in platform specific way such that the
CPUfreq driver can remain generic.

Kevin

Re: [PATCH] cpufreq: OMAP: specify range for voltage scaling

[Tero Kristo]

[-- Attachment #1: Type: text/plain, Size: 3057 bytes --]

On Thu, 2012-03-01 at 14:07 -0800, Kevin Hilman wrote:
> +Tero
> 
> Kevin Hilman <khilman@ti.com> writes:
> 
> > Afzal Mohammed <afzal@ti.com> writes:
> >
> >> Specify voltage in ranges for regulator. Range
> >> used is tolerance specified for OPP.
> >>
> >> This helps to achieve DVFS with a wider range of
> >> regulators.
> >>
> >> Cc: Kevin Hilman <khilman@ti.com>
> >> Cc: Sekhar Nori <nsekhar@ti.com>
> >> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> >
> > Thanks, will queue this with the CPUfreq changes for MPU DVFS.
> 
> Actually, not quite yet...
> 
> After some testing with the SMPS regulators, this won't quite work with
> the current SMPS regulators.  Does this actually work with the
> regulators you're using?
> 
> For OMAPs using VC/VP for voltage scaling, the TWL regulator passes on
> the voltage requested directly to the voltage layer.  When using voltage
> - tolerance, this results in voltages that the voltage layer doesn't
> know about because they do not match any of the voltages from the known
> OPPs.
> 
> The problem that we have is that while the regulators can support a
> broad range of voltages (from min to max with a some step), the on-chip
> voltage domains cannot, which is why we have defined the OPPs which are
> known to work.
> 
> Tero, for the SMPS regulators, would it be possible to configure the
> regulators so that only a discrete set voltages are availble to pick
> from?  These should be initialied from the OPP layer.

I guess this would be possible. Looking at this patch, I guess what we
want to do is that we just use the current API we have at the SMPS
regulator layer to pass the min_uV as target_uV for the regulator, and
just get the next opp voltage that is >= than the target_uV.

Here is an inline patch that does this (also attached), this applies on
top of my SMPS regulator changes:

diff --git a/arch/arm/mach-omap2/twl-common.c
b/arch/arm/mach-omap2/twl-common.c
index f19793e..7dc891f 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -41,7 +41,16 @@ static struct i2c_board_info __initdata
pmic_i2c_board_info = {
 static int twl_set_voltage(void *data, int target_uV)
 {
        struct voltagedomain *voltdm = (struct voltagedomain *)data;
-       return voltdm_scale(voltdm, target_uV);
+       struct omap_volt_data *volt_data = voltdm->volt_data;
+       
+       while (1) {
+               if (!volt_data->volt_nominal)
+                       return -EINVAL;
+               if (volt_data->volt_nominal >= target_uV)
+                       break;
+               volt_data++;
+       }
+       return voltdm_scale(voltdm, volt_data->volt_nominal);
 }
 
 static int twl_get_voltage(void *data)


Do you want a new version of the whole set or just this as a separate
one?

-Tero

> 
> Somehow we need to support something like $SUBJECT patch in order to
> support a broad range of regulators.   The OMAP voltagedomain
> limitations need to configured in platform specific way such that the
> CPUfreq driver can remain generic.
> 
> Kevin
> 


[-- Attachment #2: 0001-ARM-OMAP3-smps-regulator-match-target_uV-against-OPP.patch --]
[-- Type: text/x-patch, Size: 1372 bytes --]

>From 8b4c16fdfee63a414ebee52e674b6d9fbc16e7c9 Mon Sep 17 00:00:00 2001
From: Tero Kristo <t-kristo@ti.com>
Date: Fri, 2 Mar 2012 12:14:57 +0200
Subject: [PATCH] ARM: OMAP3+: smps regulator: match target_uV against OPP definitions

Scans through the nominal voltage for OPPs until a voltage is found
that is greater than equal to the one requested. This allows the
target_uV to have some error margin in it, and we will always scale
to the next closest OPP voltage.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/twl-common.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index f19793e..7dc891f 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -41,7 +41,16 @@ static struct i2c_board_info __initdata pmic_i2c_board_info = {
 static int twl_set_voltage(void *data, int target_uV)
 {
 	struct voltagedomain *voltdm = (struct voltagedomain *)data;
-	return voltdm_scale(voltdm, target_uV);
+	struct omap_volt_data *volt_data = voltdm->volt_data;
+	
+	while (1) {
+		if (!volt_data->volt_nominal)
+			return -EINVAL;
+		if (volt_data->volt_nominal >= target_uV)
+			break;
+		volt_data++;
+	}
+	return voltdm_scale(voltdm, volt_data->volt_nominal);
 }
 
 static int twl_get_voltage(void *data)
-- 
1.7.4.1

RE: [PATCH] cpufreq: OMAP: specify range for voltage scaling

[Mohammed, Afzal]

Hi Kevin,

On Fri, Mar 02, 2012 at 03:37:51, Hilman, Kevin wrote:
> > Thanks, will queue this with the CPUfreq changes for MPU DVFS.
> 
> Actually, not quite yet...
> 
> After some testing with the SMPS regulators, this won't quite work with
> the current SMPS regulators.  Does this actually work with the
> regulators you're using?

Yes, it works on AM335X (doesn't have VC/VP) using TPS65910, it always
sets voltage to lowest step possible within minus OPP tolerance range.

Regards
Afzal

Re: [PATCH] cpufreq: OMAP: specify range for voltage scaling

[Kevin Hilman]

+Nishanth

Tero Kristo <t-kristo@ti.com> writes:

> On Thu, 2012-03-01 at 14:07 -0800, Kevin Hilman wrote:
>> +Tero
>> 
>> Kevin Hilman <khilman@ti.com> writes:
>> 
>> > Afzal Mohammed <afzal@ti.com> writes:
>> >
>> >> Specify voltage in ranges for regulator. Range
>> >> used is tolerance specified for OPP.
>> >>
>> >> This helps to achieve DVFS with a wider range of
>> >> regulators.
>> >>
>> >> Cc: Kevin Hilman <khilman@ti.com>
>> >> Cc: Sekhar Nori <nsekhar@ti.com>
>> >> Signed-off-by: Afzal Mohammed <afzal@ti.com>
>> >
>> > Thanks, will queue this with the CPUfreq changes for MPU DVFS.
>> 
>> Actually, not quite yet...
>> 
>> After some testing with the SMPS regulators, this won't quite work with
>> the current SMPS regulators.  Does this actually work with the
>> regulators you're using?
>> 
>> For OMAPs using VC/VP for voltage scaling, the TWL regulator passes on
>> the voltage requested directly to the voltage layer.  When using voltage
>> - tolerance, this results in voltages that the voltage layer doesn't
>> know about because they do not match any of the voltages from the known
>> OPPs.
>> 
>> The problem that we have is that while the regulators can support a
>> broad range of voltages (from min to max with a some step), the on-chip
>> voltage domains cannot, which is why we have defined the OPPs which are
>> known to work.
>> 
>> Tero, for the SMPS regulators, would it be possible to configure the
>> regulators so that only a discrete set voltages are availble to pick
>> from?  These should be initialied from the OPP layer.
>
> I guess this would be possible. Looking at this patch, I guess what we
> want to do is that we just use the current API we have at the SMPS
> regulator layer to pass the min_uV as target_uV for the regulator, and
> just get the next opp voltage that is >= than the target_uV.

Yeah, this should work.

However, after discussing this on IRC with Nishanth, instead of enforcing
this in the TWL glue layer, I think we need to enforce using OPP
voltages in the voltage layer itself.  Patch below[1].

I've now added that patch to my for_3.4/pm/smps-regulator branch (and
will post to the list shortly.)

I've also added $SUBJECT patch (back) to my for_3.4/cpufreq branch. 

I'd appreciate some testing of these two branches together.  With those
two, I've tested MPU DVFS on 3530/Overo, 3630/Zoom3 and 4430/Panda, but
would appreciate any additional testing.

Kevin


>From 212024af641266c3de8b55764f84d923f238315c Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Fri, 2 Mar 2012 14:08:57 -0800
Subject: [PATCH] ARM: OMAP2+: voltage: ensure voltage used is exact voltage
 from OPP table

When using the SMPS regulators to scale voltages, the regulator
framework may pass a minimum voltage that is not an exact OPP voltage.
For the VC/VP controlled voltage domains, we must ensure that the
voltage requested is the exact voltage from the OPP table.  This is
especially critical when using SR.

To fix, voltdm_scale() uses the target voltage passed to walk through
the OPP voltages until it finds a voltage that is >= one of the OPP
voltages.

Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/voltage.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 8a36342..4dc60e8 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -73,7 +73,8 @@ unsigned long voltdm_get_voltage(struct voltagedomain *voltdm)
 int voltdm_scale(struct voltagedomain *voltdm,
 		 unsigned long target_volt)
 {
-	int ret;
+	int ret, i;
+	unsigned long volt = 0;
 
 	if (!voltdm || IS_ERR(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
@@ -86,9 +87,23 @@ int voltdm_scale(struct voltagedomain *voltdm,
 		return -ENODATA;
 	}
 
-	ret = voltdm->scale(voltdm, target_volt);
+	/* Adjust voltage to the exact voltage from the OPP table */
+	for (i = 0; voltdm->volt_data[i].volt_nominal != 0; i++) {
+		if (voltdm->volt_data[i].volt_nominal >= target_volt) {
+			volt = voltdm->volt_data[i].volt_nominal;
+			break;
+		}
+	}
+
+	if (!volt) {
+		pr_warning("%s: not scaling. OPP voltage for %lu, not found.\n",
+			   __func__, target_volt);
+		return -EINVAL;
+	}
+
+	ret = voltdm->scale(voltdm, volt);
 	if (!ret)
-		voltdm->nominal_volt = target_volt;
+		voltdm->nominal_volt = volt;
 
 	return ret;
 }
-- 
1.7.9.2