Re: [PATCH v3] hwmon: (cros_ec) Add set and get target fan RPM function

["Thomas Weißschuh" <thomas@weissschuh.net>]

On 2025-03-18 15:45:23+0800, Sung-Chi Li wrote:
> The ChromeOS embedded controller (EC) supports closed loop fan speed
> control, so add the fan target attribute under hwmon framework, such
> that kernel can expose reading and specifying the desired fan RPM for
> fans connected to the EC.
> 
> When probing the cros_ec hwmon module, we also check the supported
> command version of setting target fan RPM. This commit implements the
> version 0 of getting the target fan RPM, which can only read the target
> RPM of the first fan. This commit also implements the version 1 of
> setting the target fan RPM to each fan respectively.
> 
> Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
> ---
> ChromeOS embedded controller (EC) supports closed-loop fan control. We
> anticipate to have the fan related control from the kernel side, so this
> series register the HWMON_F_TARGET attribute, and implement the read and
> write function for setting/reading the target fan RPM from the EC side.
> ---
> Changes in v3:
> - Drop support of v0 setting target fan RPM, thus also simplify
>   implementations.
> - Align coding style to existing code, including using if-else rather
>   than switch-case, and ensure little endian conversion from read data.
> - Only log warning for failed probing get fan target command version
>   instead of fail the whole driver.
> - Link to v2: https://lore.kernel.org/r/20250317-extend_ec_hwmon_fan-v2-1-13670557afe5@chromium.org
> 
> Changes in v2:
> - Squash the read, write, and register of fan target attribute to 1
>   commit, as they are the same topic.
> - Probe the supported command version from EC for setting the target fan
>   RPM, and perform the set fan target RPM based on the supported
>   version.
> - Update the used variable type to kernel types (i.e., u32).
> - Link to v1: https://lore.kernel.org/r/20250313-extend_ec_hwmon_fan-v1-0-5c566776f2c4@chromium.org
> ---
>  drivers/hwmon/cros_ec_hwmon.c | 90 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 85 insertions(+), 5 deletions(-)

<snip>

> +static int cros_ec_hwmon_set_fan_rpm(struct cros_ec_device *cros_ec, u8 index,
> +				     u16 val)
> +{
> +	struct ec_params_pwm_set_fan_target_rpm_v1 req = {
> +		.rpm = val,
> +		.fan_idx = index,
> +	};
> +	int ret;
> +
> +	ret = cros_ec_cmd(cros_ec, 1, EC_CMD_PWM_SET_FAN_TARGET_RPM, &req,
> +			  sizeof(req), NULL, 0);

This setting does not seem to survive system suspend.
A PM handler is necessary.

> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +

<snip>