Re: [PATCH 2/3] hwmon: (cros_ec) Add reading target fan RPM function

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

On 2025-03-13 12:47:43+0800, Sung-Chi Li wrote:
> Implement the functionality of reading the target fan RPM setting from
> ChromeOS embedded controller under framework.
> 
> Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
> ---
>  drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c
> @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index
>  	return 0;
>  }
>  
> +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed)

int32_t is a userspace type. In the kernel use i32, or even better u32.

> +{
> +	int ret;
> +	struct ec_response_pwm_get_fan_rpm r;

Switch the variable declarations around.
Also call the request "req".

> +
> +	ret = cros_ec_cmd(cros_ec, 0, EC_CMD_PWM_GET_FAN_TARGET_RPM, NULL, 0, &r, sizeof(r));
> +	if (ret < 0)
> +		return ret;
> +
> +	*speed = le32_to_cpu(r.rpm);

r.rpm is not marked as __le32, I'm not sure if sparse will complain
about the usage of le32_to_cpu().

> +	return 0;
> +}
> +
>  static int cros_ec_hwmon_read_temp(struct cros_ec_device *cros_ec, u8 index, u8 *temp)
>  {
>  	unsigned int offset;
> @@ -95,6 +108,7 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  {
>  	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
>  	int ret = -EOPNOTSUPP;
> +	int32_t target_rpm;

Also u32.

>  	u16 speed;
>  	u8 temp;
>  
> @@ -111,6 +125,10 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  			ret = cros_ec_hwmon_read_fan_speed(priv->cros_ec, channel, &speed);
>  			if (ret == 0)
>  				*val = cros_ec_hwmon_is_error_fan(speed);
> +		} else if (attr == hwmon_fan_target) {
> +			ret = cros_ec_hwmon_read_fan_target(priv->cros_ec, channel, &target_rpm);
> +			if (ret == 0)
> +				*val = target_rpm;
>  		}
>  	} else if (type == hwmon_temp) {
>  		if (attr == hwmon_temp_input) {
> 
> -- 
> 2.49.0.rc0.332.g42c0ae87b1-goog
>