diff mbox series

[v2,5/5] cpufreq: vexpress-spc: fix some coding style issues

Message ID 20191017123508.26130-6-sudeep.holla@arm.com
State Superseded
Headers show
Series [v2,1/5] cpufreq: scpi: remove stale/outdated comment about the driver | expand

Commit Message

Sudeep Holla Oct. 17, 2019, 12:35 p.m. UTC 
Fix the following checkpatch checks/warnings:

CHECK: Unnecessary parentheses around the code
CHECK: Alignment should match open parenthesis
CHECK: Prefer kernel type 'u32' over 'uint32_t'
WARNING: Missing a blank line after declarations

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

---
 drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------
 1 file changed, 20 insertions(+), 23 deletions(-)

-- 
2.17.1

Comments

Viresh Kumar Oct. 18, 2019, 5:55 a.m. UTC | #1 
On 17-10-19, 13:35, Sudeep Holla wrote:
> Fix the following checkpatch checks/warnings:

> 

> CHECK: Unnecessary parentheses around the code

> CHECK: Alignment should match open parenthesis

> CHECK: Prefer kernel type 'u32' over 'uint32_t'

> WARNING: Missing a blank line after declarations

> 

> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> ---

>  drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------

>  1 file changed, 20 insertions(+), 23 deletions(-)

> 

> diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c

> index 81064430317f..8ecb2961be86 100644

> --- a/drivers/cpufreq/vexpress-spc-cpufreq.c

> +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c

> @@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)

>  	for_each_online_cpu(j) {

>  		cpu_freq = per_cpu(cpu_last_req_freq, j);

>  

> -		if ((cluster == per_cpu(physical_cluster, j)) &&

> -				(max_freq < cpu_freq))

> +		if (cluster == per_cpu(physical_cluster, j) &&

> +		    max_freq < cpu_freq)

>  			max_freq = cpu_freq;

>  	}

>  

> @@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,

>  	freqs_new = freq_table[cur_cluster][index].frequency;

>  

>  	if (is_bL_switching_enabled()) {

> -		if ((actual_cluster == A15_CLUSTER) &&

> -				(freqs_new < clk_big_min)) {

> +		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)

>  			new_cluster = A7_CLUSTER;

> -		} else if ((actual_cluster == A7_CLUSTER) &&

> -				(freqs_new > clk_little_max)) {

> +		else if (actual_cluster == A7_CLUSTER &&

> +			 freqs_new > clk_little_max)

>  			new_cluster = A15_CLUSTER;

> -		}

>  	}

>  

>  	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,

>  				      freqs_new);

>  

> -	if (!ret) {

> +	if (!ret)


That's not the standard way in Linux I believe. We do use {} even when
the body is single line but broken into two, like below.

>  		arch_set_freq_scale(policy->related_cpus, freqs_new,

>  				    policy->cpuinfo.max_freq);

> -	}

>  

>  	return ret;

>  }

> @@ -222,7 +219,8 @@ static inline u32 get_table_count(struct cpufreq_frequency_table *table)

>  static inline u32 get_table_min(struct cpufreq_frequency_table *table)

>  {

>  	struct cpufreq_frequency_table *pos;

> -	uint32_t min_freq = ~0;

> +	u32 min_freq = ~0;

> +

>  	cpufreq_for_each_entry(pos, table)

>  		if (pos->frequency < min_freq)

>  			min_freq = pos->frequency;

> @@ -233,7 +231,8 @@ static inline u32 get_table_min(struct cpufreq_frequency_table *table)

>  static inline u32 get_table_max(struct cpufreq_frequency_table *table)

>  {

>  	struct cpufreq_frequency_table *pos;

> -	uint32_t max_freq = 0;

> +	u32 max_freq = 0;

> +

>  	cpufreq_for_each_entry(pos, table)

>  		if (pos->frequency > max_freq)

>  			max_freq = pos->frequency;

> @@ -255,14 +254,11 @@ static int merge_cluster_tables(void)

>  	freq_table[MAX_CLUSTERS] = table;

>  

>  	/* Add in reverse order to get freqs in increasing order */

> -	for (i = MAX_CLUSTERS - 1; i >= 0; i--) {

> +	for (i = MAX_CLUSTERS - 1; i >= 0; i--)

>  		for (j = 0; freq_table[i][j].frequency != CPUFREQ_TABLE_END;

> -				j++) {

> -			table[k].frequency = VIRT_FREQ(i,

> -					freq_table[i][j].frequency);

> -			k++;

> -		}

> -	}

> +		     j++, k++)


same here, please keep {}.

> +			table[k].frequency =

> +				VIRT_FREQ(i, freq_table[i][j].frequency);

>  

>  	table[k].driver_data = k;

>  	table[k].frequency = CPUFREQ_TABLE_END;

> @@ -332,13 +328,13 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,

>  		return 0;

>  

>  	dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",

> -			__func__, cpu_dev->id, cluster);

> +		__func__, cpu_dev->id, cluster);

>  	ret = PTR_ERR(clk[cluster]);

>  	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);

>  

>  out:

>  	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,

> -			cluster);

> +		cluster);

>  	return ret;

>  }

>  

> @@ -406,7 +402,7 @@ static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)

>  	cpu_dev = get_cpu_device(policy->cpu);

>  	if (!cpu_dev) {

>  		pr_err("%s: failed to get cpu%d device\n", __func__,

> -				policy->cpu);

> +		       policy->cpu);

>  		return -ENODEV;

>  	}

>  

> @@ -432,7 +428,8 @@ static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)

>  	dev_pm_opp_of_register_em(policy->cpus);

>  

>  	if (is_bL_switching_enabled())

> -		per_cpu(cpu_last_req_freq, policy->cpu) = clk_get_cpu_rate(policy->cpu);

> +		per_cpu(cpu_last_req_freq, policy->cpu) =

> +						clk_get_cpu_rate(policy->cpu);

>  

>  	dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);

>  	return 0;

> @@ -451,7 +448,7 @@ static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)

>  	cpu_dev = get_cpu_device(policy->cpu);

>  	if (!cpu_dev) {

>  		pr_err("%s: failed to get cpu%d device\n", __func__,

> -				policy->cpu);

> +		       policy->cpu);

>  		return -ENODEV;

>  	}

>  

> -- 

> 2.17.1


-- 
viresh
Sudeep Holla Oct. 18, 2019, 5:59 a.m. UTC | #2 
On Fri, Oct 18, 2019 at 11:25:17AM +0530, Viresh Kumar wrote:
> On 17-10-19, 13:35, Sudeep Holla wrote:

> > Fix the following checkpatch checks/warnings:

> > 

> > CHECK: Unnecessary parentheses around the code

> > CHECK: Alignment should match open parenthesis

> > CHECK: Prefer kernel type 'u32' over 'uint32_t'

> > WARNING: Missing a blank line after declarations

> > 

> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> > ---

> >  drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------

> >  1 file changed, 20 insertions(+), 23 deletions(-)

> > 

> > diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c

> > index 81064430317f..8ecb2961be86 100644

> > --- a/drivers/cpufreq/vexpress-spc-cpufreq.c

> > +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c

> > @@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)

> >  	for_each_online_cpu(j) {

> >  		cpu_freq = per_cpu(cpu_last_req_freq, j);

> >  

> > -		if ((cluster == per_cpu(physical_cluster, j)) &&

> > -				(max_freq < cpu_freq))

> > +		if (cluster == per_cpu(physical_cluster, j) &&

> > +		    max_freq < cpu_freq)

> >  			max_freq = cpu_freq;

> >  	}

> >  

> > @@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,

> >  	freqs_new = freq_table[cur_cluster][index].frequency;

> >  

> >  	if (is_bL_switching_enabled()) {

> > -		if ((actual_cluster == A15_CLUSTER) &&

> > -				(freqs_new < clk_big_min)) {

> > +		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)

> >  			new_cluster = A7_CLUSTER;

> > -		} else if ((actual_cluster == A7_CLUSTER) &&

> > -				(freqs_new > clk_little_max)) {

> > +		else if (actual_cluster == A7_CLUSTER &&

> > +			 freqs_new > clk_little_max)

> >  			new_cluster = A15_CLUSTER;

> > -		}

> >  	}

> >  

> >  	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,

> >  				      freqs_new);

> >  

> > -	if (!ret) {

> > +	if (!ret)

> 

> That's not the standard way in Linux I believe. We do use {} even when

> the body is single line but broken into two, like below.

>


OK, wasn't aware of that. I will update. Generally I ignore checkpatch
warnings, but the list was big and fixed a bunch of them :)

--
Regards,
Sudeep
Nicolas Pitre Oct. 18, 2019, 3:51 p.m. UTC | #3 
On Fri, 18 Oct 2019, Sudeep Holla wrote:

> On Fri, Oct 18, 2019 at 11:25:17AM +0530, Viresh Kumar wrote:

> > On 17-10-19, 13:35, Sudeep Holla wrote:

> > > Fix the following checkpatch checks/warnings:

> > > 

> > > CHECK: Unnecessary parentheses around the code

> > > CHECK: Alignment should match open parenthesis

> > > CHECK: Prefer kernel type 'u32' over 'uint32_t'

> > > WARNING: Missing a blank line after declarations

> > > 

> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> > > ---

> > >  drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------

> > >  1 file changed, 20 insertions(+), 23 deletions(-)

> > > 

> > > diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c

> > > index 81064430317f..8ecb2961be86 100644

> > > --- a/drivers/cpufreq/vexpress-spc-cpufreq.c

> > > +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c

> > > @@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)

> > >  	for_each_online_cpu(j) {

> > >  		cpu_freq = per_cpu(cpu_last_req_freq, j);

> > >  

> > > -		if ((cluster == per_cpu(physical_cluster, j)) &&

> > > -				(max_freq < cpu_freq))

> > > +		if (cluster == per_cpu(physical_cluster, j) &&

> > > +		    max_freq < cpu_freq)

> > >  			max_freq = cpu_freq;

> > >  	}

> > >  

> > > @@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,

> > >  	freqs_new = freq_table[cur_cluster][index].frequency;

> > >  

> > >  	if (is_bL_switching_enabled()) {

> > > -		if ((actual_cluster == A15_CLUSTER) &&

> > > -				(freqs_new < clk_big_min)) {

> > > +		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)

> > >  			new_cluster = A7_CLUSTER;

> > > -		} else if ((actual_cluster == A7_CLUSTER) &&

> > > -				(freqs_new > clk_little_max)) {

> > > +		else if (actual_cluster == A7_CLUSTER &&

> > > +			 freqs_new > clk_little_max)

> > >  			new_cluster = A15_CLUSTER;

> > > -		}

> > >  	}

> > >  

> > >  	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,

> > >  				      freqs_new);

> > >  

> > > -	if (!ret) {

> > > +	if (!ret)

> > 

> > That's not the standard way in Linux I believe. We do use {} even when

> > the body is single line but broken into two, like below.

> >

> 

> OK, wasn't aware of that. I will update. Generally I ignore checkpatch

> warnings, but the list was big and fixed a bunch of them :)


In cases like this one, the best is to go with whatever makes checkpatch 
happy.


Nicolas
diff mbox series

Patch

diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
index 81064430317f..8ecb2961be86 100644
--- a/drivers/cpufreq/vexpress-spc-cpufreq.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -79,8 +79,8 @@  static unsigned int find_cluster_maxfreq(int cluster)
 	for_each_online_cpu(j) {
 		cpu_freq = per_cpu(cpu_last_req_freq, j);
 
-		if ((cluster == per_cpu(physical_cluster, j)) &&
-				(max_freq < cpu_freq))
+		if (cluster == per_cpu(physical_cluster, j) &&
+		    max_freq < cpu_freq)
 			max_freq = cpu_freq;
 	}
 
@@ -188,22 +188,19 @@  static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
 	freqs_new = freq_table[cur_cluster][index].frequency;
 
 	if (is_bL_switching_enabled()) {
-		if ((actual_cluster == A15_CLUSTER) &&
-				(freqs_new < clk_big_min)) {
+		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)
 			new_cluster = A7_CLUSTER;
-		} else if ((actual_cluster == A7_CLUSTER) &&
-				(freqs_new > clk_little_max)) {
+		else if (actual_cluster == A7_CLUSTER &&
+			 freqs_new > clk_little_max)
 			new_cluster = A15_CLUSTER;
-		}
 	}
 
 	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
 				      freqs_new);
 
-	if (!ret) {
+	if (!ret)
 		arch_set_freq_scale(policy->related_cpus, freqs_new,
 				    policy->cpuinfo.max_freq);
-	}
 
 	return ret;
 }
@@ -222,7 +219,8 @@  static inline u32 get_table_count(struct cpufreq_frequency_table *table)
 static inline u32 get_table_min(struct cpufreq_frequency_table *table)
 {
 	struct cpufreq_frequency_table *pos;
-	uint32_t min_freq = ~0;
+	u32 min_freq = ~0;
+
 	cpufreq_for_each_entry(pos, table)
 		if (pos->frequency < min_freq)
 			min_freq = pos->frequency;
@@ -233,7 +231,8 @@  static inline u32 get_table_min(struct cpufreq_frequency_table *table)
 static inline u32 get_table_max(struct cpufreq_frequency_table *table)
 {
 	struct cpufreq_frequency_table *pos;
-	uint32_t max_freq = 0;
+	u32 max_freq = 0;
+
 	cpufreq_for_each_entry(pos, table)
 		if (pos->frequency > max_freq)
 			max_freq = pos->frequency;
@@ -255,14 +254,11 @@  static int merge_cluster_tables(void)
 	freq_table[MAX_CLUSTERS] = table;
 
 	/* Add in reverse order to get freqs in increasing order */
-	for (i = MAX_CLUSTERS - 1; i >= 0; i--) {
+	for (i = MAX_CLUSTERS - 1; i >= 0; i--)
 		for (j = 0; freq_table[i][j].frequency != CPUFREQ_TABLE_END;
-				j++) {
-			table[k].frequency = VIRT_FREQ(i,
-					freq_table[i][j].frequency);
-			k++;
-		}
-	}
+		     j++, k++)
+			table[k].frequency =
+				VIRT_FREQ(i, freq_table[i][j].frequency);
 
 	table[k].driver_data = k;
 	table[k].frequency = CPUFREQ_TABLE_END;
@@ -332,13 +328,13 @@  static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
 		return 0;
 
 	dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",
-			__func__, cpu_dev->id, cluster);
+		__func__, cpu_dev->id, cluster);
 	ret = PTR_ERR(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
 
 out:
 	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
-			cluster);
+		cluster);
 	return ret;
 }
 
@@ -406,7 +402,7 @@  static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
 		pr_err("%s: failed to get cpu%d device\n", __func__,
-				policy->cpu);
+		       policy->cpu);
 		return -ENODEV;
 	}
 
@@ -432,7 +428,8 @@  static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
 	dev_pm_opp_of_register_em(policy->cpus);
 
 	if (is_bL_switching_enabled())
-		per_cpu(cpu_last_req_freq, policy->cpu) = clk_get_cpu_rate(policy->cpu);
+		per_cpu(cpu_last_req_freq, policy->cpu) =
+						clk_get_cpu_rate(policy->cpu);
 
 	dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);
 	return 0;
@@ -451,7 +448,7 @@  static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
 		pr_err("%s: failed to get cpu%d device\n", __func__,
-				policy->cpu);
+		       policy->cpu);
 		return -ENODEV;
 	}