[PATCH 0/2] cpufreq: cpu0: Improvements for cpu0_set_target()

[PATCH 0/2] cpufreq: cpu0: Improvements for cpu0_set_target()

[Stefan Wahren]

This patch series contains 2 improvements for cpu0_set_target().

Stefan Wahren (2):
  cpufreq: cpu0: Improve debug about matching OPP
  cpufreq: cpu0: Handle regulator_get_voltage() failure

 drivers/cpufreq/cpufreq-cpu0.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] cpufreq: cpu0: Improve debug about matching OPP

[Stefan Wahren]

During test of new DT OPPs it's very helpful to print the matching
OPP in case of frequency change. So it will be easier to find frequency
rounding issues in the dts file.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/cpufreq/cpufreq-cpu0.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index a5f8c5f..ef3d618 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -57,6 +57,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 	old_freq = clk_get_rate(cpu_clk) / 1000;
 
 	if (!IS_ERR(cpu_reg)) {
+		unsigned long opp_freq;
+
 		rcu_read_lock();
 		opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz);
 		if (IS_ERR(opp)) {
@@ -66,9 +68,12 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 			return PTR_ERR(opp);
 		}
 		volt = dev_pm_opp_get_voltage(opp);
+		opp_freq = dev_pm_opp_get_freq(opp);
 		rcu_read_unlock();
 		tol = volt * priv->voltage_tolerance / 100;
 		volt_old = regulator_get_voltage(cpu_reg);
+		dev_dbg(cpu_dev, "Found OPP: %ld kHz, %ld uV\n",
+			opp_freq / 1000, volt);
 	}
 
 	dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",
-- 
1.7.9.5

[PATCH 2/2] cpufreq: cpu0: Handle regulator_get_voltage() failure

[Stefan Wahren]

In error cases regulator_get_voltage() returns a negative value.
So take care of it.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/cpufreq/cpufreq-cpu0.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index ef3d618..6aa2104 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -77,7 +77,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 	}
 
 	dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",
-		old_freq / 1000, volt_old ? volt_old / 1000 : -1,
+		old_freq / 1000, (volt_old > 0) ? volt_old / 1000 : -1,
 		new_freq / 1000, volt ? volt / 1000 : -1);
 
 	/* scaling up?  scale voltage before frequency */
@@ -93,7 +93,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 	ret = clk_set_rate(cpu_clk, freq_exact);
 	if (ret) {
 		dev_err(cpu_dev, "failed to set clock rate: %d\n", ret);
-		if (!IS_ERR(cpu_reg))
+		if (!IS_ERR(cpu_reg) && volt_old > 0)
 			regulator_set_voltage_tol(cpu_reg, volt_old, tol);
 		return ret;
 	}
-- 
1.7.9.5

Re: [PATCH 0/2] cpufreq: cpu0: Improvements for cpu0_set_target()

[Rafael J. Wysocki]

On Saturday, October 04, 2014 08:26:02 PM Stefan Wahren wrote:
> This patch series contains 2 improvements for cpu0_set_target().
> 
> Stefan Wahren (2):
>   cpufreq: cpu0: Improve debug about matching OPP
>   cpufreq: cpu0: Handle regulator_get_voltage() failure
> 
>  drivers/cpufreq/cpufreq-cpu0.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

We renamed cpufreq-cpu0 to cpufreq-dt in the meantime.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] cpufreq: cpu0: Improve debug about matching OPP

[Viresh Kumar]

On 5 October 2014 01:56, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> During test of new DT OPPs it's very helpful to print the matching
> OPP in case of frequency change. So it will be easier to find frequency
> rounding issues in the dts file.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index a5f8c5f..ef3d618 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -57,6 +57,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>         old_freq = clk_get_rate(cpu_clk) / 1000;
>
>         if (!IS_ERR(cpu_reg)) {
> +               unsigned long opp_freq;
> +
>                 rcu_read_lock();
>                 opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz);
>                 if (IS_ERR(opp)) {
> @@ -66,9 +68,12 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>                         return PTR_ERR(opp);
>                 }
>                 volt = dev_pm_opp_get_voltage(opp);
> +               opp_freq = dev_pm_opp_get_freq(opp);
>                 rcu_read_unlock();
>                 tol = volt * priv->voltage_tolerance / 100;
>                 volt_old = regulator_get_voltage(cpu_reg);
> +               dev_dbg(cpu_dev, "Found OPP: %ld kHz, %ld uV\n",
> +                       opp_freq / 1000, volt);

What about using dev_pm_opp_get_freq() directly in dev_dbg() instead of
creating a separate variable?

>         }
>
>         dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",
> --
> 1.7.9.5
>

Re: [PATCH 2/2] cpufreq: cpu0: Handle regulator_get_voltage() failure

[Viresh Kumar]

On 5 October 2014 01:56, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> In error cases regulator_get_voltage() returns a negative value.
> So take care of it.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index ef3d618..6aa2104 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -77,7 +77,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>         }
>
>         dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",
> -               old_freq / 1000, volt_old ? volt_old / 1000 : -1,
> +               old_freq / 1000, (volt_old > 0) ? volt_old / 1000 : -1,
>                 new_freq / 1000, volt ? volt / 1000 : -1);
>
>         /* scaling up?  scale voltage before frequency */
> @@ -93,7 +93,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>         ret = clk_set_rate(cpu_clk, freq_exact);
>         if (ret) {
>                 dev_err(cpu_dev, "failed to set clock rate: %d\n", ret);
> -               if (!IS_ERR(cpu_reg))
> +               if (!IS_ERR(cpu_reg) && volt_old > 0)
>                         regulator_set_voltage_tol(cpu_reg, volt_old, tol);
>                 return ret;
>         }

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH 0/2] cpufreq: cpu0: Improvements for cpu0_set_target()

[Stefan Wahren]

Am 05.10.2014 um 23:34 schrieb Rafael J. Wysocki:
> On Saturday, October 04, 2014 08:26:02 PM Stefan Wahren wrote:
>> This patch series contains 2 improvements for cpu0_set_target().
>>
>> Stefan Wahren (2):
>>   cpufreq: cpu0: Improve debug about matching OPP
>>   cpufreq: cpu0: Handle regulator_get_voltage() failure
>>
>>  drivers/cpufreq/cpufreq-cpu0.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
> We renamed cpufreq-cpu0 to cpufreq-dt in the meantime.
>

I will fix that.

Stefan

Re: [PATCH 1/2] cpufreq: cpu0: Improve debug about matching OPP

[Stefan Wahren]

Am 06.10.2014 um 07:13 schrieb Viresh Kumar:
> On 5 October 2014 01:56, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> During test of new DT OPPs it's very helpful to print the matching
>> OPP in case of frequency change. So it will be easier to find frequency
>> rounding issues in the dts file.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>>  drivers/cpufreq/cpufreq-cpu0.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
>> index a5f8c5f..ef3d618 100644
>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> @@ -57,6 +57,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>>         old_freq = clk_get_rate(cpu_clk) / 1000;
>>
>>         if (!IS_ERR(cpu_reg)) {
>> +               unsigned long opp_freq;
>> +
>>                 rcu_read_lock();
>>                 opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz);
>>                 if (IS_ERR(opp)) {
>> @@ -66,9 +68,12 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>>                         return PTR_ERR(opp);
>>                 }
>>                 volt = dev_pm_opp_get_voltage(opp);
>> +               opp_freq = dev_pm_opp_get_freq(opp);
>>                 rcu_read_unlock();
>>                 tol = volt * priv->voltage_tolerance / 100;
>>                 volt_old = regulator_get_voltage(cpu_reg);
>> +               dev_dbg(cpu_dev, "Found OPP: %ld kHz, %ld uV\n",
>> +                       opp_freq / 1000, volt);
> What about using dev_pm_opp_get_freq() directly in dev_dbg() instead of
> creating a separate variable?

I wasn't sure because of the read lock and wanted to avoid any delays.
If it's okay i will change it.

>
>>         }
>>
>>         dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",
>> --
>> 1.7.9.5
>>

Re: [PATCH 1/2] cpufreq: cpu0: Improve debug about matching OPP

[Viresh Kumar]

On 5 October 2014 01:56, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> During test of new DT OPPs it's very helpful to print the matching
> OPP in case of frequency change. So it will be easier to find frequency
> rounding issues in the dts file.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/cpufreq/cpufreq-cpu0.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index a5f8c5f..ef3d618 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -57,6 +57,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>         old_freq = clk_get_rate(cpu_clk) / 1000;
>
>         if (!IS_ERR(cpu_reg)) {
> +               unsigned long opp_freq;
> +
>                 rcu_read_lock();
>                 opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz);
>                 if (IS_ERR(opp)) {
> @@ -66,9 +68,12 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
>                         return PTR_ERR(opp);
>                 }
>                 volt = dev_pm_opp_get_voltage(opp);
> +               opp_freq = dev_pm_opp_get_freq(opp);
>                 rcu_read_unlock();
>                 tol = volt * priv->voltage_tolerance / 100;
>                 volt_old = regulator_get_voltage(cpu_reg);
> +               dev_dbg(cpu_dev, "Found OPP: %ld kHz, %ld uV\n",
> +                       opp_freq / 1000, volt);
>         }
>
>         dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n",

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>