Re: [PATCH 1/6] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions

Tomasz Figa
2014-01-12T13:39:48Z

Hi Thomas,

Please see my comments inline.

On 09.01.2014 16:59, Thomas Abraham wrote:
[snip]
> @@ -69,12 +71,26 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
> new_freq / 1000, volt ? volt / 1000 : -1);
>
> /* scaling up? scale voltage before frequency */
> - if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
> + if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
> + new_freq >= safe_frequency) {
> ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> if (ret) {
> pr_err("failed to scale voltage up: %d\n", ret);
> return ret;
> }
> + } else if (!IS_ERR(cpu_reg) && old_freq < safe_frequency) {
> + /*
> + * the scaled up voltage level for the new_freq is lower
> + * than the safe voltage level. so set safe_voltage
> + * as the intermediate voltage level and revert it
> + * back after the frequency has been changed.
> + */
> + ret = regulator_set_voltage(cpu_reg, safe_voltage,
> + safe_voltage);

Shouldn't the tolerance be used here as well?

> + if (ret) {
> + pr_err("failed to set safe voltage: %d\n", ret);
> + return ret;
> + }
> }
>
> ret = clk_set_rate(cpu_clk, freq_exact);
> @@ -94,6 +110,19 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
> }
> }
>
> + /*
> + * if safe voltage was applied during voltage scale up, then set
> + * the correct target voltage now.
> + */
> + if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
> + new_freq < safe_frequency) {
> + ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> + if (ret) {
> + pr_err("failed to scale voltage up: %d\n", ret);
> + return ret;
> + }
> + }
> +

I believe that it would be enough to reuse the if block of scaling down
instead of repeating the same code, just the condition must be slightly
modified:

- /* scaling down? scale voltage after frequency */
- if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
+ /*
+ * scaling down or below safe frequency?
+ * scale voltage after frequency
+ */
+ if (!IS_ERR(cpu_reg)
+ && (new_freq < old_freq || new_freq < safe_frequency)) {

Best regards,
Tomasz