Re: [RFC PATCH] hwmon: it87: Add basic IT8613E fan monitoring support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional execution of PWM, limit, and voltage configuration in it87_resume() bypasses chip safety guards.
--

commit 38aeab66f2bad568170fc7c7a3bf957b47ace77f
Author: Jerome Tollet <jerome.tollet@gmail.com>

hwmon: it87: Add basic IT8613E fan monitoring support
    
The IT8613E is not recognized by the in-tree it87 driver. Full
support was proposed previously but withdrawn after problems were found
in the temperature and PWM paths.

Add conservative support limited to fan input monitoring. Detect device
ID 0x8613, check the fan pin muxes, use the existing 16-bit tachometer
layout, and expose enabled fan inputs read-only.
    
Link: https://lore.kernel.org/linux-hwmon/20260114221210.98071-1-yahoo@perenite.com/

> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 87edb1b6048bb..2f58004246c39 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c

[ ... ]

> @@ -3355,7 +3396,8 @@ static void it87_init_device(struct platform_device *pdev)
>  		data->auto_pwm[i][3] = 0x7f;	/* Full speed, hard-coded */
>  	}
>  
> -	it87_check_limit_regs(data);
> +	if (!has_fan_input_only(data))
> +		it87_check_limit_regs(data);
>  
>  	/*
>  	 * Temperature channels are not forcibly enabled, as they can be
> @@ -3364,7 +3406,8 @@ static void it87_init_device(struct platform_device *pdev)
>  	 * run-time through the temp{1-3}_type sysfs accessors if needed.
>  	 */
>  
> -	it87_check_voltage_monitors_reset(data);
> +	if (!has_fan_input_only(data))
> +		it87_check_voltage_monitors_reset(data);
>  
>  	it87_check_tachometers_reset(pdev);

[Severity: High]
Since we need to guard these register writes during initialization, do we
also need to guard them during system resume?

Looking at it87_resume() in drivers/hwmon/it87.c, these functions are called
unconditionally:

static int it87_resume(struct device *dev)
{
        ...
        it87_check_pwm(dev);
        it87_check_limit_regs(data);
        it87_check_voltage_monitors_reset(data);
        ...
}

Could this bypass the has_fan_input_only() guards on the resume path and
attempt to write to unsupported PWM, voltage limit, and VIN enable registers
on the IT8613E chip?

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