[PATCH 1/2] cpufreq: Remove extra variables

[PATCH 1/2] cpufreq: Remove extra variables

[Viresh Kumar]

We call cpufreq_cpu_get() in cpufreq_add_dev_symlink() to increase usage
refcount of policy and not to get policy for a cpu. So, we don't really need to
capture the return value of this routine and call put for it later for failure
cases. We can simply use policy passed as an argument to this routine.

Moreover debug print is rewritten to make it more clear.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 170d344..35e1a03 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -813,19 +813,18 @@ static int cpufreq_add_dev_symlink(unsigned int cpu,
 	int ret = 0;
 
 	for_each_cpu(j, policy->cpus) {
-		struct cpufreq_policy *managed_policy;
 		struct device *cpu_dev;
 
 		if (j == cpu)
 			continue;
 
-		pr_debug("CPU %u already managed, adding link\n", j);
-		managed_policy = cpufreq_cpu_get(cpu);
+		pr_debug("Adding link for CPU: %u\n", j);
+		cpufreq_cpu_get(cpu);
 		cpu_dev = get_cpu_device(j);
 		ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
 					"cpufreq");
 		if (ret) {
-			cpufreq_cpu_put(managed_policy);
+			cpufreq_cpu_put(policy);
 			return ret;
 		}
 	}
-- 
1.7.12.rc2.18.g61b472e

[PATCH 2/2] cpufreq: don't pass cpu to cpufreq_add_dev_{symlink|interface}

[Viresh Kumar]

Pointer to struct cpufreq_policy is already passed to these routines and so we
don't need to send policy->cpu to them as well. So, get rid of this extra
argument and use policy->cpu everywhere.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 35e1a03..80c0e20 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -806,8 +806,7 @@ void cpufreq_sysfs_remove_file(const struct attribute *attr)
 EXPORT_SYMBOL(cpufreq_sysfs_remove_file);
 
 /* symlink affected CPUs */
-static int cpufreq_add_dev_symlink(unsigned int cpu,
-				   struct cpufreq_policy *policy)
+static int cpufreq_add_dev_symlink(struct cpufreq_policy *policy)
 {
 	unsigned int j;
 	int ret = 0;
@@ -815,11 +814,11 @@ static int cpufreq_add_dev_symlink(unsigned int cpu,
 	for_each_cpu(j, policy->cpus) {
 		struct device *cpu_dev;
 
-		if (j == cpu)
+		if (j == policy->cpu)
 			continue;
 
 		pr_debug("Adding link for CPU: %u\n", j);
-		cpufreq_cpu_get(cpu);
+		cpufreq_cpu_get(policy->cpu);
 		cpu_dev = get_cpu_device(j);
 		ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
 					"cpufreq");
@@ -831,8 +830,7 @@ static int cpufreq_add_dev_symlink(unsigned int cpu,
 	return ret;
 }
 
-static int cpufreq_add_dev_interface(unsigned int cpu,
-				     struct cpufreq_policy *policy,
+static int cpufreq_add_dev_interface(struct cpufreq_policy *policy,
 				     struct device *dev)
 {
 	struct freq_attr **drv_attr;
@@ -868,7 +866,7 @@ static int cpufreq_add_dev_interface(unsigned int cpu,
 			goto err_out_kobj_put;
 	}
 
-	ret = cpufreq_add_dev_symlink(cpu, policy);
+	ret = cpufreq_add_dev_symlink(policy);
 	if (ret)
 		goto err_out_kobj_put;
 
@@ -1097,7 +1095,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
 	if (!frozen) {
-		ret = cpufreq_add_dev_interface(cpu, policy, dev);
+		ret = cpufreq_add_dev_interface(policy, dev);
 		if (ret)
 			goto err_out_unregister;
 	}
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH 1/2] cpufreq: Remove extra variables

[Viresh Kumar]

On 30 July 2013 19:28, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, July 30, 2013 07:06:33 PM Viresh Kumar wrote:
>> We call cpufreq_cpu_get() in cpufreq_add_dev_symlink() to increase usage
>> refcount of policy and not to get policy for a cpu. So, we don't really need to
>> capture the return value of this routine and call put for it later for failure
>> cases. We can simply use policy passed as an argument to this routine.
>>
>> Moreover debug print is rewritten to make it more clear.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Both [1-2/2] look good, but what do they apply to?  Mainline, linux-next,
> my bleeding-edge branch?

Sorry for not mentioning this, bleeding-edge :)

Re: [PATCH 1/2] cpufreq: Remove extra variables

[Rafael J. Wysocki]

On Tuesday, July 30, 2013 07:06:33 PM Viresh Kumar wrote:
> We call cpufreq_cpu_get() in cpufreq_add_dev_symlink() to increase usage
> refcount of policy and not to get policy for a cpu. So, we don't really need to
> capture the return value of this routine and call put for it later for failure
> cases. We can simply use policy passed as an argument to this routine.
> 
> Moreover debug print is rewritten to make it more clear.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Both [1-2/2] look good, but what do they apply to?  Mainline, linux-next,
my bleeding-edge branch?

Rafael


> ---
>  drivers/cpufreq/cpufreq.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 170d344..35e1a03 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -813,19 +813,18 @@ static int cpufreq_add_dev_symlink(unsigned int cpu,
>  	int ret = 0;
>  
>  	for_each_cpu(j, policy->cpus) {
> -		struct cpufreq_policy *managed_policy;
>  		struct device *cpu_dev;
>  
>  		if (j == cpu)
>  			continue;
>  
> -		pr_debug("CPU %u already managed, adding link\n", j);
> -		managed_policy = cpufreq_cpu_get(cpu);
> +		pr_debug("Adding link for CPU: %u\n", j);
> +		cpufreq_cpu_get(cpu);
>  		cpu_dev = get_cpu_device(j);
>  		ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
>  					"cpufreq");
>  		if (ret) {
> -			cpufreq_cpu_put(managed_policy);
> +			cpufreq_cpu_put(policy);
>  			return ret;
>  		}
>  	}
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.