Re: [PATCH 5/5] hwmon: (cros_ec) Allow modification of fan curves

[Thomas Weißschuh]

On 2026-05-30 18:37:32+0200, Armin Wolf wrote:
> Am 29.05.26 um 22:31 schrieb Thomas Weißschuh:

(...)

> > +static ssize_t temp_auto_point_temp_store(struct device *dev, struct device_attribute *attr,
> > +					  const char *buf, size_t size)
> > +{
> > +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> > +	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
> > +	struct ec_thermal_config config;
> > +	u32 *temp_field;
> > +	s64 temp;
> > +	int ret;
> > +
> > +	ret = kstrtos64(buf, 10, &temp);
> > +	if (ret)
> > +		return ret;
> > +
> > +	temp = cros_ec_hwmon_millicelsius_to_kelvin(temp);
> > +
> > +	if (overflows_type(temp, config.temp_fan_off))
> > +		return -ERANGE;
> > +
> > +	guard(hwmon_lock)(dev);
> > +
> > +	ret = cros_ec_hwmon_get_thermal_config(priv->cros_ec, sattr->index, &config);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (cros_ec_hwmon_attr_is_temp_fan_off(sattr))
> > +		temp_field = &config.temp_fan_off;
> > +	else /* temp_fan_max */
> > +		temp_field = &config.temp_fan_max;
> > +
> > +	/* Only allow values which are more aggressive than the current ones */
> > +	if (temp > *temp_field)
> > +		return -EINVAL;
> 
> i think it would be more practical for users to increase and later decrease the fan curve values.
> Could the driver copy the original fan curve configuration and use that instead? This would also
> require to restore the original fan curve during shutdown and removal.

That would be possible. We would would have to expose these limits
through a new UAPI as otherwise the user has no way to know about them.
Restoring the original on shutdown shouldn't be necessary, as the EC
will reset the curves at shutdown anyways.

I am not so sure that it would be generally useful though. Let's hear
what other people think about it.

> Thanks,
> Armin Wolf
> 
> > +
> > +	*temp_field = temp;
> > +
> > +	if (config.temp_fan_off > config.temp_fan_max)
> > +		return -EINVAL;
> > +
> > +	ret = cros_ec_hwmon_set_thermal_config(priv->cros_ec, sattr->index, &config);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return size;
> > +}

(...)