Re: [PATCH v4 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Software state initialization in `probe()` is not synchronized with actual hardware state.
--

commit 072171983894d237c762c5fb521a8b93d2504d1f
Author: Pei Xiao <xiaopei01@kylinos.cn>

hwmon: (gpd-fan): drop global driver data and use per-device allocation

This patch replaces the global state gpd_driver_priv with per-device private
data (struct gpd_fan_data) allocated during probe. This allows the driver to
support multiple instances in the future and aligns with kernel best practices.

> diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
> index 80de5f20781eb..7284babd4f5c9 100644
> --- a/drivers/hwmon/gpd-fan.c
> +++ b/drivers/hwmon/gpd-fan.c

[ ... ]

> @@ -624,24 +629,42 @@ static int gpd_fan_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, -EBUSY,
>  				     "Failed to request region\n");
>  
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	match = dev_get_platdata(dev);
> +	if (!match)
> +		return -EINVAL;
> +
> +	data->drvdata = match;
> +	data->pwm_enable = AUTOMATIC;
> +	data->pwm_value = 255;
> +
> +	dev_set_drvdata(dev, data);
> +

[Severity: High]
This is a pre-existing issue, but I noticed that while the software state
for pwm_enable is initialized to AUTOMATIC here in gpd_fan_probe(), there
doesn't appear to be a corresponding write to the hardware EC to actually
set this mode.

If the fan was left in manual mode by a previous driver instance, a crash,
or the BIOS, won't it remain stuck at a fixed speed while the sysfs interface
reports automatic to userspace?

Could this mismatch cause OS thermal management daemons to falsely assume
the hardware is dynamically managing the fan, potentially leading to silent
thermal throttling or shutdowns under heavy load?

>  	hwdev = devm_hwmon_device_register_with_info(dev,
>  						     DRIVER_NAME,
> -						     NULL,
> +						     data,
>  						     &gpd_fan_chip_info,
>  						     NULL);
>  	if (IS_ERR(hwdev))
>  		return dev_err_probe(dev, PTR_ERR(hwdev),
>  				     "Failed to register hwmon device\n");
>  
> -	gpd_init_ec();
> +	gpd_init_ec(data);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781055639.git.xiaopei01@kylinos.cn?part=1