Re: [PATCH] hwmon: emc2103: use min_t() for explicit type in fan target clamp

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Clamping the calculated or user-provided fan target to `0x1fff` overlaps with the 'disabled' state, inadvertently disabling fan RPM control for slow RPMs.
- [High] Lockless read of `data->fan_multiplier` leads to incorrect fan target scaling in `fan1_div_store`.
--

commit 78e3c12646374da73de8e3b7d0409be33a0d722f
Author: Animesh Rai <animeshrai853@gmail.com>

hwmon: emc2103: use min_t() for explicit type in fan target clamp

This commit replaces min() with min_t() when clamping the fan target in
fan1_div_store() to explicitly specify the comparison type and avoid
implicit type conversions.

> diff --git a/drivers/hwmon/emc2103.c b/drivers/hwmon/emc2103.c
> index 27dc149a3ed99..fbb1b4025eb7a 100644
> --- a/drivers/hwmon/emc2103.c
> +++ b/drivers/hwmon/emc2103.c
> @@ -348,7 +348,7 @@ static ssize_t fan1_div_store(struct device *dev, struct device_attribute *da,
>  	/* update fan target if high byte is not disabled */
>  	if ((data->fan_target & 0x1fe0) != 0x1fe0) {
>  		u16 new_target = (data->fan_target * old_div) / new_div;

[Severity: High]
This is a pre-existing issue, but couldn't reading data->fan_multiplier
locklessly to calculate old_div lead to an incorrect new_target calculation?

In fan1_div_store(), old_div is calculated as 8 / data->fan_multiplier
before acquiring data->update_lock. If two threads write to the fan1_div
sysfs attribute concurrently, won't the thread that acquires the lock
second use a stale old_div value, corrupting the hardware fan target RPM
state here?

> -		data->fan_target = min(new_target, (u16)0x1fff);
> +		data->fan_target = min_t(u16, new_target, 0x1fff);

[Severity: High]
This is a pre-existing issue, but does clamping to 0x1fff inadvertently
disable fan RPM control for slow RPMs?

Since values >= 0x1fe0 result in a high byte of 0xff, this overlaps with
the "disabled" state. When scaling the target beyond the 0x1fdf threshold,
or when a user writes a very low RPM value in fan1_target_store(), clamping
to 0x1fff instead of 0x1fdf seems to cause the fan to fall back to an
uncontrolled state rather than its minimum speed.

>  		write_fan_target_to_i2c(client, data->fan_target);
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706162519.6087-1-animeshrai853@gmail.com?part=1