Re: [RFT PATCH 1/3] hwmon: (nct6775) Fix target temp and tolerance for NCT6106/6116/6126

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] hwmon: (nct6775) REG_TARGET marked word-sized in
  nct6775_reg_is_word_sized()
--

commit 7460f66a45ab993c8409879e58f676a24f6e5ad1
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (nct6775) Fix target temp and tolerance for NCT6106/6116/6126

This commit initializes target_temp_mask, tolerance_mask, and
speed_tolerance_limit for NCT6106, NCT6116, and NCT6126 in nct6775_probe().

Link: https://sashiko.dev/#/bug/linux-76f191d0-4c79-4982-887e-e5c9a2ed7136

> diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
> index 6e97e4f08b90b..9340525689cff 100644
> --- a/drivers/hwmon/nct6775-core.c
> +++ b/drivers/hwmon/nct6775-core.c
> @@ -3574,6 +3574,9 @@ int nct6775_probe(struct device *dev, struct nct6775_data *data,
>  
>  		data->fan_from_reg = fan_from_reg13;
>  		data->fan_from_reg_min = fan_from_reg13;
> +		data->target_temp_mask = 0xff;
> +		data->tolerance_mask = 0x07;
> +		data->speed_tolerance_limit = 63;

[Severity: High]
Does setting target_temp_mask to 0xff expose that REG_TARGET (0x111, 0x121,
0x131, and 0x191, 0x1a1 on nct6116/nct6126) is marked word-sized in
nct6775_reg_is_word_sized()?

drivers/hwmon/nct6775-core.c:nct6775_reg_is_word_sized() {
	...
	case nct6106:
		return reg == 0x20 || reg == 0x22 || reg == 0x24 ||
		  (reg >= 0x59 && reg < 0x69 && (reg & 1)) ||
		  reg == 0xe0 || reg == 0xe2 || reg == 0xe4 ||
		  reg == 0x111 || reg == 0x121 || reg == 0x131;
	case nct6116:
	case nct6126:
		return reg == 0x20 || reg == 0x22 || reg == 0x24 ||
		  reg == 0x26 || reg == 0x28 || reg == 0x59 || reg == 0x5b ||
		  reg == 0xe0 || reg == 0xe2 || reg == 0xe4 || reg == 0xe6 ||
		  reg == 0xe8 || reg == 0x111 || reg == 0x121 || reg == 0x131 ||
		  reg == 0x191 || reg == 0x1a1;
	...
}

Because nct6775_reg_is_word_sized() returns true for REG_TARGET, a read via
nct6775_read_value() performs a 16-bit big-endian read across REG_TARGET
(placed in bits 15:8) and the adjacent REG_TOLERANCE_H at reg + 1 (placed in
bits 7:0).  In nct6775_update_pwm_limits(), masking reg_t with
target_temp_mask (0xff) then discards REG_TARGET and stores REG_TOLERANCE_H
into data->target_temp[i], while also corrupting data->target_speed[i] in
speed_cruise mode.

When writing pwm*_target_temp in thermal_cruise mode, pwm_update_registers()
passes the 8-bit data->target_temp[nr] to nct6775_write_value(), which writes
0x00 (value >> 8) to REG_TARGET and overwrites REG_TOLERANCE_H with
target_temp[nr].  Should 0x111, 0x121, 0x131, 0x191, and 0x1a1 be removed from
nct6775_reg_is_word_sized()?

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