[PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Guenter Roeck]

In corsairpsu_linear11_to_int(), the mantissa is extracted using bitwise
operations and cast to s16 before being shifted left:

static int corsairpsu_linear11_to_int(const u16 val, const int scale)
{
    ...
    const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
    ...
}

Due to C integer promotion rules, the masked value (which is always
positive) is promoted to a 32-bit integer before the left shift. As a
result, the sign bit is never extended to bit 31 of the promoted integer.

When the device hardware reports a negative temperature in Linear11 format
(such as an ambient temperature probe reporting sub-zero), the negative
mantissa is parsed incorrectly as a massive positive value. For example,
-1 becomes 2047, which scales to 2047 degrees Celsius.

Fix the problem by type casting the result of the left shift operation
to s16.

Another problem is left-shifting of negative values. In C, the result of
left-shifting negative values is undefined. Use a multiplication instead
to avoid the problem.

Also use a local s64 variable to store temporary results, change
the return value type from int to long, and clamp the final value
to LONG_MIN and LONG_MAX to avoid under- and overflow issues while
retaining as much information as possible.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Skip handling right-shift of negative values
    (since it is widely used in the kernel)

 drivers/hwmon/corsair-psu.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
index 24100519cd83..c437b469de5c 100644
--- a/drivers/hwmon/corsair-psu.c
+++ b/drivers/hwmon/corsair-psu.c
@@ -137,13 +137,18 @@ struct corsairpsu_data {
 };
 
 /* some values are SMBus LINEAR11 data which need a conversion */
-static int corsairpsu_linear11_to_int(const u16 val, const int scale)
+static long corsairpsu_linear11_to_long(const u16 val, const int scale)
 {
 	const int exp = ((s16)val) >> 11;
-	const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
-	const int result = mant * scale;
+	const int mant = ((s16)((val & 0x7ff) << 5)) >> 5;
+	s64 result = mant * scale;
 
-	return (exp >= 0) ? (result << exp) : (result >> -exp);
+	if (exp >= 0)
+		result *= (int)(1UL << exp);
+	else
+		result >>= -exp;
+
+	return clamp(result, LONG_MIN, LONG_MAX);
 }
 
 /* the micro-controller uses percentage values to control pwm */
@@ -263,13 +268,13 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8 rail, l
 	case PSU_CMD_RAIL_AMPS:
 	case PSU_CMD_TEMP0:
 	case PSU_CMD_TEMP1:
-		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000);
+		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1000);
 		break;
 	case PSU_CMD_FAN:
-		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
+		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
 		break;
 	case PSU_CMD_FAN_PWM_ENABLE:
-		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
+		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
 		/*
 		 * 0 = automatic mode, means the micro-controller controls the fan using a plan
 		 *     which can be modified, but changing this plan is not supported by this
@@ -283,12 +288,12 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8 rail, l
 			*val = 2;
 		break;
 	case PSU_CMD_FAN_PWM:
-		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
+		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
 		*val = corsairpsu_dutycycle_to_pwm(*val);
 		break;
 	case PSU_CMD_RAIL_WATTS:
 	case PSU_CMD_TOTAL_WATTS:
-		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000000);
+		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1000000);
 		break;
 	case PSU_CMD_TOTAL_UPTIME:
 	case PSU_CMD_UPTIME:
-- 
2.45.2

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[sashiko-bot]

> In corsairpsu_linear11_to_int(), the mantissa is extracted using bitwise
> operations and cast to s16 before being shifted left:
> 
> static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> {
>     ...
>     const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
>     ...
> }
> 
> Due to C integer promotion rules, the masked value (which is always
> positive) is promoted to a 32-bit integer before the left shift. As a
> result, the sign bit is never extended to bit 31 of the promoted integer.
> 
> When the device hardware reports a negative temperature in Linear11 format
> [ ... ]
> retaining as much information as possible.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804034811.2385506-1-linux@roeck-us.net?part=1

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Wilken Gottwalt]

On Mon,  3 Aug 2026 20:48:11 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> In corsairpsu_linear11_to_int(), the mantissa is extracted using bitwise
> operations and cast to s16 before being shifted left:
> 
> static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> {
>     ...
>     const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
>     ...
> }
> 
> Due to C integer promotion rules, the masked value (which is always
> positive) is promoted to a 32-bit integer before the left shift. As a
> result, the sign bit is never extended to bit 31 of the promoted integer.
> 
> When the device hardware reports a negative temperature in Linear11 format
> (such as an ambient temperature probe reporting sub-zero), the negative
> mantissa is parsed incorrectly as a massive positive value. For example,
> -1 becomes 2047, which scales to 2047 degrees Celsius.
> 
> Fix the problem by type casting the result of the left shift operation
> to s16.
> 
> Another problem is left-shifting of negative values. In C, the result of
> left-shifting negative values is undefined. Use a multiplication instead
> to avoid the problem.
> 
> Also use a local s64 variable to store temporary results, change
> the return value type from int to long, and clamp the final value
> to LONG_MIN and LONG_MAX to avoid under- and overflow issues while
> retaining as much information as possible.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Skip handling right-shift of negative values
>     (since it is widely used in the kernel)
> 
>  drivers/hwmon/corsair-psu.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index 24100519cd83..c437b469de5c 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -137,13 +137,18 @@ struct corsairpsu_data {
>  };
>  
>  /* some values are SMBus LINEAR11 data which need a conversion */
> -static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> +static long corsairpsu_linear11_to_long(const u16 val, const int scale)
>  {
>  	const int exp = ((s16)val) >> 11;
> -	const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
> -	const int result = mant * scale;
> +	const int mant = ((s16)((val & 0x7ff) << 5)) >> 5;


> +	s64 result = mant * scale;

Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
in our company. https://godbolt.org/z/eM6TbGG5E

> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
> +	if (exp >= 0)
> +		result *= (int)(1UL << exp);
> +	else
> +		result >>= -exp;
> +
> +	return clamp(result, LONG_MIN, LONG_MAX);
>  }
>  
>  /* the micro-controller uses percentage values to control pwm */
> @@ -263,13 +268,13 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8
> rail, l case PSU_CMD_RAIL_AMPS:
>  	case PSU_CMD_TEMP0:
>  	case PSU_CMD_TEMP1:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1000);
>  		break;
>  	case PSU_CMD_FAN:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
>  		break;
>  	case PSU_CMD_FAN_PWM_ENABLE:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
>  		/*
>  		 * 0 = automatic mode, means the micro-controller controls the fan using a plan
>  		 *     which can be modified, but changing this plan is not supported by this
> @@ -283,12 +288,12 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8
> rail, l *val = 2;
>  		break;
>  	case PSU_CMD_FAN_PWM:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
>  		*val = corsairpsu_dutycycle_to_pwm(*val);
>  		break;
>  	case PSU_CMD_RAIL_WATTS:
>  	case PSU_CMD_TOTAL_WATTS:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000000);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1000000);
>  		break;
>  	case PSU_CMD_TOTAL_UPTIME:
>  	case PSU_CMD_UPTIME:

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Guenter Roeck]

On 8/6/26 08:34, Wilken Gottwalt wrote:
> On Mon,  3 Aug 2026 20:48:11 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
>> In corsairpsu_linear11_to_int(), the mantissa is extracted using bitwise
>> operations and cast to s16 before being shifted left:
>>
>> static int corsairpsu_linear11_to_int(const u16 val, const int scale)
>> {
>>      ...
>>      const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
>>      ...
>> }
>>
>> Due to C integer promotion rules, the masked value (which is always
>> positive) is promoted to a 32-bit integer before the left shift. As a
>> result, the sign bit is never extended to bit 31 of the promoted integer.
>>
>> When the device hardware reports a negative temperature in Linear11 format
>> (such as an ambient temperature probe reporting sub-zero), the negative
>> mantissa is parsed incorrectly as a massive positive value. For example,
>> -1 becomes 2047, which scales to 2047 degrees Celsius.
>>
>> Fix the problem by type casting the result of the left shift operation
>> to s16.
>>
>> Another problem is left-shifting of negative values. In C, the result of
>> left-shifting negative values is undefined. Use a multiplication instead
>> to avoid the problem.
>>
>> Also use a local s64 variable to store temporary results, change
>> the return value type from int to long, and clamp the final value
>> to LONG_MIN and LONG_MAX to avoid under- and overflow issues while
>> retaining as much information as possible.
>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> v2: Skip handling right-shift of negative values
>>      (since it is widely used in the kernel)
>>
>>   drivers/hwmon/corsair-psu.c | 23 ++++++++++++++---------
>>   1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
>> index 24100519cd83..c437b469de5c 100644
>> --- a/drivers/hwmon/corsair-psu.c
>> +++ b/drivers/hwmon/corsair-psu.c
>> @@ -137,13 +137,18 @@ struct corsairpsu_data {
>>   };
>>   
>>   /* some values are SMBus LINEAR11 data which need a conversion */
>> -static int corsairpsu_linear11_to_int(const u16 val, const int scale)
>> +static long corsairpsu_linear11_to_long(const u16 val, const int scale)
>>   {
>>   	const int exp = ((s16)val) >> 11;
>> -	const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
>> -	const int result = mant * scale;
>> +	const int mant = ((s16)((val & 0x7ff) << 5)) >> 5;
> 
> 
>> +	s64 result = mant * scale;
> 
> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
> in our company. https://godbolt.org/z/eM6TbGG5E
> 

It is, but that is ok and intentional: both mant and exp are guaranteed to be
no larger than s16, meaning the result is never larger than s32 and will never
overflow.

>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
>> +	if (exp >= 0)
>> +		result *= (int)(1UL << exp);

This is the calculation that can overflow, making it necessary for result to be s64.

Thanks,
Guenter

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Wilken Gottwalt]

On Thu, 6 Aug 2026 08:52:41 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 8/6/26 08:34, Wilken Gottwalt wrote:
> > On Mon,  3 Aug 2026 20:48:11 -0700
> > Guenter Roeck <linux@roeck-us.net> wrote:
> > 
> >> In corsairpsu_linear11_to_int(), the mantissa is extracted using bitwise
> >> operations and cast to s16 before being shifted left:
> >>
> >> static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> >> {
> >>      ...
> >>      const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
> >>      ...
> >> }
> >>
> >> Due to C integer promotion rules, the masked value (which is always
> >> positive) is promoted to a 32-bit integer before the left shift. As a
> >> result, the sign bit is never extended to bit 31 of the promoted integer.
> >>
> >> When the device hardware reports a negative temperature in Linear11 format
> >> (such as an ambient temperature probe reporting sub-zero), the negative
> >> mantissa is parsed incorrectly as a massive positive value. For example,
> >> -1 becomes 2047, which scales to 2047 degrees Celsius.
> >>
> >> Fix the problem by type casting the result of the left shift operation
> >> to s16.
> >>
> >> Another problem is left-shifting of negative values. In C, the result of
> >> left-shifting negative values is undefined. Use a multiplication instead
> >> to avoid the problem.
> >>
> >> Also use a local s64 variable to store temporary results, change
> >> the return value type from int to long, and clamp the final value
> >> to LONG_MIN and LONG_MAX to avoid under- and overflow issues while
> >> retaining as much information as possible.
> >>
> >> Reported-by: Sashiko <sashiko-bot@kernel.org>
> >> Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >> ---
> >> v2: Skip handling right-shift of negative values
> >>      (since it is widely used in the kernel)
> >>
> >>   drivers/hwmon/corsair-psu.c | 23 ++++++++++++++---------
> >>   1 file changed, 14 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> >> index 24100519cd83..c437b469de5c 100644
> >> --- a/drivers/hwmon/corsair-psu.c
> >> +++ b/drivers/hwmon/corsair-psu.c
> >> @@ -137,13 +137,18 @@ struct corsairpsu_data {
> >>   };
> >>   
> >>   /* some values are SMBus LINEAR11 data which need a conversion */
> >> -static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> >> +static long corsairpsu_linear11_to_long(const u16 val, const int scale)
> >>   {
> >>   	const int exp = ((s16)val) >> 11;
> >> -	const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
> >> -	const int result = mant * scale;
> >> +	const int mant = ((s16)((val & 0x7ff) << 5)) >> 5;
> > 
> > 
> >> +	s64 result = mant * scale;
> > 
> > Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
> > in our company. https://godbolt.org/z/eM6TbGG5E
> > 
> 
> It is, but that is ok and intentional: both mant and exp are guaranteed to be
> no larger than s16, meaning the result is never larger than s32 and will never
> overflow.
> 
> >> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
> >> +	if (exp >= 0)
> >> +		result *= (int)(1UL << exp);
> 
> This is the calculation that can overflow, making it necessary for result to be s64.

Yeah, it was just funny to see in the wild. It made my day. :D

greetings,
Wilken

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Guenter Roeck]

On 8/6/26 08:56, Wilken Gottwalt wrote:
...
>>>> +	s64 result = mant * scale;
>>>
>>> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
>>> in our company. https://godbolt.org/z/eM6TbGG5E
>>>
>>
>> It is, but that is ok and intentional: both mant and exp are guaranteed to be
>> no larger than s16, meaning the result is never larger than s32 and will never
>> overflow.
>>
>>>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
>>>> +	if (exp >= 0)
>>>> +		result *= (int)(1UL << exp);
>>
>> This is the calculation that can overflow, making it necessary for result to be s64.
> 
> Yeah, it was just funny to see in the wild. It made my day. :D
> 

Guess I lost you there. Do you want me to change it ? I could add a comment, or just
type cast mant to s64. Please let me know.

Thanks,
Guenter

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Wilken Gottwalt]

On Thu, 6 Aug 2026 09:28:34 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 8/6/26 08:56, Wilken Gottwalt wrote:
> ...
> >>>> +	s64 result = mant * scale;
> >>>
> >>> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
> >>> in our company. https://godbolt.org/z/eM6TbGG5E
> >>>
> >>
> >> It is, but that is ok and intentional: both mant and exp are guaranteed to be
> >> no larger than s16, meaning the result is never larger than s32 and will never
> >> overflow.
> >>
> >>>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
> >>>> +	if (exp >= 0)
> >>>> +		result *= (int)(1UL << exp);
> >>
> >> This is the calculation that can overflow, making it necessary for result to be s64.
> > 
> > Yeah, it was just funny to see in the wild. It made my day. :D
> > 
> 
> Guess I lost you there. Do you want me to change it ? I could add a comment, or just
> type cast mant to s64. Please let me know.

No no, it is fine. I just was surprised to see that famous gotcha. But I guess
some AIs may jump on it, ignoring the context. Changing it maybe would prevent
noise in the future. It is like that famous mathematical "iff" term, which is
used in some places of the kernel. A lot of non-native English speakers report
that as a typo. Ahh... just ignore my gibberish.

greetings,
Wilken

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Guenter Roeck]

On 8/6/26 09:42, Wilken Gottwalt wrote:
> On Thu, 6 Aug 2026 09:28:34 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
>> On 8/6/26 08:56, Wilken Gottwalt wrote:
>> ...
>>>>>> +	s64 result = mant * scale;
>>>>>
>>>>> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
>>>>> in our company. https://godbolt.org/z/eM6TbGG5E
>>>>>
>>>>
>>>> It is, but that is ok and intentional: both mant and exp are guaranteed to be
>>>> no larger than s16, meaning the result is never larger than s32 and will never
>>>> overflow.
>>>>
>>>>>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
>>>>>> +	if (exp >= 0)
>>>>>> +		result *= (int)(1UL << exp);
>>>>
>>>> This is the calculation that can overflow, making it necessary for result to be s64.
>>>
>>> Yeah, it was just funny to see in the wild. It made my day. :D
>>>
>>
>> Guess I lost you there. Do you want me to change it ? I could add a comment, or just
>> type cast mant to s64. Please let me know.
> 
> No no, it is fine. I just was surprised to see that famous gotcha. But I guess
> some AIs may jump on it, ignoring the context. Changing it maybe would prevent
> noise in the future. It is like that famous mathematical "iff" term, which is

FWIW, one could argue that it is often misused in the Linux kernel.

> used in some places of the kernel. A lot of non-native English speakers report
> that as a typo. Ahh... just ignore my gibberish.
> 

Actually, any reasonable AI should be able to find that this isn't a problem
given the constraints. Sashiko didn't flag it, and if it did I'd have submitted
a prompt update. Sashiko is explicitly directed to check for over- and underflows
in hwmon patches because it happens so often, so I am (reasonably ;-) sure that it
would flag it if there was a problem.

If you are ok with the patch, any chance for a Tested-by/Acked-by/Reviewed-by ?

Thanks,
Guenter

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Wilken Gottwalt]

On Thu, 6 Aug 2026 11:04:50 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 8/6/26 09:42, Wilken Gottwalt wrote:
> > On Thu, 6 Aug 2026 09:28:34 -0700
> > Guenter Roeck <linux@roeck-us.net> wrote:
> > 
> >> On 8/6/26 08:56, Wilken Gottwalt wrote:
> >> ...
> >>>>>> +	s64 result = mant * scale;
> >>>>>
> >>>>> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
> >>>>> in our company. https://godbolt.org/z/eM6TbGG5E
> >>>>>
> >>>>
> >>>> It is, but that is ok and intentional: both mant and exp are guaranteed to be
> >>>> no larger than s16, meaning the result is never larger than s32 and will never
> >>>> overflow.
> >>>>
> >>>>>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
> >>>>>> +	if (exp >= 0)
> >>>>>> +		result *= (int)(1UL << exp);
> >>>>
> >>>> This is the calculation that can overflow, making it necessary for result to be s64.
> >>>
> >>> Yeah, it was just funny to see in the wild. It made my day. :D
> >>>
> >>
> >> Guess I lost you there. Do you want me to change it ? I could add a comment, or just
> >> type cast mant to s64. Please let me know.
> > 
> > No no, it is fine. I just was surprised to see that famous gotcha. But I guess
> > some AIs may jump on it, ignoring the context. Changing it maybe would prevent
> > noise in the future. It is like that famous mathematical "iff" term, which is
> 
> FWIW, one could argue that it is often misused in the Linux kernel.

I actually don't know. I'm one of the people who considered it a typo. :D

> > used in some places of the kernel. A lot of non-native English speakers report
> > that as a typo. Ahh... just ignore my gibberish.
> > 
> 
> Actually, any reasonable AI should be able to find that this isn't a problem
> given the constraints. Sashiko didn't flag it, and if it did I'd have submitted
> a prompt update. Sashiko is explicitly directed to check for over- and underflows
> in hwmon patches because it happens so often, so I am (reasonably ;-) sure that it
> would flag it if there was a problem.

Yeah, a reasonable AI... So, I guess we will find out in the future.

> If you are ok with the patch, any chance for a Tested-by/Acked-by/Reviewed-by ?

Sorry, I did not test it yet, but I will tomorrow.

Hmm, I really wonder if something will actually change. I never encountered odd
values. Well, except for the really obvious ones. Picking every value one by one
results in every value being from a different sample. I think internally the MCU
samples with about 1000 Hz. But getting that right would be some serious work.

greetings,
Wilken

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Guenter Roeck]

On 8/6/26 11:28, Wilken Gottwalt wrote:
> On Thu, 6 Aug 2026 11:04:50 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
>> On 8/6/26 09:42, Wilken Gottwalt wrote:
>>> On Thu, 6 Aug 2026 09:28:34 -0700
>>> Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>>> On 8/6/26 08:56, Wilken Gottwalt wrote:
>>>> ...
>>>>>>>> +	s64 result = mant * scale;
>>>>>>>
>>>>>>> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
>>>>>>> in our company. https://godbolt.org/z/eM6TbGG5E
>>>>>>>
>>>>>>
>>>>>> It is, but that is ok and intentional: both mant and exp are guaranteed to be
>>>>>> no larger than s16, meaning the result is never larger than s32 and will never
>>>>>> overflow.
>>>>>>
>>>>>>>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
>>>>>>>> +	if (exp >= 0)
>>>>>>>> +		result *= (int)(1UL << exp);
>>>>>>
>>>>>> This is the calculation that can overflow, making it necessary for result to be s64.
>>>>>
>>>>> Yeah, it was just funny to see in the wild. It made my day. :D
>>>>>
>>>>
>>>> Guess I lost you there. Do you want me to change it ? I could add a comment, or just
>>>> type cast mant to s64. Please let me know.
>>>
>>> No no, it is fine. I just was surprised to see that famous gotcha. But I guess
>>> some AIs may jump on it, ignoring the context. Changing it maybe would prevent
>>> noise in the future. It is like that famous mathematical "iff" term, which is
>>
>> FWIW, one could argue that it is often misused in the Linux kernel.
> 
> I actually don't know. I'm one of the people who considered it a typo. :D
> 
>>> used in some places of the kernel. A lot of non-native English speakers report
>>> that as a typo. Ahh... just ignore my gibberish.
>>>
>>
>> Actually, any reasonable AI should be able to find that this isn't a problem
>> given the constraints. Sashiko didn't flag it, and if it did I'd have submitted
>> a prompt update. Sashiko is explicitly directed to check for over- and underflows
>> in hwmon patches because it happens so often, so I am (reasonably ;-) sure that it
>> would flag it if there was a problem.
> 
> Yeah, a reasonable AI... So, I guess we will find out in the future.
> 
>> If you are ok with the patch, any chance for a Tested-by/Acked-by/Reviewed-by ?
> 
> Sorry, I did not test it yet, but I will tomorrow.
> 
> Hmm, I really wonder if something will actually change. I never encountered odd
> values. Well, except for the really obvious ones. Picking every value one by one
> results in every value being from a different sample. I think internally the MCU
> samples with about 1000 Hz. But getting that right would be some serious work.
> 

You probably won't see a difference. This is about the possibility of encountering
issues, after all, not about something that is actually seen (unless you can manage
to run the system in freezing temperatures ...). All we can do here is to ensure that
there is no subtle regression under normal operation.

In the I2C world I have test scripts which can simulate borderline conditions (such
as negative temperatures), but for USB that would require something more sophisticated
than the i2c-stub driver.

Guenter

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Wilken Gottwalt]

On Mon,  3 Aug 2026 20:48:11 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> In corsairpsu_linear11_to_int(), the mantissa is extracted using bitwise
> operations and cast to s16 before being shifted left:
> 
> static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> {
>     ...
>     const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
>     ...
> }
> 
> Due to C integer promotion rules, the masked value (which is always
> positive) is promoted to a 32-bit integer before the left shift. As a
> result, the sign bit is never extended to bit 31 of the promoted integer.
> 
> When the device hardware reports a negative temperature in Linear11 format
> (such as an ambient temperature probe reporting sub-zero), the negative
> mantissa is parsed incorrectly as a massive positive value. For example,
> -1 becomes 2047, which scales to 2047 degrees Celsius.
> 
> Fix the problem by type casting the result of the left shift operation
> to s16.
> 
> Another problem is left-shifting of negative values. In C, the result of
> left-shifting negative values is undefined. Use a multiplication instead
> to avoid the problem.
> 
> Also use a local s64 variable to store temporary results, change
> the return value type from int to long, and clamp the final value
> to LONG_MIN and LONG_MAX to avoid under- and overflow issues while
> retaining as much information as possible.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Skip handling right-shift of negative values
>     (since it is widely used in the kernel)
> 
>  drivers/hwmon/corsair-psu.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index 24100519cd83..c437b469de5c 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -137,13 +137,18 @@ struct corsairpsu_data {
>  };
>  
>  /* some values are SMBus LINEAR11 data which need a conversion */
> -static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> +static long corsairpsu_linear11_to_long(const u16 val, const int scale)
>  {
>  	const int exp = ((s16)val) >> 11;
> -	const int mant = (((s16)(val & 0x7ff)) << 5) >> 5;
> -	const int result = mant * scale;
> +	const int mant = ((s16)((val & 0x7ff) << 5)) >> 5;
> +	s64 result = mant * scale;
>  
> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
> +	if (exp >= 0)
> +		result *= (int)(1UL << exp);
> +	else
> +		result >>= -exp;
> +
> +	return clamp(result, LONG_MIN, LONG_MAX);
>  }
>  
>  /* the micro-controller uses percentage values to control pwm */
> @@ -263,13 +268,13 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8
> rail, l case PSU_CMD_RAIL_AMPS:
>  	case PSU_CMD_TEMP0:
>  	case PSU_CMD_TEMP1:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1000);
>  		break;
>  	case PSU_CMD_FAN:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
>  		break;
>  	case PSU_CMD_FAN_PWM_ENABLE:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
>  		/*
>  		 * 0 = automatic mode, means the micro-controller controls the fan using a plan
>  		 *     which can be modified, but changing this plan is not supported by this
> @@ -283,12 +288,12 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8
> rail, l *val = 2;
>  		break;
>  	case PSU_CMD_FAN_PWM:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1);
>  		*val = corsairpsu_dutycycle_to_pwm(*val);
>  		break;
>  	case PSU_CMD_RAIL_WATTS:
>  	case PSU_CMD_TOTAL_WATTS:
> -		*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000000);
> +		*val = corsairpsu_linear11_to_long(tmp & 0xFFFF, 1000000);
>  		break;
>  	case PSU_CMD_TOTAL_UPTIME:
>  	case PSU_CMD_UPTIME:

Don't see any anomalies here, so the change is fine. Though, my test is limited,
I can not simulate negative temperatures to fully test this. But that should not
matter, because the allowed continuous operating temperature is 0°C - 50°C anyway.

Tested-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>

greetings,
Wilken

Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

[Guenter Roeck]

On 8/6/26 21:26, Wilken Gottwalt wrote:
...
> Don't see any anomalies here, so the change is fine. Though, my test is limited,
> I can not simulate negative temperatures to fully test this. But that should not
> matter, because the allowed continuous operating temperature is 0°C - 50°C anyway.
> 
> Tested-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> 

Thanks a lot!

Guenter