* [PATCH] cpufreq: Use int type to store negative error codes
@ 2025-08-29 8:44 Qianfeng Rong
2025-08-29 16:52 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Qianfeng Rong @ 2025-08-29 8:44 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, linux-pm, linux-kernel; +Cc: Qianfeng Rong
Change the 'ret' variable from unsigned int to int to store negative error
codes directly or returned by other functions. Change the return type of
the speedstep_get_freqs() function from unsigned int to int as well.
Storing the negative error codes in unsigned type, doesn't cause an issue
at runtime but it's ugly as pants. Additionally, assigning negative error
codes to unsigned type may trigger a GCC warning when the -Wsign-conversion
flag is enabled.
No effect on runtime.
Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
---
drivers/cpufreq/cpufreq.c | 2 +-
drivers/cpufreq/powernow-k7.c | 2 +-
drivers/cpufreq/speedstep-lib.c | 12 ++++++------
drivers/cpufreq/speedstep-lib.h | 10 +++++-----
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a615c98d80ca..f47096683abb 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -914,7 +914,7 @@ static ssize_t store_scaling_setspeed(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
unsigned int freq = 0;
- unsigned int ret;
+ int ret;
if (!policy->governor || !policy->governor->store_setspeed)
return -EINVAL;
diff --git a/drivers/cpufreq/powernow-k7.c b/drivers/cpufreq/powernow-k7.c
index 31039330a3ba..88616cd14353 100644
--- a/drivers/cpufreq/powernow-k7.c
+++ b/drivers/cpufreq/powernow-k7.c
@@ -451,7 +451,7 @@ static int powernow_decode_bios(int maxfid, int startvid)
unsigned int i, j;
unsigned char *p;
unsigned int etuple;
- unsigned int ret;
+ int ret;
etuple = cpuid_eax(0x80000001);
diff --git a/drivers/cpufreq/speedstep-lib.c b/drivers/cpufreq/speedstep-lib.c
index 0b66df4ed513..f8b42e981635 100644
--- a/drivers/cpufreq/speedstep-lib.c
+++ b/drivers/cpufreq/speedstep-lib.c
@@ -378,16 +378,16 @@ EXPORT_SYMBOL_GPL(speedstep_detect_processor);
* DETECT SPEEDSTEP SPEEDS *
*********************************************************************/
-unsigned int speedstep_get_freqs(enum speedstep_processor processor,
- unsigned int *low_speed,
- unsigned int *high_speed,
- unsigned int *transition_latency,
- void (*set_state) (unsigned int state))
+int speedstep_get_freqs(enum speedstep_processor processor,
+ unsigned int *low_speed,
+ unsigned int *high_speed,
+ unsigned int *transition_latency,
+ void (*set_state)(unsigned int state))
{
unsigned int prev_speed;
- unsigned int ret = 0;
unsigned long flags;
ktime_t tv1, tv2;
+ int ret = 0;
if ((!processor) || (!low_speed) || (!high_speed) || (!set_state))
return -EINVAL;
diff --git a/drivers/cpufreq/speedstep-lib.h b/drivers/cpufreq/speedstep-lib.h
index dc762ea786be..48329647d4c4 100644
--- a/drivers/cpufreq/speedstep-lib.h
+++ b/drivers/cpufreq/speedstep-lib.h
@@ -41,8 +41,8 @@ extern unsigned int speedstep_get_frequency(enum speedstep_processor processor);
* SPEEDSTEP_LOW; the second argument is zero so that no
* cpufreq_notify_transition calls are initiated.
*/
-extern unsigned int speedstep_get_freqs(enum speedstep_processor processor,
- unsigned int *low_speed,
- unsigned int *high_speed,
- unsigned int *transition_latency,
- void (*set_state) (unsigned int state));
+extern int speedstep_get_freqs(enum speedstep_processor processor,
+ unsigned int *low_speed,
+ unsigned int *high_speed,
+ unsigned int *transition_latency,
+ void (*set_state)(unsigned int state));
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cpufreq: Use int type to store negative error codes
2025-08-29 8:44 [PATCH] cpufreq: Use int type to store negative error codes Qianfeng Rong
@ 2025-08-29 16:52 ` Rafael J. Wysocki
2025-08-30 7:32 ` Qianfeng Rong
0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2025-08-29 16:52 UTC (permalink / raw)
To: Qianfeng Rong; +Cc: Rafael J. Wysocki, Viresh Kumar, linux-pm, linux-kernel
On Fri, Aug 29, 2025 at 10:44 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote:
>
> Change the 'ret' variable from unsigned int to int to store negative error
> codes directly or returned by other functions.
You need to say upfront that this is about speedstep_get_freqs() specifically.
> Change the return type of
> the speedstep_get_freqs() function from unsigned int to int as well.
>
> Storing the negative error codes in unsigned type, doesn't cause an issue
> at runtime but it's ugly as pants.
Which isn't really a technical term.
> Additionally, assigning negative error codes to unsigned type may trigger a GCC warning
> when the -Wsign-conversion flag is enabled.
Is the latter a motivation for this change?
> No effect on runtime.
>
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> ---
> drivers/cpufreq/cpufreq.c | 2 +-
> drivers/cpufreq/powernow-k7.c | 2 +-
> drivers/cpufreq/speedstep-lib.c | 12 ++++++------
> drivers/cpufreq/speedstep-lib.h | 10 +++++-----
> 4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index a615c98d80ca..f47096683abb 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -914,7 +914,7 @@ static ssize_t store_scaling_setspeed(struct cpufreq_policy *policy,
> const char *buf, size_t count)
> {
> unsigned int freq = 0;
> - unsigned int ret;
> + int ret;
>
> if (!policy->governor || !policy->governor->store_setspeed)
> return -EINVAL;
> diff --git a/drivers/cpufreq/powernow-k7.c b/drivers/cpufreq/powernow-k7.c
> index 31039330a3ba..88616cd14353 100644
> --- a/drivers/cpufreq/powernow-k7.c
> +++ b/drivers/cpufreq/powernow-k7.c
> @@ -451,7 +451,7 @@ static int powernow_decode_bios(int maxfid, int startvid)
> unsigned int i, j;
> unsigned char *p;
> unsigned int etuple;
> - unsigned int ret;
> + int ret;
>
> etuple = cpuid_eax(0x80000001);
>
> diff --git a/drivers/cpufreq/speedstep-lib.c b/drivers/cpufreq/speedstep-lib.c
> index 0b66df4ed513..f8b42e981635 100644
> --- a/drivers/cpufreq/speedstep-lib.c
> +++ b/drivers/cpufreq/speedstep-lib.c
> @@ -378,16 +378,16 @@ EXPORT_SYMBOL_GPL(speedstep_detect_processor);
> * DETECT SPEEDSTEP SPEEDS *
> *********************************************************************/
>
> -unsigned int speedstep_get_freqs(enum speedstep_processor processor,
> - unsigned int *low_speed,
> - unsigned int *high_speed,
> - unsigned int *transition_latency,
> - void (*set_state) (unsigned int state))
> +int speedstep_get_freqs(enum speedstep_processor processor,
> + unsigned int *low_speed,
> + unsigned int *high_speed,
> + unsigned int *transition_latency,
> + void (*set_state)(unsigned int state))
> {
> unsigned int prev_speed;
> - unsigned int ret = 0;
> unsigned long flags;
> ktime_t tv1, tv2;
> + int ret = 0;
>
> if ((!processor) || (!low_speed) || (!high_speed) || (!set_state))
> return -EINVAL;
> diff --git a/drivers/cpufreq/speedstep-lib.h b/drivers/cpufreq/speedstep-lib.h
> index dc762ea786be..48329647d4c4 100644
> --- a/drivers/cpufreq/speedstep-lib.h
> +++ b/drivers/cpufreq/speedstep-lib.h
> @@ -41,8 +41,8 @@ extern unsigned int speedstep_get_frequency(enum speedstep_processor processor);
> * SPEEDSTEP_LOW; the second argument is zero so that no
> * cpufreq_notify_transition calls are initiated.
> */
> -extern unsigned int speedstep_get_freqs(enum speedstep_processor processor,
> - unsigned int *low_speed,
> - unsigned int *high_speed,
> - unsigned int *transition_latency,
> - void (*set_state) (unsigned int state));
> +extern int speedstep_get_freqs(enum speedstep_processor processor,
> + unsigned int *low_speed,
> + unsigned int *high_speed,
> + unsigned int *transition_latency,
> + void (*set_state)(unsigned int state));
> --
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cpufreq: Use int type to store negative error codes
2025-08-29 16:52 ` Rafael J. Wysocki
@ 2025-08-30 7:32 ` Qianfeng Rong
0 siblings, 0 replies; 3+ messages in thread
From: Qianfeng Rong @ 2025-08-30 7:32 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Viresh Kumar, linux-pm, linux-kernel
在 2025/8/30 0:52, Rafael J. Wysocki 写道:
> On Fri, Aug 29, 2025 at 10:44 AM Qianfeng Rong <rongqianfeng@vivo.com> wrote:
>> Change the 'ret' variable from unsigned int to int to store negative error
>> codes directly or returned by other functions.
> You need to say upfront that this is about speedstep_get_freqs() specifically.
Yes, my commit message was a little unclear. I will improve it.
>
>> Change the return type of
>> the speedstep_get_freqs() function from unsigned int to int as well.
>>
>> Storing the negative error codes in unsigned type, doesn't cause an issue
>> at runtime but it's ugly as pants.
> Which isn't really a technical term.
Yes, I should have used more normal words.
>
>> Additionally, assigning negative error codes to unsigned type may trigger a GCC warning
>> when the -Wsign-conversion flag is enabled.
> Is the latter a motivation for this change?
That's one aspect. On the other hand, I think code that assigns negative
values to an unsigned type is confusing, so I want to fix such code.
Best regards, Qianfeng
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-30 7:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 8:44 [PATCH] cpufreq: Use int type to store negative error codes Qianfeng Rong
2025-08-29 16:52 ` Rafael J. Wysocki
2025-08-30 7:32 ` Qianfeng Rong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).