linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence
@ 2013-06-19  8:52 Viresh Kumar
  2013-06-19  8:52 ` [PATCH 01/13] cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases Viresh Kumar
                   ` (13 more replies)
  0 siblings, 14 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:52 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar

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 isn't obeyed by many of the drivers and core wasn't forcing it.

This patchset first fixes all the driver to follow it strictly and then adds
some protection against this. Now, we keep track of the last transaction and see
if something went wrong.

Last patch is intentionally kept towards the end, so that git bisect still works
well for all the faulty drivers.

This is pushed here and a pull request for 3.11 would be sent after few days.

git://git.linaro.org/people/vireshk/linux.git cpufreq-fix-notification

Viresh Kumar (13):
  cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: arm-big-little: call CPUFREQ_POSTCHANGE notfier in error
    cases
  cpufreq: davinci: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: dbx500: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: exynos: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: imx6q: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: omap: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: pcc: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: powernow-k8: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: s3c64xx: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: tegra: call CPUFREQ_POSTCHANGE notfier in error cases
  cpufreq: make sure frequency transitions are serialized

 drivers/cpufreq/acpi-cpufreq.c    |  6 ++++--
 drivers/cpufreq/arm_big_little.c  |  4 +---
 drivers/cpufreq/cpufreq.c         |  9 +++++++++
 drivers/cpufreq/davinci-cpufreq.c |  3 +++
 drivers/cpufreq/dbx500-cpufreq.c  |  4 ++--
 drivers/cpufreq/e_powersaver.c    |  3 +++
 drivers/cpufreq/exynos-cpufreq.c  | 10 ++++++++--
 drivers/cpufreq/imx6q-cpufreq.c   | 17 +++++++++++------
 drivers/cpufreq/omap-cpufreq.c    |  6 +++---
 drivers/cpufreq/pcc-cpufreq.c     |  2 ++
 drivers/cpufreq/powernow-k8.c     |  6 +++---
 drivers/cpufreq/s3c64xx-cpufreq.c |  8 ++++++--
 drivers/cpufreq/tegra-cpufreq.c   |  4 ++--
 13 files changed, 57 insertions(+), 25 deletions(-)

-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 01/13] cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
@ 2013-06-19  8:52 ` Viresh Kumar
  2013-06-19  8:52 ` [PATCH 02/13] cpufreq: arm-big-little: " Viresh Kumar
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:52 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Borislav Petkov

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 patch fixes it.

Cc: Borislav Petkov <bp@suse.de>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/acpi-cpufreq.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index edc089e..223ddf4 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -494,12 +494,14 @@ static int acpi_cpufreq_target(struct cpufreq_policy *policy,
 			pr_debug("acpi_cpufreq_target failed (%d)\n",
 				policy->cpu);
 			result = -EAGAIN;
-			goto out;
+			freqs.new = freqs.old;
 		}
 	}
 
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
-	perf->state = next_perf_state;
+
+	if (!result)
+		perf->state = next_perf_state;
 
 out:
 	return result;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 02/13] cpufreq: arm-big-little: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
  2013-06-19  8:52 ` [PATCH 01/13] cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases Viresh Kumar
@ 2013-06-19  8:52 ` Viresh Kumar
  2013-06-19  8:52 ` [PATCH 03/13] cpufreq: davinci: " Viresh Kumar
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:52 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar

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 patch fixes it.

This also removes code setting policy->cur as this is also done by POSTCHANGE
notifier.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/arm_big_little.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 5d7f53f..3549f07 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -84,11 +84,9 @@ static int bL_cpufreq_set_target(struct cpufreq_policy *policy,
 	ret = clk_set_rate(clk[cur_cluster], freqs.new * 1000);
 	if (ret) {
 		pr_err("clk_set_rate failed: %d\n", ret);
-		return ret;
+		freqs.new = freqs.old;
 	}
 
-	policy->cur = freqs.new;
-
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 
 	return ret;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 03/13] cpufreq: davinci: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
  2013-06-19  8:52 ` [PATCH 01/13] cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases Viresh Kumar
  2013-06-19  8:52 ` [PATCH 02/13] cpufreq: arm-big-little: " Viresh Kumar
@ 2013-06-19  8:52 ` Viresh Kumar
  2013-06-19  8:58   ` Sekhar Nori
  2013-06-19  8:52 ` [PATCH 04/13] cpufreq: dbx500: " Viresh Kumar
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:52 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Sekhar Nori

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.

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

This patch fixes it.

Cc: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/davinci-cpufreq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
index c33c76c..551dd65 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -114,6 +114,9 @@ static int davinci_target(struct cpufreq_policy *policy,
 		pdata->set_voltage(idx);
 
 out:
+	if (ret)
+		freqs.new = freqs.old;
+
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 
 	return ret;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 04/13] cpufreq: dbx500: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (2 preceding siblings ...)
  2013-06-19  8:52 ` [PATCH 03/13] cpufreq: davinci: " Viresh Kumar
@ 2013-06-19  8:52 ` Viresh Kumar
  2013-06-19 19:42   ` Linus Walleij
  2013-06-19  8:52 ` [PATCH 05/13] cpufreq: e_powersave: " Viresh Kumar
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:52 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Linus Walleij

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 patch fixes it.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/dbx500-cpufreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/dbx500-cpufreq.c b/drivers/cpufreq/dbx500-cpufreq.c
index 6ec6539..1fdb02b 100644
--- a/drivers/cpufreq/dbx500-cpufreq.c
+++ b/drivers/cpufreq/dbx500-cpufreq.c
@@ -57,13 +57,13 @@ static int dbx500_cpufreq_target(struct cpufreq_policy *policy,
 	if (ret) {
 		pr_err("dbx500-cpufreq: Failed to set armss_clk to %d Hz: error %d\n",
 		       freqs.new * 1000, ret);
-		return ret;
+		freqs.new = freqs.old;
 	}
 
 	/* post change notification */
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 
-	return 0;
+	return ret;
 }
 
 static unsigned int dbx500_cpufreq_getspeed(unsigned int cpu)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 05/13] cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (3 preceding siblings ...)
  2013-06-19  8:52 ` [PATCH 04/13] cpufreq: dbx500: " Viresh Kumar
@ 2013-06-19  8:52 ` Viresh Kumar
  2013-06-19 12:22   ` Simon Horman
  2013-06-19  8:53 ` [PATCH 06/13] cpufreq: exynos: " Viresh Kumar
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:52 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Simon Horman

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(+)

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


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 06/13] cpufreq: exynos: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (4 preceding siblings ...)
  2013-06-19  8:52 ` [PATCH 05/13] cpufreq: e_powersave: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-19  8:53 ` [PATCH 07/13] cpufreq: imx6q: " Viresh Kumar
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Kukjin Kim

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 patch fixes it.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/exynos-cpufreq.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index 475b4f6..0d32f02 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -113,7 +113,8 @@ static int exynos_cpufreq_scale(unsigned int target_freq)
 		if (ret) {
 			pr_err("%s: failed to set cpu voltage to %d\n",
 				__func__, arm_volt);
-			goto out;
+			freqs.new = freqs.old;
+			goto post_notify;
 		}
 	}
 
@@ -123,14 +124,19 @@ static int exynos_cpufreq_scale(unsigned int target_freq)
 		if (ret) {
 			pr_err("%s: failed to set cpu voltage to %d\n",
 				__func__, safe_arm_volt);
-			goto out;
+			freqs.new = freqs.old;
+			goto post_notify;
 		}
 	}
 
 	exynos_info->set_freq(old_index, index);
 
+post_notify:
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 
+	if (ret)
+		goto out;
+
 	/* When the new frequency is lower than current frequency */
 	if ((freqs.new < freqs.old) ||
 	   ((freqs.new > freqs.old) && safe_arm_volt)) {
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 07/13] cpufreq: imx6q: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (5 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 06/13] cpufreq: exynos: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-20  2:52   ` Shawn Guo
  2013-06-19  8:53 ` [PATCH 08/13] cpufreq: omap: " Viresh Kumar
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Shawn Guo

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 patch fixes it.

This also moves PRECHANGE notifier down so that we call it just before starting
frequency transition.

Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/imx6q-cpufreq.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index b78bc35..e37cdae 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -68,8 +68,6 @@ static int imx6q_set_target(struct cpufreq_policy *policy,
 	if (freqs.old == freqs.new)
 		return 0;
 
-	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
-
 	rcu_read_lock();
 	opp = opp_find_freq_ceil(cpu_dev, &freq_hz);
 	if (IS_ERR(opp)) {
@@ -86,13 +84,16 @@ static int imx6q_set_target(struct cpufreq_policy *policy,
 		freqs.old / 1000, volt_old / 1000,
 		freqs.new / 1000, volt / 1000);
 
+	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
+
 	/* scaling up?  scale voltage before frequency */
 	if (freqs.new > freqs.old) {
 		ret = regulator_set_voltage_tol(arm_reg, volt, 0);
 		if (ret) {
 			dev_err(cpu_dev,
 				"failed to scale vddarm up: %d\n", ret);
-			return ret;
+			freqs.new = freqs.old;
+			goto post_notify;
 		}
 
 		/*
@@ -145,15 +146,18 @@ static int imx6q_set_target(struct cpufreq_policy *policy,
 	if (ret) {
 		dev_err(cpu_dev, "failed to set clock rate: %d\n", ret);
 		regulator_set_voltage_tol(arm_reg, volt_old, 0);
-		return ret;
+		freqs.new = freqs.old;
+		goto post_notify;
 	}
 
 	/* scaling down?  scale voltage after frequency */
 	if (freqs.new < freqs.old) {
 		ret = regulator_set_voltage_tol(arm_reg, volt, 0);
-		if (ret)
+		if (ret) {
 			dev_warn(cpu_dev,
 				 "failed to scale vddarm down: %d\n", ret);
+			ret = 0;
+		}
 
 		if (freqs.old == FREQ_1P2_GHZ / 1000) {
 			regulator_set_voltage_tol(pu_reg,
@@ -163,9 +167,10 @@ static int imx6q_set_target(struct cpufreq_policy *policy,
 		}
 	}
 
+post_notify:
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 
-	return 0;
+	return ret;
 }
 
 static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 08/13] cpufreq: omap: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (6 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 07/13] cpufreq: imx6q: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-19 14:44   ` Santosh Shilimkar
  2013-06-19  8:53 ` [PATCH 09/13] cpufreq: pcc: " Viresh Kumar
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Santosh Shilimkar

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.

Omap driver was taking care of it well, but wasn't restoring freqs.new to
freqs.old in some cases. I wasn't required to add code for it as moving
PRECHANGE notifier down was a better option, so that we call it just before
starting frequency transition.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/omap-cpufreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 0279d18..29468a5 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -93,9 +93,6 @@ static int omap_target(struct cpufreq_policy *policy,
 	if (freqs.old == freqs.new && policy->cur == freqs.new)
 		return ret;
 
-	/* notifiers */
-	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
-
 	freq = freqs.new * 1000;
 	ret = clk_round_rate(mpu_clk, freq);
 	if (IS_ERR_VALUE(ret)) {
@@ -125,6 +122,9 @@ static int omap_target(struct cpufreq_policy *policy,
 		freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
 		freqs.new / 1000, volt ? volt / 1000 : -1);
 
+	/* notifiers */
+	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
+
 	/* scaling up?  scale voltage before frequency */
 	if (mpu_reg && (freqs.new > freqs.old)) {
 		r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 09/13] cpufreq: pcc: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (7 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 08/13] cpufreq: omap: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-19  8:53 ` [PATCH 10/13] cpufreq: powernow-k8: " Viresh Kumar
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Dave Jones

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 patch fixes it.

Cc: Dave Jones <davej@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/pcc-cpufreq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index 0de0008..1581fcc4 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -243,6 +243,8 @@ static int pcc_cpufreq_target(struct cpufreq_policy *policy,
 	return 0;
 
 cmd_incomplete:
+	freqs.new = freqs.old;
+	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 	iowrite16(0, &pcch_hdr->status);
 	spin_unlock(&pcc_lock);
 	return -EINVAL;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 10/13] cpufreq: powernow-k8: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (8 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 09/13] cpufreq: pcc: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-19  8:53 ` [PATCH 11/13] cpufreq: s3c64xx: " Viresh Kumar
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar

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 patch fixes it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/powernow-k8.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index b828efe..c916320 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -967,9 +967,9 @@ static int transition_frequency_fidvid(struct powernow_k8_data *data,
 
 	res = transition_fid_vid(data, fid, vid);
 	if (res)
-		return res;
-
-	freqs.new = find_khz_freq_from_fid(data->currfid);
+		freqs.new = freqs.old;
+	else
+		freqs.new = find_khz_freq_from_fid(data->currfid);
 
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 	return res;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 11/13] cpufreq: s3c64xx: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (9 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 10/13] cpufreq: powernow-k8: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-19  8:53 ` [PATCH 12/13] cpufreq: tegra: " Viresh Kumar
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Mark Brown

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 patch fixes it.

Cc: Mark Brown <broonie@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/s3c64xx-cpufreq.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/s3c64xx-cpufreq.c b/drivers/cpufreq/s3c64xx-cpufreq.c
index 27cacb5..017ade0 100644
--- a/drivers/cpufreq/s3c64xx-cpufreq.c
+++ b/drivers/cpufreq/s3c64xx-cpufreq.c
@@ -104,7 +104,8 @@ static int s3c64xx_cpufreq_set_target(struct cpufreq_policy *policy,
 		if (ret != 0) {
 			pr_err("Failed to set VDDARM for %dkHz: %d\n",
 			       freqs.new, ret);
-			goto err;
+			freqs.new = freqs.old;
+			goto post_notify;
 		}
 	}
 #endif
@@ -113,10 +114,13 @@ static int s3c64xx_cpufreq_set_target(struct cpufreq_policy *policy,
 	if (ret < 0) {
 		pr_err("Failed to set rate %dkHz: %d\n",
 		       freqs.new, ret);
-		goto err;
+		freqs.new = freqs.old;
 	}
 
+post_notify:
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
+	if (ret)
+		goto err;
 
 #ifdef CONFIG_REGULATOR
 	if (vddarm && freqs.new < freqs.old) {
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 12/13] cpufreq: tegra: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (10 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 11/13] cpufreq: s3c64xx: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-19 17:11   ` Stephen Warren
  2013-06-19  8:53 ` [PATCH 13/13] cpufreq: make sure frequency transitions are serialized Viresh Kumar
  2013-06-24 11:58 ` [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Rafael J. Wysocki
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar,
	Stephen Warren

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 patch fixes it.

Cc: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/tegra-cpufreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index c74c0e1..e74d777 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -138,12 +138,12 @@ static int tegra_update_cpu_speed(struct cpufreq_policy *policy,
 	if (ret) {
 		pr_err("cpu-tegra: Failed to set cpu frequency to %d kHz\n",
 			freqs.new);
-		return ret;
+		freqs.new = freqs.old;
 	}
 
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
 
-	return 0;
+	return ret;
 }
 
 static unsigned long tegra_cpu_highest_speed(void)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (11 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 12/13] cpufreq: tegra: " Viresh Kumar
@ 2013-06-19  8:53 ` Viresh Kumar
  2013-06-24 11:43   ` Rafael J. Wysocki
  2013-06-24 11:58 ` [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Rafael J. Wysocki
  13 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19  8:53 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin, Viresh Kumar

Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
shouldn't be called twice contiguously.

This can happen due to bugs in users of __cpufreq_driver_target() or actual
cpufreq drivers who are sending these notifiers.

This patch adds some protection against this. Now, we keep track of the last
transaction and see if something went wrong.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..92cb8b3 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
 static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
 static struct srcu_notifier_head cpufreq_transition_notifier_list;
 
+/* Tracks status of transition */
+static int transition_ongoing;
+
 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
 {
@@ -264,6 +267,8 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 	switch (state) {
 
 	case CPUFREQ_PRECHANGE:
+		WARN_ON(transition_ongoing++);
+
 		/* detect if the driver reported a value as "old frequency"
 		 * which is not equal to what the cpufreq core thinks is
 		 * "old frequency".
@@ -283,6 +288,8 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
+		WARN_ON(!transition_ongoing--);
+
 		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
 		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
 			(unsigned long)freqs->cpu);
@@ -1458,6 +1465,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
 
 	if (cpufreq_disabled())
 		return -ENODEV;
+	if (transition_ongoing)
+		return -EBUSY;
 
 	/* Make sure that target_freq is within supported range */
 	if (target_freq > policy->max)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 03/13] cpufreq: davinci: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 ` [PATCH 03/13] cpufreq: davinci: " Viresh Kumar
@ 2013-06-19  8:58   ` Sekhar Nori
  0 siblings, 0 replies; 32+ messages in thread
From: Sekhar Nori @ 2013-06-19  8:58 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On 6/19/2013 2:22 PM, 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.
> 
> Davinci driver was taking care of it but frequency isn't restored to freqs.old.
> 
> This patch fixes it.
> 
> Cc: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Looks good to me.

Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 05/13] cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 ` [PATCH 05/13] cpufreq: e_powersave: " Viresh Kumar
@ 2013-06-19 12:22   ` Simon Horman
  2013-06-19 14:54     ` Viresh Kumar
  0 siblings, 1 reply; 32+ messages in thread
From: Simon Horman @ 2013-06-19 12:22 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

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
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 08/13] cpufreq: omap: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:53 ` [PATCH 08/13] cpufreq: omap: " Viresh Kumar
@ 2013-06-19 14:44   ` Santosh Shilimkar
  0 siblings, 0 replies; 32+ messages in thread
From: Santosh Shilimkar @ 2013-06-19 14:44 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Wednesday 19 June 2013 04:53 AM, 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.
> 
> Omap driver was taking care of it well, but wasn't restoring freqs.new to
> freqs.old in some cases. I wasn't required to add code for it as moving
> PRECHANGE notifier down was a better option, so that we call it just before
> starting frequency transition.
> 
> Cca: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Looks safe to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 05/13] cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19 12:22   ` Simon Horman
@ 2013-06-19 14:54     ` Viresh Kumar
  2013-06-19 15:08       ` Dave Jones
  0 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-19 14:54 UTC (permalink / raw)
  To: Simon Horman, Dave Jones
  Cc: rjw, linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

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?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 05/13] cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19 14:54     ` Viresh Kumar
@ 2013-06-19 15:08       ` Dave Jones
  0 siblings, 0 replies; 32+ messages in thread
From: Dave Jones @ 2013-06-19 15:08 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Simon Horman, rjw, linaro-kernel, patches, cpufreq, linux-pm,
	linux-kernel, robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 12/13] cpufreq: tegra: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:53 ` [PATCH 12/13] cpufreq: tegra: " Viresh Kumar
@ 2013-06-19 17:11   ` Stephen Warren
  0 siblings, 0 replies; 32+ messages in thread
From: Stephen Warren @ 2013-06-19 17:11 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin,
	Stephen Warren

On 06/19/2013 02:53 AM, 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 patch fixes it.

Acked-by: Stephen Warren <swarren@nvidia.com>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 04/13] cpufreq: dbx500: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:52 ` [PATCH 04/13] cpufreq: dbx500: " Viresh Kumar
@ 2013-06-19 19:42   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2013-06-19 19:42 UTC (permalink / raw)
  To: Viresh Kumar, Rickard ANDERSSON
  Cc: Rafael J. Wysocki, linaro-kernel, Patch Tracking, cpufreq,
	linux-pm, linux-kernel, Robin Randhawa, Steve.Bannister,
	Liviu Dudau, Charles Garcia-Tobin, Arvind Chauhan, dave.martin

On Wed, Jun 19, 2013 at 10:52 AM, Viresh Kumar <viresh.kumar@linaro.org> 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 patch fixes it.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Looks good to me.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 07/13] cpufreq: imx6q: call CPUFREQ_POSTCHANGE notfier in error cases
  2013-06-19  8:53 ` [PATCH 07/13] cpufreq: imx6q: " Viresh Kumar
@ 2013-06-20  2:52   ` Shawn Guo
  0 siblings, 0 replies; 32+ messages in thread
From: Shawn Guo @ 2013-06-20  2:52 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rjw, linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Wed, Jun 19, 2013 at 02:23:01PM +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 patch fixes it.
> 
> This also moves PRECHANGE notifier down so that we call it just before starting
> frequency transition.
> 
> Cc: Shawn Guo <shawn.guo@linaro.org>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

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


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-19  8:53 ` [PATCH 13/13] cpufreq: make sure frequency transitions are serialized Viresh Kumar
@ 2013-06-24 11:43   ` Rafael J. Wysocki
  2013-06-24 13:08     ` Viresh Kumar
  0 siblings, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2013-06-24 11:43 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Wednesday, June 19, 2013 02:23:07 PM Viresh Kumar wrote:
> Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
> POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
> shouldn't be called twice contiguously.
> 
> This can happen due to bugs in users of __cpufreq_driver_target() or actual
> cpufreq drivers who are sending these notifiers.
> 
> This patch adds some protection against this. Now, we keep track of the last
> transaction and see if something went wrong.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 2d53f47..92cb8b3 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
>  static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
>  static struct srcu_notifier_head cpufreq_transition_notifier_list;
>  
> +/* Tracks status of transition */
> +static int transition_ongoing;
> +
>  static bool init_cpufreq_transition_notifier_list_called;
>  static int __init init_cpufreq_transition_notifier_list(void)
>  {
> @@ -264,6 +267,8 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
>  	switch (state) {
>  
>  	case CPUFREQ_PRECHANGE:
> +		WARN_ON(transition_ongoing++);
> +
>  		/* detect if the driver reported a value as "old frequency"
>  		 * which is not equal to what the cpufreq core thinks is
>  		 * "old frequency".
> @@ -283,6 +288,8 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
>  		break;
>  
>  	case CPUFREQ_POSTCHANGE:
> +		WARN_ON(!transition_ongoing--);

Shouldn't we try to avoid going into the negative range here?

> +
>  		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
>  		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
>  			(unsigned long)freqs->cpu);
> @@ -1458,6 +1465,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
>  
>  	if (cpufreq_disabled())
>  		return -ENODEV;
> +	if (transition_ongoing)
> +		return -EBUSY;
>  
>  	/* Make sure that target_freq is within supported range */
>  	if (target_freq > policy->max)
> 

Rafael


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence
  2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
                   ` (12 preceding siblings ...)
  2013-06-19  8:53 ` [PATCH 13/13] cpufreq: make sure frequency transitions are serialized Viresh Kumar
@ 2013-06-24 11:58 ` Rafael J. Wysocki
  13 siblings, 0 replies; 32+ messages in thread
From: Rafael J. Wysocki @ 2013-06-24 11:58 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Wednesday, June 19, 2013 02:22:54 PM 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 isn't obeyed by many of the drivers and core wasn't forcing it.
> 
> This patchset first fixes all the driver to follow it strictly and then adds
> some protection against this. Now, we keep track of the last transaction and see
> if something went wrong.
> 
> Last patch is intentionally kept towards the end, so that git bisect still works
> well for all the faulty drivers.
> 
> This is pushed here and a pull request for 3.11 would be sent after few days.
> 
> git://git.linaro.org/people/vireshk/linux.git cpufreq-fix-notification
> 
> Viresh Kumar (13):
>   cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: arm-big-little: call CPUFREQ_POSTCHANGE notfier in error
>     cases
>   cpufreq: davinci: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: dbx500: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: e_powersave: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: exynos: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: imx6q: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: omap: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: pcc: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: powernow-k8: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: s3c64xx: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: tegra: call CPUFREQ_POSTCHANGE notfier in error cases
>   cpufreq: make sure frequency transitions are serialized

Patches [1,5,9-10/13] applied to bleeding-edge, the ARM ones I'm expecting
to get from you and [13/13] can wait.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-24 11:43   ` Rafael J. Wysocki
@ 2013-06-24 13:08     ` Viresh Kumar
  2013-06-24 13:23       ` Rafael J. Wysocki
  0 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-24 13:08 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]

On 24 June 2013 17:13, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday, June 19, 2013 02:23:07 PM Viresh Kumar wrote:
>>       case CPUFREQ_POSTCHANGE:
>> +             WARN_ON(!transition_ongoing--);
>
> Shouldn't we try to avoid going into the negative range here?

What about this patch? Find it attached to apply.

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..6624694 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
 static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
 static struct srcu_notifier_head cpufreq_transition_notifier_list;

+/* Tracks status of transition */
+static int transition_ongoing;
+
 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
 {
@@ -264,6 +267,13 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
        switch (state) {

        case CPUFREQ_PRECHANGE:
+               if (transition_ongoing) {
+                       WARN(1, "In middle of another frequency transition\n");
+                       return;
+               }
+
+               transition_ongoing++;
+
                /* detect if the driver reported a value as "old frequency"
                 * which is not equal to what the cpufreq core thinks is
                 * "old frequency".
@@ -283,6 +293,13 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
                break;

        case CPUFREQ_POSTCHANGE:
+               if (!transition_ongoing) {
+                       WARN(1, "No frequency transition in progress\n");
+                       return;
+               }
+
+               transition_ongoing--;
+
                adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
                pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
                        (unsigned long)freqs->cpu);
@@ -1458,6 +1475,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,

        if (cpufreq_disabled())
                return -ENODEV;
+       if (transition_ongoing)
+               return -EBUSY;

        /* Make sure that target_freq is within supported range */
        if (target_freq > policy->max)

[-- Attachment #2: 0001-cpufreq-make-sure-frequency-transitions-are-serializ.patch --]
[-- Type: application/octet-stream, Size: 2571 bytes --]

From 25fb6e6164b107f1674ec32f0d0e47328c2c11a6 Mon Sep 17 00:00:00 2001
Message-Id: <25fb6e6164b107f1674ec32f0d0e47328c2c11a6.1372079234.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 19 Jun 2013 10:16:55 +0530
Subject: [PATCH] cpufreq: make sure frequency transitions are serialized

Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
shouldn't be called twice contiguously.

This can happen due to bugs in users of __cpufreq_driver_target() or actual
cpufreq drivers who are sending these notifiers.

This patch adds some protection against this. Now, we keep track of the last
transaction and see if something went wrong.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..6624694 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
 static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
 static struct srcu_notifier_head cpufreq_transition_notifier_list;
 
+/* Tracks status of transition */
+static int transition_ongoing;
+
 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
 {
@@ -264,6 +267,13 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 	switch (state) {
 
 	case CPUFREQ_PRECHANGE:
+		if (transition_ongoing) {
+			WARN(1, "In middle of another frequency transition\n");
+			return;
+		}
+
+		transition_ongoing++;
+
 		/* detect if the driver reported a value as "old frequency"
 		 * which is not equal to what the cpufreq core thinks is
 		 * "old frequency".
@@ -283,6 +293,13 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
+		if (!transition_ongoing) {
+			WARN(1, "No frequency transition in progress\n");
+			return;
+		}
+
+		transition_ongoing--;
+
 		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
 		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
 			(unsigned long)freqs->cpu);
@@ -1458,6 +1475,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
 
 	if (cpufreq_disabled())
 		return -ENODEV;
+	if (transition_ongoing)
+		return -EBUSY;
 
 	/* Make sure that target_freq is within supported range */
 	if (target_freq > policy->max)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-24 13:23       ` Rafael J. Wysocki
@ 2013-06-24 13:16         ` Viresh Kumar
  2013-06-24 13:33           ` Rafael J. Wysocki
  0 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-24 13:16 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

[-- Attachment #1: Type: text/plain, Size: 1354 bytes --]

On 24 June 2013 18:53, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> You can do
>
>         if (WARN(transition_ongoing, "<text>"))
>                 return;
>
> and below analogously.

Ahh.. stupid code.. Check if this fixup is fine (attached too)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6624694..6ca7eac 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -267,10 +267,9 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
        switch (state) {

        case CPUFREQ_PRECHANGE:
-               if (transition_ongoing) {
-                       WARN(1, "In middle of another frequency transition\n");
+               if (WARN(transition_ongoing,
+                               "In middle of another frequency transition\n"))
                        return;
-               }

                transition_ongoing++;

@@ -293,10 +292,9 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
                break;

        case CPUFREQ_POSTCHANGE:
-               if (!transition_ongoing) {
-                       WARN(1, "No frequency transition in progress\n");
+               if (WARN(!transition_ongoing,
+                               "No frequency transition in progress\n"))
                        return;
-               }

                transition_ongoing--;

[-- Attachment #2: 0001-cpufreq-make-sure-frequency-transitions-are-serializ.patch --]
[-- Type: application/octet-stream, Size: 2551 bytes --]

From 01a85c5e860f4b52a09418559fd27f480357272f Mon Sep 17 00:00:00 2001
Message-Id: <01a85c5e860f4b52a09418559fd27f480357272f.1372079781.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 19 Jun 2013 10:16:55 +0530
Subject: [PATCH] cpufreq: make sure frequency transitions are serialized

Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
shouldn't be called twice contiguously.

This can happen due to bugs in users of __cpufreq_driver_target() or actual
cpufreq drivers who are sending these notifiers.

This patch adds some protection against this. Now, we keep track of the last
transaction and see if something went wrong.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..6ca7eac 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
 static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
 static struct srcu_notifier_head cpufreq_transition_notifier_list;
 
+/* Tracks status of transition */
+static int transition_ongoing;
+
 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
 {
@@ -264,6 +267,12 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 	switch (state) {
 
 	case CPUFREQ_PRECHANGE:
+		if (WARN(transition_ongoing,
+				"In middle of another frequency transition\n"))
+			return;
+
+		transition_ongoing++;
+
 		/* detect if the driver reported a value as "old frequency"
 		 * which is not equal to what the cpufreq core thinks is
 		 * "old frequency".
@@ -283,6 +292,12 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
+		if (WARN(!transition_ongoing,
+				"No frequency transition in progress\n"))
+			return;
+
+		transition_ongoing--;
+
 		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
 		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
 			(unsigned long)freqs->cpu);
@@ -1458,6 +1473,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
 
 	if (cpufreq_disabled())
 		return -ENODEV;
+	if (transition_ongoing)
+		return -EBUSY;
 
 	/* Make sure that target_freq is within supported range */
 	if (target_freq > policy->max)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-24 13:08     ` Viresh Kumar
@ 2013-06-24 13:23       ` Rafael J. Wysocki
  2013-06-24 13:16         ` Viresh Kumar
  0 siblings, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2013-06-24 13:23 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Monday, June 24, 2013 06:38:17 PM Viresh Kumar wrote:
> On 24 June 2013 17:13, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Wednesday, June 19, 2013 02:23:07 PM Viresh Kumar wrote:
> >>       case CPUFREQ_POSTCHANGE:
> >> +             WARN_ON(!transition_ongoing--);
> >
> > Shouldn't we try to avoid going into the negative range here?
> 
> What about this patch? Find it attached to apply.
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 2d53f47..6624694 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
>  static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
>  static struct srcu_notifier_head cpufreq_transition_notifier_list;
> 
> +/* Tracks status of transition */
> +static int transition_ongoing;
> +
>  static bool init_cpufreq_transition_notifier_list_called;
>  static int __init init_cpufreq_transition_notifier_list(void)
>  {
> @@ -264,6 +267,13 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>         switch (state) {
> 
>         case CPUFREQ_PRECHANGE:
> +               if (transition_ongoing) {
> +                       WARN(1, "In middle of another frequency transition\n");
> +                       return;
> +               }

You can do

	if (WARN(transition_ongoing, "<text>"))
		return;

and below analogously.

> +
> +               transition_ongoing++;
> +
>                 /* detect if the driver reported a value as "old frequency"
>                  * which is not equal to what the cpufreq core thinks is
>                  * "old frequency".
> @@ -283,6 +293,13 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>                 break;
> 
>         case CPUFREQ_POSTCHANGE:
> +               if (!transition_ongoing) {
> +                       WARN(1, "No frequency transition in progress\n");
> +                       return;
> +               }
> +
> +               transition_ongoing--;
> +
>                 adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
>                 pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
>                         (unsigned long)freqs->cpu);
> @@ -1458,6 +1475,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
> 
>         if (cpufreq_disabled())
>                 return -ENODEV;
> +       if (transition_ongoing)
> +               return -EBUSY;
> 
>         /* Make sure that target_freq is within supported range */
>         if (target_freq > policy->max)

Thanks,
Rafael


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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-24 13:33           ` Rafael J. Wysocki
@ 2013-06-24 13:31             ` Viresh Kumar
  2013-06-26 21:57               ` Rafael J. Wysocki
  0 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-24 13:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

On 24 June 2013 19:03, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Looks OK, but since transition_ongoing is either 0 or 1 now, as far as I can
> say, it would be better to make it a bool and use = true/false instead of
> ++/-- I suppose.

Another fixup:

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6ca7eac..49d942a 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -108,7 +108,7 @@ static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
 static struct srcu_notifier_head cpufreq_transition_notifier_list;

 /* Tracks status of transition */
-static int transition_ongoing;
+static bool transition_ongoing;

 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
@@ -271,7 +271,7 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
                                "In middle of another frequency transition\n"))
                        return;

-               transition_ongoing++;
+               transition_ongoing = true;

                /* detect if the driver reported a value as "old frequency"
                 * which is not equal to what the cpufreq core thinks is
@@ -296,7 +296,7 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
                                "No frequency transition in progress\n"))
                        return;

-               transition_ongoing--;
+               transition_ongoing = false;

                adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
                pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,

[-- Attachment #2: 0001-cpufreq-make-sure-frequency-transitions-are-serializ.patch --]
[-- Type: application/octet-stream, Size: 2563 bytes --]

From b8c3bd9fcb521f1b66d52f157fc26eccf6cc4f29 Mon Sep 17 00:00:00 2001
Message-Id: <b8c3bd9fcb521f1b66d52f157fc26eccf6cc4f29.1372080656.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 19 Jun 2013 10:16:55 +0530
Subject: [PATCH] cpufreq: make sure frequency transitions are serialized

Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
shouldn't be called twice contiguously.

This can happen due to bugs in users of __cpufreq_driver_target() or actual
cpufreq drivers who are sending these notifiers.

This patch adds some protection against this. Now, we keep track of the last
transaction and see if something went wrong.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..49d942a 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -107,6 +107,9 @@ static void handle_update(struct work_struct *work);
 static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
 static struct srcu_notifier_head cpufreq_transition_notifier_list;
 
+/* Tracks status of transition */
+static bool transition_ongoing;
+
 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
 {
@@ -264,6 +267,12 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 	switch (state) {
 
 	case CPUFREQ_PRECHANGE:
+		if (WARN(transition_ongoing,
+				"In middle of another frequency transition\n"))
+			return;
+
+		transition_ongoing = true;
+
 		/* detect if the driver reported a value as "old frequency"
 		 * which is not equal to what the cpufreq core thinks is
 		 * "old frequency".
@@ -283,6 +292,12 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
+		if (WARN(!transition_ongoing,
+				"No frequency transition in progress\n"))
+			return;
+
+		transition_ongoing = false;
+
 		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
 		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
 			(unsigned long)freqs->cpu);
@@ -1458,6 +1473,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
 
 	if (cpufreq_disabled())
 		return -ENODEV;
+	if (transition_ongoing)
+		return -EBUSY;
 
 	/* Make sure that target_freq is within supported range */
 	if (target_freq > policy->max)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-24 13:16         ` Viresh Kumar
@ 2013-06-24 13:33           ` Rafael J. Wysocki
  2013-06-24 13:31             ` Viresh Kumar
  0 siblings, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2013-06-24 13:33 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Monday, June 24, 2013 06:46:39 PM Viresh Kumar wrote:
> On 24 June 2013 18:53, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > You can do
> >
> >         if (WARN(transition_ongoing, "<text>"))
> >                 return;
> >
> > and below analogously.
> 
> Ahh.. stupid code.. Check if this fixup is fine (attached too)

Looks OK, but since transition_ongoing is either 0 or 1 now, as far as I can
say, it would be better to make it a bool and use = true/false instead of
++/-- I suppose.

Thanks,
Rafael


> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6624694..6ca7eac 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -267,10 +267,9 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>         switch (state) {
> 
>         case CPUFREQ_PRECHANGE:
> -               if (transition_ongoing) {
> -                       WARN(1, "In middle of another frequency transition\n");
> +               if (WARN(transition_ongoing,
> +                               "In middle of another frequency transition\n"))
>                         return;
> -               }
> 
>                 transition_ongoing++;
> 
> @@ -293,10 +292,9 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>                 break;
> 
>         case CPUFREQ_POSTCHANGE:
> -               if (!transition_ongoing) {
> -                       WARN(1, "No frequency transition in progress\n");
> +               if (WARN(!transition_ongoing,
> +                               "No frequency transition in progress\n"))
>                         return;
> -               }
> 
>                 transition_ongoing--;
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-24 13:31             ` Viresh Kumar
@ 2013-06-26 21:57               ` Rafael J. Wysocki
  2013-06-27  4:56                 ` Viresh Kumar
  0 siblings, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2013-06-26 21:57 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Monday, June 24, 2013 07:01:59 PM Viresh Kumar wrote:
> On 24 June 2013 19:03, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Looks OK, but since transition_ongoing is either 0 or 1 now, as far as I can
> > say, it would be better to make it a bool and use = true/false instead of
> > ++/-- I suppose.
> 
> Another fixup:
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6ca7eac..49d942a 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -108,7 +108,7 @@ static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
>  static struct srcu_notifier_head cpufreq_transition_notifier_list;
> 
>  /* Tracks status of transition */
> -static int transition_ongoing;
> +static bool transition_ongoing;
> 
>  static bool init_cpufreq_transition_notifier_list_called;
>  static int __init init_cpufreq_transition_notifier_list(void)
> @@ -271,7 +271,7 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>                                 "In middle of another frequency transition\n"))
>                         return;
> 
> -               transition_ongoing++;
> +               transition_ongoing = true;
> 
>                 /* detect if the driver reported a value as "old frequency"
>                  * which is not equal to what the cpufreq core thinks is
> @@ -296,7 +296,7 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>                                 "No frequency transition in progress\n"))
>                         return;
> 
> -               transition_ongoing--;
> +               transition_ongoing = false;
> 
>                 adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
>                 pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,

Well, now, seeing that the locking around this seems to be kind of haphazard,
I'm wondering what prevents two different threads from doing CPUFREQ_PRECHANGE
concurrently in such a way that thread A will check transition_ongoing
and thread B will check transition_ongoing and then both will set it if it
was 'false' before.  And then one of them will trigger the WARN() in
CPUFREQ_POSTCHANGE.

Is there any protection in place and if so then how does it work?

Rafael


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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-26 21:57               ` Rafael J. Wysocki
@ 2013-06-27  4:56                 ` Viresh Kumar
  2013-06-27 12:15                   ` Rafael J. Wysocki
  0 siblings, 1 reply; 32+ messages in thread
From: Viresh Kumar @ 2013-06-27  4:56 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

[-- Attachment #1: Type: text/plain, Size: 3701 bytes --]

On 27 June 2013 03:27, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Well, now, seeing that the locking around this seems to be kind of haphazard,
> I'm wondering what prevents two different threads from doing CPUFREQ_PRECHANGE
> concurrently in such a way that thread A will check transition_ongoing
> and thread B will check transition_ongoing and then both will set it if it
> was 'false' before.  And then one of them will trigger the WARN() in
> CPUFREQ_POSTCHANGE.
>
> Is there any protection in place and if so then how does it work?

cpufreq_notify_transition() is called from driver->target() which is
called from __cpufreq_driver_target(). __cpufreq_driver_target()
is called directly by governors and cpufreq_driver_target() otherwise.

cpufreq_driver_target() implements proper locking and so it is fine.
__cpufreq_driver_target() is called from governors. From governors
it is is serialized in the sense two threads wouldn't call it at the same
time.

And so I thought this will work. But I just found a mistake in my code.
For multi-socket platforms with clock domains for sockets/clusters,
a single instance of transition_ongoing isn't enough and so this must
be embedded in struct cpufreq_policy.

Below patch must get this fixed (Attached).

-------------x---------------------x-----------------

From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 19 Jun 2013 10:16:55 +0530
Subject: [PATCH] cpufreq: make sure frequency transitions are serialized

Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
shouldn't be called twice contiguously.

This can happen due to bugs in users of __cpufreq_driver_target() or actual
cpufreq drivers who are sending these notifiers.

This patch adds some protection against this. Now, we keep track of the last
transaction and see if something went wrong.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 14 ++++++++++++++
 include/linux/cpufreq.h   |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..75715f1 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -264,6 +264,12 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
 	switch (state) {

 	case CPUFREQ_PRECHANGE:
+		if (WARN(policy->transition_ongoing,
+				"In middle of another frequency transition\n"))
+			return;
+
+		policy->transition_ongoing = true;
+
 		/* detect if the driver reported a value as "old frequency"
 		 * which is not equal to what the cpufreq core thinks is
 		 * "old frequency".
@@ -283,6 +289,12 @@ void __cpufreq_notify_transition(struct
cpufreq_policy *policy,
 		break;

 	case CPUFREQ_POSTCHANGE:
+		if (WARN(!policy->transition_ongoing,
+				"No frequency transition in progress\n"))
+			return;
+
+		policy->transition_ongoing = false;
+
 		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
 		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
 			(unsigned long)freqs->cpu);
@@ -1458,6 +1470,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,

 	if (cpufreq_disabled())
 		return -ENODEV;
+	if (policy->transition_ongoing)
+		return -EBUSY;

 	/* Make sure that target_freq is within supported range */
 	if (target_freq > policy->max)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 037d36a..8c13a45 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -115,6 +115,7 @@ struct cpufreq_policy {

 	struct kobject		kobj;
 	struct completion	kobj_unregister;
+	bool			transition_ongoing; /* Tracks transition status */
 };

 #define CPUFREQ_ADJUST			(0)

[-- Attachment #2: 0001-cpufreq-make-sure-frequency-transitions-are-serializ.patch --]
[-- Type: application/octet-stream, Size: 2591 bytes --]

From f0573570c85b0fd14db1130ee4885a50ed7bdb94 Mon Sep 17 00:00:00 2001
Message-Id: <f0573570c85b0fd14db1130ee4885a50ed7bdb94.1372308892.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 19 Jun 2013 10:16:55 +0530
Subject: [PATCH] cpufreq: make sure frequency transitions are serialized

Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
shouldn't be called twice contiguously.

This can happen due to bugs in users of __cpufreq_driver_target() or actual
cpufreq drivers who are sending these notifiers.

This patch adds some protection against this. Now, we keep track of the last
transaction and see if something went wrong.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 14 ++++++++++++++
 include/linux/cpufreq.h   |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2d53f47..75715f1 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -264,6 +264,12 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 	switch (state) {
 
 	case CPUFREQ_PRECHANGE:
+		if (WARN(policy->transition_ongoing,
+				"In middle of another frequency transition\n"))
+			return;
+
+		policy->transition_ongoing = true;
+
 		/* detect if the driver reported a value as "old frequency"
 		 * which is not equal to what the cpufreq core thinks is
 		 * "old frequency".
@@ -283,6 +289,12 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
+		if (WARN(!policy->transition_ongoing,
+				"No frequency transition in progress\n"))
+			return;
+
+		policy->transition_ongoing = false;
+
 		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
 		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
 			(unsigned long)freqs->cpu);
@@ -1458,6 +1470,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
 
 	if (cpufreq_disabled())
 		return -ENODEV;
+	if (policy->transition_ongoing)
+		return -EBUSY;
 
 	/* Make sure that target_freq is within supported range */
 	if (target_freq > policy->max)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 037d36a..8c13a45 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -115,6 +115,7 @@ struct cpufreq_policy {
 
 	struct kobject		kobj;
 	struct completion	kobj_unregister;
+	bool			transition_ongoing; /* Tracks transition status */
 };
 
 #define CPUFREQ_ADJUST			(0)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 13/13] cpufreq: make sure frequency transitions are serialized
  2013-06-27  4:56                 ` Viresh Kumar
@ 2013-06-27 12:15                   ` Rafael J. Wysocki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafael J. Wysocki @ 2013-06-27 12:15 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, arvind.chauhan, dave.martin

On Thursday, June 27, 2013 10:26:27 AM Viresh Kumar wrote:
> On 27 June 2013 03:27, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Well, now, seeing that the locking around this seems to be kind of haphazard,
> > I'm wondering what prevents two different threads from doing CPUFREQ_PRECHANGE
> > concurrently in such a way that thread A will check transition_ongoing
> > and thread B will check transition_ongoing and then both will set it if it
> > was 'false' before.  And then one of them will trigger the WARN() in
> > CPUFREQ_POSTCHANGE.
> >
> > Is there any protection in place and if so then how does it work?
> 
> cpufreq_notify_transition() is called from driver->target() which is
> called from __cpufreq_driver_target(). __cpufreq_driver_target()
> is called directly by governors and cpufreq_driver_target() otherwise.
> 
> cpufreq_driver_target() implements proper locking and so it is fine.
> __cpufreq_driver_target() is called from governors. From governors
> it is is serialized in the sense two threads wouldn't call it at the same
> time.
> 
> And so I thought this will work. But I just found a mistake in my code.
> For multi-socket platforms with clock domains for sockets/clusters,
> a single instance of transition_ongoing isn't enough and so this must
> be embedded in struct cpufreq_policy.
> 
> Below patch must get this fixed (Attached).
> 
> -------------x---------------------x-----------------
> 
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Date: Wed, 19 Jun 2013 10:16:55 +0530
> Subject: [PATCH] cpufreq: make sure frequency transitions are serialized
> 
> Whenever we are changing frequency of a cpu, we are calling PRECHANGE and
> POSTCHANGE notifiers. They must be serialized. i.e. PRECHANGE or POSTCHANGE
> shouldn't be called twice contiguously.
> 
> This can happen due to bugs in users of __cpufreq_driver_target() or actual
> cpufreq drivers who are sending these notifiers.
> 
> This patch adds some protection against this. Now, we keep track of the last
> transaction and see if something went wrong.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

OK, queued up for 3.11.

Thanks,
Rafael


> ---
>  drivers/cpufreq/cpufreq.c | 14 ++++++++++++++
>  include/linux/cpufreq.h   |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 2d53f47..75715f1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -264,6 +264,12 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>  	switch (state) {
> 
>  	case CPUFREQ_PRECHANGE:
> +		if (WARN(policy->transition_ongoing,
> +				"In middle of another frequency transition\n"))
> +			return;
> +
> +		policy->transition_ongoing = true;
> +
>  		/* detect if the driver reported a value as "old frequency"
>  		 * which is not equal to what the cpufreq core thinks is
>  		 * "old frequency".
> @@ -283,6 +289,12 @@ void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>  		break;
> 
>  	case CPUFREQ_POSTCHANGE:
> +		if (WARN(!policy->transition_ongoing,
> +				"No frequency transition in progress\n"))
> +			return;
> +
> +		policy->transition_ongoing = false;
> +
>  		adjust_jiffies(CPUFREQ_POSTCHANGE, freqs);
>  		pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new,
>  			(unsigned long)freqs->cpu);
> @@ -1458,6 +1470,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
> 
>  	if (cpufreq_disabled())
>  		return -ENODEV;
> +	if (policy->transition_ongoing)
> +		return -EBUSY;
> 
>  	/* Make sure that target_freq is within supported range */
>  	if (target_freq > policy->max)
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 037d36a..8c13a45 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -115,6 +115,7 @@ struct cpufreq_policy {
> 
>  	struct kobject		kobj;
>  	struct completion	kobj_unregister;
> +	bool			transition_ongoing; /* Tracks transition status */
>  };
> 
>  #define CPUFREQ_ADJUST			(0)
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2013-06-27 12:06 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-19  8:52 [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Viresh Kumar
2013-06-19  8:52 ` [PATCH 01/13] cpufreq: acpi: call CPUFREQ_POSTCHANGE notfier in error cases Viresh Kumar
2013-06-19  8:52 ` [PATCH 02/13] cpufreq: arm-big-little: " Viresh Kumar
2013-06-19  8:52 ` [PATCH 03/13] cpufreq: davinci: " Viresh Kumar
2013-06-19  8:58   ` Sekhar Nori
2013-06-19  8:52 ` [PATCH 04/13] cpufreq: dbx500: " Viresh Kumar
2013-06-19 19:42   ` Linus Walleij
2013-06-19  8:52 ` [PATCH 05/13] cpufreq: e_powersave: " Viresh Kumar
2013-06-19 12:22   ` Simon Horman
2013-06-19 14:54     ` Viresh Kumar
2013-06-19 15:08       ` Dave Jones
2013-06-19  8:53 ` [PATCH 06/13] cpufreq: exynos: " Viresh Kumar
2013-06-19  8:53 ` [PATCH 07/13] cpufreq: imx6q: " Viresh Kumar
2013-06-20  2:52   ` Shawn Guo
2013-06-19  8:53 ` [PATCH 08/13] cpufreq: omap: " Viresh Kumar
2013-06-19 14:44   ` Santosh Shilimkar
2013-06-19  8:53 ` [PATCH 09/13] cpufreq: pcc: " Viresh Kumar
2013-06-19  8:53 ` [PATCH 10/13] cpufreq: powernow-k8: " Viresh Kumar
2013-06-19  8:53 ` [PATCH 11/13] cpufreq: s3c64xx: " Viresh Kumar
2013-06-19  8:53 ` [PATCH 12/13] cpufreq: tegra: " Viresh Kumar
2013-06-19 17:11   ` Stephen Warren
2013-06-19  8:53 ` [PATCH 13/13] cpufreq: make sure frequency transitions are serialized Viresh Kumar
2013-06-24 11:43   ` Rafael J. Wysocki
2013-06-24 13:08     ` Viresh Kumar
2013-06-24 13:23       ` Rafael J. Wysocki
2013-06-24 13:16         ` Viresh Kumar
2013-06-24 13:33           ` Rafael J. Wysocki
2013-06-24 13:31             ` Viresh Kumar
2013-06-26 21:57               ` Rafael J. Wysocki
2013-06-27  4:56                 ` Viresh Kumar
2013-06-27 12:15                   ` Rafael J. Wysocki
2013-06-24 11:58 ` [PATCH 00/13] CPUFreq: Fix {PRE|POST}CHANGE notification sequence Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).