| Message ID | 5674598B.6020000@users.sourceforge.net (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 18-12-15, 20:07, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > 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 <elfring@users.sourceforge.net> > --- > drivers/cpufreq/blackfin-cpufreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/blackfin-cpufreq.c b/drivers/cpufreq/blackfin-cpufreq.c > index a9f8e5b..2a6f3ac 100644 > --- a/drivers/cpufreq/blackfin-cpufreq.c > +++ b/drivers/cpufreq/blackfin-cpufreq.c > @@ -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 <viresh.kumar@linaro.org>
>> From: Markus Elfring <elfring@users.sourceforge.net> >> 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 <elfring@users.sourceforge.net> >> --- >> drivers/cpufreq/blackfin-cpufreq.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/cpufreq/blackfin-cpufreq.c b/drivers/cpufreq/blackfin-cpufreq.c >> index a9f8e5b..2a6f3ac 100644 >> --- a/drivers/cpufreq/blackfin-cpufreq.c >> +++ b/drivers/cpufreq/blackfin-cpufreq.c >> @@ -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 <viresh.kumar@linaro.org> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19-12-15, 09:23, SF Markus Elfring wrote: > >> From: Markus Elfring <elfring@users.sourceforge.net> > >> 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 <elfring@users.sourceforge.net> > >> --- > >> drivers/cpufreq/blackfin-cpufreq.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/cpufreq/blackfin-cpufreq.c b/drivers/cpufreq/blackfin-cpufreq.c > >> index a9f8e5b..2a6f3ac 100644 > >> --- a/drivers/cpufreq/blackfin-cpufreq.c > >> +++ b/drivers/cpufreq/blackfin-cpufreq.c > >> @@ -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 <viresh.kumar@linaro.org> > > 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.
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21-12-15, 10:20, SF Markus Elfring wrote: > > 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.
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21-12-15, 10:50, SF Markus Elfring wrote: > 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. > * 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.
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21-12-15, 11:00, SF Markus Elfring wrote: > 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 :)
From: Markus Elfring <elfring@users.sourceforge.net>
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(-)
diff --git a/drivers/cpufreq/blackfin-cpufreq.c b/drivers/cpufreq/blackfin-cpufreq.c index a9f8e5b..2a6f3ac 100644 --- a/drivers/cpufreq/blackfin-cpufreq.c +++ b/drivers/cpufreq/blackfin-cpufreq.c @@ -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;