diff mbox series

cpufreq: longhaul: Make array speeds static const

Message ID 20221103132141.79671-1-colin.i.king@gmail.com
State New
Headers
Series cpufreq: longhaul: Make array speeds static const |

Commit Message

Colin Ian King Nov. 3, 2022, 1:21 p.m. UTC 
  Don't populate the read-only array speeds on the stack but instead
make it static. Also makes the object code a little smaller.

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
---
 drivers/cpufreq/longhaul.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Joe Perches Nov. 3, 2022, 2:15 p.m. UTC | #1 
On Thu, 2022-11-03 at 13:21 +0000, Colin Ian King wrote:
> Don't populate the read-only array speeds on the stack but instead
> make it static. Also makes the object code a little smaller.
[]
> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
[]
> @@ -407,7 +407,7 @@ static int guess_fsb(int mult)
>  {
>  	int speed = cpu_khz / 1000;
>  	int i;
> -	int speeds[] = { 666, 1000, 1333, 2000 };
> +	static const int speeds[] = { 666, 1000, 1333, 2000 };
>  	int f_max, f_min;
>  
>  	for (i = 0; i < 4; i++) {

style trivia:  the loop test is probably better using ARRAY_SIZE

	for (i = 0; i < ARRAY_SIZE(speeds); i++)
Colin Ian King Nov. 3, 2022, 2:30 p.m. UTC | #2 
On 03/11/2022 14:15, Joe Perches wrote:
> On Thu, 2022-11-03 at 13:21 +0000, Colin Ian King wrote:
>> Don't populate the read-only array speeds on the stack but instead
>> make it static. Also makes the object code a little smaller.
> []
>> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
> []
>> @@ -407,7 +407,7 @@ static int guess_fsb(int mult)
>>   {
>>   	int speed = cpu_khz / 1000;
>>   	int i;
>> -	int speeds[] = { 666, 1000, 1333, 2000 };
>> +	static const int speeds[] = { 666, 1000, 1333, 2000 };
>>   	int f_max, f_min;
>>   
>>   	for (i = 0; i < 4; i++) {
> 
> style trivia:  the loop test is probably better using ARRAY_SIZE
> 
> 	for (i = 0; i < ARRAY_SIZE(speeds); i++)
> 
I'll send a V2 for that. Good idea.

Colin
Viresh Kumar Nov. 3, 2022, 2:37 p.m. UTC | #3 
On 03-11-22, 13:21, Colin Ian King wrote:
> Don't populate the read-only array speeds on the stack but instead
> make it static. Also makes the object code a little smaller.

How will that benefit ? I am just looking for a valid answer in commit log.

> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> ---
>  drivers/cpufreq/longhaul.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
> index 3e000e1a75c6..25f8ef7bac47 100644
> --- a/drivers/cpufreq/longhaul.c
> +++ b/drivers/cpufreq/longhaul.c
> @@ -407,7 +407,7 @@ static int guess_fsb(int mult)
>  {
>  	int speed = cpu_khz / 1000;
>  	int i;
> -	int speeds[] = { 666, 1000, 1333, 2000 };
> +	static const int speeds[] = { 666, 1000, 1333, 2000 };

Why not make it global then ?
Colin Ian King Nov. 3, 2022, 2:46 p.m. UTC | #4 
On 03/11/2022 14:37, Viresh Kumar wrote:
> On 03-11-22, 13:21, Colin Ian King wrote:
>> Don't populate the read-only array speeds on the stack but instead
>> make it static. Also makes the object code a little smaller.
> 
> How will that benefit ? I am just looking for a valid answer in commit log.

When the array is non-static there will be some executable code to put 
these values into the array that's on the stack (e.g. at run time). When 
it is static the array is filled at compile time and there is no 
executable code required to populate the array at run time.



> 
>> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
>> ---
>>   drivers/cpufreq/longhaul.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
>> index 3e000e1a75c6..25f8ef7bac47 100644
>> --- a/drivers/cpufreq/longhaul.c
>> +++ b/drivers/cpufreq/longhaul.c
>> @@ -407,7 +407,7 @@ static int guess_fsb(int mult)
>>   {
>>   	int speed = cpu_khz / 1000;
>>   	int i;
>> -	int speeds[] = { 666, 1000, 1333, 2000 };
>> +	static const int speeds[] = { 666, 1000, 1333, 2000 };
> 
> Why not make it global then ?

Making it static inside the function limits the scope to the function 
and so it's not globally visible. We don't like making stuff global 
unless we really need to.

Colin
diff mbox series

Patch

diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
index 3e000e1a75c6..25f8ef7bac47 100644
--- a/drivers/cpufreq/longhaul.c
+++ b/drivers/cpufreq/longhaul.c
@@ -407,7 +407,7 @@  static int guess_fsb(int mult)
 {
 	int speed = cpu_khz / 1000;
 	int i;
-	int speeds[] = { 666, 1000, 1333, 2000 };
+	static const int speeds[] = { 666, 1000, 1333, 2000 };
 	int f_max, f_min;
 
 	for (i = 0; i < 4; i++) {