On Tue, Dec 06, 2022 at 01:53:31PM +0800, Xingjiang Qiao wrote: > There are fields 'last_hwmon_state' and 'last_thermal_state' in the > structure 'emc2305_cdev_data', which respectively store the cooling state > set by the 'hwmon' and 'thermal' subsystem, and the driver author hopes > that if the state set by 'hwmon' is lower than the value set by 'thermal', > the driver will just save it without actually setting the pwm. Currently, > the 'last_thermal_state' also be updated by 'hwmon', which will cause the > cooling state to never be set to a lower value. This patch fixes that. > > Signed-off-by: Xingjiang Qiao <nanpuyue@xxxxxxxxx> Applied, after renaming emc2305_set_cur_state_shim to __emc2305_set_cur_state. Thanks, Guenter > --- > drivers/hwmon/emc2305.c | 37 ++++++++++++++++++++++++------------- > 1 file changed, 24 insertions(+), 13 deletions(-) > > diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c > index 9a78ca22541e..e0f6eb8d72fc 100644 > --- a/drivers/hwmon/emc2305.c > +++ b/drivers/hwmon/emc2305.c > @@ -171,22 +171,12 @@ static int emc2305_get_max_state(struct thermal_cooling_device *cdev, unsigned l > return 0; > } > > -static int emc2305_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state) > +static int emc2305_set_cur_state_shim(struct emc2305_data *data, int cdev_idx, unsigned long state) > { > - int cdev_idx, ret; > - struct emc2305_data *data = cdev->devdata; > + int ret; > struct i2c_client *client = data->client; > u8 val, i; > > - if (state > data->max_state) > - return -EINVAL; > - > - cdev_idx = emc2305_get_cdev_idx(cdev); > - if (cdev_idx < 0) > - return cdev_idx; > - > - /* Save thermal state. */ > - data->cdev_data[cdev_idx].last_thermal_state = state; > state = max_t(unsigned long, state, data->cdev_data[cdev_idx].last_hwmon_state); > > val = EMC2305_PWM_STATE2DUTY(state, data->max_state, EMC2305_FAN_MAX); > @@ -211,6 +201,27 @@ static int emc2305_set_cur_state(struct thermal_cooling_device *cdev, unsigned l > return 0; > } > > +static int emc2305_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state) > +{ > + int cdev_idx, ret; > + struct emc2305_data *data = cdev->devdata; > + > + if (state > data->max_state) > + return -EINVAL; > + > + cdev_idx = emc2305_get_cdev_idx(cdev); > + if (cdev_idx < 0) > + return cdev_idx; > + > + /* Save thermal state. */ > + data->cdev_data[cdev_idx].last_thermal_state = state; > + ret = emc2305_set_cur_state_shim(data, cdev_idx, state); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > static const struct thermal_cooling_device_ops emc2305_cooling_ops = { > .get_max_state = emc2305_get_max_state, > .get_cur_state = emc2305_get_cur_state, > @@ -401,7 +412,7 @@ emc2305_write(struct device *dev, enum hwmon_sensor_types type, u32 attr, int ch > */ > if (data->cdev_data[cdev_idx].last_hwmon_state >= > data->cdev_data[cdev_idx].last_thermal_state) > - return emc2305_set_cur_state(data->cdev_data[cdev_idx].cdev, > + return emc2305_set_cur_state_shim(data, cdev_idx, > data->cdev_data[cdev_idx].last_hwmon_state); > return 0; > }