Skip to contentSkip to conversation

blackfin-cpufreq: Change return type of cpu_set_cclk() to that of clk_set_rate()

PATCH In review SF Markus Elfring 3 participants 17 messages Dec 18, 2015 – Jan 3, 2016

Subsystems (2)
Kernel janitors CPU frequency scaling framework
lore
Share & tools
lore

SF Markus Elfring

author
#0 lore raw

PATCH 1/1 blackfin-cpufreq: Change return type of cpu_set_cclk() to that of clk_set_rate()

From: Markus Elfring <[email protected]>
Date: Fri, 18 Dec 2015 19:43:27 +0100

The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/blackfin-cpufreq.c +1 -1
index a9f8e5b..2a6f3ac 100644
@@ -112,7 +112,7 @@ static unsigned int bfin_getfreq_khz(unsigned int cpu)
 }
 
 #ifdef CONFIG_BF60x
-unsigned long cpu_set_cclk(int cpu, unsigned long new)
+int cpu_set_cclk(int cpu, unsigned long new)
 {
 	struct clk *clk;
 	int ret;
Signature · 1 line
2.6.3
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 19 lines · SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Fri, 18 Dec 2015 19:43:27 +0100

The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/blackfin-cpufreq.c +0 -0
index a9f8e5b..2a6f3ac 100644

drivers/cpufreq/blackfin-cpufreq.c static unsigned int bfin_getfreq_khz(unsigned int cpu) · line 117

Quoted 7 lines
drivers/cpufreq/blackfin-cpufreq.c +1 -1
@@ -112,7 +112,7 @@ static unsigned int bfin_getfreq_khz(unsigned int cpu)
 }
 
 #ifdef CONFIG_BF60x
-unsigned long cpu_set_cclk(int cpu, unsigned long new)
+int cpu_set_cclk(int cpu, unsigned long new)
 {
 	struct clk *clk;
Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh
author
#2 lore raw
Quoted 21 lines
Quoted 21 lines

From: Markus Elfring <[email protected]>
Date: Fri, 18 Dec 2015 19:43:27 +0100

The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/blackfin-cpufreq.c +0 -0
index a9f8e5b..2a6f3ac 100644
@@ -112,7 +112,7 @@ static unsigned int bfin_getfreq_khz(unsigned int cpu)
 }
Quoted 7 lines

drivers/cpufreq/blackfin-cpufreq.c static unsigned int bfin_getfreq_khz(unsigned int cpu) · line 117

Quoted 5 lines
drivers/cpufreq/blackfin-cpufreq.c +1 -1
 #ifdef CONFIG_BF60x
-unsigned long cpu_set_cclk(int cpu, unsigned long new)
+int cpu_set_cclk(int cpu, unsigned long new)
 {
 	struct clk *clk;
Acked-by Viresh Kumar <[email protected]>

Thanks for your acceptance.

I would appreciate if another implementation detail can also be clarified there.
http://lxr.free-electrons.com/ident?v=4.3;i=cpu_set_cclk

* Do you want to reuse such a function in other modules?
* Should it eventually marked as "static"?

Regards,
Markus

maintainer
#3 lore raw
Quoted 29 lines · SF Markus Elfring
Quoted 21 lines
Quoted 21 lines

From: Markus Elfring <[email protected]>
Date: Fri, 18 Dec 2015 19:43:27 +0100

The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/blackfin-cpufreq.c +0 -0
index a9f8e5b..2a6f3ac 100644
@@ -112,7 +112,7 @@ static unsigned int bfin_getfreq_khz(unsigned int cpu)
 }
Quoted 7 lines

drivers/cpufreq/blackfin-cpufreq.c static unsigned int bfin_getfreq_khz(unsigned int cpu) · line 117

Quoted 5 lines
drivers/cpufreq/blackfin-cpufreq.c +1 -1
 #ifdef CONFIG_BF60x
-unsigned long cpu_set_cclk(int cpu, unsigned long new)
+int cpu_set_cclk(int cpu, unsigned long new)
 {
 	struct clk *clk;
Acked-by Viresh Kumar <[email protected]>
Quoted 7 lines

Thanks for your acceptance.

I would appreciate if another implementation detail can also be clarified there.
http://lxr.free-electrons.com/ident?v=4.3;i=cpu_set_cclk

* Do you want to reuse such a function in other modules?
* Should it eventually marked as "static"?

This should be static, yeah.

Signature · 1 line
viresh
author
#4 lore raw

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

Quoted 1 line

This should be static, yeah.

Would you like to integrate a corresponding small source code change yourself? (Do you need a separate patch from me?)

Regards,
Markus

maintainer
#5 lore raw

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

Quoted 4 lines · SF Markus Elfring
Quoted 1 line

This should be static, yeah.

Would you like to integrate a corresponding small source code change yourself? (Do you need a separate patch from me?)

Since you reported the issue, it will be good if you can send a patch for this as well.

Signature · 1 line
viresh
author
#6 lore raw

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

Quoted 2 lines

Since you reported the issue, it will be good if you can send a patch for this as well.

I'm sorry if I become a bit picky for this implementation detail.

In which order would you prefer that the properties of the function "cpu_set_cclk" will be improved?
* Should the linkage specifier be added on a separate source code line?
* Can this fix be combined with my previous update suggestion for the return type?

Regards,
Markus

maintainer
#7 lore raw

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

Quoted 3 lines · SF Markus Elfring

In which order would you prefer that the properties of the function "cpu_set_cclk" will be improved?
* Should the linkage specifier be added on a separate source code line?

No, it can be there in the same line.

Quoted 2 lines

* Can this fix be combined with my previous update suggestion for the return type?

I think the first patch is already Acked now and can be applied as it is. Just send the next patch based on the previous one.

Signature · 1 line
viresh
author
#8 lore raw

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

Quoted 2 lines

I think the first patch is already Acked now and can be applied as it is. Just send the next patch based on the previous one.

Does it matter to provide the proposed changes as a tiny patch series with two update steps?

Regards,
Markus

maintainer
#9 lore raw

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

Quoted 2 lines · SF Markus Elfring

Does it matter to provide the proposed changes as a tiny patch series with two update steps?

Yeah, you can do it that way as well :)

Signature · 1 line
viresh
author
#10 lore raw

cover blackfin-cpufreq: Fine-tuning for properties of one function

In reply to SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Mon, 21 Dec 2015 22:24:18 +0100

Another update suggestion was taken into account after a patch was applied from static source code analysis.

Markus Elfring (2):
Change return type of cpu_set_cclk() to that of clk_set_rate() Mark cpu_set_cclk() as static

 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Signature · 1 line
2.6.3
author
#11 lore raw

PATCH 1/2 blackfin-cpufreq: Change return type of cpu_set_cclk() to that of clk_set_rate()

From: Markus Elfring <[email protected]>
Date: Mon, 21 Dec 2015 21:56:27 +0100

The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
Acked-by Viresh Kumar <[email protected]>
---
 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/blackfin-cpufreq.c +1 -1
index a9f8e5b..2a6f3ac 100644
@@ -112,7 +112,7 @@ static unsigned int bfin_getfreq_khz(unsigned int cpu)
 }
 
 #ifdef CONFIG_BF60x
-unsigned long cpu_set_cclk(int cpu, unsigned long new)
+int cpu_set_cclk(int cpu, unsigned long new)
 {
 	struct clk *clk;
 	int ret;
Signature · 1 line
2.6.3
author
#12 lore raw

PATCH 2/2 blackfin-cpufreq: Mark cpu_set_cclk() as static

In reply to SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Mon, 21 Dec 2015 22:12:26 +0100

The cpu_set_cclk() function was only used in a single source file so far. Indicate this setting also by the corresponding linkage specifier.

Signed-off-by Markus Elfring <[email protected]>
Acked-by Viresh Kumar <[email protected]>
---
 drivers/cpufreq/blackfin-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/blackfin-cpufreq.c +1 -1
index 2a6f3ac..12e97d8 100644
@@ -112,7 +112,7 @@ static unsigned int bfin_getfreq_khz(unsigned int cpu)
 }
 
 #ifdef CONFIG_BF60x
-int cpu_set_cclk(int cpu, unsigned long new)
+static int cpu_set_cclk(int cpu, unsigned long new)
 {
 	struct clk *clk;
 	int ret;
Signature · 1 line
2.6.3
maintainer Acked-by on v1 2/2
#13 lore raw

Re: [PATCH 2/2] blackfin-cpufreq: Mark cpu_set_cclk() as static

Quoted 8 lines · SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Mon, 21 Dec 2015 22:12:26 +0100

The cpu_set_cclk() function was only used in a single source file so far. Indicate this setting also by the corresponding linkage specifier.

Signed-off-by Markus Elfring <[email protected]>
Acked-by Viresh Kumar <[email protected]>

Well, I never Acked this patch as such and you shouldn't have mentioned it by yourself. But it looks fine otherwise and so

Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh
author
#14 lore raw

Re: blackfin-cpufreq: Mark cpu_set_cclk() as static

Quoted 2 lines

Well, I never Acked this patch as such and you shouldn't have mentioned it by yourself.

Did I interpret your positive feedback too early in the way that I can add this tag myself already?

Quoted 2 lines

But it looks fine otherwise and so

Acked-by Viresh Kumar <[email protected]>

Thanks for another acknowledgement.

Regards,
Markus

maintainer
#15 lore raw

Re: blackfin-cpufreq: Mark cpu_set_cclk() as static

Quoted 5 lines · SF Markus Elfring
Quoted 2 lines

Well, I never Acked this patch as such and you shouldn't have mentioned it by yourself.

Did I interpret your positive feedback too early in the way that I can add this tag myself already?

You aren't supposed to add a Tag on someone else's behalf, even if they said that the change is okay. Others have to send this separately.

Signature · 1 line
viresh
#16 lore raw

Re: [PATCH 2/2] blackfin-cpufreq: Mark cpu_set_cclk() as static

In reply to Viresh Kumar
Quoted 7 lines · Viresh Kumar
Quoted 6 lines · SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Mon, 21 Dec 2015 22:12:26 +0100

The cpu_set_cclk() function was only used in a single source file so far. Indicate this setting also by the corresponding linkage specifier.

Quoted 7 lines
Quoted 2 lines
Signed-off-by Markus Elfring <[email protected]>
Acked-by Viresh Kumar <[email protected]>

Well, I never Acked this patch as such and you shouldn't have mentioned it by yourself. But it looks fine otherwise and so

Acked-by Viresh Kumar <[email protected]>

Series applied, thanks!