On 02/17/2014 06:36 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> 
> Reduce the rampant usage of goto and the indentation level in
> cpufreq_set_policy() to improve the readability of that code.
> 
> No functional changes should result from that.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>

Reviewed-by: Srivatsa S. Bhat <srivatsa.b...@linux.vnet.ibm.com>

Regards,
Srivatsa S. Bhat

> ---
>  drivers/cpufreq/cpufreq.c |  102 
> ++++++++++++++++++++--------------------------
>  1 file changed, 45 insertions(+), 57 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -2018,22 +2018,21 @@ EXPORT_SYMBOL(cpufreq_get_policy);
>  static int cpufreq_set_policy(struct cpufreq_policy *policy,
>                               struct cpufreq_policy *new_policy)
>  {
> -     int ret = 0, failed = 1;
> +     struct cpufreq_governor *old_gov;
> +     int ret;
> 
>       pr_debug("setting new policy for CPU %u: %u - %u kHz\n", 
> new_policy->cpu,
>               new_policy->min, new_policy->max);
> 
>       memcpy(&new_policy->cpuinfo, &policy->cpuinfo, sizeof(policy->cpuinfo));
> 
> -     if (new_policy->min > policy->max || new_policy->max < policy->min) {
> -             ret = -EINVAL;
> -             goto error_out;
> -     }
> +     if (new_policy->min > policy->max || new_policy->max < policy->min)
> +             return -EINVAL;
> 
>       /* verify the cpu speed can be set within this limit */
>       ret = cpufreq_driver->verify(new_policy);
>       if (ret)
> -             goto error_out;
> +             return ret;
> 
>       /* adjust if necessary - all reasons */
>       blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
> @@ -2049,7 +2048,7 @@ static int cpufreq_set_policy(struct cpu
>        */
>       ret = cpufreq_driver->verify(new_policy);
>       if (ret)
> -             goto error_out;
> +             return ret;
> 
>       /* notification of the new policy */
>       blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
> @@ -2064,58 +2063,47 @@ static int cpufreq_set_policy(struct cpu
>       if (cpufreq_driver->setpolicy) {
>               policy->policy = new_policy->policy;
>               pr_debug("setting range\n");
> -             ret = cpufreq_driver->setpolicy(new_policy);
> -     } else {
> -             if (new_policy->governor != policy->governor) {
> -                     /* save old, working values */
> -                     struct cpufreq_governor *old_gov = policy->governor;
> -
> -                     pr_debug("governor switch\n");
> -
> -                     /* end old governor */
> -                     if (policy->governor) {
> -                             __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> -                             up_write(&policy->rwsem);
> -                             __cpufreq_governor(policy,
> -                                             CPUFREQ_GOV_POLICY_EXIT);
> -                             down_write(&policy->rwsem);
> -                     }
> -
> -                     /* start new governor */
> -                     policy->governor = new_policy->governor;
> -                     if (!__cpufreq_governor(policy, 
> CPUFREQ_GOV_POLICY_INIT)) {
> -                             if (!__cpufreq_governor(policy, 
> CPUFREQ_GOV_START)) {
> -                                     failed = 0;
> -                             } else {
> -                                     up_write(&policy->rwsem);
> -                                     __cpufreq_governor(policy,
> -                                                     
> CPUFREQ_GOV_POLICY_EXIT);
> -                                     down_write(&policy->rwsem);
> -                             }
> -                     }
> -
> -                     if (failed) {
> -                             /* new governor failed, so re-start old one */
> -                             pr_debug("starting governor %s failed\n",
> -                                                     policy->governor->name);
> -                             if (old_gov) {
> -                                     policy->governor = old_gov;
> -                                     __cpufreq_governor(policy,
> -                                                     
> CPUFREQ_GOV_POLICY_INIT);
> -                                     __cpufreq_governor(policy,
> -                                                        CPUFREQ_GOV_START);
> -                             }
> -                             ret = -EINVAL;
> -                             goto error_out;
> -                     }
> -                     /* might be a policy change, too, so fall through */
> -             }
> -             pr_debug("governor: change or update limits\n");
> -             ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
> +             return cpufreq_driver->setpolicy(new_policy);
>       }
> +     if (new_policy->governor == policy->governor)
> +             goto out;
> +
> +     pr_debug("governor switch\n");
> +
> +     /* save old, working values */
> +     old_gov = policy->governor;
> +     /* end old governor */
> +     if (old_gov) {
> +             __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> +             up_write(&policy->rwsem);
> +             __cpufreq_governor(policy,CPUFREQ_GOV_POLICY_EXIT);
> +             down_write(&policy->rwsem);
> +     }
> +
> +     /* start new governor */
> +     policy->governor = new_policy->governor;
> +     if (!__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT)) {
> +             if (!__cpufreq_governor(policy, CPUFREQ_GOV_START))
> +                     goto out;
> +
> +             up_write(&policy->rwsem);
> +             __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
> +             down_write(&policy->rwsem);
> +     }
> +
> +     /* new governor failed, so re-start old one */
> +     pr_debug("starting governor %s failed\n", policy->governor->name);
> +     if (old_gov) {
> +             policy->governor = old_gov;
> +             __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT);
> +             __cpufreq_governor(policy, CPUFREQ_GOV_START);
> +     }
> +
> +     return -EINVAL;
> 
> -error_out:
> -     return ret;
> + out:
> +     pr_debug("governor: change or update limits\n");
> +     return __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
>  }
> 
>  /**
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to