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

On Mon,  3 Aug 2026 20:48:11 -0700
Guenter Roeck <[email protected]> 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 <[email protected]>
> Cc: Wilken Gottwalt <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> 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: