Re: [PATCH v2] cpufreq: conservative: Decrease frequency faster when the update deferred

[Viresh Kumar <viresh.kumar@linaro.org>]

On 12-11-16, 23:04, Stratos Karafotis wrote:
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index fa5ece3..d787772 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -73,7 +73,19 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  	 */
>  	if (cs_tuners->freq_step == 0)
>  		goto out;
> -
> +	/*
> +	 * Decrease requested_freq for each idle period that we didn't
> +	 * update the frequency

Add a full stop (.) here.

> +	 */

I am wondering if we should be adding this code after the below 'if' block where
we check if the policy->min/max have changed ?

> +	if (policy_dbs->idle_periods < UINT_MAX) {
> +		unsigned int freq_target = policy_dbs->idle_periods *
> +				get_freq_target(cs_tuners, policy);

I get confused every time I look at this routine (get_freq_target()). I have
sent an update to this file just now to get that fixed. If Rafael applies that
one, please rebase over it.

> +		if (requested_freq > freq_target)
> +			requested_freq -= freq_target;
> +		else
> +			requested_freq = policy->min;
> +		policy_dbs->idle_periods = UINT_MAX;
> +	}

Need a blank line here.

-- 
viresh