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;
> }
