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

On 8/3/26 17:15, Ali Ahmet Memis wrote:
> On Mon,  3 Aug 2026 16:21:55 -0700 Guenter Roeck wrote:
>> +	if (exp >= 0)
>> +		result *= (int)(1UL << exp);
>> +	else
>> +		result /= (int)(1UL << -exp);
> 
> This breaks the build on 32 bit. result is s64 and the divisor is only known
> at runtime, so the compiler emits a libgcc call instead of a shift, and
> __divdi3 is only provided by sparc and parisc. SENSORS_CORSAIR_PSU just
> depends on HID, so i386 and 32 bit arm are reachable.
> 
> On this patch applied to 2d2338c93da7:
> 
>    $ make ARCH=i386 defconfig
>    $ ./scripts/config --set-val CONFIG_SENSORS_CORSAIR_PSU y
>    $ make ARCH=i386 olddefconfig
>    $ make ARCH=i386 vmlinux
>    [...]
>      LD      .tmp_vmlinux1
>    ld: drivers/hwmon/corsair-psu.o: in function `corsairpsu_get_value':
>    corsair-psu.c:(.text+0xa0b): undefined reference to `__divdi3'
>    ld: corsair-psu.c:(.text+0xa63): undefined reference to `__divdi3'
>    ld: corsair-psu.c:(.text+0xa83): undefined reference to `__divdi3'
>    ld: corsair-psu.c:(.text+0xaa3): undefined reference to `__divdi3'
>    make[2]: *** [scripts/Makefile.vmlinux:72: vmlinux.unstripped] Error 1
> 
> Only the divide is a problem, the multiply builds fine. div_s64() would do
> it, or keeping a shift on the negative branch, since the divisor is a power
> of two anyway.
> 
> The mantissa fix itself looks right to me. ((s16)((val & 0x7ff) << 5)) >> 5
> puts bit 10 of the mantissa into the sign bit of the s16 before the
> arithmetic shift brings it back, which is what the old order failed to do.
> 
> One small thing on the changelog: it says right shifting negative values is
> implementation defined and that multiply and divide are used to avoid that,
> but exp and mant are still computed with >> on values that can be negative.
> That is the normal sign extension idiom and fine in practice, it just reads
> as if those had been converted too.

Good point. Supposedly is implementation defined, but even sign_extend32()
uses it, so I'll just undo that part.

Thanks,
Guenter