diff mbox

[1/2] cpufreq: don't allow governor limits to be changed when it is disabled

Message ID 085013f4e584e3fef97187bcb349c3fa76942e19.1378012620.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar Sept. 1, 2013, 5:26 a.m. UTC
__cpufreq_governor() returns with -EBUSY when governor is already stopped and we
try to stop it again, but when it is stopped we must not allow calls to
CPUFREQ_GOV_LIMITS event as well.

This patch adds this check in __cpufreq_governor().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Hi Rafael,

Its better if we can get these in for 3.11, otherwise we need to get them in the
stable tree..

Anyway, we will get these in 3.10 stable tree but that requires us to identify
few more patches that will go with these. I will do that later.

This must fix the issues reported by Stephen.

Tested on my thinkpad over your linux-next branch.

 drivers/cpufreq/cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Viresh Kumar Sept. 1, 2013, 6:26 a.m. UTC | #1
On 1 September 2013 10:56, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Its better if we can get these in for 3.11, otherwise we need to get them in the
> stable tree..

They can't get into 3.11 due to two reasons..
- I haven't tested them over 3.11 but linux-next/master.
- Few more dependency patches might also be required for 3.11..

So get them into 3.12-rc1 if they look okay to you :)
Rafael J. Wysocki Sept. 1, 2013, 1:26 p.m. UTC | #2
On Sunday, September 01, 2013 10:56:01 AM Viresh Kumar wrote:
> __cpufreq_governor() returns with -EBUSY when governor is already stopped and we
> try to stop it again, but when it is stopped we must not allow calls to
> CPUFREQ_GOV_LIMITS event as well.
> 
> This patch adds this check in __cpufreq_governor().
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Hi Rafael,
> 
> Its better if we can get these in for 3.11, otherwise we need to get them in the
> stable tree..

There's no way they could go into 3.11 or even 3.12 without speding time
in linux-next.  I'll queue them up for the second part of the 3.12 merge
window, unless there is 3.11-rc8 (which I doubt will happen).

> Anyway, we will get these in 3.10 stable tree but that requires us to identify
> few more patches that will go with these. I will do that later.
> 
> This must fix the issues reported by Stephen.
> 
> Tested on my thinkpad over your linux-next branch.
> 
>  drivers/cpufreq/cpufreq.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 5c75e31..f320a20 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1692,8 +1692,9 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
>  						policy->cpu, event);
>  
>  	mutex_lock(&cpufreq_governor_lock);
> -	if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
> -	    (policy->governor_enabled && (event == CPUFREQ_GOV_START))) {
> +	if ((policy->governor_enabled && (event == CPUFREQ_GOV_START)) ||
> +		(!policy->governor_enabled && ((event == CPUFREQ_GOV_LIMITS) ||
> +					       (event == CPUFREQ_GOV_STOP)))) {

Broken white space, but never mind.

>  		mutex_unlock(&cpufreq_governor_lock);
>  		return -EBUSY;
>  	}

Thanks,
Rafael
Rafael J. Wysocki Sept. 1, 2013, 1:29 p.m. UTC | #3
On Sunday, September 01, 2013 11:56:09 AM Viresh Kumar wrote:
> On 1 September 2013 10:56, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > Its better if we can get these in for 3.11, otherwise we need to get them in the
> > stable tree..
> 
> They can't get into 3.11 due to two reasons..
> - I haven't tested them over 3.11 but linux-next/master.
> - Few more dependency patches might also be required for 3.11..
> 
> So get them into 3.12-rc1 if they look okay to you :)

I'm going to do that, but please add explanation to the [2/2] changelog.

Thanks!
diff mbox

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 5c75e31..f320a20 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1692,8 +1692,9 @@  static int __cpufreq_governor(struct cpufreq_policy *policy,
 						policy->cpu, event);
 
 	mutex_lock(&cpufreq_governor_lock);
-	if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
-	    (policy->governor_enabled && (event == CPUFREQ_GOV_START))) {
+	if ((policy->governor_enabled && (event == CPUFREQ_GOV_START)) ||
+		(!policy->governor_enabled && ((event == CPUFREQ_GOV_LIMITS) ||
+					       (event == CPUFREQ_GOV_STOP)))) {
 		mutex_unlock(&cpufreq_governor_lock);
 		return -EBUSY;
 	}