Re: [hwmon PATCH v2 4/8] hwmon: (adt7470) Fix swapped PWM3 and PWM4 auto mode masks

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is an invalid bitwise index check in `hwmon_temp_alarm` leading to incorrect or missing temperature alarms.
- [High] This problem wasn't introduced by this patch, but a typo in `pwm_auto_temp_store` mistakenly assigns to `data->pwm_automatic` instead of `data->pwm_auto_temp`, corrupting driver cache state.
--

--- Patch [4]: [hwmon PATCH v2 4/8] hwmon: (adt7470) Fix swapped PWM3 and PWM4 auto mode masks ---
commit 5ed1ddc6459d007f79bfd20aad146b331e6158a6
Author: Luiz Angelo Daros de Luca <[email protected]>

hwmon: (adt7470) Fix swapped PWM3 and PWM4 auto mode masks

This commit swaps the values for ADT7470_PWM3_AUTO_MASK and
ADT7470_PWM4_AUTO_MASK to correctly match the datasheet. The driver
relies on the modulo parity of the channel index to configure the hardware,
so this fix does not alter driver behavior.

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 0b19b0925d1c7..a34635526aaa2 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c

[ ... ]

[Severity: High]
This problem wasn't introduced by this patch, but there is an invalid bitwise
index check in adt7470_temp_read() leading to incorrect or missing temperature
alarms:

adt7470_temp_read() {
    ...
	case hwmon_temp_alarm:
		*val = !!(data->alarm & channel);
		break;
    ...
}

Does this code use the integer channel index directly as a bitmask against
the data->alarm state? If so, sensor 0 would never report an alarm, and
other sensors would report false alarms based on incorrect bitmask evaluation
rather than a bit shift like (1 << channel).

[Severity: High]
This is a pre-existing issue, but a typo in pwm_auto_temp_store() mistakenly
assigns to data->pwm_automatic instead of data->pwm_auto_temp:

pwm_auto_temp_store() {
    ...
	mutex_lock(&data->lock);
	data->pwm_automatic[attr->index] = temp;
    ...
}

Is this assignment intended? It appears to write the parsed temperature
channel mapping value to the PWM enable state array (pwm_automatic) instead
of pwm_auto_temp. Writing to pwmN_auto_channels_temp would silently corrupt
the cached pwm_automatic state, which tracks whether the PWM is in manual
or auto mode, potentially causing subsequent reads of pwmN_enable to return
out-of-spec values to userspace.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4