[05/13] cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases

Message ID 859db6d3cfc0efbd5b96783967c10cfcb8b5a51a.1371630975.git.viresh.kumar@linaro.org
State Accepted
Headers show

Commit Message

Viresh Kumar June 19, 2013, 8:52 a.m.
PRECHANGE and POSTCHANGE notifiers must be called in groups, i.e either both
should be called or both shouldn't be.

In case we have started PRECHANGE notifier and found an error, we must call
POSTCHANGE notifier with freqs.new = freqs.old to guarantee that sequence of
calling notifiers is complete.

This driver was taking care of it but frequency isn't restored to freqs.old.

This patch fixes it.

Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/e_powersaver.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Simon Horman June 19, 2013, 12:22 p.m. | #1
On Wed, Jun 19, 2013 at 02:22:59PM +0530, Viresh Kumar wrote:
> PRECHANGE and POSTCHANGE notifiers must be called in groups, i.e either both
> should be called or both shouldn't be.
> 
> In case we have started PRECHANGE notifier and found an error, we must call
> POSTCHANGE notifier with freqs.new = freqs.old to guarantee that sequence of
> calling notifiers is complete.
> 
> This driver was taking care of it but frequency isn't restored to freqs.old.
> 
> This patch fixes it.
> 
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

I have no objections to this change but at the same time I don't
feel that I know the code well enough to review it.

> ---
>  drivers/cpufreq/e_powersaver.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c
> index 37380fb..3d03626 100644
> --- a/drivers/cpufreq/e_powersaver.c
> +++ b/drivers/cpufreq/e_powersaver.c
> @@ -161,6 +161,9 @@ postchange:
>  		current_multiplier);
>  	}
>  #endif
> +	if (err)
> +		freqs.new = freqs.old;
> +
>  	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
>  	return err;
>  }
> -- 
> 1.7.12.rc2.18.g61b472e
>
Viresh Kumar June 19, 2013, 2:54 p.m. | #2
On 19 June 2013 17:52, Simon Horman <horms@verge.net.au> wrote:
> I have no objections to this change but at the same time I don't
> feel that I know the code well enough to review it.

Probably I made a mistake adding your name. Don't know how I
made up my mind that you are the main contributor for this.

Probably Dave is?
Dave Jones June 19, 2013, 3:08 p.m. | #3
On Wed, Jun 19, 2013 at 08:24:41PM +0530, Viresh Kumar wrote:
 > On 19 June 2013 17:52, Simon Horman <horms@verge.net.au> wrote:
 > > I have no objections to this change but at the same time I don't
 > > feel that I know the code well enough to review it.
 > 
 > Probably I made a mistake adding your name. Don't know how I
 > made up my mind that you are the main contributor for this.
 > 
 > Probably Dave is?

e_powersaver ? Rafael.

	Dave

Patch

diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c
index 37380fb..3d03626 100644
--- a/drivers/cpufreq/e_powersaver.c
+++ b/drivers/cpufreq/e_powersaver.c
@@ -161,6 +161,9 @@  postchange:
 		current_multiplier);
 	}
 #endif
+	if (err)
+		freqs.new = freqs.old;
+
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 	return err;
 }