Re: [PATCH 2/2] cpufreq: governor: CPUFREQ_GOV_STOP never fails

[Viresh Kumar]

On 12-05-16, 15:14, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> None of the cpufreq governors currently in the tree will ever fail
> an invocation of the ->governor() callback with the event argument
> equal to CPUFREQ_GOV_STOP (unless invoked with incorrect arguments
> which doesn't matter anyway) and it is rather difficult to imagine
> a valid reason for such a failure.
> 
> Accordingly, rearrange the code in the core to make it clear that
> this call never fails.
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/cpufreq/cpufreq.c |   40 +++-
>  1 file changed, 11 insertions(+), 29 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH 2/2] cpufreq: governor: CPUFREQ_GOV_STOP never fails

[Viresh Kumar]

On 12-05-16, 15:14, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> None of the cpufreq governors currently in the tree will ever fail
> an invocation of the ->governor() callback with the event argument
> equal to CPUFREQ_GOV_STOP (unless invoked with incorrect arguments
> which doesn't matter anyway) and it is rather difficult to imagine
> a valid reason for such a failure.
> 
> Accordingly, rearrange the code in the core to make it clear that
> this call never fails.
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/cpufreq/cpufreq.c |   40 +++-
>  1 file changed, 11 insertions(+), 29 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

[PATCH 2/2] cpufreq: governor: CPUFREQ_GOV_STOP never fails

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

None of the cpufreq governors currently in the tree will ever fail
an invocation of the ->governor() callback with the event argument
equal to CPUFREQ_GOV_STOP (unless invoked with incorrect arguments
which doesn't matter anyway) and it is rather difficult to imagine
a valid reason for such a failure.

Accordingly, rearrange the code in the core to make it clear that
this call never fails.

Signed-off-by: Rafael J. Wysocki 
---
 drivers/cpufreq/cpufreq.c |   40 +++-
 1 file changed, 11 insertions(+), 29 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -83,6 +83,11 @@ static inline void cpufreq_exit_governor
(void)cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
 }
 
+static inline void cpufreq_stop_governor(struct cpufreq_policy *policy)
+{
+   (void)cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+}
+
 /**
  * Two notifier lists: the "policy" list is involved in the
  * validation process for a new CPU frequency policy; the
@@ -1026,13 +1031,8 @@ static int cpufreq_add_policy_cpu(struct
return 0;
 
down_write(>rwsem);
-   if (has_target()) {
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-   if (ret) {
-   pr_err("%s: Failed to stop governor\n", __func__);
-   goto unlock;
-   }
-   }
+   if (has_target())
+   cpufreq_stop_governor(policy);
 
cpumask_set_cpu(cpu, policy->cpus);
 
@@ -1041,8 +1041,6 @@ static int cpufreq_add_policy_cpu(struct
if (ret)
pr_err("%s: Failed to start governor\n", __func__);
}
-
-unlock:
up_write(>rwsem);
return ret;
 }
@@ -1354,11 +1352,8 @@ static void cpufreq_offline(unsigned int
}
 
down_write(>rwsem);
-   if (has_target()) {
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-   if (ret)
-   pr_err("%s: Failed to stop governor\n", __func__);
-   }
+   if (has_target())
+   cpufreq_stop_governor(policy);
 
cpumask_clear_cpu(cpu, policy->cpus);
 
@@ -1622,7 +1617,6 @@ EXPORT_SYMBOL(cpufreq_generic_suspend);
 void cpufreq_suspend(void)
 {
struct cpufreq_policy *policy;
-   int ret;
 
if (!cpufreq_driver)
return;
@@ -1635,14 +1629,8 @@ void cpufreq_suspend(void)
for_each_active_policy(policy) {
if (has_target()) {
down_write(>rwsem);
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+   cpufreq_stop_governor(policy);
up_write(>rwsem);
-
-   if (ret) {
-   pr_err("%s: Failed to stop governor for policy: 
%p\n",
-   __func__, policy);
-   continue;
-   }
}
 
if (cpufreq_driver->suspend && cpufreq_driver->suspend(policy))
@@ -2216,13 +2204,7 @@ static int cpufreq_set_policy(struct cpu
old_gov = policy->governor;
/* end old governor */
if (old_gov) {
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-   if (ret) {
-   /* This can happen due to race with other operations */
-   pr_debug("%s: Failed to Stop Governor: %s (%d)\n",
-__func__, old_gov->name, ret);
-   return ret;
-   }
+   cpufreq_stop_governor(policy);
cpufreq_exit_governor(policy);
}
 

[PATCH 2/2] cpufreq: governor: CPUFREQ_GOV_STOP never fails

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

None of the cpufreq governors currently in the tree will ever fail
an invocation of the ->governor() callback with the event argument
equal to CPUFREQ_GOV_STOP (unless invoked with incorrect arguments
which doesn't matter anyway) and it is rather difficult to imagine
a valid reason for such a failure.

Accordingly, rearrange the code in the core to make it clear that
this call never fails.

Signed-off-by: Rafael J. Wysocki 
---
 drivers/cpufreq/cpufreq.c |   40 +++-
 1 file changed, 11 insertions(+), 29 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -83,6 +83,11 @@ static inline void cpufreq_exit_governor
(void)cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
 }
 
+static inline void cpufreq_stop_governor(struct cpufreq_policy *policy)
+{
+   (void)cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+}
+
 /**
  * Two notifier lists: the "policy" list is involved in the
  * validation process for a new CPU frequency policy; the
@@ -1026,13 +1031,8 @@ static int cpufreq_add_policy_cpu(struct
return 0;
 
down_write(>rwsem);
-   if (has_target()) {
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-   if (ret) {
-   pr_err("%s: Failed to stop governor\n", __func__);
-   goto unlock;
-   }
-   }
+   if (has_target())
+   cpufreq_stop_governor(policy);
 
cpumask_set_cpu(cpu, policy->cpus);
 
@@ -1041,8 +1041,6 @@ static int cpufreq_add_policy_cpu(struct
if (ret)
pr_err("%s: Failed to start governor\n", __func__);
}
-
-unlock:
up_write(>rwsem);
return ret;
 }
@@ -1354,11 +1352,8 @@ static void cpufreq_offline(unsigned int
}
 
down_write(>rwsem);
-   if (has_target()) {
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-   if (ret)
-   pr_err("%s: Failed to stop governor\n", __func__);
-   }
+   if (has_target())
+   cpufreq_stop_governor(policy);
 
cpumask_clear_cpu(cpu, policy->cpus);
 
@@ -1622,7 +1617,6 @@ EXPORT_SYMBOL(cpufreq_generic_suspend);
 void cpufreq_suspend(void)
 {
struct cpufreq_policy *policy;
-   int ret;
 
if (!cpufreq_driver)
return;
@@ -1635,14 +1629,8 @@ void cpufreq_suspend(void)
for_each_active_policy(policy) {
if (has_target()) {
down_write(>rwsem);
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+   cpufreq_stop_governor(policy);
up_write(>rwsem);
-
-   if (ret) {
-   pr_err("%s: Failed to stop governor for policy: 
%p\n",
-   __func__, policy);
-   continue;
-   }
}
 
if (cpufreq_driver->suspend && cpufreq_driver->suspend(policy))
@@ -2216,13 +2204,7 @@ static int cpufreq_set_policy(struct cpu
old_gov = policy->governor;
/* end old governor */
if (old_gov) {
-   ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-   if (ret) {
-   /* This can happen due to race with other operations */
-   pr_debug("%s: Failed to Stop Governor: %s (%d)\n",
-__func__, old_gov->name, ret);
-   return ret;
-   }
+   cpufreq_stop_governor(policy);
cpufreq_exit_governor(policy);
}